[PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[Markus Elfring]

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

The functions “platform_get_resource_byname” and “devm_ioremap_resource”
are called together in 181 source files.
This implementation detail can be determined also with the help
of the semantic patch language (Coccinelle software).

Wrap these two calls into another helper function.
Thus a local variable does not need to be declared for a resource
structure pointer before and a redundant argument can be omitted
for the resource type.

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

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4d1729853d1a..c1f19a479dd7 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -97,6 +97,24 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
 	return devm_ioremap_resource(&pdev->dev, res);
 }
 EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
+
+/**
+ * devm_platform_ioremap_resource_byname
+ * Call devm_ioremap_resource() for a platform device
+ *
+ * @pdev: platform device to use both for memory resource lookup as well as
+ *        resource management
+ * @name: resource name
+ */
+void __iomem *devm_platform_ioremap_resource_byname(struct platform_device *pdev,
+						    const char *name)
+{
+	struct resource *res;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
+	return devm_ioremap_resource(&pdev->dev, res);
+}
+EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
 #endif /* CONFIG_HAS_IOMEM */

 /**
--
2.22.0

Re: [PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[Andy Shevchenko]

+Cc: Jack Ping, who did internally the same

On Fri, Jun 14, 2019 at 03:26:25PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 14 Jun 2019 15:15:14 +0200
> 
> The functions “platform_get_resource_byname” and “devm_ioremap_resource”
> are called together in 181 source files.
> This implementation detail can be determined also with the help
> of the semantic patch language (Coccinelle software).
> 
> Wrap these two calls into another helper function.
> Thus a local variable does not need to be declared for a resource
> structure pointer before and a redundant argument can be omitted
> for the resource type.

This one makes sense.
Though I'm not sure Greg will see your message.

Rafael, maybe you can apply this one?

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/base/platform.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 4d1729853d1a..c1f19a479dd7 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -97,6 +97,24 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
>  	return devm_ioremap_resource(&pdev->dev, res);
>  }
>  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> +
> +/**
> + * devm_platform_ioremap_resource_byname
> + * Call devm_ioremap_resource() for a platform device
> + *
> + * @pdev: platform device to use both for memory resource lookup as well as
> + *        resource management
> + * @name: resource name
> + */
> +void __iomem *devm_platform_ioremap_resource_byname(struct platform_device *pdev,
> +						    const char *name)
> +{
> +	struct resource *res;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> +	return devm_ioremap_resource(&pdev->dev, res);
> +}
> +EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
>  #endif /* CONFIG_HAS_IOMEM */
> 
>  /**
> --
> 2.22.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[Greg Kroah-Hartman]

On Fri, Jun 14, 2019 at 04:38:40PM +0300, Andy Shevchenko wrote:
> +Cc: Jack Ping, who did internally the same
> 
> On Fri, Jun 14, 2019 at 03:26:25PM +0200, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Fri, 14 Jun 2019 15:15:14 +0200
> > 
> > The functions “platform_get_resource_byname” and “devm_ioremap_resource”
> > are called together in 181 source files.
> > This implementation detail can be determined also with the help
> > of the semantic patch language (Coccinelle software).
> > 
> > Wrap these two calls into another helper function.
> > Thus a local variable does not need to be declared for a resource
> > structure pointer before and a redundant argument can be omitted
> > for the resource type.
> 
> This one makes sense.
> Though I'm not sure Greg will see your message.

Nope, didn't see it, don't want to see it, it will only cause more work
in the longrun...

> Rafael, maybe you can apply this one?

Um, don't go around maintainers please, that's rude.  There is a reason
this specific developer is in my blacklist, and perhaps they should be
in yours as well :)

> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/base/platform.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 4d1729853d1a..c1f19a479dd7 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -97,6 +97,24 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> >  	return devm_ioremap_resource(&pdev->dev, res);
> >  }
> >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > +
> > +/**
> > + * devm_platform_ioremap_resource_byname
> > + * Call devm_ioremap_resource() for a platform device
> > + *
> > + * @pdev: platform device to use both for memory resource lookup as well as
> > + *        resource management
> > + * @name: resource name
> > + */
> > +void __iomem *devm_platform_ioremap_resource_byname(struct platform_device *pdev,
> > +						    const char *name)
> > +{
> > +	struct resource *res;
> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> > +	return devm_ioremap_resource(&pdev->dev, res);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
> >  #endif /* CONFIG_HAS_IOMEM */

I don't like adding new apis with no user.

thanks,

greg k-h

Re: [PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[Andy Shevchenko]

On Fri, Jun 14, 2019 at 04:10:04PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jun 14, 2019 at 04:38:40PM +0300, Andy Shevchenko wrote:
> > +Cc: Jack Ping, who did internally the same
> > 
> > On Fri, Jun 14, 2019 at 03:26:25PM +0200, Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Fri, 14 Jun 2019 15:15:14 +0200
> > > 
> > > The functions “platform_get_resource_byname” and “devm_ioremap_resource”
> > > are called together in 181 source files.
> > > This implementation detail can be determined also with the help
> > > of the semantic patch language (Coccinelle software).
> > > 
> > > Wrap these two calls into another helper function.
> > > Thus a local variable does not need to be declared for a resource
> > > structure pointer before and a redundant argument can be omitted
> > > for the resource type.
> > 
> > This one makes sense.
> > Though I'm not sure Greg will see your message.
> 
> Nope, didn't see it, don't want to see it, it will only cause more work
> in the longrun...
> 
> > Rafael, maybe you can apply this one?
> 
> Um, don't go around maintainers please, that's rude.  

I won't do it, how should we proceed if de facto this functionality is good to
have besides the fact of coming new user in the future?

> There is a reason
> this specific developer is in my blacklist, and perhaps they should be
> in yours as well :)

Perhaps.

> I don't like adding new apis with no user.

Perhaps Jack Ping will send it as a first patch of his series where he utilizes
this functionality. Would it be acceptable?

That's why I Cc'ed him as well.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[Greg Kroah-Hartman]

On Fri, Jun 14, 2019 at 05:47:06PM +0300, Andy Shevchenko wrote:
> On Fri, Jun 14, 2019 at 04:10:04PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jun 14, 2019 at 04:38:40PM +0300, Andy Shevchenko wrote:
> > > +Cc: Jack Ping, who did internally the same
> > > 
> > > On Fri, Jun 14, 2019 at 03:26:25PM +0200, Markus Elfring wrote:
> > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > Date: Fri, 14 Jun 2019 15:15:14 +0200
> > > > 
> > > > The functions “platform_get_resource_byname” and “devm_ioremap_resource”
> > > > are called together in 181 source files.
> > > > This implementation detail can be determined also with the help
> > > > of the semantic patch language (Coccinelle software).
> > > > 
> > > > Wrap these two calls into another helper function.
> > > > Thus a local variable does not need to be declared for a resource
> > > > structure pointer before and a redundant argument can be omitted
> > > > for the resource type.
> > > 
> > > This one makes sense.
> > > Though I'm not sure Greg will see your message.
> > 
> > Nope, didn't see it, don't want to see it, it will only cause more work
> > in the longrun...
> > 
> > > Rafael, maybe you can apply this one?
> > 
> > Um, don't go around maintainers please, that's rude.  
> 
> I won't do it, how should we proceed if de facto this functionality is good to
> have besides the fact of coming new user in the future?
> 
> > There is a reason
> > this specific developer is in my blacklist, and perhaps they should be
> > in yours as well :)
> 
> Perhaps.
> 
> > I don't like adding new apis with no user.
> 
> Perhaps Jack Ping will send it as a first patch of his series where he utilizes
> this functionality. Would it be acceptable?

