[Cocci] Checking the replacement of two specific function calls

[Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

Hello,

I noticed the suggestion “i2c: busses: convert to devm_platform_get_and_ioremap_resource”.
https://lore.kernel.org/linux-i2c/20200414134827.18674-1-zhengdejin5@gmail.com/
https://lore.kernel.org/patchwork/patch/1223734/

I got into the mood to try another script out for the semantic patch language.


@replacement@
expression* base, device, resource;
@@
-resource = platform_get_resource(device, IORESOURCE_MEM, 0);
 base =
-       devm_ioremap_resource(&device->dev, resource)
+       devm_platform_get_and_ioremap_resource(device, 0, &resource)
 ;


Source file example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rcar.c?id=8632e9b5645bbc2331d21d892b0d6961c1a08429#n921
https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/i2c/busses/i2c-rcar.c#L921

// SPDX-License-Identifier: GPL-2.0
// deleted part
static int rcar_i2c_probe(struct platform_device *pdev)
{
	struct rcar_i2c_priv *priv;
	struct i2c_adapter *adap;
	struct device *dev = &pdev->dev;
// deleted part
	priv->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

	priv->io = devm_ioremap_resource(dev, priv->res);
	if (IS_ERR(priv->io))
		return PTR_ERR(priv->io);
// deleted part
}
// deleted part


Now I wonder why the expected patch is not generated by the software
“Coccinelle 1.0.8-00029-ga549b9f0” at the moment.
How should this situation be improved?

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

Re: [Cocci] Checking the replacement of two specific function calls

[Julia Lawall]

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



On Wed, 15 Apr 2020, Markus Elfring wrote:

> Hello,
>
> I noticed the suggestion “i2c: busses: convert to devm_platform_get_and_ioremap_resource”.
> https://lore.kernel.org/linux-i2c/20200414134827.18674-1-zhengdejin5@gmail.com/
> https://lore.kernel.org/patchwork/patch/1223734/
>
> I got into the mood to try another script out for the semantic patch language.
>
>
> @replacement@
> expression* base, device, resource;
> @@
> -resource = platform_get_resource(device, IORESOURCE_MEM, 0);
>  base =
> -       devm_ioremap_resource(&device->dev, resource)
> +       devm_platform_get_and_ioremap_resource(device, 0, &resource)
>  ;
>
>
> Source file example:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rcar.c?id=8632e9b5645bbc2331d21d892b0d6961c1a08429#n921
> https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/i2c/busses/i2c-rcar.c#L921
>
> // SPDX-License-Identifier: GPL-2.0
> // deleted part
> static int rcar_i2c_probe(struct platform_device *pdev)
> {
> 	struct rcar_i2c_priv *priv;
> 	struct i2c_adapter *adap;
> 	struct device *dev = &pdev->dev;
> // deleted part
> 	priv->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> 	priv->io = devm_ioremap_resource(dev, priv->res);
> 	if (IS_ERR(priv->io))
> 		return PTR_ERR(priv->io);
> // deleted part
> }
> // deleted part
>
>
> Now I wonder why the expected patch is not generated by the software
> “Coccinelle 1.0.8-00029-ga549b9f0” at the moment.
> How should this situation be improved?

Probably Coccinelle is not able to determine that priv->res and priv->io
are pointers, if you have not given it enough encouragement to look for
header files to find the defintion of the type of the priv structure.

I don't think that there is any need for the * in the metavariable
declaration.  If the various expressions are not pointers, the C compiler
would have complained.

julia


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

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

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>> static int rcar_i2c_probe(struct platform_device *pdev)
>> {
>> 	struct rcar_i2c_priv *priv;
…
>> 	struct device *dev = &pdev->dev;
>> // deleted part
>> 	priv->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>
>> 	priv->io = devm_ioremap_resource(dev, priv->res);
…
>> }
…
> Probably Coccinelle is not able to determine that priv->res and priv->io
> are pointers, if you have not given it enough encouragement to look for
> header files to find the defintion of the type of the priv structure.

The system configuration might need another bit of fine-tuning for
my test approach.


> I don't think that there is any need for the * in the metavariable declaration.

I tried out to specify the desired change by a specification which
can be a bit more precise.


> If the various expressions are not pointers, the C compiler
> would have complained.

The omission of the asterisk did also not trigger a known patch
for the mentioned function implementation while patches are generated
for code variations from other source files as expected.

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>> @replacement@
>> expression* base, device, resource;
>> @@
>> -resource = platform_get_resource(device, IORESOURCE_MEM, 0);
>>  base =
>> -       devm_ioremap_resource(&device->dev, resource)
>> +       devm_platform_get_and_ioremap_resource(device, 0, &resource)
>>  ;
…
>> static int rcar_i2c_probe(struct platform_device *pdev)
>> {
>> 	struct rcar_i2c_priv *priv;
…
>> 	struct device *dev = &pdev->dev;
…
>> 	priv->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>
>> 	priv->io = devm_ioremap_resource(dev, priv->res);
>> 	if (IS_ERR(priv->io))
>> 		return PTR_ERR(priv->io);
…
>> }
…
> Probably Coccinelle is not able to determine that priv->res and priv->io
> are pointers, if you have not given it enough encouragement to look for
> header files to find the defintion of the type of the priv structure.

Would you find the following test command encouraging enough?

elfring@Sonne:~/Projekte/Linux/next-patched> spatch --include-headers-for-types ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c


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

Re: [Cocci] Checking the replacement of two specific function calls

[Julia Lawall]

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



On Thu, 16 Apr 2020, Markus Elfring wrote:

> >> @replacement@
> >> expression* base, device, resource;
> >> @@
> >> -resource = platform_get_resource(device, IORESOURCE_MEM, 0);
> >>  base =
> >> -       devm_ioremap_resource(&device->dev, resource)
> >> +       devm_platform_get_and_ioremap_resource(device, 0, &resource)
> >>  ;
> …
> >> static int rcar_i2c_probe(struct platform_device *pdev)
> >> {
> >> 	struct rcar_i2c_priv *priv;
> …
> >> 	struct device *dev = &pdev->dev;
> …
> >> 	priv->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>
> >> 	priv->io = devm_ioremap_resource(dev, priv->res);
> >> 	if (IS_ERR(priv->io))
> >> 		return PTR_ERR(priv->io);
> …
> >> }
> …
> > Probably Coccinelle is not able to determine that priv->res and priv->io
> > are pointers, if you have not given it enough encouragement to look for
> > header files to find the defintion of the type of the priv structure.
>
> Would you find the following test command encouraging enough?
>
> elfring@Sonne:~/Projekte/Linux/next-patched> spatch --include-headers-for-types ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c

No, include headers for types doesn't have any impact on how many header
files are included.  You need options like --all-includes or
--recursive-includes.  Include headers for types means only look at the
included files to collect type information, but ignore these files in the
matching and transformation process.

julia

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

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>> elfring@Sonne:~/Projekte/Linux/next-patched> spatch --include-headers-for-types ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c
>
> No, include headers for types doesn't have any impact on how many header
> files are included.  You need options like --all-includes or
> --recursive-includes.

I still observe that the known patch hunk is not generated even with the addition
of one of these command options.
https://lore.kernel.org/patchwork/patch/1223734/
https://lore.kernel.org/linux-i2c/20200414134827.18674-1-zhengdejin5@gmail.com/

Would I need any extra parameters here?


Will the Coccinelle software need any further improvements also around the
desired processing for pointers and corresponding data structures?

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

> I don't think that there is any need for the * in the metavariable declaration.

I can try another SmPL script variant out also without the specification
of an extra asterisk. If I test with the following source file example,
a patch is generated by the Coccinelle software in the expected way.


// SPDX-License-Identifier: GPL-2.0
static int rcar_i2c_probe(struct platform_device *pdev)
{
	struct rcar_i2c_priv *priv;

	priv->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	priv->io = devm_ioremap_resource(&pdev->dev, priv->res);
	if (IS_ERR(priv->io))
		return PTR_ERR(priv->io);
}


Command example:
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch use_devm_platform_get_and_ioremap_resource3.cocci ../Probe/i2c-rcar-variant-20200416.c


Implementation details from the original source file trigger some
software development challenges.
https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/i2c/busses/i2c-rcar.c#L921
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rcar.c?id=8632e9b5645bbc2331d21d892b0d6961c1a08429#n921

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

Re: [Cocci] Checking the replacement of two specific function calls

[Julia Lawall]

On Thu, 16 Apr 2020, Markus Elfring wrote:

> >> elfring@Sonne:~/Projekte/Linux/next-patched> spatch --include-headers-for-types ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c
> >
> > No, include headers for types doesn't have any impact on how many header
> > files are included.  You need options like --all-includes or
> > --recursive-includes.
>
> I still observe that the known patch hunk is not generated even with the addition
> of one of these command options.
> https://lore.kernel.org/patchwork/patch/1223734/
> https://lore.kernel.org/linux-i2c/20200414134827.18674-1-zhengdejin5@gmail.com/
>
> Would I need any extra parameters here?

Perhaps -I options to help it find the relevant .h file.

julia

>
>
> Will the Coccinelle software need any further improvements also around the
> desired processing for pointers and corresponding data structures?
>
> Regards,
> Markus
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>> Would I need any extra parameters here?
>
> Perhaps -I options to help it find the relevant .h file.

I wonder why the relevant header files would not be found directly
from the corresponding Git repository.

Can a file inclusion failure become an immediate SmPL script execution error
when a special program parameter would be passed?


Can it be that the pointer variable “priv” to the data structure “rcar_i2c_priv”
distracts from the desired understanding of a questionable software situation
when the relevant declaration is provided in the same source file?
https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/i2c/busses/i2c-rcar.c#L119
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rcar.c?id=8632e9b5645bbc2331d21d892b0d6961c1a08429#n119

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

Re: [Cocci] Checking the replacement of two specific function calls

[Julia Lawall]

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



On Thu, 16 Apr 2020, Markus Elfring wrote:

> >> Would I need any extra parameters here?
> >
> > Perhaps -I options to help it find the relevant .h file.
>
> I wonder why the relevant header files would not be found directly
> from the corresponding Git repository.

OK, it seems that the structure is defined in the same file so no include
options should be needed.  But I have now lost track of what the
semantic patch actually is, so I don't know what is the problem.

julia

>
> Can a file inclusion failure become an immediate SmPL script execution error
> when a special program parameter would be passed?
>
>
> Can it be that the pointer variable “priv” to the data structure “rcar_i2c_priv”
> distracts from the desired understanding of a questionable software situation
> when the relevant declaration is provided in the same source file?
> https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/i2c/busses/i2c-rcar.c#L119
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rcar.c?id=8632e9b5645bbc2331d21d892b0d6961c1a08429#n119
>
> Regards,
> Markus
>

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

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>> Would I need any extra parameters here?
>
> Perhaps -I options to help it find the relevant .h file.

The dependencies for the data structure “device” can become also more interesting.
https://elixir.bootlin.com/linux/v5.7-rc1/source/include/linux/device.h#L451
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/device.h?id=00086336a8d96a04aa960f912287692a258f6cf5#n451

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

> OK, it seems that the structure is defined in the same file so no include
> options should be needed.  But I have now lost track of what the
> semantic patch actually is, so I don't know what is the problem.

1. Unfortunately, I needed a bit longer to become more aware of relevant
   differences in source code variations.
   Another case distinction would be needed. Thus a corresponding
   SmPL disjunction is probably required.
   I have tried out to express the desired change by a SmPL specification
   which can be a bit more precise.

@replacement@
expression base, device, private, resource;
@@
-resource = platform_get_resource(device, IORESOURCE_MEM, 0);
 base =
-       devm_ioremap_resource
+       devm_platform_get_and_ioremap_resource
                             (
(
-                             &
                               device
-                                    ->dev
                              ,
+                             0, &
                              resource
|
                              device,
+                                     0, &
                                      private->res
)
                             );


elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch --parse-cocci use_devm_platform_get_and_ioremap_resource6.cocci
…
warning: iso ptr_to_array does not match the code below on line 11
device->dev

the following code matched is not uniformly minus or context,
or contains a disjunction:
Exp:
device->dev
…


2. Would it be occasionally nice to intentionally express restrictions on
   pointer data types by using the asterisk in metavariable declarations?


3. If we would require information from header files:
   Is a file inclusion failure treated as an immediate SmPL script
   execution error?


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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

> But I have now lost track of what the semantic patch actually is,
> so I don't know what is the problem.

Do you find the following transformation approach easier to clarify?

SmPL script:
@replacement@
expression base, device, index, private;
@@
-private->res = platform_get_resource(device, IORESOURCE_MEM, index);
 base =
-       devm_ioremap_resource
+       devm_platform_get_and_ioremap_resource
                             (device,
+                                     index, &
                                              private->res
                             );


Command example:
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch use_devm_platform_get_and_ioremap_resource8.cocci ../Probe/i2c-rcar-excerpt-20200415.c


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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>> But I have now lost track of what the semantic patch actually is,
>> so I don't know what is the problem.

Another possibility should be taken into account for source code variations.

SmPL script example:
@replacement@
expression base, device1, device2, index, private;
@@
-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
 base =
-       devm_ioremap_resource
+       devm_platform_get_and_ioremap_resource
                             (device2,
+                                     index, &
                                              private->res
                             );


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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

> Another possibility should be taken into account for source code variations.
>
> SmPL script example:
> @replacement@
> expression base, device1, device2, index, private;
> @@
> -private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
>  base =
> -       devm_ioremap_resource
> +       devm_platform_get_and_ioremap_resource
>                              (device2,
> +                                     index, &
>                                               private->res
>                              );

Such a transformation approach can work to some degree.
Thus I would expect that it can be reused for a case distinction in the way
of a SmPL disjunction like the following.

Another SmPL script example:
@replacement@
expression base, device1, device2, index, private, resource;
@@
-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
 base =
-       devm_ioremap_resource
+       devm_platform_get_and_ioremap_resource
                             (
(
-                             &
                               device1
-                                     ->dev
                              ,
+                             index, &
                                      resource
|
                              device2,
+                                      index, &
                                               private->res
)
                             );


Test command:
elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch use_devm_platform_get_and_ioremap_resource10.cocci ../Probe/i2c-rcar-excerpt-20200415.c


Now I wonder again why the known patch is not generated as expected so far.
How would you like to clarify this software situation?

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

Re: [Cocci] Checking the replacement of two specific function calls

[Markus Elfring]

>>>> elfring@Sonne:~/Projekte/Linux/next-patched> spatch --include-headers-for-types ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c
>>>
>>> No, include headers for types doesn't have any impact on how many header
>>> files are included.  You need options like --all-includes or
>>> --recursive-includes.
>>
>> I still observe that the known patch hunk is not generated even with the addition
>> of one of these command options.
>> https://lore.kernel.org/patchwork/patch/1223734/
>> https://lore.kernel.org/linux-i2c/20200414134827.18674-1-zhengdejin5@gmail.com/
>>
>> Would I need any extra parameters here?
>
> Perhaps -I options to help it find the relevant .h file.

I became curious if the software situation can be clarified better.

Thus I have tested my transformation approach again with the commit
“i2c: altera: use proper variable to hold errno” before the referenced change.


@replacement@
expression base, device, resource;
@@
-resource = platform_get_resource(device, IORESOURCE_MEM, 0);
 base =
-       devm_ioremap_resource(&device->dev, resource)
+       devm_platform_get_and_ioremap_resource(device, 0, &resource)
 ;



elfring@Sonne:~/Projekte/Linux/next-patched> git checkout edb2c9dd3948738ef030c32b948543e84f4d3f81 && LANG=C make MODE=patch COCCI=~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci M=drivers/i2c/busses V=1 coccicheck
Running (4 in parallel): /usr/local/bin/spatch -D patch --very-quiet --cocci-file /home/elfring/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci --patch . --dir drivers/i2c/busses -I ./arch/x86/include -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi --include ./include/linux/kconfig.h --jobs 4 --chunksize 1
coccicheck failed
make: *** [Makefile:1781: coccicheck] Error 255


I can try another customised command out with the shown parameters.

elfring@Sonne:~/Projekte/Linux/next-patched> spatch -I ./arch/x86/include -I ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi -I ./include/uapi -I ./include/generated/uapi --include ./include/linux/kconfig.h ~/Projekte/Coccinelle/janitor/use_devm_platform_get_and_ioremap_resource3.cocci drivers/i2c/busses/i2c-rcar.c
init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
HANDLING: drivers/i2c/busses/i2c-rcar.c


May I expect the generation of a patch by the Coccinelle software here?

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

[Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 7 Sep 2020 13:14:44 +0200

Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
("drivers: provide devm_platform_get_and_ioremap_resource()").
Provide design options for the adjustment of affected source code
by the means of the semantic patch language (Coccinelle software).

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 ...vm_platform_get_and_ioremap_resource.cocci | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci

diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
new file mode 100644
index 000000000000..8e67359f6b76
--- /dev/null
+++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Simplify a function call combination by using a known wrapper function.
+//
+// Keywords: wrapper function conversion ioremap resources
+// Confidence: High
+
+virtual context, patch, report, org
+
+@display depends on context@
+expression base, device1, device2, index, private, resource;
+@@
+(
+*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+*       devm_ioremap_resource
+                             (&device1->dev, resource);
+|
+*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+*       devm_ioremap_resource
+                             (device2, private->res);
+)
+
+@replacement depends on patch@
+expression base, device1, device2, index, private, resource;
+@@
+(
+-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+-       devm_ioremap_resource
++       devm_platform_get_and_ioremap_resource
+                             (
+-                             &
+                               device1
+-                                     ->dev
+                              ,
+-                             resource
++                             index, &resource
+                             );
+|
+-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base =
+-       devm_ioremap_resource
++       devm_platform_get_and_ioremap_resource
+                             (device2,
+-                             private->res
++                             index, &private->res
+                             );
+)
+
+@or depends on org || report@
+expression base, device1, device2, index, private, resource;
+position p;
+@@
+(
+ resource = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource@p(&device1->dev, resource);
+|
+ private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource@p(device2, private->res);
+)
+
+@script:python to_do depends on org@
+p << or.p;
+@@
+coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
+
+@script:python reporting depends on report@
+p << or.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
--
2.28.0

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

Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Sep 2020 13:14:44 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  ...vm_platform_get_and_ioremap_resource.cocci | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..8e67359f6b76
> --- /dev/null
> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Simplify a function call combination by using a known wrapper function.
> +//
> +// Keywords: wrapper function conversion ioremap resources
> +// Confidence: High
> +
> +virtual context, patch, report, org
> +
> +@display depends on context@
> +expression base, device1, device2, index, private, resource;
> +@@
> +(
> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +*       devm_ioremap_resource
> +                             (&device1->dev, resource);

Why do you require these statements to be next to each other?

> +|
> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +*       devm_ioremap_resource
> +                             (device2, private->res);

Why do you have this special case?

> +)
> +
> +@replacement depends on patch@
> +expression base, device1, device2, index, private, resource;
> +@@
> +(
> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (
> +-                             &
> +                               device1
> +-                                     ->dev
> +                              ,
> +-                             resource
> ++                             index, &resource
> +                             );
> +|
> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (device2,

It is very suspicious that in one case you change the first argument of
devm_platform_get_and_ioremap_resource and in one case you don't.  If you
don't know how to make the change in some cases, it would be better to do
nothing at all.

julia

> +-                             private->res
> ++                             index, &private->res
> +                             );
> +)
> +
> +@or depends on org || report@
> +expression base, device1, device2, index, private, resource;
> +position p;
> +@@
> +(
> + resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(&device1->dev, resource);
> +|
> + private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(device2, private->res);
> +)
> +
> +@script:python to_do depends on org@
> +p << or.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> +
> +@script:python reporting depends on report@
> +p << or.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> --
> 2.28.0
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

Re: [Cocci] [PATCH] Coccinelle: api: Ad =?UTF-8?Q?d_SmPL_script_=E2=80=9Cu

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Sep 2020 13:14:44 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  ...vm_platform_get_and_ioremap_resource.cocci | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..8e67359f6b76
> --- /dev/null
> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Simplify a function call combination by using a known wrapper function.
> +//
> +// Keywords: wrapper function conversion ioremap resources
> +// Confidence: High
> +
> +virtual context, patch, report, org
> +
> +@display depends on context@
> +expression base, device1, device2, index, private, resource;
> +@@
> +(
> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base > +*       devm_ioremap_resource
> +                             (&device1->dev, resource);

Why do you require these statements to be next to each other?

> +|
> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base > +*       devm_ioremap_resource
> +                             (device2, private->res);

Why do you have this special case?

> +)
> +
> +@replacement depends on patch@
> +expression base, device1, device2, index, private, resource;
> +@@
> +(
> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base > +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (
> +-                             &
> +                               device1
> +-                                     ->dev
> +                              ,
> +-                             resource
> ++                             index, &resource
> +                             );
> +|
> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base > +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (device2,

It is very suspicious that in one case you change the first argument of
devm_platform_get_and_ioremap_resource and in one case you don't.  If you
don't know how to make the change in some cases, it would be better to do
nothing at all.

julia

> +-                             private->res
> ++                             index, &private->res
> +                             );
> +)
> +
> +@or depends on org || report@
> +expression base, device1, device2, index, private, resource;
> +position p;
> +@@
> +(
> + resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(&device1->dev, resource);
> +|
> + private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(device2, private->res);
> +)
> +
> +@script:python to_do depends on org@
> +p << or.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> +
> +@script:python reporting depends on report@
> +p << or.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> --
> 2.28.0
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Sep 2020 13:14:44 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  ...vm_platform_get_and_ioremap_resource.cocci | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..8e67359f6b76
> --- /dev/null
> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Simplify a function call combination by using a known wrapper function.
> +//
> +// Keywords: wrapper function conversion ioremap resources
> +// Confidence: High
> +
> +virtual context, patch, report, org
> +
> +@display depends on context@
> +expression base, device1, device2, index, private, resource;
> +@@
> +(
> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +*       devm_ioremap_resource
> +                             (&device1->dev, resource);

Why do you require these statements to be next to each other?

> +|
> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +*       devm_ioremap_resource
> +                             (device2, private->res);

Why do you have this special case?

> +)
> +
> +@replacement depends on patch@
> +expression base, device1, device2, index, private, resource;
> +@@
> +(
> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (
> +-                             &
> +                               device1
> +-                                     ->dev
> +                              ,
> +-                             resource
> ++                             index, &resource
> +                             );
> +|
> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base =
> +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (device2,

It is very suspicious that in one case you change the first argument of
devm_platform_get_and_ioremap_resource and in one case you don't.  If you
don't know how to make the change in some cases, it would be better to do
nothing at all.

julia

> +-                             private->res
> ++                             index, &private->res
> +                             );
> +)
> +
> +@or depends on org || report@
> +expression base, device1, device2, index, private, resource;
> +position p;
> +@@
> +(
> + resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(&device1->dev, resource);
> +|
> + private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(device2, private->res);
> +)
> +
> +@script:python to_do depends on org@
> +p << or.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> +
> +@script:python reporting depends on report@
> +p << or.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> --
> 2.28.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

Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Markus Elfring]

>> +@display depends on context@
>> +expression base, device1, device2, index, private, resource;
>> +@@
>> +(
>> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
>> + base =
>> +*       devm_ioremap_resource
>> +                             (&device1->dev, resource);
>
> Why do you require these statements to be next to each other?

I would appreciate indications for a general change acceptance
also according to such a simple transformation approach.
I imagine that it will become more challenging to tolerate extra
source code between these function calls (by the specification
of special SmPL filters).


>> +|
>> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
>> + base =
>> +*       devm_ioremap_resource
>> +                             (device2, private->res);
>
> Why do you have this special case?

The usage of the data structure member “res” triggers corresponding
software design consequences.
The expressions which are passed as the first function call parameters
can be different.


>> +@replacement depends on patch@
>> +expression base, device1, device2, index, private, resource;
>> +@@
>> +(
>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
>> + base =
>> +-       devm_ioremap_resource
>> ++       devm_platform_get_and_ioremap_resource
>> +                             (
>> +-                             &
>> +                               device1
>> +-                                     ->dev
>> +                              ,
>> +-                             resource
>> ++                             index, &resource
>> +                             );
>> +|
>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
>> + base =
>> +-       devm_ioremap_resource
>> ++       devm_platform_get_and_ioremap_resource
>> +                             (device2,
>
> It is very suspicious that in one case you change the first argument of
> devm_platform_get_and_ioremap_resource and in one case you don't.

I noticed a few special cases during my source code analysis approach.


> If you don't know how to make the change in some cases, it would be better
> to do nothing at all.

How do you think about to take another look at any update candidates?

Examples:
* mvebu_sei_probe
  https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/irqchip/irq-mvebu-sei.c#L368
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-mvebu-sei.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n368

* hi655x_pmic_probe
  https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/mfd/hi655x-pmic.c#L92
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/hi655x-pmic.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n92

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

Re: [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

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



On Mon, 7 Sep 2020, Markus Elfring wrote:

> >> +@display depends on context@
> >> +expression base, device1, device2, index, private, resource;
> >> +@@
> >> +(
> >> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +*       devm_ioremap_resource
> >> +                             (&device1->dev, resource);
> >
> > Why do you require these statements to be next to each other?
>
> I would appreciate indications for a general change acceptance
> also according to such a simple transformation approach.
> I imagine that it will become more challenging to tolerate extra
> source code between these function calls (by the specification
> of special SmPL filters).
>
>
> >> +|
> >> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +*       devm_ioremap_resource
> >> +                             (device2, private->res);
> >
> > Why do you have this special case?
>
> The usage of the data structure member “res” triggers corresponding
> software design consequences.

I don't think this is a reliable rule.  You included two examples below.
They involve structures of different types. It seems unlikely that there
is a guarantee that the res field of all structures is used in the same
way.  Furthermore, this is the context case.  What is the purpose of
making the distinction?  The user will have to figure out what to do by
hand in any case.

> The expressions which are passed as the first function call parameters
> can be different.
>
>
> >> +@replacement depends on patch@
> >> +expression base, device1, device2, index, private, resource;
> >> +@@
> >> +(
> >> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +-       devm_ioremap_resource
> >> ++       devm_platform_get_and_ioremap_resource
> >> +                             (
> >> +-                             &
> >> +                               device1
> >> +-                                     ->dev
> >> +                              ,
> >> +-                             resource
> >> ++                             index, &resource
> >> +                             );
> >> +|
> >> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +-       devm_ioremap_resource
> >> ++       devm_platform_get_and_ioremap_resource
> >> +                             (device2,
> >
> > It is very suspicious that in one case you change the first argument of
> > devm_platform_get_and_ioremap_resource and in one case you don't.
>
> I noticed a few special cases during my source code analysis approach.

This is not a reasonable answer.  Does the rule work correctly or not?  If
it doesn't work correctly, it needs to be removed.

> > If you don't know how to make the change in some cases, it would be better
> > to do nothing at all.
>
> How do you think about to take another look at any update candidates?
>
> Examples:
> * mvebu_sei_probe
>   https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/irqchip/irq-mvebu-sei.c#L368
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-mvebu-sei.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n368

I don't see any purpose to providing two links for everything.

julia

>
> * hi655x_pmic_probe
>   https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/mfd/hi655x-pmic.c#L92
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/hi655x-pmic.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n92
>
> Regards,
> Markus
>

Re: [PATCH] Coccinelle: api: Add SmPL scr =?UTF-8?Q?ipt_=E2=80=9Cuse=5Fdevm=5F

[Julia Lawall]

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



On Mon, 7 Sep 2020, Markus Elfring wrote:

> >> +@display depends on context@
> >> +expression base, device1, device2, index, private, resource;
> >> +@@
> >> +(
> >> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +*       devm_ioremap_resource
> >> +                             (&device1->dev, resource);
> >
> > Why do you require these statements to be next to each other?
>
> I would appreciate indications for a general change acceptance
> also according to such a simple transformation approach.
> I imagine that it will become more challenging to tolerate extra
> source code between these function calls (by the specification
> of special SmPL filters).
>
>
> >> +|
> >> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +*       devm_ioremap_resource
> >> +                             (device2, private->res);
> >
> > Why do you have this special case?
>
> The usage of the data structure member “res” triggers corresponding
> software design consequences.

I don't think this is a reliable rule.  You included two examples below.
They involve structures of different types. It seems unlikely that there
is a guarantee that the res field of all structures is used in the same
way.  Furthermore, this is the context case.  What is the purpose of
making the distinction?  The user will have to figure out what to do by
hand in any case.

> The expressions which are passed as the first function call parameters
> can be different.
>
>
> >> +@replacement depends on patch@
> >> +expression base, device1, device2, index, private, resource;
> >> +@@
> >> +(
> >> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +-       devm_ioremap_resource
> >> ++       devm_platform_get_and_ioremap_resource
> >> +                             (
> >> +-                             &
> >> +                               device1
> >> +-                                     ->dev
> >> +                              ,
> >> +-                             resource
> >> ++                             index, &resource
> >> +                             );
> >> +|
> >> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +-       devm_ioremap_resource
> >> ++       devm_platform_get_and_ioremap_resource
> >> +                             (device2,
> >
> > It is very suspicious that in one case you change the first argument of
> > devm_platform_get_and_ioremap_resource and in one case you don't.
>
> I noticed a few special cases during my source code analysis approach.

This is not a reasonable answer.  Does the rule work correctly or not?  If
it doesn't work correctly, it needs to be removed.

> > If you don't know how to make the change in some cases, it would be better
> > to do nothing at all.
>
> How do you think about to take another look at any update candidates?
>
> Examples:
> * mvebu_sei_probe
>   https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/irqchip/irq-mvebu-sei.c#L368
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-mvebu-sei.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n368

I don't see any purpose to providing two links for everything.

julia

>
> * hi655x_pmic_probe
>   https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/mfd/hi655x-pmic.c#L92
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/hi655x-pmic.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n92
>
> Regards,
> Markus
>

Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

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



On Mon, 7 Sep 2020, Markus Elfring wrote:

> >> +@display depends on context@
> >> +expression base, device1, device2, index, private, resource;
> >> +@@
> >> +(
> >> +*resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +*       devm_ioremap_resource
> >> +                             (&device1->dev, resource);
> >
> > Why do you require these statements to be next to each other?
>
> I would appreciate indications for a general change acceptance
> also according to such a simple transformation approach.
> I imagine that it will become more challenging to tolerate extra
> source code between these function calls (by the specification
> of special SmPL filters).
>
>
> >> +|
> >> +*private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +*       devm_ioremap_resource
> >> +                             (device2, private->res);
> >
> > Why do you have this special case?
>
> The usage of the data structure member “res” triggers corresponding
> software design consequences.

I don't think this is a reliable rule.  You included two examples below.
They involve structures of different types. It seems unlikely that there
is a guarantee that the res field of all structures is used in the same
way.  Furthermore, this is the context case.  What is the purpose of
making the distinction?  The user will have to figure out what to do by
hand in any case.

> The expressions which are passed as the first function call parameters
> can be different.
>
>
> >> +@replacement depends on patch@
> >> +expression base, device1, device2, index, private, resource;
> >> +@@
> >> +(
> >> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +-       devm_ioremap_resource
> >> ++       devm_platform_get_and_ioremap_resource
> >> +                             (
> >> +-                             &
> >> +                               device1
> >> +-                                     ->dev
> >> +                              ,
> >> +-                             resource
> >> ++                             index, &resource
> >> +                             );
> >> +|
> >> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >> + base =
> >> +-       devm_ioremap_resource
> >> ++       devm_platform_get_and_ioremap_resource
> >> +                             (device2,
> >
> > It is very suspicious that in one case you change the first argument of
> > devm_platform_get_and_ioremap_resource and in one case you don't.
>
> I noticed a few special cases during my source code analysis approach.

This is not a reasonable answer.  Does the rule work correctly or not?  If
it doesn't work correctly, it needs to be removed.

> > If you don't know how to make the change in some cases, it would be better
> > to do nothing at all.
>
> How do you think about to take another look at any update candidates?
>
> Examples:
> * mvebu_sei_probe
>   https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/irqchip/irq-mvebu-sei.c#L368
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-mvebu-sei.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n368

I don't see any purpose to providing two links for everything.

julia

>
> * hi655x_pmic_probe
>   https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/mfd/hi655x-pmic.c#L92
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mfd/hi655x-pmic.c?id=f4d51dffc6c01a9e94650d95ce0104964f8ae822#n92
>
> Regards,
> Markus
>

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

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

Re: [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

> >>>> +@replacement depends on patch@
> >>>> +expression base, device1, device2, index, private, resource;
> >>>> +@@
> >>>> +(
> >>>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>> + base =
> >>>> +-       devm_ioremap_resource
> >>>> ++       devm_platform_get_and_ioremap_resource
> >>>> +                             (
> >>>> +-                             &
> >>>> +                               device1
> >>>> +-                                     ->dev
> >>>> +                              ,
> >>>> +-                             resource
> >>>> ++                             index, &resource
> >>>> +                             );
> >>>> +|
> >>>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>> + base =
> >>>> +-       devm_ioremap_resource
> >>>> ++       devm_platform_get_and_ioremap_resource
> >>>> +                             (device2,
> >>>
> >>> It is very suspicious that in one case you change the first argument of
> >>> devm_platform_get_and_ioremap_resource and in one case you don't.
> >>
> >> I noticed a few special cases during my source code analysis approach.
> >
> > This is not a reasonable answer.  Does the rule work correctly or not?
> > If it doesn't work correctly, it needs to be removed.
>
> Both source code change patterns from these branches of a SmPL disjunction
> work as I would expect it by the current Coccinelle software.
> Would you like to clarify any remaining related open issues?

In one case the first argument of devm_ioremap_resource has to be changed
to produce the first argument of devm_platform_get_and_ioremap_resource
and in the other case there is no such change.  "work as I would expect"
is not a proper explanation of why this is correct.  Maybe you can point
to some previous commits that have made the change in this way.

julia

Re: [PATCH] Coccinelle: api: Add SmPL scr =?UTF-8?Q?ipt_=E2=80=9Cuse=5Fdevm=5F

[Julia Lawall]

> >>>> +@replacement depends on patch@
> >>>> +expression base, device1, device2, index, private, resource;
> >>>> +@@
> >>>> +(
> >>>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>> + base > >>>> +-       devm_ioremap_resource
> >>>> ++       devm_platform_get_and_ioremap_resource
> >>>> +                             (
> >>>> +-                             &
> >>>> +                               device1
> >>>> +-                                     ->dev
> >>>> +                              ,
> >>>> +-                             resource
> >>>> ++                             index, &resource
> >>>> +                             );
> >>>> +|
> >>>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>> + base > >>>> +-       devm_ioremap_resource
> >>>> ++       devm_platform_get_and_ioremap_resource
> >>>> +                             (device2,
> >>>
> >>> It is very suspicious that in one case you change the first argument of
> >>> devm_platform_get_and_ioremap_resource and in one case you don't.
> >>
> >> I noticed a few special cases during my source code analysis approach.
> >
> > This is not a reasonable answer.  Does the rule work correctly or not?
> > If it doesn't work correctly, it needs to be removed.
>
> Both source code change patterns from these branches of a SmPL disjunction
> work as I would expect it by the current Coccinelle software.
> Would you like to clarify any remaining related open issues?

In one case the first argument of devm_ioremap_resource has to be changed
to produce the first argument of devm_platform_get_and_ioremap_resource
and in the other case there is no such change.  "work as I would expect"
is not a proper explanation of why this is correct.  Maybe you can point
to some previous commits that have made the change in this way.

julia

Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

> >>>> +@replacement depends on patch@
> >>>> +expression base, device1, device2, index, private, resource;
> >>>> +@@
> >>>> +(
> >>>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>> + base =
> >>>> +-       devm_ioremap_resource
> >>>> ++       devm_platform_get_and_ioremap_resource
> >>>> +                             (
> >>>> +-                             &
> >>>> +                               device1
> >>>> +-                                     ->dev
> >>>> +                              ,
> >>>> +-                             resource
> >>>> ++                             index, &resource
> >>>> +                             );
> >>>> +|
> >>>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>> + base =
> >>>> +-       devm_ioremap_resource
> >>>> ++       devm_platform_get_and_ioremap_resource
> >>>> +                             (device2,
> >>>
> >>> It is very suspicious that in one case you change the first argument of
> >>> devm_platform_get_and_ioremap_resource and in one case you don't.
> >>
> >> I noticed a few special cases during my source code analysis approach.
> >
> > This is not a reasonable answer.  Does the rule work correctly or not?
> > If it doesn't work correctly, it needs to be removed.
>
> Both source code change patterns from these branches of a SmPL disjunction
> work as I would expect it by the current Coccinelle software.
> Would you like to clarify any remaining related open issues?

In one case the first argument of devm_ioremap_resource has to be changed
to produce the first argument of devm_platform_get_and_ioremap_resource
and in the other case there is no such change.  "work as I would expect"
is not a proper explanation of why this is correct.  Maybe you can point
to some previous commits that have made the change in this way.

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

Re: [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

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



On Mon, 7 Sep 2020, Markus Elfring wrote:

> >>>>>> +@replacement depends on patch@
> >>>>>> +expression base, device1, device2, index, private, resource;
> >>>>>> +@@
> >>>>>> +(
> >>>>>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>>>> + base =
> >>>>>> +-       devm_ioremap_resource
> >>>>>> ++       devm_platform_get_and_ioremap_resource
> >>>>>> +                             (
> >>>>>> +-                             &
> >>>>>> +                               device1
> >>>>>> +-                                     ->dev
> >>>>>> +                              ,
> >>>>>> +-                             resource
> >>>>>> ++                             index, &resource
> >>>>>> +                             );
> >>>>>> +|
> >>>>>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>>>> + base =
> >>>>>> +-       devm_ioremap_resource
> >>>>>> ++       devm_platform_get_and_ioremap_resource
> >>>>>> +                             (device2,
> …
> >> Both source code change patterns from these branches of a SmPL disjunction
> >> work as I would expect it by the current Coccinelle software.
> >> Would you like to clarify any remaining related open issues?
> >
> > In one case the first argument of devm_ioremap_resource has to be changed
> > to produce the first argument of devm_platform_get_and_ioremap_resource
> > and in the other case there is no such change.  "work as I would expect"
> > is not a proper explanation of why this is correct.
>
> Would like to achieve that a SmPL script (like the one which I present here)
> can help to adjust the implementation of the functions “mvebu_sei_probe”
> and “hi655x_pmic_probe” (for example) for the desired reuse of the wrapper
> function “devm_platform_get_and_ioremap_resource”?
>
>
> > Maybe you can point to some previous commits that have made the change in this way.

I looked at some of the provided examples, but none of them leaves the
first argument of devm_ioremap_resource unchanged.

julia

>
> Not yet for these source code examples.
>
> Some patches were contributed.
> https://lore.kernel.org/patchwork/project/lkml/list/?q=devm_platform_get_and_ioremap_resource&archive=both&state=*
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=devm_platform_get_and_ioremap_resource
>
> Regards,
> Markus
>

Re: [PATCH] Coccinelle: api: Add SmPL scr =?UTF-8?Q?ipt_=E2=80=9Cuse=5Fdevm=5F

[Julia Lawall]

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



On Mon, 7 Sep 2020, Markus Elfring wrote:

> >>>>>> +@replacement depends on patch@
> >>>>>> +expression base, device1, device2, index, private, resource;
> >>>>>> +@@
> >>>>>> +(
> >>>>>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>>>> + base =
> >>>>>> +-       devm_ioremap_resource
> >>>>>> ++       devm_platform_get_and_ioremap_resource
> >>>>>> +                             (
> >>>>>> +-                             &
> >>>>>> +                               device1
> >>>>>> +-                                     ->dev
> >>>>>> +                              ,
> >>>>>> +-                             resource
> >>>>>> ++                             index, &resource
> >>>>>> +                             );
> >>>>>> +|
> >>>>>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>>>> + base =
> >>>>>> +-       devm_ioremap_resource
> >>>>>> ++       devm_platform_get_and_ioremap_resource
> >>>>>> +                             (device2,
> …
> >> Both source code change patterns from these branches of a SmPL disjunction
> >> work as I would expect it by the current Coccinelle software.
> >> Would you like to clarify any remaining related open issues?
> >
> > In one case the first argument of devm_ioremap_resource has to be changed
> > to produce the first argument of devm_platform_get_and_ioremap_resource
> > and in the other case there is no such change.  "work as I would expect"
> > is not a proper explanation of why this is correct.
>
> Would like to achieve that a SmPL script (like the one which I present here)
> can help to adjust the implementation of the functions “mvebu_sei_probe”
> and “hi655x_pmic_probe” (for example) for the desired reuse of the wrapper
> function “devm_platform_get_and_ioremap_resource”?
>
>
> > Maybe you can point to some previous commits that have made the change in this way.

I looked at some of the provided examples, but none of them leaves the
first argument of devm_ioremap_resource unchanged.

julia

>
> Not yet for these source code examples.
>
> Some patches were contributed.
> https://lore.kernel.org/patchwork/project/lkml/list/?q=devm_platform_get_and_ioremap_resource&archive=both&state=*
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=devm_platform_get_and_ioremap_resource
>
> Regards,
> Markus
>

Re: [Cocci] [PATCH] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

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



On Mon, 7 Sep 2020, Markus Elfring wrote:

> >>>>>> +@replacement depends on patch@
> >>>>>> +expression base, device1, device2, index, private, resource;
> >>>>>> +@@
> >>>>>> +(
> >>>>>> +-resource = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>>>> + base =
> >>>>>> +-       devm_ioremap_resource
> >>>>>> ++       devm_platform_get_and_ioremap_resource
> >>>>>> +                             (
> >>>>>> +-                             &
> >>>>>> +                               device1
> >>>>>> +-                                     ->dev
> >>>>>> +                              ,
> >>>>>> +-                             resource
> >>>>>> ++                             index, &resource
> >>>>>> +                             );
> >>>>>> +|
> >>>>>> +-private->res = platform_get_resource(device1, IORESOURCE_MEM, index);
> >>>>>> + base =
> >>>>>> +-       devm_ioremap_resource
> >>>>>> ++       devm_platform_get_and_ioremap_resource
> >>>>>> +                             (device2,
> …
> >> Both source code change patterns from these branches of a SmPL disjunction
> >> work as I would expect it by the current Coccinelle software.
> >> Would you like to clarify any remaining related open issues?
> >
> > In one case the first argument of devm_ioremap_resource has to be changed
> > to produce the first argument of devm_platform_get_and_ioremap_resource
> > and in the other case there is no such change.  "work as I would expect"
> > is not a proper explanation of why this is correct.
>
> Would like to achieve that a SmPL script (like the one which I present here)
> can help to adjust the implementation of the functions “mvebu_sei_probe”
> and “hi655x_pmic_probe” (for example) for the desired reuse of the wrapper
> function “devm_platform_get_and_ioremap_resource”?
>
>
> > Maybe you can point to some previous commits that have made the change in this way.

I looked at some of the provided examples, but none of them leaves the
first argument of devm_ioremap_resource unchanged.

julia

>
> Not yet for these source code examples.
>
> Some patches were contributed.
> https://lore.kernel.org/patchwork/project/lkml/list/?q=devm_platform_get_and_ioremap_resource&archive=both&state=*
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=devm_platform_get_and_ioremap_resource
>
> Regards,
> Markus
>

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

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

[Cocci] [PATCH v2] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 7 Sep 2020 18:38:04 +0200

Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
("drivers: provide devm_platform_get_and_ioremap_resource()").
Provide safe design options for the adjustment of affected source code
by the means of the semantic patch language (Coccinelle software).

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

v2:
Julia Lawall requested to omit case distinctions (disjunctions) from
the first SmPL script.
The usage of different expression metavariables for the first parameter
of function calls was too questionable for the proposed source
code transformation.


 ...vm_platform_get_and_ioremap_resource.cocci | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci

diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
new file mode 100644
index 000000000000..319583716ac8
--- /dev/null
+++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Simplify a function call combination by using a known wrapper function.
+//
+// Keywords: wrapper function conversion ioremap resources
+// Confidence: High
+
+virtual context, patch, report, org
+
+@display depends on context@
+expression base, device, index, resource;
+@@
+*resource = platform_get_resource(device, IORESOURCE_MEM, index);
+ base =
+*       devm_ioremap_resource
+                             (&device->dev, resource);
+
+@replacement depends on patch@
+expression base, device, index, resource;
+@@
+-resource = platform_get_resource(device, IORESOURCE_MEM, index);
+ base =
+-       devm_ioremap_resource
++       devm_platform_get_and_ioremap_resource
+                             (
+-                             &
+                               device
+-                                    ->dev
+                              ,
+-                             resource
++                             index, &resource
+                             );
+
+@or depends on org || report@
+expression base, device, index, resource;
+position p;
+@@
+ resource = platform_get_resource(device, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource@p(&device->dev, resource);
+
+@script:python to_do depends on org@
+p << or.p;
+@@
+coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
+
+@script:python reporting depends on report@
+p << or.p;
+@@
+coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
--
2.28.0

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

Re: [Cocci] [PATCH v2] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Sep 2020 18:38:04 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide safe design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>
> v2:
> Julia Lawall requested to omit case distinctions (disjunctions) from
> the first SmPL script.
> The usage of different expression metavariables for the first parameter
> of function calls was too questionable for the proposed source
> code transformation.
>
>
>  ...vm_platform_get_and_ioremap_resource.cocci | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..319583716ac8
> --- /dev/null
> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Simplify a function call combination by using a known wrapper function.
> +//
> +// Keywords: wrapper function conversion ioremap resources
> +// Confidence: High

Shouldn't there be some options?  --no-includes and perhaps
--include-headers would seem reasonable.

Please also remove all of the rule names except for "or" which is
necessary.  The rest just add clutter and make it harder to find the
useful information, which is the depends on.  Rules only need names if
some other rule inherits their metavariables.

julia

> +
> +virtual context, patch, report, org
> +
> +@display depends on context@
> +expression base, device, index, resource;
> +@@
> +*resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base =
> +*       devm_ioremap_resource
> +                             (&device->dev, resource);
> +
> +@replacement depends on patch@
> +expression base, device, index, resource;
> +@@
> +-resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base =
> +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (
> +-                             &
> +                               device
> +-                                    ->dev
> +                              ,
> +-                             resource
> ++                             index, &resource
> +                             );
> +
> +@or depends on org || report@
> +expression base, device, index, resource;
> +position p;
> +@@
> + resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(&device->dev, resource);
> +
> +@script:python to_do depends on org@
> +p << or.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> +
> +@script:python reporting depends on report@
> +p << or.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> --
> 2.28.0
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

Re: [Cocci] [PATCH v2] Coccinelle: api: =?UTF-8?Q?_Add_SmPL_script_=E2=80=

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Sep 2020 18:38:04 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide safe design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>
> v2:
> Julia Lawall requested to omit case distinctions (disjunctions) from
> the first SmPL script.
> The usage of different expression metavariables for the first parameter
> of function calls was too questionable for the proposed source
> code transformation.
>
>
>  ...vm_platform_get_and_ioremap_resource.cocci | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..319583716ac8
> --- /dev/null
> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Simplify a function call combination by using a known wrapper function.
> +//
> +// Keywords: wrapper function conversion ioremap resources
> +// Confidence: High

Shouldn't there be some options?  --no-includes and perhaps
--include-headers would seem reasonable.

Please also remove all of the rule names except for "or" which is
necessary.  The rest just add clutter and make it harder to find the
useful information, which is the depends on.  Rules only need names if
some other rule inherits their metavariables.

julia

> +
> +virtual context, patch, report, org
> +
> +@display depends on context@
> +expression base, device, index, resource;
> +@@
> +*resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base > +*       devm_ioremap_resource
> +                             (&device->dev, resource);
> +
> +@replacement depends on patch@
> +expression base, device, index, resource;
> +@@
> +-resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base > +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (
> +-                             &
> +                               device
> +-                                    ->dev
> +                              ,
> +-                             resource
> ++                             index, &resource
> +                             );
> +
> +@or depends on org || report@
> +expression base, device, index, resource;
> +position p;
> +@@
> + resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(&device->dev, resource);
> +
> +@script:python to_do depends on org@
> +p << or.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> +
> +@script:python reporting depends on report@
> +p << or.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> --
> 2.28.0
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

Re: [Cocci] [PATCH v2] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Sep 2020 18:38:04 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide safe design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>
> v2:
> Julia Lawall requested to omit case distinctions (disjunctions) from
> the first SmPL script.
> The usage of different expression metavariables for the first parameter
> of function calls was too questionable for the proposed source
> code transformation.
>
>
>  ...vm_platform_get_and_ioremap_resource.cocci | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
>
> diff --git a/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> new file mode 100644
> index 000000000000..319583716ac8
> --- /dev/null
> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Simplify a function call combination by using a known wrapper function.
> +//
> +// Keywords: wrapper function conversion ioremap resources
> +// Confidence: High

Shouldn't there be some options?  --no-includes and perhaps
--include-headers would seem reasonable.

Please also remove all of the rule names except for "or" which is
necessary.  The rest just add clutter and make it harder to find the
useful information, which is the depends on.  Rules only need names if
some other rule inherits their metavariables.

julia

> +
> +virtual context, patch, report, org
> +
> +@display depends on context@
> +expression base, device, index, resource;
> +@@
> +*resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base =
> +*       devm_ioremap_resource
> +                             (&device->dev, resource);
> +
> +@replacement depends on patch@
> +expression base, device, index, resource;
> +@@
> +-resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base =
> +-       devm_ioremap_resource
> ++       devm_platform_get_and_ioremap_resource
> +                             (
> +-                             &
> +                               device
> +-                                    ->dev
> +                              ,
> +-                             resource
> ++                             index, &resource
> +                             );
> +
> +@or depends on org || report@
> +expression base, device, index, resource;
> +position p;
> +@@
> + resource = platform_get_resource(device, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource@p(&device->dev, resource);
> +
> +@script:python to_do depends on org@
> +p << or.p;
> +@@
> +coccilib.org.print_todo(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> +
> +@script:python reporting depends on report@
> +p << or.p;
> +@@
> +coccilib.report.print_report(p[0], "WARNING: opportunity for devm_platform_get_and_ioremap_resource()")
> --
> 2.28.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

Re: [PATCH v2] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> >> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> >> @@ -0,0 +1,48 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/// Simplify a function call combination by using a known wrapper function.
> >> +//
> >> +// Keywords: wrapper function conversion ioremap resources
> >> +// Confidence: High
> >
> > Shouldn't there be some options?  --no-includes and perhaps
> > --include-headers would seem reasonable.
>
> I imagine that the proposed source code transformation can work also without
> additional semantic patch command options.

It can.  But it will be more efficient and more exhaustive if you include
them.

julia

Re: [PATCH v2] Coccinelle: api: Add SmPL =?UTF-8?Q?_script_=E2=80=9Cuse=5Fdevm

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> >> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> >> @@ -0,0 +1,48 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/// Simplify a function call combination by using a known wrapper function.
> >> +//
> >> +// Keywords: wrapper function conversion ioremap resources
> >> +// Confidence: High
> >
> > Shouldn't there be some options?  --no-includes and perhaps
> > --include-headers would seem reasonable.
>
> I imagine that the proposed source code transformation can work also without
> additional semantic patch command options.

It can.  But it will be more efficient and more exhaustive if you include
them.

julia

Re: [Cocci] [PATCH v2] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Julia Lawall]

On Mon, 7 Sep 2020, Markus Elfring wrote:

> >> +++ b/scripts/coccinelle/api/use_devm_platform_get_and_ioremap_resource.cocci
> >> @@ -0,0 +1,48 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/// Simplify a function call combination by using a known wrapper function.
> >> +//
> >> +// Keywords: wrapper function conversion ioremap resources
> >> +// Confidence: High
> >
> > Shouldn't there be some options?  --no-includes and perhaps
> > --include-headers would seem reasonable.
>
> I imagine that the proposed source code transformation can work also without
> additional semantic patch command options.

It can.  But it will be more efficient and more exhaustive if you include
them.

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

Re: [cocci] [PATCH v3] Coccinelle: api: Add SmPL script “use_devm_platform_get_and_ioremap_resource.cocci”

[Markus Elfring]

> Date: Mon, 7 Sep 2020 20:00:09 +0200
>
> Another wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Provide safe design options for the adjustment of affected source code
> by the means of the semantic patch language (Coccinelle software).

Do you find this transformation approach variant acceptable?

How are the chances for integration of this SmPL script version?

Regards,
Markus