Coccinelle archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script
@ 2019-04-06  6:11 Himanshu Jha
  2019-04-06  6:31 ` Julia Lawall
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Himanshu Jha @ 2019-04-06  6:11 UTC (permalink / raw)
  To: yamada.masahiro, Julia.Lawall
  Cc: kernel-janitors, michal.lkml, gregkh, linus.walleij,
	nicolas.palix, linux-kernel, bgolaszewski, andriy.shevchenko,
	cocci

Use recently introduced devm_platform_ioremap_resource
helper which wraps platform_get_resource() and
devm_ioremap_resource() together. This helps produce much
cleaner code while removing local `struct resource` declaration.

Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---

Tree wide changes has been tested through 0-day test service
with build success.

BUILD SUCCESS 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0
tree/branch: https://github.com/himanshujha199640/linux-next  20190401-devm_platform_ioremap_resource-final
branch HEAD: 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0  Coccinelle: api: Add devm_platform_ioremap_resource.cocci

elapsed time: 385m
configs tested: 162


Stats:
916 files changed, 1028 insertions(+), 2921 deletions(-)

Note: cases where the `struct resource *res` variable is
used subsequently in the function have been ignored out because
those cases produce:

eg., drivers/bus/da8xx-mstpri.c

warning: 'res' may be used uninitialized in this function [-Wmaybe-uninitialized]

due to: 
	if (prio_descr->reg + sizeof(u32) > resource_size(res)) {

which seems correct as `res` isn't initialized in the scope of
the function(da8xx_mstpri_probe) and instead initialized inside:

   void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
                                                unsigned int index)
   {
           struct resource *res;
   
           res = platform_get_resource(pdev, IORESOURCE_MEM, index);
           return devm_ioremap_resource(&pdev->dev, res);
   }
   EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);


 .../api/devm_platform_ioremap_resource.cocci  | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci

diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
new file mode 100644
index 000000000000..a28274af14df
--- /dev/null
+++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
@@ -0,0 +1,63 @@
+/// Use devm_platform_ioremap_resource helper which wraps
+/// platform_get_resource() and devm_ioremap_resource() together.
+///
+// Confidence: High
+// Copyright: (C) 2019 Himanshu Jha GPLv2.
+// Copyright: (C) 2019 Julia Lawall, Inria/LIP6. GPLv2.
+// Keywords: platform_get_resource, devm_ioremap_resource,
+// Keywords: devm_platform_ioremap_resource
+
+virtual patch
+virtual report
+
+@r depends on patch && !report@
+expression e1, e2, arg1, arg2, arg3, arg4;
+identifier id;
+@@
+
+(
+- id = platform_get_resource(arg1, arg2, arg3);
+|
+- struct resource *id = platform_get_resource(arg1, arg2, arg3);
+)
+  ... when != id
+- e1 = devm_ioremap_resource(arg4, id);
++ e1 = devm_platform_ioremap_resource(arg1, arg3);
+  ... when != id
+? id = e2
+
+@r1 depends on patch && !report@
+identifier r.id;
+type T;
+@@
+
+- T *id;
+  ...when != id
+
+// ----------------------------------------------------------------------------
+
+@r2 depends on report && !patch@
+identifier id;
+expression e1, e2, arg1, arg2, arg3, arg4;
+position j0;
+@@
+
+(
+  id = platform_get_resource(arg1, arg2, arg3);
+|
+  struct resource *id = platform_get_resource(arg1, arg2, arg3);
+)
+  ... when != id
+  e1@j0 = devm_ioremap_resource(arg4, id);
+  ... when != id
+? id = e2
+
+// ----------------------------------------------------------------------------
+
+@script:python depends on report && !patch@
+e1 << r2.e1;
+j0 << r2.j0;
+@@
+
+msg = "WARNING: Use devm_platform_ioremap_resource for %s" % (e1)
+coccilib.report.print_report(j0[0], msg)
-- 
2.17.1

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script
  2019-04-06  6:11 [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script Himanshu Jha
@ 2019-04-06  6:31 ` Julia Lawall
  2019-04-06  6:34   ` Julia Lawall
  2019-04-06 12:36 ` [Cocci] [PATCH] coccinelle: " Markus Elfring
  2019-06-08 15:24 ` [Cocci] Coccinelle: " Markus Elfring
  2 siblings, 1 reply; 24+ messages in thread
From: Julia Lawall @ 2019-04-06  6:31 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: michal.lkml, kernel-janitors, gregkh, linus.walleij,
	nicolas.palix, linux-kernel, bgolaszewski, andriy.shevchenko,
	cocci



On Sat, 6 Apr 2019, Himanshu Jha wrote:

> Use recently introduced devm_platform_ioremap_resource
> helper which wraps platform_get_resource() and
> devm_ioremap_resource() together. This helps produce much
> cleaner code while removing local `struct resource` declaration.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

Thanks for taking up this issue.

julia

> ---
>
> Tree wide changes has been tested through 0-day test service
> with build success.
>
> BUILD SUCCESS 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0
> tree/branch: https://github.com/himanshujha199640/linux-next  20190401-devm_platform_ioremap_resource-final
> branch HEAD: 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0  Coccinelle: api: Add devm_platform_ioremap_resource.cocci
>
> elapsed time: 385m
> configs tested: 162
>
>
> Stats:
> 916 files changed, 1028 insertions(+), 2921 deletions(-)
>
> Note: cases where the `struct resource *res` variable is
> used subsequently in the function have been ignored out because
> those cases produce:
>
> eg., drivers/bus/da8xx-mstpri.c
>
> warning: 'res' may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> due to:
> 	if (prio_descr->reg + sizeof(u32) > resource_size(res)) {
>
> which seems correct as `res` isn't initialized in the scope of
> the function(da8xx_mstpri_probe) and instead initialized inside:
>
>    void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
>                                                 unsigned int index)
>    {
>            struct resource *res;
>
>            res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>            return devm_ioremap_resource(&pdev->dev, res);
>    }
>    EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
>
>
>  .../api/devm_platform_ioremap_resource.cocci  | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..a28274af14df
> --- /dev/null
> +++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> @@ -0,0 +1,63 @@
> +/// Use devm_platform_ioremap_resource helper which wraps
> +/// platform_get_resource() and devm_ioremap_resource() together.
> +///
> +// Confidence: High
> +// Copyright: (C) 2019 Himanshu Jha GPLv2.
> +// Copyright: (C) 2019 Julia Lawall, Inria/LIP6. GPLv2.
> +// Keywords: platform_get_resource, devm_ioremap_resource,
> +// Keywords: devm_platform_ioremap_resource
> +
> +virtual patch
> +virtual report
> +
> +@r depends on patch && !report@
> +expression e1, e2, arg1, arg2, arg3, arg4;
> +identifier id;
> +@@
> +
> +(
> +- id = platform_get_resource(arg1, arg2, arg3);
> +|
> +- struct resource *id = platform_get_resource(arg1, arg2, arg3);
> +)
> +  ... when != id
> +- e1 = devm_ioremap_resource(arg4, id);
> ++ e1 = devm_platform_ioremap_resource(arg1, arg3);
> +  ... when != id
> +? id = e2
> +
> +@r1 depends on patch && !report@
> +identifier r.id;
> +type T;
> +@@
> +
> +- T *id;
> +  ...when != id
> +
> +// ----------------------------------------------------------------------------
> +
> +@r2 depends on report && !patch@
> +identifier id;
> +expression e1, e2, arg1, arg2, arg3, arg4;
> +position j0;
> +@@
> +
> +(
> +  id = platform_get_resource(arg1, arg2, arg3);
> +|
> +  struct resource *id = platform_get_resource(arg1, arg2, arg3);
> +)
> +  ... when != id
> +  e1@j0 = devm_ioremap_resource(arg4, id);
> +  ... when != id
> +? id = e2
> +
> +// ----------------------------------------------------------------------------
> +
> +@script:python depends on report && !patch@
> +e1 << r2.e1;
> +j0 << r2.j0;
> +@@
> +
> +msg = "WARNING: Use devm_platform_ioremap_resource for %s" % (e1)
> +coccilib.report.print_report(j0[0], msg)
> --
> 2.17.1
>
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script
  2019-04-06  6:31 ` Julia Lawall
