| Message ID | 20170920053020.6860-5-andrew@aj.id.au |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | watchdog: aspeed: Retain enabled state and move to arch_initcall | expand |
On Wed, Sep 20, 2017 at 3:00 PM, Andrew Jeffery <andrew@aj.id.au> wrote: > Probing at device_initcall time lead to perverse cases where the > watchdog was probed after, say, I2C devices, which then leaves a > potentially running watchdog at the mercy of I2C device behaviour and > bus conditions. > > Load the watchdog driver early to ensure that the kernel is patting it > well before initialising peripherals. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> I agree that we need to make sure the watchdog driver is loaded earlier. I think this is the correct method, but I'll defer to Guenter on this one. Cheers, Joel
On Wed, 2017-09-20 at 15:43 +0930, Joel Stanley wrote: > > On Wed, Sep 20, 2017 at 3:00 PM, Andrew Jeffery <andrew@aj.id.au> wrote: > > Probing at device_initcall time lead to perverse cases where the > > watchdog was probed after, say, I2C devices, which then leaves a > > potentially running watchdog at the mercy of I2C device behaviour and > > bus conditions. > > > > Load the watchdog driver early to ensure that the kernel is patting it > > well before initialising peripherals. > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > I agree that we need to make sure the watchdog driver is loaded > earlier. I think this is the correct method, but I'll defer to Guenter > on this one. Just following up on Joel's comments: Is there anything else I need to address? Cheers, Andrew
On Wed, Sep 20, 2017 at 03:00:20PM +0930, Andrew Jeffery wrote: > Probing at device_initcall time lead to perverse cases where the > watchdog was probed after, say, I2C devices, which then leaves a > potentially running watchdog at the mercy of I2C device behaviour and > bus conditions. > > Load the watchdog driver early to ensure that the kernel is patting it > well before initialising peripherals. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> At some point we'll need to come up with a generic means to handle those situations. Until then, I don't have a better idea. Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/aspeed_wdt.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > index 6c6dd3f4c48d..ca5b91e2eb92 100644 > --- a/drivers/watchdog/aspeed_wdt.c > +++ b/drivers/watchdog/aspeed_wdt.c > @@ -316,7 +316,18 @@ static struct platform_driver aspeed_watchdog_driver = { > .of_match_table = of_match_ptr(aspeed_wdt_of_table), > }, > }; > -module_platform_driver(aspeed_watchdog_driver); > + > +static int __init aspeed_wdt_init(void) > +{ > + return platform_driver_register(&aspeed_watchdog_driver); > +} > +arch_initcall(aspeed_wdt_init); > + > +static void __exit aspeed_wdt_exit(void) > +{ > + platform_driver_unregister(&aspeed_watchdog_driver); > +} > +module_exit(aspeed_wdt_exit); > > MODULE_DESCRIPTION("Aspeed Watchdog Driver"); > MODULE_LICENSE("GPL");
On 10/17/2017 04:35 AM, Andrew Jeffery wrote: > On Wed, 2017-09-20 at 15:43 +0930, Joel Stanley wrote: >>> On Wed, Sep 20, 2017 at 3:00 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >>> Probing at device_initcall time lead to perverse cases where the >>> watchdog was probed after, say, I2C devices, which then leaves a >>> potentially running watchdog at the mercy of I2C device behaviour and >>> bus conditions. >>> >>> Load the watchdog driver early to ensure that the kernel is patting it >>> well before initialising peripherals. >>> >>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> >> >> I agree that we need to make sure the watchdog driver is loaded >> earlier. I think this is the correct method, but I'll defer to Guenter >> on this one. > > Just following up on Joel's comments: Is there anything else I need to > address? > No. Sorry, I have been terribly busy. Digging through patch backlog today. Guenter
diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index 6c6dd3f4c48d..ca5b91e2eb92 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -316,7 +316,18 @@ static struct platform_driver aspeed_watchdog_driver = { .of_match_table = of_match_ptr(aspeed_wdt_of_table), }, }; -module_platform_driver(aspeed_watchdog_driver); + +static int __init aspeed_wdt_init(void) +{ + return platform_driver_register(&aspeed_watchdog_driver); +} +arch_initcall(aspeed_wdt_init); + +static void __exit aspeed_wdt_exit(void) +{ + platform_driver_unregister(&aspeed_watchdog_driver); +} +module_exit(aspeed_wdt_exit); MODULE_DESCRIPTION("Aspeed Watchdog Driver"); MODULE_LICENSE("GPL");
Probing at device_initcall time lead to perverse cases where the watchdog was probed after, say, I2C devices, which then leaves a potentially running watchdog at the mercy of I2C device behaviour and bus conditions. Load the watchdog driver early to ensure that the kernel is patting it well before initialising peripherals. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- drivers/watchdog/aspeed_wdt.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)