diff mbox series

drivers: depend on HAS_IOMEM for devm_platform_ioremap_resource()

Message ID 20190221162627.3476-1-brgl@bgdev.pl
State New
Headers show
Series drivers: depend on HAS_IOMEM for devm_platform_ioremap_resource() | expand

Commit Message

Bartosz Golaszewski Feb. 21, 2019, 4:26 p.m. UTC 
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
dependency must cascade down to devm_platform_ioremap_resource().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/base/platform.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Greg KH Feb. 21, 2019, 4:47 p.m. UTC | #1 
On Thu, Feb 21, 2019 at 05:26:27PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
> dependency must cascade down to devm_platform_ioremap_resource().
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/base/platform.c | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Andy Shevchenko Feb. 21, 2019, 7:55 p.m. UTC | #2 
On Thu, Feb 21, 2019 at 6:27 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
> dependency must cascade down to devm_platform_ioremap_resource().

> +#ifdef CONFIG_HAS_IOMEM
>  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
>                                              unsigned int index)
>  {
> @@ -96,6 +97,7 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
>         return devm_ioremap_resource(&pdev->dev, res);
>  }
>  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> +#endif /* CONFIG_HAS_IOMEM */

What about declaration in *.h?

Perhaps you may just do this inside the function?
Andy Shevchenko Feb. 21, 2019, 7:56 p.m. UTC | #3 
On Thu, Feb 21, 2019 at 9:55 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Feb 21, 2019 at 6:27 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
> > dependency must cascade down to devm_platform_ioremap_resource().
>
> > +#ifdef CONFIG_HAS_IOMEM
> >  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> >                                              unsigned int index)
> >  {
> > @@ -96,6 +97,7 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> >         return devm_ioremap_resource(&pdev->dev, res);
> >  }
> >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > +#endif /* CONFIG_HAS_IOMEM */
>
> What about declaration in *.h?
>
> Perhaps you may just do this inside the function?

#ifdef ...
#else
return ERR_PTR(-ENOTSUPP);
#endif
Bartosz Golaszewski Feb. 22, 2019, 9:04 a.m. UTC | #4 
czw., 21 lut 2019 o 20:56 Andy Shevchenko <andy.shevchenko@gmail.com>
napisał(a):
>
> On Thu, Feb 21, 2019 at 9:55 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Thu, Feb 21, 2019 at 6:27 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
> > > dependency must cascade down to devm_platform_ioremap_resource().
> >
> > > +#ifdef CONFIG_HAS_IOMEM
> > >  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > >                                              unsigned int index)
> > >  {
> > > @@ -96,6 +97,7 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > >         return devm_ioremap_resource(&pdev->dev, res);
> > >  }
> > >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > > +#endif /* CONFIG_HAS_IOMEM */
> >
> > What about declaration in *.h?
> >
> > Perhaps you may just do this inside the function?
>
> #ifdef ...
> #else
> return ERR_PTR(-ENOTSUPP);
> #endif
>

I followed the example of devm_ioremap_resource() which doesn't do
this - it just expects never to be used by systems without IOMEM.

Bart
Andy Shevchenko Feb. 22, 2019, 10:24 a.m. UTC | #5 
On Fri, Feb 22, 2019 at 11:04 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> czw., 21 lut 2019 o 20:56 Andy Shevchenko <andy.shevchenko@gmail.com>
> napisał(a):
> >
> > On Thu, Feb 21, 2019 at 9:55 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Thu, Feb 21, 2019 at 6:27 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
> > > > dependency must cascade down to devm_platform_ioremap_resource().
> > >
> > > > +#ifdef CONFIG_HAS_IOMEM
> > > >  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > > >                                              unsigned int index)
> > > >  {
> > > > @@ -96,6 +97,7 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> > > >         return devm_ioremap_resource(&pdev->dev, res);
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > > > +#endif /* CONFIG_HAS_IOMEM */
> > >
> > > What about declaration in *.h?
> > >
> > > Perhaps you may just do this inside the function?
> >
> > #ifdef ...
> > #else
> > return ERR_PTR(-ENOTSUPP);
> > #endif
> >
>
> I followed the example of devm_ioremap_resource() which doesn't do
> this - it just expects never to be used by systems without IOMEM.

Okay, that's fine!
Rob Herring Feb. 22, 2019, 2:22 p.m. UTC | #6 
On Thu, Feb 21, 2019 at 1:56 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Feb 21, 2019 at 6:27 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > We only build devm_ioremap_resource() if HAS_IOMEM is selected, so this
> > dependency must cascade down to devm_platform_ioremap_resource().
>
> > +#ifdef CONFIG_HAS_IOMEM
> >  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> >                                              unsigned int index)
> >  {
> > @@ -96,6 +97,7 @@ void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
> >         return devm_ioremap_resource(&pdev->dev, res);
> >  }
> >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
> > +#endif /* CONFIG_HAS_IOMEM */
>
> What about declaration in *.h?
>
> Perhaps you may just do this inside the function?

Just a drive by comment, but IMO we should just get rid of HAS_IOMEM
altogether. It is really just a !UML option as I think every other
arch enables it. If folks really want to disable drivers on UML, just
disable the subsystems.

Though now with kunit mocking, there's a desire to enable HAS_IOMEM on UML, too.

Rob
Linus Walleij Feb. 22, 2019, 4:22 p.m. UTC | #7 
On Fri, Feb 22, 2019 at 3:22 PM Rob Herring <rob.herring@linaro.org> wrote:

> Just a drive by comment, but IMO we should just get rid of HAS_IOMEM
> altogether. It is really just a !UML option as I think every other
> arch enables it. If folks really want to disable drivers on UML, just
> disable the subsystems.

I just got a build failure from S390 for the same thing so apparently
there is S390 Linux without IOMEM. I have no idea how that works
but whenever Arnd start talking to me about how S390 works
my head start spinning.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f82691e1c26c..5f837f2e4f41 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -87,6 +87,7 @@  EXPORT_SYMBOL_GPL(platform_get_resource);
  *        resource managemend
  * @index: resource index
  */
+#ifdef CONFIG_HAS_IOMEM
 void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
 					     unsigned int index)
 {
@@ -96,6 +97,7 @@  void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
 	return devm_ioremap_resource(&pdev->dev, res);
 }
 EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
+#endif /* CONFIG_HAS_IOMEM */
 
 /**
  * platform_get_irq - get an IRQ for a device