@ 2019-04-06  6:34   ` Julia Lawall
  2019-07-06 12:16     ` Masahiro Yamada
  0 siblings, 1 reply; 24+ messages in thread
From: Julia Lawall @ 2019-04-06  6:34 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: kernel-janitors, michal.lkml, gregkh, linus.walleij,
	nicolas.palix, linux-kernel, bgolaszewski, andriy.shevchenko,
	cocci



On Sat, 6 Apr 2019, Julia Lawall wrote:

>
>
> On Sat, 6 Apr 2019, Himanshu Jha wrote:
>
> > Use recently introduced devm_platform_ioremap_resource
> > helper which wraps platform_get_resource() and
> > devm_ioremap_resource() together. This helps produce much
> > cleaner code while removing local `struct resource` declaration.
> >
> > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
>
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>
> Thanks for taking up this issue.

Maybe this should be

Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>

since I contributed two lines to the script :)

julia

>
> julia
>
> > ---
> >
> > Tree wide changes has been tested through 0-day test service
> > with build success.
> >
> > BUILD SUCCESS 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0
> > tree/branch: https://github.com/himanshujha199640/linux-next  20190401-devm_platform_ioremap_resource-final
> > branch HEAD: 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0  Coccinelle: api: Add devm_platform_ioremap_resource.cocci
> >
> > elapsed time: 385m
> > configs tested: 162
> >
> >
> > Stats:
> > 916 files changed, 1028 insertions(+), 2921 deletions(-)
> >
> > Note: cases where the `struct resource *res` variable is
> > used subsequently in the function have been ignored out because
> > those cases produce:
> >
> > eg., drivers/bus/da8xx-mstpri.c
> >
> > warning: 'res' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >
> > due to:
> > 	if (prio_descr->reg + sizeof(u32) > resource_size(res)) {
> >
> > which seems correct as `res` isn't initialized in the scope of
> > the function(da8xx_mstpri_probe) and instead initialized inside:
> >
> >    void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> >                                                 unsigned int index)
> >    {
> >            struct resource *res;
> >
> >            res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> >            return devm_ioremap_resource(&pdev->dev, res);
> >    }
> >    EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> >
> >
> >  .../api/devm_platform_ioremap_resource.cocci  | 63 +++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >  create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> >
> > diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > new file mode 100644
> > index 000000000000..a28274af14df
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > @@ -0,0 +1,63 @@
> > +/// Use devm_platform_ioremap_resource helper which wraps
> > +/// platform_get_resource() and devm_ioremap_resource() together.
> > +///
> > +// Confidence: High
> > +// Copyright: (C) 2019 Himanshu Jha GPLv2.
> > +// Copyright: (C) 2019 Julia Lawall, Inria/LIP6. GPLv2.
> > +// Keywords: platform_get_resource, devm_ioremap_resource,
> > +// Keywords: devm_platform_ioremap_resource
> > +
> > +virtual patch
> > +virtual report
> > +
> > +@r depends on patch && !report@
> > +expression e1, e2, arg1, arg2, arg3, arg4;
> > +identifier id;
> > +@@
> > +
> > +(
> > +- id = platform_get_resource(arg1, arg2, arg3);
> > +|
> > +- struct resource *id = platform_get_resource(arg1, arg2, arg3);
> > +)
> > +  ... when != id
> > +- e1 = devm_ioremap_resource(arg4, id);
> > ++ e1 = devm_platform_ioremap_resource(arg1, arg3);
> > +  ... when != id
> > +? id = e2
> > +
> > +@r1 depends on patch && !report@
> > +identifier r.id;
> > +type T;
> > +@@
> > +
> > +- T *id;
> > +  ...when != id
> > +
> > +// ----------------------------------------------------------------------------
> > +
> > +@r2 depends on report && !patch@
> > +identifier id;
> > +expression e1, e2, arg1, arg2, arg3, arg4;
> > +position j0;
> > +@@
> > +
> > +(
> > +  id = platform_get_resource(arg1, arg2, arg3);
> > +|
> > +  struct resource *id = platform_get_resource(arg1, arg2, arg3);
> > +)
> > +  ... when != id
> > +  e1@j0 = devm_ioremap_resource(arg4, id);
> > +  ... when != id
> > +? id = e2
> > +
> > +// ----------------------------------------------------------------------------
> > +
> > +@script:python depends on report && !patch@
> > +e1 << r2.e1;
> > +j0 << r2.j0;
> > +@@
> > +
> > +msg = "WARNING: Use devm_platform_ioremap_resource for %s" % (e1)
> > +coccilib.report.print_report(j0[0], msg)
> > --
> > 2.17.1
> >
> >
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script
  2019-04-06  6:11 [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script Himanshu Jha
  2019-04-06  6:31 ` Julia Lawall
@ 2019-04-06 12:36 ` " Markus Elfring
  2019-06-08 15:24 ` [Cocci] Coccinelle: " Markus Elfring
  2 siblings, 0 replies; 24+ messages in thread
From: Markus Elfring @ 2019-04-06 12:36 UTC (permalink / raw)
  To: Himanshu Jha, Julia Lawall
  Cc: Nicolas Palix, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci

> +// Copyright: (C) 2019 Himanshu Jha GPLv2.

How do you think about to use a SPDX identifier?


> +// ---…

I would prefer a SmPL script without such comment lines as delimiters here.


> +position j0;

Would the variable name “p” be nicer?


> +@script:python depends on report && !patch@
> +e1 << r2.e1;
> +j0 << r2.j0;
> +@@
> +
> +msg = "WARNING: Use devm_platform_ioremap_resource for %s" % (e1)
> +coccilib.report.print_report(j0[0], msg)

I suggest to print such a message without the extra variable “msg”
because the string format expression can be passed directly.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-04-06  6:11 [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script Himanshu Jha
  2019-04-06  6:31 ` Julia Lawall
  2019-04-06 12:36 ` [Cocci] [PATCH] coccinelle: " Markus Elfring
@ 2019-06-08 15:24 ` " Markus Elfring
  2019-06-08 17:26   ` Julia Lawall
  2 siblings, 1 reply; 24+ messages in thread
From: Markus Elfring @ 2019-06-08 15:24 UTC (permalink / raw)
  To: Himanshu Jha, Julia Lawall
  Cc: Nicolas Palix, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci

> +- e1 = devm_ioremap_resource(arg4, id);
> ++ e1 = devm_platform_ioremap_resource(arg1, arg3);

Can the following specification variant matter for the shown SmPL
change approach?

+ e1 =
+-     devm_ioremap_resource(arg4, id
++     devm_platform_ioremap_resource(arg1, arg3
+                           );


Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-06-08 15:24 ` [Cocci] Coccinelle: " Markus Elfring
@ 2019-06-08 17:26   ` Julia Lawall
  2019-06-09  8:55     ` Markus Elfring
  0 siblings, 1 reply; 24+ messages in thread
From: Julia Lawall @ 2019-06-08 17:26 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Nicolas Palix, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci



On Sat, 8 Jun 2019, Markus Elfring wrote:

> > +- e1 = devm_ioremap_resource(arg4, id);
> > ++ e1 = devm_platform_ioremap_resource(arg1, arg3);
>
> Can the following specification variant matter for the shown SmPL
> change approach?
>
> + e1 =
> +-     devm_ioremap_resource(arg4, id
> ++     devm_platform_ioremap_resource(arg1, arg3
> +                           );

In the latter case, the original formatting of e1 will be preserved.  But
there is not usually any interesting formatting on the left side of an
assignment (ie typically no newlines or comments).  I can see no purpose
to factorizing the right parenthesis.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-06-08 17:26   ` Julia Lawall
@ 2019-06-09  8:55     ` Markus Elfring
  2019-06-11 20:40       ` Enrico Weigelt, metux IT consult
  0 siblings, 1 reply; 24+ messages in thread
From: Markus Elfring @ 2019-06-09  8:55 UTC (permalink / raw)
  To: Julia Lawall, Himanshu Jha
  Cc: Nicolas Palix, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci

>>> +- e1 = devm_ioremap_resource(arg4, id);
>>> ++ e1 = devm_platform_ioremap_resource(arg1, arg3);
>>
>> Can the following specification variant matter for the shown SmPL
>> change approach?
>>
>> + e1 =
>> +-     devm_ioremap_resource(arg4, id
>> ++     devm_platform_ioremap_resource(arg1, arg3
>> +                           );
>
> In the latter case, the original formatting of e1 will be preserved.

I would like to point the possibility out to express only required changes
also by SmPL specifications.


> But there is not usually any interesting formatting on the left side of an
> assignment (ie typically no newlines or comments).

Is there any need to trigger additional source code reformatting?


> I can see no purpose to factorizing the right parenthesis.

These characters at the end of such a function call should be kept unchanged.


I got another software development concern according to the discussed
software update “drivers: provide devm_platform_ioremap_resource()”
(from 2019-02-21).
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/base/platform.c?id=7945f929f1a77a1c8887a97ca07f87626858ff42

The flag “IORESOURCE_MEM” is passed as the second parameter for the call
of the function “platform_get_resource” in this refactoring.
Should this detail be specified also in the proposed script for the
semantic patch language instead of using the metavariable “arg2”
in SmPL disjunctions?

How do you think about to delete error detection and corresponding
exception handling code for the previous function call?


Is the SmPL code specification “when != id” really sufficient for
the exclusion of variable reassignments here?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-06-09  8:55     ` Markus Elfring
@ 2019-06-11 20:40       ` Enrico Weigelt, metux IT consult
  2019-06-12  5:28         ` Julia Lawall
                           ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2019-06-11 20:40 UTC (permalink / raw)
  To: Markus Elfring, Julia Lawall, Himanshu Jha
  Cc: Nicolas Palix, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci

On 09.06.19 10:55, Markus Elfring wrote:

<snip>

>> But there is not usually any interesting formatting on the left side of an
>> assignment (ie typically no newlines or comments).
> 
> Is there any need to trigger additional source code reformatting?
> 
>> I can see no purpose to factorizing the right parenthesis.
> 
> These characters at the end of such a function call should be kept unchanged.

Agreed. OTOH, we all know that spatch results still need to be carefully
checked. I suspect trying to teach it all the formatting rules of the
kernel isn't an easy task.

> The flag “IORESOURCE_MEM” is passed as the second parameter for the call
> of the function “platform_get_resource” in this refactoring.

In that particular case, we maybe should consider separate inline
helpers instead of passing this is a parameter.

Maybe it would even be more efficient to have completely separate
versions of devm_platform_ioremap_resource(), so we don't even have
to pass that parameter on stack.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-06-11 20:40       ` Enrico Weigelt, metux IT consult
@ 2019-06-12  5:28         ` Julia Lawall
  2019-06-12  6:24         ` Markus Elfring
  2019-06-14  9:22         ` [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions Markus Elfring
  2 siblings, 0 replies; 24+ messages in thread
From: Julia Lawall @ 2019-06-12  5:28 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Nicolas Palix, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Markus Elfring, Andy Shevchenko, cocci

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]



On Tue, 11 Jun 2019, Enrico Weigelt, metux IT consult wrote:

> On 09.06.19 10:55, Markus Elfring wrote:
>
> <snip>
>
> >> But there is not usually any interesting formatting on the left side of an
> >> assignment (ie typically no newlines or comments).
> >
> > Is there any need to trigger additional source code reformatting?
> >
> >> I can see no purpose to factorizing the right parenthesis.
> >
> > These characters at the end of such a function call should be kept unchanged.
>
> Agreed. OTOH, we all know that spatch results still need to be carefully
> checked. I suspect trying to teach it all the formatting rules of the
> kernel isn't an easy task.
>
> > The flag “IORESOURCE_MEM” is passed as the second parameter for the call
> > of the function “platform_get_resource” in this refactoring.
>
> In that particular case, we maybe should consider separate inline
> helpers instead of passing this is a parameter.
>
> Maybe it would even be more efficient to have completely separate
> versions of devm_platform_ioremap_resource(), so we don't even have
> to pass that parameter on stack.

I'm lost as to why this discussion suddenly appeared.  What problem is
actually being discussed?

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-06-11 20:40       ` Enrico Weigelt, metux IT consult
  2019-06-12  5:28         ` Julia Lawall
@ 2019-06-12  6:24         ` Markus Elfring
  2019-06-14  9:22         ` [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions Markus Elfring
  2 siblings, 0 replies; 24+ messages in thread
From: Markus Elfring @ 2019-06-12  6:24 UTC (permalink / raw)
  To: Enrico Weigelt
  Cc: Nicolas Palix, Michal Marek, Bartosz Golaszewski,
	Greg Kroah-Hartman, Linus Walleij, kernel-janitors, linux-kernel,
	Andy Shevchenko, cocci

>> The flag “IORESOURCE_MEM” is passed as the second parameter for the call
>> of the function “platform_get_resource” in this refactoring.
>
> In that particular case, we maybe should consider separate inline
> helpers instead of passing this is a parameter.
>
> Maybe it would even be more efficient to have completely separate
> versions of devm_platform_ioremap_resource(), so we don't even have
> to pass that parameter on stack.

Would you like to add another function which should be called instead of
the combination of the functions “platform_get_resource” and “devm_ioremap_resource”?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-11 20:40       ` Enrico Weigelt, metux IT consult
  2019-06-12  5:28         ` Julia Lawall
  2019-06-12  6:24         ` Markus Elfring
@ 2019-06-14  9:22         ` Markus Elfring
  2019-06-14  9:27           ` Julia Lawall
                             ` (2 more replies)
  2 siblings, 3 replies; 24+ messages in thread
From: Markus Elfring @ 2019-06-14  9:22 UTC (permalink / raw)
  To: Enrico Weigelt, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: Michal Marek, Nicolas Palix, Linus Walleij, kernel-janitors,
	linux-kernel, Bartosz Golaszewski, Andy Shevchenko, cocci

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 14 Jun 2019 11:05:33 +0200

Two function calls were combined in this function implementation.
Inline corresponding code so that extra error checks can be avoided here.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/base/platform.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4d1729853d1a..baadca72f949 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -80,8 +80,8 @@ struct resource *platform_get_resource(struct platform_device *dev,
 EXPORT_SYMBOL_GPL(platform_get_resource);

 /**
- * devm_platform_ioremap_resource - call devm_ioremap_resource() for a platform
- *				    device
+ * devm_platform_ioremap_resource
+ * Achieve devm_ioremap_resource() functionality for a platform device
  *
  * @pdev: platform device to use both for memory resource lookup as well as
  *        resource management
@@ -91,10 +91,39 @@ EXPORT_SYMBOL_GPL(platform_get_resource);
 void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
 					     unsigned int index)
 {
-	struct resource *res;
+	u32 i;

-	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
-	return devm_ioremap_resource(&pdev->dev, res);
+	for (i = 0; i < pdev->num_resources; i++) {
+		struct resource *res = &pdev->resource[i];
+
+		if (resource_type(res) == IORESOURCE_MEM && index-- == 0) {
+			struct device *dev = &pdev->dev;
+			resource_size_t size = resource_size(res);
+			void __iomem *dest;
+
+			if (!devm_request_mem_region(dev,
+						     res->start,
+						     size,
+						     dev_name(dev))) {
+				dev_err(dev,
+					"can't request region for resource %pR\n",
+					res);
+				return IOMEM_ERR_PTR(-EBUSY);
+			}
+
+			dest = devm_ioremap(dev, res->start, size);
+			if (!dest) {
+				dev_err(dev,
+					"ioremap failed for resource %pR\n",
+					res);
+				devm_release_mem_region(dev, res->start, size);
+				dest = IOMEM_ERR_PTR(-ENOMEM);
+			}
+
+			return dest;
+		}
+	}
+	return IOMEM_ERR_PTR(-EINVAL);
 }
 EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
 #endif /* CONFIG_HAS_IOMEM */
--
2.22.0

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-14  9:22         ` [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions Markus Elfring
@ 2019-06-14  9:27           ` Julia Lawall
  2019-06-15 11:00             ` [Cocci] " Markus Elfring
  2019-06-14 10:07           ` [Cocci] [PATCH] " Greg Kroah-Hartman
  2019-06-17 20:21           ` Enrico Weigelt, metux IT consult
  2 siblings, 1 reply; 24+ messages in thread
From: Julia Lawall @ 2019-06-14  9:27 UTC (permalink / raw)
  To: Markus Elfring
  Cc: kernel-janitors, Michal Marek, Rafael J. Wysocki,
	Greg Kroah-Hartman, Linus Walleij, Nicolas Palix, linux-kernel,
	Bartosz Golaszewski, Andy Shevchenko, cocci



On Fri, 14 Jun 2019, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 14 Jun 2019 11:05:33 +0200
>
> Two function calls were combined in this function implementation.
> Inline corresponding code so that extra error checks can be avoided here.

I don't see any point to this at all.  By inlining the code, you have
created a clone, which will introduce extra work to maintain in the
future.

julia


>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/base/platform.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 4d1729853d1a..baadca72f949 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -80,8 +80,8 @@ struct resource *platform_get_resource(struct platform_device *dev,
>  EXPORT_SYMBOL_GPL(platform_get_resource);
>
>  /**
> - * devm_platform_ioremap_resource - call devm_ioremap_resource() for a platform
> - *				    device
> + * devm_platform_ioremap_resource
> + * Achieve devm_ioremap_resource() functionality for a platform device
>   *
>   * @pdev: platform device to use both for memory resource lookup as well as
>   *        resource management
> @@ -91,10 +91,39 @@ EXPORT_SYMBOL_GPL(platform_get_resource);
>  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
>  					     unsigned int index)
>  {
> -	struct resource *res;
> +	u32 i;
>
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> -	return devm_ioremap_resource(&pdev->dev, res);
> +	for (i = 0; i < pdev->num_resources; i++) {
> +		struct resource *res = &pdev->resource[i];
> +
> +		if (resource_type(res) == IORESOURCE_MEM && index-- == 0) {
> +			struct device *dev = &pdev->dev;
> +			resource_size_t size = resource_size(res);
> +			void __iomem *dest;
> +
> +			if (!devm_request_mem_region(dev,
> +						     res->start,
> +						     size,
> +						     dev_name(dev))) {
> +				dev_err(dev,
> +					"can't request region for resource %pR\n",
> +					res);
> +				return IOMEM_ERR_PTR(-EBUSY);
> +			}
> +
> +			dest = devm_ioremap(dev, res->start, size);
> +			if (!dest) {
> +				dev_err(dev,
> +					"ioremap failed for resource %pR\n",
> +					res);
> +				devm_release_mem_region(dev, res->start, size);
> +				dest = IOMEM_ERR_PTR(-ENOMEM);
> +			}
> +
> +			return dest;
> +		}
> +	}
> +	return IOMEM_ERR_PTR(-EINVAL);
>  }
>  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
>  #endif /* CONFIG_HAS_IOMEM */
> --
> 2.22.0
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-14  9:22         ` [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions Markus Elfring
  2019-06-14  9:27           ` Julia Lawall
@ 2019-06-14 10:07           ` " Greg Kroah-Hartman
  2019-06-17 20:21           ` Enrico Weigelt, metux IT consult
  2 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-14 10:07 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Michal Marek, Rafael J. Wysocki, Nicolas Palix, Linus Walleij,
	kernel-janitors, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci

On Fri, Jun 14, 2019 at 11:22:40AM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 14 Jun 2019 11:05:33 +0200
> 
> Two function calls were combined in this function implementation.
> Inline corresponding code so that extra error checks can be avoided here.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/base/platform.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)

Hey, looks like you timed out from my kill-file and this snuck through
somehow.  Let me go add you again to it, so I'm not bothered by
pointless stuff like this anymore.

*plonk*

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-14  9:27           ` Julia Lawall
@ 2019-06-15 11:00             ` " Markus Elfring
  0 siblings, 0 replies; 24+ messages in thread
From: Markus Elfring @ 2019-06-15 11:00 UTC (permalink / raw)
  To: Julia Lawall, kernel-janitors
  Cc: Michal Marek, Rafael J. Wysocki, Greg Kroah-Hartman,
	Linus Walleij, Nicolas Palix, linux-kernel, Bartosz Golaszewski,
	Andy Shevchenko, cocci

>> Two function calls were combined in this function implementation.
>> Inline corresponding code so that extra error checks can be avoided here.
>
> I don't see any point to this at all.

Would you like to take another look at corresponding design options?

How do you think about to check run time characteristics any more?


> By inlining the code, you have created a clone,
> which will introduce extra work to maintain in the future.

Would you find the shown software transformation acceptable
if a C compiler will be able to generate a similar code structure?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-14  9:22         ` [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions Markus Elfring
  2019-06-14  9:27           ` Julia Lawall
  2019-06-14 10:07           ` [Cocci] [PATCH] " Greg Kroah-Hartman
@ 2019-06-17 20:21           ` Enrico Weigelt, metux IT consult
  2019-06-18  5:37             ` [Cocci] " Markus Elfring
  2 siblings, 1 reply; 24+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2019-06-17 20:21 UTC (permalink / raw)
  To: Markus Elfring, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: Michal Marek, Nicolas Palix, Linus Walleij, kernel-janitors,
	linux-kernel, Bartosz Golaszewski, Andy Shevchenko, cocci

On 14.06.19 11:22, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 14 Jun 2019 11:05:33 +0200
> 
> Two function calls were combined in this function implementation.
> Inline corresponding code so that extra error checks can be avoided here.

What exactly is the purpose of this ?

Looks like a notable code duplication ... I thought we usually try to
reduce this, instead of introducing new ones.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-17 20:21           ` Enrico Weigelt, metux IT consult
@ 2019-06-18  5:37             ` " Markus Elfring
  2019-06-18  9:35               ` Enrico Weigelt, metux IT consult
  0 siblings, 1 reply; 24+ messages in thread
From: Markus Elfring @ 2019-06-18  5:37 UTC (permalink / raw)
  To: Enrico Weigelt, Greg Kroah-Hartman, Rafael J. Wysocki,
	Bartosz Golaszewski
  Cc: Michal Marek, Nicolas Palix, Linus Walleij, kernel-janitors,
	linux-kernel, Andy Shevchenko, cocci

>> Two function calls were combined in this function implementation.
>> Inline corresponding code so that extra error checks can be avoided here.
>
> What exactly is the purpose of this ?

I suggest to take another look at the need and relevance of involved
error checks in the discussed function combination.


> Looks like a notable code duplication ...

This can be.


> I thought we usually try to reduce this, instead of introducing new ones.

Would you like to check the software circumstances once more
for the generation of a similar code structure by a C compiler
(or optimiser)?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-18  5:37             ` [Cocci] " Markus Elfring
@ 2019-06-18  9:35               ` Enrico Weigelt, metux IT consult
  2019-06-18 10:21                 ` Dan Carpenter
  2019-06-18 11:44                 ` Markus Elfring
  0 siblings, 2 replies; 24+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2019-06-18  9:35 UTC (permalink / raw)
  To: Markus Elfring, Greg Kroah-Hartman, Rafael J. Wysocki,
	Bartosz Golaszewski
  Cc: Michal Marek, Nicolas Palix, Linus Walleij, kernel-janitors,
	linux-kernel, Andy Shevchenko, cocci

On 18.06.19 07:37, Markus Elfring wrote:
>>> Two function calls were combined in this function implementation.
>>> Inline corresponding code so that extra error checks can be avoided here.
>>
>> What exactly is the purpose of this ?
> 
> I suggest to take another look at the need and relevance of involved
> error checks in the discussed function combination.

Sorry, don't have the time for guessing and trying to reproduce your
thoughts. That's why we have patch descriptions / commit messages.
It would be a lot easier for all of us if you just desribe the exact
problem you'd like to solve and your approach to do so.

>> Looks like a notable code duplication ...
> 
> This can be.

I doubt that code duplication is appreciated, as this increases the
maintenance overhead. (actually, we're usually trying to reduce that,
eg. by using lots of generic helpers).

>> I thought we usually try to reduce this, instead of introducing new ones.
> 
> Would you like to check the software circumstances once more
> for the generation of a similar code structure by a C compiler
> (or optimiser)?

As said: unfortunately, I don't have the time to do that - you'd have to
tell us, what exactly you've got in mind.

If it's just about some error checks which happen to be redundant in a
particular case, you'll have to show that this case is a *really* hot
path (eg. irq, syscall, scheduling, etc) - but I don't see that here.

What's the exact scenario you're trying to optimize ? Any actual
measurements on how your patch improves that ?


Look, I understand that you'd like to squeeze out maximum performance,
but this has to be practically maintainable. I could list a lot of
things that I don't need in particular use cases and would like to
introduce build knobs for, but I have to understand that maintainers
have to be pretty reluctant towards those things.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-18  9:35               ` Enrico Weigelt, metux IT consult
@ 2019-06-18 10:21                 ` Dan Carpenter
  2019-06-18 11:44                 ` Markus Elfring
  1 sibling, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2019-06-18 10:21 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Nicolas Palix, Michal Marek, Rafael J. Wysocki,
	Greg Kroah-Hartman, Linus Walleij, kernel-janitors, linux-kernel,
	Bartosz Golaszewski, Markus Elfring, Andy Shevchenko, cocci

Greg already commented on this thread.  No need to discuss it further.

regards,
dan carpenter
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] drivers: Inline code in devm_platform_ioremap_resource() from two functions
  2019-06-18  9:35               ` Enrico Weigelt, metux IT consult
  2019-06-18 10:21                 ` Dan Carpenter
@ 2019-06-18 11:44                 ` Markus Elfring
  1 sibling, 0 replies; 24+ messages in thread
From: Markus Elfring @ 2019-06-18 11:44 UTC (permalink / raw)
  To: Enrico Weigelt, Greg Kroah-Hartman, Rafael J. Wysocki,
	Bartosz Golaszewski
  Cc: Michal Marek, Nicolas Palix, Linus Walleij, kernel-janitors,
	linux-kernel, Andy Shevchenko, cocci

>> Would you like to check the software circumstances once more
>> for the generation of a similar code structure by a C compiler
>> (or optimiser)?
>
> As said: unfortunately, I don't have the time to do that

I became curious if you would like to adjust your software development
attention a bit more also in this area.


> - you'd have to tell us, what exactly you've got in mind.

I try to point possibilities out to improve the combination of
two functions.


> If it's just about some error checks which happen to be redundant in a
> particular case, you'll have to show that this case is a *really* hot
> path (eg. irq, syscall, scheduling, etc) - but I don't see that here.

1. May the check “resource_type(res) == IORESOURCE_MEM” be performed
   in a local loop?

2. How hot do you find the null pointer check for the device
   input parameter of the function “devm_ioremap_resource”?


> Any actual measurements on how your patch improves that ?

Not yet. - Which benchmarks would you trust here?


> Look, I understand that you'd like to squeeze out maximum performance,

I hope so.


> but this has to be practically maintainable.

This can be achieved if more contributors would find proposed
adjustments helpful for another software transformation.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script
  2019-04-06  6:34   ` Julia Lawall
@ 2019-07-06 12:16     ` Masahiro Yamada
  2019-07-06 13:39       ` Julia Lawall
  0 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2019-07-06 12:16 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	Nicolas Palix, Linux Kernel Mailing List, Bartosz Golaszewski,
	Andy Shevchenko, cocci

On Sat, Apr 6, 2019 at 3:34 PM Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>
> On Sat, 6 Apr 2019, Julia Lawall wrote:
>
> >
> >
> > On Sat, 6 Apr 2019, Himanshu Jha wrote:
> >
> > > Use recently introduced devm_platform_ioremap_resource
> > > helper which wraps platform_get_resource() and
> > > devm_ioremap_resource() together. This helps produce much
> > > cleaner code while removing local `struct resource` declaration.
> > >
> > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> >
> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> >
> > Thanks for taking up this issue.
>
> Maybe this should be
>
> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
>
> since I contributed two lines to the script :)

I will apply with Julia's Signed-off-by instead of Acked-by.
I will also add SPDX tag.

Is this OK?



> julia
>
> >
> > julia
> >
> > > ---
> > >
> > > Tree wide changes has been tested through 0-day test service
> > > with build success.
> > >
> > > BUILD SUCCESS 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0
> > > tree/branch: https://github.com/himanshujha199640/linux-next  20190401-devm_platform_ioremap_resource-final
> > > branch HEAD: 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0  Coccinelle: api: Add devm_platform_ioremap_resource.cocci
> > >
> > > elapsed time: 385m
> > > configs tested: 162
> > >
> > >
> > > Stats:
> > > 916 files changed, 1028 insertions(+), 2921 deletions(-)
> > >
> > > Note: cases where the `struct resource *res` variable is
> > > used subsequently in the function have been ignored out because
> > > those cases produce:
> > >
> > > eg., drivers/bus/da8xx-mstpri.c
> > >
> > > warning: 'res' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >
> > > due to:
> > >     if (prio_descr->reg + sizeof(u32) > resource_size(res)) {
> > >
> > > which seems correct as `res` isn't initialized in the scope of
> > > the function(da8xx_mstpri_probe) and instead initialized inside:
> > >
> > >    void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > >                                                 unsigned int index)
> > >    {
> > >            struct resource *res;
> > >
> > >            res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> > >            return devm_ioremap_resource(&pdev->dev, res);
> > >    }
> > >    EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > >
> > >
> > >  .../api/devm_platform_ioremap_resource.cocci  | 63 +++++++++++++++++++
> > >  1 file changed, 63 insertions(+)
> > >  create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > >
> > > diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > > new file mode 100644
> > > index 000000000000..a28274af14df
> > > --- /dev/null
> > > +++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > > @@ -0,0 +1,63 @@
> > > +/// Use devm_platform_ioremap_resource helper which wraps
> > > +/// platform_get_resource() and devm_ioremap_resource() together.
> > > +///
> > > +// Confidence: High
> > > +// Copyright: (C) 2019 Himanshu Jha GPLv2.
> > > +// Copyright: (C) 2019 Julia Lawall, Inria/LIP6. GPLv2.
> > > +// Keywords: platform_get_resource, devm_ioremap_resource,
> > > +// Keywords: devm_platform_ioremap_resource
> > > +
> > > +virtual patch
> > > +virtual report
> > > +
> > > +@r depends on patch && !report@
> > > +expression e1, e2, arg1, arg2, arg3, arg4;
> > > +identifier id;
> > > +@@
> > > +
> > > +(
> > > +- id = platform_get_resource(arg1, arg2, arg3);
> > > +|
> > > +- struct resource *id = platform_get_resource(arg1, arg2, arg3);
> > > +)
> > > +  ... when != id
> > > +- e1 = devm_ioremap_resource(arg4, id);
> > > ++ e1 = devm_platform_ioremap_resource(arg1, arg3);
> > > +  ... when != id
> > > +? id = e2
> > > +
> > > +@r1 depends on patch && !report@
> > > +identifier r.id;
> > > +type T;
> > > +@@
> > > +
> > > +- T *id;
> > > +  ...when != id
> > > +
> > > +// ----------------------------------------------------------------------------
> > > +
> > > +@r2 depends on report && !patch@
> > > +identifier id;
> > > +expression e1, e2, arg1, arg2, arg3, arg4;
> > > +position j0;
> > > +@@
> > > +
> > > +(
> > > +  id = platform_get_resource(arg1, arg2, arg3);
> > > +|
> > > +  struct resource *id = platform_get_resource(arg1, arg2, arg3);
> > > +)
> > > +  ... when != id
> > > +  e1@j0 = devm_ioremap_resource(arg4, id);
> > > +  ... when != id
> > > +? id = e2
> > > +
> > > +// ----------------------------------------------------------------------------
> > > +
> > > +@script:python depends on report && !patch@
> > > +e1 << r2.e1;
> > > +j0 << r2.j0;
> > > +@@
> > > +
> > > +msg = "WARNING: Use devm_platform_ioremap_resource for %s" % (e1)
> > > +coccilib.report.print_report(j0[0], msg)
> > > --
> > > 2.17.1
> > >
> > >
> >
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci



-- 
Best Regards
Masahiro Yamada
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script
  2019-07-06 12:16     ` Masahiro Yamada
@ 2019-07-06 13:39       ` Julia Lawall
  2019-07-07  9:55         ` [Cocci] Coccinelle: " Markus Elfring
  0 siblings, 1 reply; 24+ messages in thread
From: Julia Lawall @ 2019-07-06 13:39 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: kernel-janitors, Michal Marek, Greg Kroah-Hartman, Linus Walleij,
	Nicolas Palix, Linux Kernel Mailing List, Bartosz Golaszewski,
	Andy Shevchenko, cocci



On Sat, 6 Jul 2019, Masahiro Yamada wrote:

> On Sat, Apr 6, 2019 at 3:34 PM Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> >
> > On Sat, 6 Apr 2019, Julia Lawall wrote:
> >
> > >
> > >
> > > On Sat, 6 Apr 2019, Himanshu Jha wrote:
> > >
> > > > Use recently introduced devm_platform_ioremap_resource
> > > > helper which wraps platform_get_resource() and
> > > > devm_ioremap_resource() together. This helps produce much
> > > > cleaner code while removing local `struct resource` declaration.
> > > >
> > > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > >
> > > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> > >
> > > Thanks for taking up this issue.
> >
> > Maybe this should be
> >
> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> >
> > since I contributed two lines to the script :)
>
> I will apply with Julia's Signed-off-by instead of Acked-by.
> I will also add SPDX tag.
>
> Is this OK?

Yes, thanks.

julia

>
>
>
> > julia
> >
> > >
> > > julia
> > >
> > > > ---
> > > >
> > > > Tree wide changes has been tested through 0-day test service
> > > > with build success.
> > > >
> > > > BUILD SUCCESS 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0
> > > > tree/branch: https://github.com/himanshujha199640/linux-next  20190401-devm_platform_ioremap_resource-final
> > > > branch HEAD: 74ebaaca5d14d3d9b03e911f0b4995b78a4d60f0  Coccinelle: api: Add devm_platform_ioremap_resource.cocci
> > > >
> > > > elapsed time: 385m
> > > > configs tested: 162
> > > >
> > > >
> > > > Stats:
> > > > 916 files changed, 1028 insertions(+), 2921 deletions(-)
> > > >
> > > > Note: cases where the `struct resource *res` variable is
> > > > used subsequently in the function have been ignored out because
> > > > those cases produce:
> > > >
> > > > eg., drivers/bus/da8xx-mstpri.c
> > > >
> > > > warning: 'res' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >
> > > > due to:
> > > >     if (prio_descr->reg + sizeof(u32) > resource_size(res)) {
> > > >
> > > > which seems correct as `res` isn't initialized in the scope of
> > > > the function(da8xx_mstpri_probe) and instead initialized inside:
> > > >
> > > >    void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > > >                                                 unsigned int index)
> > > >    {
> > > >            struct resource *res;
> > > >
> > > >            res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> > > >            return devm_ioremap_resource(&pdev->dev, res);
> > > >    }
> > > >    EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > > >
> > > >
> > > >  .../api/devm_platform_ioremap_resource.cocci  | 63 +++++++++++++++++++
> > > >  1 file changed, 63 insertions(+)
> > > >  create mode 100644 scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > > >
> > > > diff --git a/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > > > new file mode 100644
> > > > index 000000000000..a28274af14df
> > > > --- /dev/null
> > > > +++ b/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci
> > > > @@ -0,0 +1,63 @@
> > > > +/// Use devm_platform_ioremap_resource helper which wraps
> > > > +/// platform_get_resource() and devm_ioremap_resource() together.
> > > > +///
> > > > +// Confidence: High
> > > > +// Copyright: (C) 2019 Himanshu Jha GPLv2.
> > > > +// Copyright: (C) 2019 Julia Lawall, Inria/LIP6. GPLv2.
> > > > +// Keywords: platform_get_resource, devm_ioremap_resource,
> > > > +// Keywords: devm_platform_ioremap_resource
> > > > +
> > > > +virtual patch
> > > > +virtual report
> > > > +
> > > > +@r depends on patch && !report@
> > > > +expression e1, e2, arg1, arg2, arg3, arg4;
> > > > +identifier id;
> > > > +@@
> > > > +
> > > > +(
> > > > +- id = platform_get_resource(arg1, arg2, arg3);
> > > > +|
> > > > +- struct resource *id = platform_get_resource(arg1, arg2, arg3);
> > > > +)
> > > > +  ... when != id
> > > > +- e1 = devm_ioremap_resource(arg4, id);
> > > > ++ e1 = devm_platform_ioremap_resource(arg1, arg3);
> > > > +  ... when != id
> > > > +? id = e2
> > > > +
> > > > +@r1 depends on patch && !report@
> > > > +identifier r.id;
> > > > +type T;
> > > > +@@
> > > > +
> > > > +- T *id;
> > > > +  ...when != id
> > > > +
> > > > +// ----------------------------------------------------------------------------
> > > > +
> > > > +@r2 depends on report && !patch@
> > > > +identifier id;
> > > > +expression e1, e2, arg1, arg2, arg3, arg4;
> > > > +position j0;
> > > > +@@
> > > > +
> > > > +(
> > > > +  id = platform_get_resource(arg1, arg2, arg3);
> > > > +|
> > > > +  struct resource *id = platform_get_resource(arg1, arg2, arg3);
> > > > +)
> > > > +  ... when != id
> > > > +  e1@j0 = devm_ioremap_resource(arg4, id);
> > > > +  ... when != id
> > > > +? id = e2
> > > > +
> > > > +// ----------------------------------------------------------------------------
> > > > +
> > > > +@script:python depends on report && !patch@
> > > > +e1 << r2.e1;
> > > > +j0 << r2.j0;
> > > > +@@
> > > > +
> > > > +msg = "WARNING: Use devm_platform_ioremap_resource for %s" % (e1)
> > > > +coccilib.report.print_report(j0[0], msg)
> > > > --
> > > > 2.17.1
> > > >
> > > >
> > >
> > _______________________________________________
> > Cocci mailing list
> > Cocci@systeme.lip6.fr
> > https://systeme.lip6.fr/mailman/listinfo/cocci
>
>
>
> --
> Best Regards
> Masahiro Yamada
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-07-06 13:39       ` Julia Lawall
@ 2019-07-07  9:55         ` " Markus Elfring
  2019-07-07 12:38           ` Masahiro Yamada
  0 siblings, 1 reply; 24+ messages in thread
From: Markus Elfring @ 2019-07-07  9:55 UTC (permalink / raw)
  To: Julia Lawall, Masahiro Yamada, kernel-janitors
  Cc: Greg Kroah-Hartman, Linus Walleij, Nicolas Palix, LKML,
	Bartosz Golaszewski, Andy Shevchenko, Coccinelle

>> I will apply with Julia's Signed-off-by instead of Acked-by.

>> I will also add SPDX tag.

>>

>> Is this OK?


>
> Yes, thanks.


Will the clarification for following implementation details get any more
software development attention?
https://systeme.lip6.fr/pipermail/cocci/2019-June/005975.html
https://lore.kernel.org/lkml/7b4fe770-dadd-80ba-2ba4-0f2bc90984ef@web.de/

* The flag “IORESOURCE_MEM”

* Exclusion of variable assignments by SmPL when constraints

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-07-07  9:55         ` [Cocci] Coccinelle: " Markus Elfring
@ 2019-07-07 12:38           ` Masahiro Yamada
  2019-09-19  7:21             ` Markus Elfring
  0 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2019-07-07 12:38 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Nicolas Palix, Bartosz Golaszewski, Greg Kroah-Hartman,
	Linus Walleij, kernel-janitors, LKML, Andy Shevchenko,
	Coccinelle

On Sun, Jul 7, 2019 at 6:56 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> >> I will apply with Julia's Signed-off-by instead of Acked-by.
>
> >> I will also add SPDX tag.
>
> >>
>
> >> Is this OK?
>
>
> >
> > Yes, thanks.
>
>
> Will the clarification for following implementation details get any more
> software development attention?
> https://systeme.lip6.fr/pipermail/cocci/2019-June/005975.html
> https://lore.kernel.org/lkml/7b4fe770-dadd-80ba-2ba4-0f2bc90984ef@web.de/
>
> * The flag “IORESOURCE_MEM”
>
> * Exclusion of variable assignments by SmPL when constraints


OK, for this refactoring to happen,
the second argument should be IORESOURCE_MEM
instead of generic 'arg2'.

Himanshu,
Will you send v2?



-- 
Best Regards
Masahiro Yamada
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Cocci] Coccinelle: api: add devm_platform_ioremap_resource script
  2019-07-07 12:38           ` Masahiro Yamada
@ 2019-09-19  7:21             ` Markus Elfring
  0 siblings, 0 replies; 24+ messages in thread
From: Markus Elfring @ 2019-09-19  7:21 UTC (permalink / raw)
  To: Masahiro Yamada, kernel-janitors, Coccinelle
  Cc: Greg Kroah-Hartman, Linus Walleij, Nicolas Palix, LKML,
	Bartosz Golaszewski, Andy Shevchenko

> OK, for this refactoring to happen,
> the second argument should be IORESOURCE_MEM
> instead of generic 'arg2'.

A corresponding adjustment was committed on 2019-07-17.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/scripts/coccinelle/api/devm_platform_ioremap_resource.cocci?id=d09778d16e20bc4f1f4971cc9a9fd7ff6ba898ff

I constructed another variant of a script for the semantic patch language
based on this contribution.
I tried out to delete a bit of exception handling code after a call
of the function “platform_get_resource”.
Yesterday I sent a selection of patches from this transformation approach.
(An analysis based on “Linux next-20190916” pointed 56 source files
as update candidates out.)

Will it be useful to integrate such a case distinction into
the SmPL script directory?


The analysis based on the committed SmPL script pointed 657 source files
as update candidates out.
So there are further opportunities for collateral software evolution.
Can it become easier to check how many update suggestions are already
in corresponding patch review queues?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-06  6:11 [Cocci] [PATCH] coccinelle: api: add devm_platform_ioremap_resource script Himanshu Jha
2019-04-06  6:31 ` Julia Lawall
2019-04-06  6:34   ` Julia Lawall
2019-07-06 12:16     ` Masahiro Yamada
2019-07-06 13:39       ` Julia Lawall
2019-07-07  9:55         ` [Cocci] Coccinelle: " Markus Elfring
2019-07-07 12:38           ` Masahiro Yamada
2019-09-19  7:21             ` Markus Elfring
2019-04-06 12:36 ` [Cocci] [PATCH] coccinelle: " Markus Elfring
2019-06-08 15:24 ` [Cocci] Coccinelle: " Markus Elfring
2019-06-08 17:26   ` Julia Lawall
2019-06-09  8:55     ` Markus Elfring
2019-06-11 20:40       ` Enrico Weigelt, metux IT consult
2019-06-12  5:28         ` Julia Lawall
2019-06-12  6:24         ` Markus Elfring
2019-06-14  9:22         ` [Cocci] [PATCH] drivers: Inline code in devm_platform_ioremap_resource() from two functions Markus Elfring
2019-06-14  9:27           ` Julia Lawall
2019-06-15 11:00             ` [Cocci] " Markus Elfring
2019-06-14 10:07           ` [Cocci] [PATCH] " Greg Kroah-Hartman
2019-06-17 20:21           ` Enrico Weigelt, metux IT consult
2019-06-18  5:37             ` [Cocci] " Markus Elfring
2019-06-18  9:35               ` Enrico Weigelt, metux IT consult
2019-06-18 10:21                 ` Dan Carpenter
2019-06-18 11:44                 ` Markus Elfring

Coccinelle archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr cocci@archiver.kernel.org
	public-inbox-index cocci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox