linux-kernel - Re: [PATCH 3/3] drivers/rtc/rtc-snvs: Add clock support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8e48ef6e52a8c6eaa1cda95fbd3c1bc7@agner.ch>
Date:	Mon, 29 Sep 2014 15:05:35 +0200
From:	Stefan Agner <stefan@...er.ch>
To:	Sanchayan Maity <maitysanchayan@...il.com>
Cc:	kernel@...gutronix.de, a.zummo@...ertech.it, b35083@...escale.com,
	rtc-linux@...glegroups.com, shawn.guo@...escale.com,
	linux@....linux.org.uk, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] drivers/rtc/rtc-snvs: Add clock support

Am 2014-09-26 15:14, schrieb Sanchayan Maity:
> This patch adds clock enable and disable support
> for the SNVS peripheral which is required by the
> SNVS RTC.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@...il.com>
> ---
>  drivers/rtc/rtc-snvs.c |   48 +++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-snvs.c b/drivers/rtc/rtc-snvs.c
> index fa384fe..f3e8f05 100644
> --- a/drivers/rtc/rtc-snvs.c
> +++ b/drivers/rtc/rtc-snvs.c
> @@ -17,6 +17,7 @@
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtc.h>
> +#include <linux/clk.h>
>  
>  /* These register offsets are relative to LP (Low Power) range */
>  #define SNVS_LPCR		0x04
> @@ -39,6 +40,7 @@ struct snvs_rtc_data {
>  	void __iomem *ioaddr;
>  	int irq;
>  	spinlock_t lock;
> +	struct clk *clk;
>  };
>  
>  static u32 rtc_read_lp_counter(void __iomem *ioaddr)
> @@ -260,8 +262,31 @@ static int snvs_rtc_probe(struct platform_device *pdev)
>  	if (data->irq < 0)
>  		return data->irq;
>  
> +	ret = devm_request_irq(&pdev->dev, data->irq, snvs_rtc_irq_handler,
> +			       IRQF_SHARED, "rtc alarm", &pdev->dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq %d: %d\n",
> +			data->irq, ret);
> +		return ret;
> +	}
> +
> +	data->clk = devm_clk_get(&pdev->dev, "snvs");
> +	if (IS_ERR(data->clk)) {
> +		dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
> +						PTR_ERR(data->clk));
> +		ret = PTR_ERR(data->clk);
> +		return ret;
> +	}
> +
>  	platform_set_drvdata(pdev, data);
>  
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Could not prepare or enable the snvs clock\n");
> +		return ret;
> +	}
> +
>  	spin_lock_init(&data->lock);
>  
>  	/* Initialize glitch detect */
> @@ -275,23 +300,20 @@ static int snvs_rtc_probe(struct platform_device *pdev)
>  
>  	device_init_wakeup(&pdev->dev, true);
>  
> -	ret = devm_request_irq(&pdev->dev, data->irq, snvs_rtc_irq_handler,
> -			       IRQF_SHARED, "rtc alarm", &pdev->dev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to request irq %d: %d\n",
> -			data->irq, ret);
> -		return ret;
> -	}
> -

Is there a reason why you moved the irq request before enabling/getting
the clocks? AFAIK the irq is quite independent from the clock of that
IP, so you could that leave here. Try to make as few changes to the code
as possible to archive your goal.

>  	data->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
>  					&snvs_rtc_ops, THIS_MODULE);
>  	if (IS_ERR(data->rtc)) {
>  		ret = PTR_ERR(data->rtc);
>  		dev_err(&pdev->dev, "failed to register rtc: %d\n", ret);
> -		return ret;
> +		goto error_rtc_device_register;
>  	}
>  
>  	return 0;
> +
> +error_rtc_device_register:
> +	clk_disable_unprepare(data->clk);
> +
> +	return ret;
>  }
>  
>  #ifdef CONFIG_PM_SLEEP
> @@ -302,16 +324,24 @@ static int snvs_rtc_suspend(struct device *dev)
>  	if (device_may_wakeup(dev))
>  		enable_irq_wake(data->irq);
>  
> +	clk_disable_unprepare(data->clk);
> +
>  	return 0;
>  }
>  
>  static int snvs_rtc_resume(struct device *dev)
>  {
> +	int ret;
> +
>  	struct snvs_rtc_data *data = dev_get_drvdata(dev);
>  
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(data->irq);
>  
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }
>  #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/