Re: [PATCH v3 1/5] drivers: provide devm_platform_get_and_ioremap_resource()

[Dejin Zheng <zhengdejin5@gmail.com>]

On Wed, Mar 18, 2020 at 11:10:10AM +0100, Greg KH wrote:
> On Tue, Mar 17, 2020 at 08:35:16PM +0100, Geert Uytterhoeven wrote:
> > Hi Greg,
> > 
> > On Tue, Mar 17, 2020 at 8:20 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Sun, Mar 15, 2020 at 10:05:21PM +0800, Dejin Zheng wrote:
> > > > Since commit "drivers: provide devm_platform_ioremap_resource()",
> > > > it was wrap platform_get_resource() and devm_ioremap_resource() as
> > > > single helper devm_platform_ioremap_resource(). but now, many drivers
> > > > still used platform_get_resource() and devm_ioremap_resource()
> > > > together in the kernel tree. The reason can not be replaced is they
> > > > still need use the resource variables obtained by platform_get_resource().
> > > > so provide this helper.
> > > >
> > > > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> > > > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
> > > > ---
> > > > v2 -> v3:
> > > >       - rename the function to
> > > >         devm_platform_get_and_ioremap_resource() by Sergei's suggestion.
> > > >       - make the last parameter res as optional by Geert's suggestion.
> > > >
> > > > v1 -> v2:
> > > >       - No change.
> > > >
> > > >  drivers/base/platform.c         | 22 ++++++++++++++++++++++
> > > >  include/linux/platform_device.h |  3 +++
> > > >  2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > > > index 7fa654f1288b..9f6a78f79235 100644
> > > > --- a/drivers/base/platform.c
> > > > +++ b/drivers/base/platform.c
> > > > @@ -62,6 +62,28 @@ struct resource *platform_get_resource(struct platform_device *dev,
> > > >  EXPORT_SYMBOL_GPL(platform_get_resource);
> > > >
> > > >  #ifdef CONFIG_HAS_IOMEM
> > > > +/**
> > > > + * devm_platform_get_and_ioremap_resource - call devm_ioremap_resource() for a
> > > > + *                                       platform device and get resource
> > > > + *
> > > > + * @pdev: platform device to use both for memory resource lookup as well as
> > > > + *        resource management
> > > > + * @index: resource index
> > > > + * @res: get the resource
> > > > + */
> > > > +void __iomem *
> > > > +devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
> > > > +                             unsigned int index, struct resource **res)
> > > > +{
> > > > +     struct resource *r;
> > > > +
> > > > +     r = platform_get_resource(pdev, IORESOURCE_MEM, index);
> > > > +     if (res)
> > > > +             *res = r;
> > >
> > > What happens if that call fails?  Shouldn't that be checked?
> > 
> > Then devm_ioremap_resource() will print an error message, and return
> > an error.
> > It's designed to be pipelined that way, so you have to check for an error
> > only once.
> 
> Ok, thanks.  Can I get an ack/reviewed-by for this series then?
> 
> thanks,
> 
> greg k-h

Hi Geert:

Could you help me review these patches if you have free time? Your comments and
suggestions will be of great help to me. It is important to me and thanks
again for your help.

Hi Greg, Mathias and Minas:

Thank you very much for taking your time to help me review my code.

BR,
Dejin