[PATCH] iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[PATCH] iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Feb 2024 11:36:51 +0100

A wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
("drivers: provide devm_platform_get_and_ioremap_resource()").
Thus reuse existing functionality instead of keeping duplicate source code.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/iommu/ipmmu-vmsa.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ace1fc4bd34b..12685dd2dd31 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1025,8 +1025,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 		return ret;

 	/* Map I/O memory and request IRQ. */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mmu->base = devm_ioremap_resource(&pdev->dev, res);
+	mmu->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(mmu->base))
 		return PTR_ERR(mmu->base);

--
2.43.0

Re: [PATCH] iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Robin Murphy]

On 2024-02-05 10:42 am, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 5 Feb 2024 11:36:51 +0100
> 
> A wrapper function is available since the commit 890cc39a879906b63912482dfc41944579df2dc6
> ("drivers: provide devm_platform_get_and_ioremap_resource()").
> Thus reuse existing functionality instead of keeping duplicate source code.

Much as I detest the get_and_ioremap_resource obfuscator, it's not even 
appropriate here since nothing else is using "res".

> This issue was detected by using the Coccinelle software.

Please improve your script, or at least manually review your patches to 
ensure they actually make sense in context. If you want to do cleanup, 
do thorough, meaningful cleanup - leaving weird dangling local variables 
does not make the code clearer, if anything it makes it subtly worse 
since now the reader's wondering why we've gone out of our way to make 
this unused assignment, so maybe something's missing and there's a bug?

Thanks,
Robin.

> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   drivers/iommu/ipmmu-vmsa.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index ace1fc4bd34b..12685dd2dd31 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1025,8 +1025,7 @@ static int ipmmu_probe(struct platform_device *pdev)
>   		return ret;
> 
>   	/* Map I/O memory and request IRQ. */
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	mmu->base = devm_ioremap_resource(&pdev->dev, res);
> +	mmu->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
>   	if (IS_ERR(mmu->base))
>   		return PTR_ERR(mmu->base);
> 
> --
> 2.43.0
> 

Re: iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Markus Elfring]

>> Thus reuse existing functionality instead of keeping duplicate source code.
>
> Much as I detest the get_and_ioremap_resource obfuscator, it's not even appropriate here since nothing else is using "res".

I got the impression that this local variable is needed to perform
a desired function call.

Regards,
Markus

Re: iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Robin Murphy]

On 2024-02-05 11:52 am, Markus Elfring wrote:
>>> Thus reuse existing functionality instead of keeping duplicate source code.
>>
>> Much as I detest the get_and_ioremap_resource obfuscator, it's not even appropriate here since nothing else is using "res".
> 
> I got the impression that this local variable is needed to perform
> a desired function call.

Yes, the call to devm_ioremap_resource(). Which you're proposing to 
remove...

Thanks,
Robin.

Re: iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Markus Elfring]

>>>> Thus reuse existing functionality instead of keeping duplicate source code.
>>>
>>> Much as I detest the get_and_ioremap_resource obfuscator, it's not even appropriate here since nothing else is using "res".
>>
>> I got the impression that this local variable is needed to perform
>> a desired function call.
>
> Yes, the call to devm_ioremap_resource(). Which you're proposing to remove...

I propose to replace a specific function call combination
by an other single call for a known wrapper function.
The mentioned variable is preserved for this purpose.

Regards,
Markus

Re: iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Robin Murphy]

On 2024-02-05 1:02 pm, Markus Elfring wrote:
>>>>> Thus reuse existing functionality instead of keeping duplicate source code.
>>>>
>>>> Much as I detest the get_and_ioremap_resource obfuscator, it's not even appropriate here since nothing else is using "res".
>>>
>>> I got the impression that this local variable is needed to perform
>>> a desired function call.
>>
>> Yes, the call to devm_ioremap_resource(). Which you're proposing to remove...
> 
> I propose to replace a specific function call combination
> by an other single call for a known wrapper function.
> The mentioned variable is preserved for this purpose.

No. If you believe in cleaning up code, please apply your brain and 
clean up code *meaningfully* - others seem to have managed it just fine 
(e.g. [1][2] at random from a quick search on lore). Hell, I think even 
the original now-deleted script could auto-generate a proper patch for 
situations where devm_platform_ioremap_resource() was appropriate - 
there is zero excuse for doing a *worse* job 4 years later.

If on the other hand your interest is not to actually clean up the code 
for the good of the community, but to stoke your own ego as a 
"contributor" of shitty patches that annoy people and reinforce the 
popular impression that you are in fact an AI, please stop, or at the 
very least please refrain from doing so in any areas I review or maintain.

Thanks,
Robin.

[1] https://lore.kernel.org/all/20191013152716.1830911-1-jic23@kernel.org/
[2] 
https://lore.kernel.org/all/20200412135023.3831-1-aishwaryarj100@gmail.com/

Re: iommu/ipmmu-vmsa: Use devm_platform_get_and_ioremap_resource() in ipmmu_probe()

[Markus Elfring]

>> Thus reuse existing functionality instead of keeping duplicate source code.
>
> Much as I detest the get_and_ioremap_resource obfuscator, it's not even appropriate here since nothing else is using "res".

Another adjustment might follow for the application of the function “devm_platform_ioremap_resource”.
https://elixir.bootlin.com/linux/v6.8-rc3/source/drivers/base/platform.c#L112

Regards,
Markus

[PATCH v2] iommu/ipmmu-vmsa: Use devm_platform_ioremap_resource() in ipmmu_probe()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 6 Feb 2024 15:00:23 +0100

A wrapper function is available since the commit 7945f929f1a77a1c8887a97ca07f87626858ff42
("drivers: provide devm_platform_ioremap_resource()").

* Thus reuse existing functionality instead of keeping duplicate source code.

* Delete a local variable which became unnecessary with this refactoring.


This issue was transformed by using the Coccinelle software.

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

v2:
The transformation pattern was adjusted based on advices by known contributors.

Examples:
* Doug Anderson
* Geert Uytterhoeven
* Robin Murphy


 drivers/iommu/ipmmu-vmsa.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ace1fc4bd34b..7e28aac1e383 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1005,7 +1005,6 @@ static const struct of_device_id ipmmu_of_ids[] = {
 static int ipmmu_probe(struct platform_device *pdev)
 {
 	struct ipmmu_vmsa_device *mmu;
-	struct resource *res;
 	int irq;
 	int ret;

@@ -1025,8 +1024,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 		return ret;

 	/* Map I/O memory and request IRQ. */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mmu->base = devm_ioremap_resource(&pdev->dev, res);
+	mmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(mmu->base))
 		return PTR_ERR(mmu->base);

--
2.43.0

Re: [PATCH v2] iommu/ipmmu-vmsa: Use devm_platform_ioremap_resource() in ipmmu_probe()

[Geert Uytterhoeven]

On Tue, Feb 6, 2024 at 3:16 PM Markus Elfring <Markus.Elfring@web.de> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 6 Feb 2024 15:00:23 +0100
>
> A wrapper function is available since the commit 7945f929f1a77a1c8887a97ca07f87626858ff42
> ("drivers: provide devm_platform_ioremap_resource()").
>
> * Thus reuse existing functionality instead of keeping duplicate source code.
>
> * Delete a local variable which became unnecessary with this refactoring.
>
>
> This issue was transformed by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>
> v2:
> The transformation pattern was adjusted based on advices by known contributors.
>
> Examples:
> * Doug Anderson
> * Geert Uytterhoeven
> * Robin Murphy

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds