| Message ID | 1474312335-20997-4-git-send-email-robert.jarzmik@free.fr (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Stephen Boyd |
| Headers | show |
On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote: > The OS timer rate used for the watchdog can now be fetched from the > standard clock API. This will remove the last user of > get_clock_tick_rate() in both pxa and sa11x0 architectures. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Did you test this ? Potential problem, if built into the kernel, could be that the clocks might not be ready by the time the driver is instantiated. Unless this is converted to a platform driver, it won't be able to handle a -EPROBE_DEFER from the clock subsystem. Guenter > --- > drivers/watchdog/sa1100_wdt.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/sa1100_wdt.c b/drivers/watchdog/sa1100_wdt.c > index e1d39a1e9628..8965e3f536c3 100644 > --- a/drivers/watchdog/sa1100_wdt.c > +++ b/drivers/watchdog/sa1100_wdt.c > @@ -22,6 +22,7 @@ > > #include <linux/module.h> > #include <linux/moduleparam.h> > +#include <linux/clk.h> > #include <linux/types.h> > #include <linux/kernel.h> > #include <linux/fs.h> > @@ -155,12 +156,27 @@ static struct miscdevice sa1100dog_miscdev = { > }; > > static int margin __initdata = 60; /* (secs) Default is 1 minute */ > +static struct clk *clk; > > static int __init sa1100dog_init(void) > { > int ret; > > - oscr_freq = get_clock_tick_rate(); > + clk = clk_get(NULL, "OSTIMER0"); > + if (IS_ERR(clk)) { > + pr_err("SA1100/PXA2xx Watchdog Timer: clock not found: %d\n", > + (int) PTR_ERR(clk)); > + return PTR_ERR(clk); > + } > + > + ret = clk_prepare_enable(clk); > + if (ret) { > + pr_err("SA1100/PXA2xx Watchdog Timer: clock failed to prepare+enable: %d\n", > + ret); > + goto err; > + } > + > + oscr_freq = clk_get_rate(clk); > > /* > * Read the reset status, and save it for later. If > @@ -176,11 +192,17 @@ static int __init sa1100dog_init(void) > pr_info("SA1100/PXA2xx Watchdog Timer: timer margin %d sec\n", > margin); > return ret; > +err: > + clk_disable_unprepare(clk); > + clk_put(clk); > + return ret; > } > > static void __exit sa1100dog_exit(void) > { > misc_deregister(&sa1100dog_miscdev); > + clk_disable_unprepare(clk); > + clk_put(clk); > } > > module_init(sa1100dog_init); > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 19, 2016 at 01:08:16PM -0700, Guenter Roeck wrote: > On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote: > > The OS timer rate used for the watchdog can now be fetched from the > > standard clock API. This will remove the last user of > > get_clock_tick_rate() in both pxa and sa11x0 architectures. > > > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > > Did you test this ? Potential problem, if built into the kernel, could be that > the clocks might not be ready by the time the driver is instantiated. Unless > this is converted to a platform driver, it won't be able to handle a > -EPROBE_DEFER from the clock subsystem. Really not a problem at all. The OSTIMER0 is required for the system tick, and if that's not present, the kernel will be without any kind of time keeping, so a missing watchdog driver is the least of the problems. Therefore, both PXA and SA11x0 register their clocks really early to ensure that OSTIMER0 is available by the time_init() stage, which is way before driver probe time.
On 09/19/2016 12:12 PM, Robert Jarzmik wrote: > The OS timer rate used for the watchdog can now be fetched from the > standard clock API. This will remove the last user of > get_clock_tick_rate() in both pxa and sa11x0 architectures. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Tested-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/sa1100_wdt.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/sa1100_wdt.c b/drivers/watchdog/sa1100_wdt.c > index e1d39a1e9628..8965e3f536c3 100644 > --- a/drivers/watchdog/sa1100_wdt.c > +++ b/drivers/watchdog/sa1100_wdt.c > @@ -22,6 +22,7 @@ > > #include <linux/module.h> > #include <linux/moduleparam.h> > +#include <linux/clk.h> > #include <linux/types.h> > #include <linux/kernel.h> > #include <linux/fs.h> > @@ -155,12 +156,27 @@ static struct miscdevice sa1100dog_miscdev = { > }; > > static int margin __initdata = 60; /* (secs) Default is 1 minute */ > +static struct clk *clk; > > static int __init sa1100dog_init(void) > { > int ret; > > - oscr_freq = get_clock_tick_rate(); > + clk = clk_get(NULL, "OSTIMER0"); > + if (IS_ERR(clk)) { > + pr_err("SA1100/PXA2xx Watchdog Timer: clock not found: %d\n", > + (int) PTR_ERR(clk)); > + return PTR_ERR(clk); > + } > + > + ret = clk_prepare_enable(clk); > + if (ret) { > + pr_err("SA1100/PXA2xx Watchdog Timer: clock failed to prepare+enable: %d\n", > + ret); > + goto err; > + } > + > + oscr_freq = clk_get_rate(clk); > > /* > * Read the reset status, and save it for later. If > @@ -176,11 +192,17 @@ static int __init sa1100dog_init(void) > pr_info("SA1100/PXA2xx Watchdog Timer: timer margin %d sec\n", > margin); > return ret; > +err: > + clk_disable_unprepare(clk); > + clk_put(clk); > + return ret; > } > > static void __exit sa1100dog_exit(void) > { > misc_deregister(&sa1100dog_miscdev); > + clk_disable_unprepare(clk); > + clk_put(clk); > } > > module_init(sa1100dog_init); > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/19/2016 03:36 PM, Russell King - ARM Linux wrote: > On Mon, Sep 19, 2016 at 01:08:16PM -0700, Guenter Roeck wrote: >> On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote: >>> The OS timer rate used for the watchdog can now be fetched from the >>> standard clock API. This will remove the last user of >>> get_clock_tick_rate() in both pxa and sa11x0 architectures. >>> >>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> >> >> Did you test this ? Potential problem, if built into the kernel, could be that >> the clocks might not be ready by the time the driver is instantiated. Unless >> this is converted to a platform driver, it won't be able to handle a >> -EPROBE_DEFER from the clock subsystem. > > Really not a problem at all. The OSTIMER0 is required for the system > tick, and if that's not present, the kernel will be without any kind > of time keeping, so a missing watchdog driver is the least of the > problems. > > Therefore, both PXA and SA11x0 register their clocks really early to > ensure that OSTIMER0 is available by the time_init() stage, which is > way before driver probe time. > You are right. And, at least in qemu, it actually works. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Guenter Roeck <linux@roeck-us.net> writes: > On 09/19/2016 03:36 PM, Russell King - ARM Linux wrote: >> On Mon, Sep 19, 2016 at 01:08:16PM -0700, Guenter Roeck wrote: >>> On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote: >>>> The OS timer rate used for the watchdog can now be fetched from the >>>> standard clock API. This will remove the last user of >>>> get_clock_tick_rate() in both pxa and sa11x0 architectures. >>>> >>>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> >>> >>> Did you test this ? Potential problem, if built into the kernel, could be that >>> the clocks might not be ready by the time the driver is instantiated. Unless >>> this is converted to a platform driver, it won't be able to handle a >>> -EPROBE_DEFER from the clock subsystem. >> >> Really not a problem at all. The OSTIMER0 is required for the system >> tick, and if that's not present, the kernel will be without any kind >> of time keeping, so a missing watchdog driver is the least of the >> problems. >> >> Therefore, both PXA and SA11x0 register their clocks really early to >> ensure that OSTIMER0 is available by the time_init() stage, which is >> way before driver probe time. >> > > You are right. And, at least in qemu, it actually works. Hi Guenter, Yes, it was tested on pxa25x (lubbock) and pxa27x (mainstone). Cheers.
On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote: > The OS timer rate used for the watchdog can now be fetched from the > standard clock API. This will remove the last user of > get_clock_tick_rate() in both pxa and sa11x0 architectures. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Acked-by: Russell King <rmk+kernel@armlinux.org.uk> Thanks.
Hi Robert, > The OS timer rate used for the watchdog can now be fetched from the > standard clock API. This will remove the last user of > get_clock_tick_rate() in both pxa and sa11x0 architectures. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> Acked-by: Wim Van Sebroeck <wim@iguana.be> Kind regards, Wim. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/watchdog/sa1100_wdt.c b/drivers/watchdog/sa1100_wdt.c index e1d39a1e9628..8965e3f536c3 100644 --- a/drivers/watchdog/sa1100_wdt.c +++ b/drivers/watchdog/sa1100_wdt.c @@ -22,6 +22,7 @@ #include <linux/module.h> #include <linux/moduleparam.h> +#include <linux/clk.h> #include <linux/types.h> #include <linux/kernel.h> #include <linux/fs.h> @@ -155,12 +156,27 @@ static struct miscdevice sa1100dog_miscdev = { }; static int margin __initdata = 60; /* (secs) Default is 1 minute */ +static struct clk *clk; static int __init sa1100dog_init(void) { int ret; - oscr_freq = get_clock_tick_rate(); + clk = clk_get(NULL, "OSTIMER0"); + if (IS_ERR(clk)) { + pr_err("SA1100/PXA2xx Watchdog Timer: clock not found: %d\n", + (int) PTR_ERR(clk)); + return PTR_ERR(clk); + } + + ret = clk_prepare_enable(clk); + if (ret) { + pr_err("SA1100/PXA2xx Watchdog Timer: clock failed to prepare+enable: %d\n", + ret); + goto err; + } + + oscr_freq = clk_get_rate(clk); /* * Read the reset status, and save it for later. If @@ -176,11 +192,17 @@ static int __init sa1100dog_init(void) pr_info("SA1100/PXA2xx Watchdog Timer: timer margin %d sec\n", margin); return ret; +err: + clk_disable_unprepare(clk); + clk_put(clk); + return ret; } static void __exit sa1100dog_exit(void) { misc_deregister(&sa1100dog_miscdev); + clk_disable_unprepare(clk); + clk_put(clk); } module_init(sa1100dog_init);
The OS timer rate used for the watchdog can now be fetched from the standard clock API. This will remove the last user of get_clock_tick_rate() in both pxa and sa11x0 architectures. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- drivers/watchdog/sa1100_wdt.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)