Messages in this thread | | | Date | Wed, 12 Nov 2014 14:26:51 +0530 | From | Sanchayan Maity <> | Subject | Re: [[PATCHv2] 3/3] drivers/rtc/rtc-snvs: Add clock support |
| |
On Wednesday 12 November 2014 02:57 AM, Stefan Agner wrote: > On 2014-11-07 14:04, Sanchayan Maity wrote: >> This patch adds clock enable and disable support for >> the SNVS peripheral, which is required for using the >> RTC within the SNVS block. > > What happens if the device tree node is there while this patch is not > applied? I guess the driver would load, but since the clocks of the > peripheral are not enabled the first register access would lead to bus > error or similar. If this is the case, this would break bisectability. > You should move the device tree patch to the end. >
If the DT node was present with this patch not applied, the driver would still crash with a bus error, since the clocks for SNVS are not explicitly enabled anywhere else. I had not thought about any problems which might occur later with git bisect. Will move this to the end with v3.
Thanks.
Regards, Sanchayan.
> Other than that > Acked-by: Stefan Agner <stefan@agner.ch> > > -- > Stefan > >> >> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com> >> --- >> drivers/rtc/rtc-snvs.c | 34 ++++++++++++++++++++++++++++++++-- >> 1 file changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-snvs.c b/drivers/rtc/rtc-snvs.c >> index fa384fe..d4a6512 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,6 +262,18 @@ static int snvs_rtc_probe(struct platform_device *pdev) >> if (data->irq < 0) >> return data->irq; >> >> + data->clk = devm_clk_get(&pdev->dev, "snvs-rtc"); >> + if (IS_ERR(data->clk)) { >> + data->clk = NULL; >> + } else { >> + ret = clk_prepare_enable(data->clk); >> + if (ret) { >> + dev_err(&pdev->dev, >> + "Could not prepare or enable the snvs clock\n"); >> + return ret; >> + } >> + } >> + >> platform_set_drvdata(pdev, data); >> >> spin_lock_init(&data->lock); >> @@ -280,7 +294,7 @@ static int snvs_rtc_probe(struct platform_device *pdev) >> if (ret) { >> dev_err(&pdev->dev, "failed to request irq %d: %d\n", >> data->irq, ret); >> - return ret; >> + goto error_rtc_device_register; >> } >> >> data->rtc = devm_rtc_device_register(&pdev->dev, pdev->name, >> @@ -288,10 +302,16 @@ static int snvs_rtc_probe(struct platform_device *pdev) >> 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: >> + if (data->clk) >> + clk_disable_unprepare(data->clk); >> + >> + return ret; >> } >> >> #ifdef CONFIG_PM_SLEEP >> @@ -302,16 +322,26 @@ static int snvs_rtc_suspend(struct device *dev) >> if (device_may_wakeup(dev)) >> enable_irq_wake(data->irq); >> >> + if (data->clk) >> + clk_disable_unprepare(data->clk); >> + >> return 0; >> } >> >> static int snvs_rtc_resume(struct device *dev) >> { >> struct snvs_rtc_data *data = dev_get_drvdata(dev); >> + int ret; >> >> if (device_may_wakeup(dev)) >> disable_irq_wake(data->irq); >> >> + if (data->clk) { >> + ret = clk_prepare_enable(data->clk); >> + if (ret) >> + return ret; >> + } >> + >> return 0; >> } >> #endif >
| |