Yes, that would be ok.

thanks,

greg k-h

Re: drivers: Provide devm_platform_ioremap_resource_byname()

[Markus Elfring]

>> I don't like adding new apis with no user.
>
> Perhaps Jack Ping will send it as a first patch of his series where he utilizes
> this functionality. Would it be acceptable?

The proposed function can get wider application in a reasonable time frame
if a corresponding script for the semantic patch language (Coccinelle software)
would perform an automatic source code conversion (for example)
according to your change acceptance.
How do you think about to support a bit more collateral software evolution?

Regards,
Markus

Re: drivers: Provide devm_platform_ioremap_resource_byname()

[Julia Lawall]

On Fri, 14 Jun 2019, Markus Elfring wrote:

> >> I don't like adding new apis with no user.
> >
> > Perhaps Jack Ping will send it as a first patch of his series where he utilizes
> > this functionality. Would it be acceptable?
>
> The proposed function can get wider application in a reasonable time frame
> if a corresponding script for the semantic patch language (Coccinelle software)
> would perform an automatic source code conversion (for example)
> according to your change acceptance.
> How do you think about to support a bit more collateral software evolution?

Why don't you just do what is asked for?

julia

Re: drivers: Provide devm_platform_ioremap_resource_byname()

[Markus Elfring]

