| Message ID | 1554752326-13319-13-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | watchdog: Convert to use device managed functions and other improvements | expand |
Hi Guenter, On 08 April 2019 20:39, Guenter Roeck: > Subject: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of > dereferencing it repeatedly > > Introduce local variable 'struct device *dev' and use it instead of > dereferencing it repeatedly. > > The conversion was done automatically with coccinelle using the > following semantic patches. The semantic patches and the scripts > used to generate this commit log are available at > https://github.com/groeck/coccinelle-patches > > Cc: Support Opensource <support.opensource@diasemi.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/da9063_wdt.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c > index 384dca16af8b..06eb9070203c 100644 > --- a/drivers/watchdog/da9063_wdt.c > +++ b/drivers/watchdog/da9063_wdt.c > @@ -188,17 +188,18 @@ static const struct watchdog_ops > da9063_watchdog_ops = { > > static int da9063_wdt_probe(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > struct da9063 *da9063; > struct watchdog_device *wdd; > > - if (!pdev->dev.parent) > + if (!dev->parent) > return -EINVAL; None of my previous Acked e-mails in this patch set considered whether the dev->parent was NULL. But this DA9063 driver does. Logically, this is correct to check, but ... any thoughts? Otherwise, Acked-by: Steve Twiss <stwiss.opensource@diasemi.com> Regards, Steve
Hi Steve, On 4/10/19 5:50 AM, Steve Twiss wrote: > Hi Guenter, > > On 08 April 2019 20:39, Guenter Roeck: > >> Subject: [PATCH 12/22] watchdog: da9063_wdt: Use 'dev' instead of >> dereferencing it repeatedly >> >> Introduce local variable 'struct device *dev' and use it instead of >> dereferencing it repeatedly. >> >> The conversion was done automatically with coccinelle using the >> following semantic patches. The semantic patches and the scripts >> used to generate this commit log are available at >> https://github.com/groeck/coccinelle-patches >> >> Cc: Support Opensource <support.opensource@diasemi.com> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> drivers/watchdog/da9063_wdt.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c >> index 384dca16af8b..06eb9070203c 100644 >> --- a/drivers/watchdog/da9063_wdt.c >> +++ b/drivers/watchdog/da9063_wdt.c >> @@ -188,17 +188,18 @@ static const struct watchdog_ops >> da9063_watchdog_ops = { >> >> static int da9063_wdt_probe(struct platform_device *pdev) >> { >> + struct device *dev = &pdev->dev; >> struct da9063 *da9063; >> struct watchdog_device *wdd; >> >> - if (!pdev->dev.parent) >> + if (!dev->parent) >> return -EINVAL; > > None of my previous Acked e-mails in this patch set considered whether the > dev->parent was NULL. But this DA9063 driver does. > > Logically, this is correct to check, but ... any thoughts? The check is not really necessary. All da90xx drivers are instantiated from mfd drivers and do provide a parent. Anyone changing that code or trying to instantiate the drivers from some other place without providing a parent really deserves the resulting crash (it would be a bug). Either case, I don't think this warrants changing this driver to drop the check, or changing the other drivers to add unnecessary checks just to make the code consistent. > Otherwise, > > Acked-by: Steve Twiss <stwiss.opensource@diasemi.com> > Thanks, Guenter
diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c index 384dca16af8b..06eb9070203c 100644 --- a/drivers/watchdog/da9063_wdt.c +++ b/drivers/watchdog/da9063_wdt.c @@ -188,17 +188,18 @@ static const struct watchdog_ops da9063_watchdog_ops = { static int da9063_wdt_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; struct da9063 *da9063; struct watchdog_device *wdd; - if (!pdev->dev.parent) + if (!dev->parent) return -EINVAL; - da9063 = dev_get_drvdata(pdev->dev.parent); + da9063 = dev_get_drvdata(dev->parent); if (!da9063) return -EINVAL; - wdd = devm_kzalloc(&pdev->dev, sizeof(*wdd), GFP_KERNEL); + wdd = devm_kzalloc(dev, sizeof(*wdd), GFP_KERNEL); if (!wdd) return -ENOMEM; @@ -208,7 +209,7 @@ static int da9063_wdt_probe(struct platform_device *pdev) wdd->max_timeout = DA9063_WDT_MAX_TIMEOUT; wdd->min_hw_heartbeat_ms = DA9063_RESET_PROTECTION_MS; wdd->timeout = DA9063_WDG_TIMEOUT; - wdd->parent = &pdev->dev; + wdd->parent = dev; wdd->status = WATCHDOG_NOWAYOUT_INIT_STATUS; @@ -222,7 +223,7 @@ static int da9063_wdt_probe(struct platform_device *pdev) set_bit(WDOG_HW_RUNNING, &wdd->status); } - return devm_watchdog_register_device(&pdev->dev, wdd); + return devm_watchdog_register_device(dev, wdd); } static struct platform_driver da9063_wdt_driver = {
Introduce local variable 'struct device *dev' and use it instead of dereferencing it repeatedly. The conversion was done automatically with coccinelle using the following semantic patches. The semantic patches and the scripts used to generate this commit log are available at https://github.com/groeck/coccinelle-patches Cc: Support Opensource <support.opensource@diasemi.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/watchdog/da9063_wdt.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)