| Message ID | 1554913683-25454-19-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | watchdog: Expand use of device managed functions (series 3 of 3) | expand |
On Wed, 2019-04-10 at 09:27 -0700, Guenter Roeck wrote: > 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 Interesting collection. It would be useful to specify which particular script generated or enabled this patch. Just scanning briefly, it might have been this one: https://github.com/groeck/coccinelle-patches/blob/master/common/deref.cocci But it looks like some manual bit might have been required too. And trivially: > diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c [] > @@ -133,18 +133,19 @@ static struct watchdog_device mt7621_wdt_dev = { [] > watchdog_init_timeout(&mt7621_wdt_dev, mt7621_wdt_dev.max_timeout, > - &pdev->dev); > + dev); This could be on one line.
On Wed, Apr 10, 2019 at 11:46:24AM -0700, Joe Perches wrote: > On Wed, 2019-04-10 at 09:27 -0700, Guenter Roeck wrote: > > 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 > > Interesting collection. It would be useful to specify which > particular script generated or enabled this patch. > It is pdev-addvar.cocci, rule 'new'. deref.cocci wasn't used for the watchdog patches. The script to apply the various rules is watchdog/make.sh. Pointing to the actual scripts used is a good idea. I'll see if I can add this for subsequent series. After all, the commit log is also auto-generated, so this should be straightforward. > Just scanning briefly, it might have been this one: > https://github.com/groeck/coccinelle-patches/blob/master/common/deref.cocci > But it looks like some manual bit might have been required too. Not for this one. There were a couple of situations where I had to manually split long lines to avoid checkpatch warnings, and I manually updated a few of the commit logs, but not in this patch. > > And trivially: > > > diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c > [] > > @@ -133,18 +133,19 @@ static struct watchdog_device mt7621_wdt_dev = { > [] > > watchdog_init_timeout(&mt7621_wdt_dev, mt7621_wdt_dev.max_timeout, > > - &pdev->dev); > > + dev); > > This could be on one line. > Coccinelle isn't perfect. The rule doesn't modify the entire argument list, only the last argument, so coccinelle missed that it could have merged the two lines into one. A checkpatch rule suggesting that multiple extension lines can be merged might be useful to help finding such situations. Just a thought. Thanks, Guenter
On Wed, 2019-04-10 at 12:54 -0700, Guenter Roeck wrote: > A checkpatch rule suggesting that multiple extension lines can be merged > might be useful to help finding such situations. Just a thought. You are welcome to try to write that one. It's likely not a trivial task to make it sensible. cheers, Joe
Hi Joe, On Wed, Apr 10, 2019 at 03:10:09PM -0700, Joe Perches wrote: > On Wed, 2019-04-10 at 12:54 -0700, Guenter Roeck wrote: > > A checkpatch rule suggesting that multiple extension lines can be merged > > might be useful to help finding such situations. Just a thought. > > You are welcome to try to write that one. > It's likely not a trivial task to make it sensible. > No chance - if _you_ think it is difficult, it would be absolutely hopeless for me, and I am not going to touch it. Cheers, Guenter
diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c index 9c943f1d57ec..cbb3c0dde136 100644 --- a/drivers/watchdog/mt7621_wdt.c +++ b/drivers/watchdog/mt7621_wdt.c @@ -133,18 +133,19 @@ static struct watchdog_device mt7621_wdt_dev = { static int mt7621_wdt_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; mt7621_wdt_base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(mt7621_wdt_base)) return PTR_ERR(mt7621_wdt_base); - mt7621_wdt_reset = devm_reset_control_get_exclusive(&pdev->dev, NULL); + mt7621_wdt_reset = devm_reset_control_get_exclusive(dev, NULL); if (!IS_ERR(mt7621_wdt_reset)) reset_control_deassert(mt7621_wdt_reset); mt7621_wdt_dev.bootstatus = mt7621_wdt_bootcause(); watchdog_init_timeout(&mt7621_wdt_dev, mt7621_wdt_dev.max_timeout, - &pdev->dev); + dev); watchdog_set_nowayout(&mt7621_wdt_dev, nowayout); if (mt7621_wdt_is_running(&mt7621_wdt_dev)) { /* @@ -161,7 +162,7 @@ static int mt7621_wdt_probe(struct platform_device *pdev) set_bit(WDOG_HW_RUNNING, &mt7621_wdt_dev.status); } - return devm_watchdog_register_device(&pdev->dev, &mt7621_wdt_dev); + return devm_watchdog_register_device(dev, &mt7621_wdt_dev); } static void mt7621_wdt_shutdown(struct platform_device *pdev)
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: Matthias Brugger <matthias.bgg@gmail.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/watchdog/mt7621_wdt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)