> Why don't you just do what is asked for?

It seems that a previous update suggestion was easier to integrate
(on the first glance), wasn't it?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/base/platform.c?idy45f929f1a77a1c8887a97ca07f87626858ff42

Regards,
Markus

Re: [PATCH] drivers: Provide devm_platform_ioremap_resource_byname()

[Rafael J. Wysocki]

On Fri, Jun 14, 2019 at 4:10 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jun 14, 2019 at 04:38:40PM +0300, Andy Shevchenko wrote:
> > +Cc: Jack Ping, who did internally the same
> >
> > On Fri, Jun 14, 2019 at 03:26:25PM +0200, Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Fri, 14 Jun 2019 15:15:14 +0200
> > >
> > > The functions “platform_get_resource_byname” and “devm_ioremap_resource”
> > > are called together in 181 source files.
> > > This implementation detail can be determined also with the help
> > > of the semantic patch language (Coccinelle software).
> > >
> > > Wrap these two calls into another helper function.
> > > Thus a local variable does not need to be declared for a resource
> > > structure pointer before and a redundant argument can be omitted
> > > for the resource type.
> >
> > This one makes sense.
> > Though I'm not sure Greg will see your message.
>
> Nope, didn't see it, don't want to see it, it will only cause more work
> in the longrun...
>
> > Rafael, maybe you can apply this one?
>
> Um, don't go around maintainers please, that's rude.

Totally agreed.

And there would be no reason for me to even consider applying it, really.

> There is a reason this specific developer is in my blacklist, and perhaps they should be
> in yours as well :)
>
> > FWIW,
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > >
> > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > > ---
> > >  drivers/base/platform.c | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > > index 4d1729853d1a..c1f19a479dd7 100644
> > > --- a/drivers/base/platform.c
> > > +++ b/drivers/base/platform.c
> > > @@ -97,6 +97,24 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > >     return devm_ioremap_resource(&pdev->dev, res);
> > >  }
> > >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > > +
> > > +/**
> > > + * devm_platform_ioremap_resource_byname
> > > + * Call devm_ioremap_resource() for a platform device
> > > + *
> > > + * @pdev: platform device to use both for memory resource lookup as well as
> > > + *        resource management
> > > + * @name: resource name
> > > + */
> > > +void __iomem *devm_platform_ioremap_resource_byname(struct platform_device *pdev,
> > > +                                               const char *name)
> > > +{
> > > +   struct resource *res;
> > > +
> > > +   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> > > +   return devm_ioremap_resource(&pdev->dev, res);
> > > +}
> > > +EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
> > >  #endif /* CONFIG_HAS_IOMEM */
>
> I don't like adding new apis with no user.

I agree with that too.

Cheers!

Re: drivers: Provide devm_platform_ioremap_resource_byname()

[Markus Elfring]

>> I don't like adding new apis with no user.
>
> I agree with that too.

How do you think about the possibility to let an improved script
for the semantic patch language (Coccinelle software) perform
the desired software transformation so that the usage of
an additional function would get introduced by a known algorithm?

Can an agreement be achieved for the function name before?

Regards,
Markus