Message ID | 20220422183012.444674-1-j.neuschaefer@gmx.net |
---|---|
Headers | show |
Series | Nuvoton WPCM450 clock and reset driver | expand |
On Fri, Apr 22, 2022 at 11:34:17AM -0700, Guenter Roeck wrote: > On Fri, Apr 22, 2022 at 08:30:08PM +0200, Jonathan Neuschäfer wrote: > > On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral > > clocks are individually gated and ungated. Therefore, the watchdog > > driver must be able to ungate the watchdog clock. > > > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> > > --- > > drivers/watchdog/npcm_wdt.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c > > index 28a24caa2627c..6d27f0e16188e 100644 > > --- a/drivers/watchdog/npcm_wdt.c > > +++ b/drivers/watchdog/npcm_wdt.c > > @@ -3,6 +3,7 @@ > > // Copyright (c) 2018 IBM Corp. > > > > #include <linux/bitops.h> > > +#include <linux/clk.h> > > #include <linux/delay.h> > > #include <linux/interrupt.h> > > #include <linux/kernel.h> > > @@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > struct npcm_wdt *wdt; > > + struct clk *clk; > > int irq; > > int ret; > > > > @@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev) > > if (IS_ERR(wdt->reg)) > > return PTR_ERR(wdt->reg); > > > > + clk = devm_clk_get_optional(&pdev->dev, NULL); > > + if (IS_ERR(clk)) > > + return PTR_ERR(clk); > > + > > + if (clk) > > + clk_prepare_enable(clk); > > + > > This needs a matching clk_disable_unprepare(). Good point. It's probably easiest if I move the clk calls to the watchdog start/stop callbacks, then. Thanks, Jonathan
On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote: >In the WPCM450 SoC, the clocks for each timer can be gated individually. >To prevent the timer 1 clock from being gated, enable it explicitly. > >Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> >--- > drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > >diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c >index a00520cbb660a..974269b6b0c36 100644 >--- a/drivers/clocksource/timer-npcm7xx.c >+++ b/drivers/clocksource/timer-npcm7xx.c >@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void) > > static int __init npcm7xx_timer_init(struct device_node *np) > { >+ struct clk *clk; > int ret; > > ret = timer_of_init(np, &npcm7xx_to); >- if (ret) >+ if (ret) { >+ pr_warn("timer_of_init failed: %d\n", ret); This seems like a somewhat opaque message to emit, especially given this file's lack of a pr_fmt() definition -- maybe add a %pOF so it's slightly easier to trace back to the device it stems from? > return ret; >+ } > > /* Clock input is divided by PRESCALE + 1 before it is fed */ > /* to the counter */ > npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate / > (NPCM7XX_Tx_MIN_PRESCALE + 1); > >+ /* Enable the clock for timer1, if it exists */ >+ clk = of_clk_get(np, 1); >+ if (clk) { >+ if (!IS_ERR(clk)) >+ clk_prepare_enable(clk); >+ else >+ pr_warn("Failed to get clock for timer1: %pe", clk); Likewise here (though to a slightly lesser extent). >+ } >+ > npcm7xx_clocksource_init(); > npcm7xx_clockevents_init(); > >-- >2.35.1 >
On Thu, Apr 28, 2022 at 09:11:58AM +0000, Zev Weiss wrote: > On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote: > >In the WPCM450 SoC, the clocks for each timer can be gated individually. > >To prevent the timer 1 clock from being gated, enable it explicitly. > > > >Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> > >--- > > drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c > >index a00520cbb660a..974269b6b0c36 100644 > >--- a/drivers/clocksource/timer-npcm7xx.c > >+++ b/drivers/clocksource/timer-npcm7xx.c > >@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void) > > > > static int __init npcm7xx_timer_init(struct device_node *np) > > { > >+ struct clk *clk; > > int ret; > > > > ret = timer_of_init(np, &npcm7xx_to); > >- if (ret) > >+ if (ret) { > >+ pr_warn("timer_of_init failed: %d\n", ret); > > This seems like a somewhat opaque message to emit, especially given this > file's lack of a pr_fmt() definition -- maybe add a %pOF so it's > slightly easier to trace back to the device it stems from? Now that I look at this code again, I think I should just drop the pr_warn entirely, since I didn't mention it in the description, and it's unrelated to enabling the clock. > > return ret; > >+ } > > > > /* Clock input is divided by PRESCALE + 1 before it is fed */ > > /* to the counter */ > > npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate / > > (NPCM7XX_Tx_MIN_PRESCALE + 1); > > > >+ /* Enable the clock for timer1, if it exists */ > >+ clk = of_clk_get(np, 1); > >+ if (clk) { > >+ if (!IS_ERR(clk)) > >+ clk_prepare_enable(clk); > >+ else > >+ pr_warn("Failed to get clock for timer1: %pe", clk); > > Likewise here (though to a slightly lesser extent). I'll add %pOF here. Thanks, Jonathan