[PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Arnd Bergmann]

The pci_register_io_range() and pci_pio_to_address() were recently
introduced to generalize the handling of memory mapped PCI I/O space,
but they are only valid when CONFIG_OF is set, leading to a possible
build error:

drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
   res_start = pci_pio_to_address(res->start);
   ^
drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
   err = of_pci_range_to_resource(&range, pdev->dev.of_node,
   ^
cc1: some warnings being treated as errors

This provides inline dummy implementations for the case that
CONFIG_OF is disabled, to allow better build testing.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")

diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 7ebb877b07c2..851097aab115 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -71,6 +71,11 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
 	return NULL;
 }
 
+static inline phys_addr_t pci_pio_to_address(unsigned long pio)
+{
+	return 0;
+}
+
 static inline int of_pci_range_parser_init(struct of_pci_range_parser *parser,
 			struct device_node *node)
 {
@@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(struct device_node *dev,
 {
 	return NULL;
 }
+static inline int of_pci_range_to_resource(struct of_pci_range *range,
+					   struct device_node *np,
+					   struct resource *res)
+{
+	return -ENOSYS;
+}
 #endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
 
 #endif /* __OF_ADDRESS_H */

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Liviu Dudau]

On Tue, Sep 30, 2014 at 02:19:05PM +0100, Arnd Bergmann wrote:
> The pci_register_io_range() and pci_pio_to_address() were recently
> introduced to generalize the handling of memory mapped PCI I/O space,
> but they are only valid when CONFIG_OF is set, leading to a possible
> build error:
> 
> drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
> drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
>    res_start = pci_pio_to_address(res->start);
>    ^
> drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
> drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
>    err = of_pci_range_to_resource(&range, pdev->dev.of_node,
>    ^
> cc1: some warnings being treated as errors
> 
> This provides inline dummy implementations for the case that
> CONFIG_OF is disabled, to allow better build testing.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")

Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>

Is that triggered by shmobile_defconfig?

Best regards,
Liviu

> 
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 7ebb877b07c2..851097aab115 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -71,6 +71,11 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
>  	return NULL;
>  }
>  
> +static inline phys_addr_t pci_pio_to_address(unsigned long pio)
> +{
> +	return 0;
> +}
> +
>  static inline int of_pci_range_parser_init(struct of_pci_range_parser *parser,
>  			struct device_node *node)
>  {
> @@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(struct device_node *dev,
>  {
>  	return NULL;
>  }
> +static inline int of_pci_range_to_resource(struct of_pci_range *range,
> +					   struct device_node *np,
> +					   struct resource *res)
> +{
> +	return -ENOSYS;
> +}
>  #endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
>  
>  #endif /* __OF_ADDRESS_H */
> 
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Arnd Bergmann]

On Tuesday 30 September 2014 15:45:06 Liviu Dudau wrote:
> 
> Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
> 
> Is that triggered by shmobile_defconfig?
> 
> 

No, I only found it using randconfig testing, shmobile enables CONFIG_OF by
default.

	Arnd

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Liviu Dudau]

On Tue, Sep 30, 2014 at 04:15:19PM +0100, Arnd Bergmann wrote:
> On Tuesday 30 September 2014 15:45:06 Liviu Dudau wrote:
> > 
> > Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > 
> > Is that triggered by shmobile_defconfig?
> > 
> > 
> 
> No, I only found it using randconfig testing, shmobile enables CONFIG_OF by
> default.

Hmm, I was looking at who defines CONFIG_PCI_RCAR_GEN2_PCIE and it's only
present in shmobile_defconfig, but that one does not select any OF options (other
than CONFIG_ARM_APPENDED_DTB). So there is no way of triggering this other
than by randconfig?

Best regards,
Liviu

> 
> 	Arnd
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Arnd Bergmann]

On Tuesday 30 September 2014 16:42:02 Liviu Dudau wrote:
> On Tue, Sep 30, 2014 at 04:15:19PM +0100, Arnd Bergmann wrote:
> > On Tuesday 30 September 2014 15:45:06 Liviu Dudau wrote:
> > > 
> > > Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > > 
> > > Is that triggered by shmobile_defconfig?
> > > 
> > > 
> > 
> > No, I only found it using randconfig testing, shmobile enables CONFIG_OF by
> > default.
> 
> Hmm, I was looking at who defines CONFIG_PCI_RCAR_GEN2_PCIE and it's only
> present in shmobile_defconfig, but that one does not select any OF options (other
> than CONFIG_ARM_APPENDED_DTB). So there is no way of triggering this other
> than by randconfig?

It should be easy enough to build a kernel without CONFIG_OF that enables
this driver manually on another platform.

	Arnd

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Bjorn Helgaas]

On Tue, Sep 30, 2014 at 03:19:05PM +0200, Arnd Bergmann wrote:
> The pci_register_io_range() and pci_pio_to_address() were recently
> introduced to generalize the handling of memory mapped PCI I/O space,
> but they are only valid when CONFIG_OF is set, leading to a possible
> build error:
> 
> drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
> drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
>    res_start = pci_pio_to_address(res->start);
>    ^
> drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
> drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
>    err = of_pci_range_to_resource(&range, pdev->dev.of_node,
>    ^
> cc1: some warnings being treated as errors
> 
> This provides inline dummy implementations for the case that
> CONFIG_OF is disabled, to allow better build testing.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")

I'm rebuilding the pci/host-generic branch to pick up the other v13 fixes,
so I folded this fix directly into the "of/pci: Add pci_register_io_range()
and pci_pio_to_address()" patch.  Thanks for finding this; the config
dependencies are a bit of a mess.

> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 7ebb877b07c2..851097aab115 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -71,6 +71,11 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
>  	return NULL;
>  }
>  
> +static inline phys_addr_t pci_pio_to_address(unsigned long pio)
> +{
> +	return 0;
> +}
> +
>  static inline int of_pci_range_parser_init(struct of_pci_range_parser *parser,
>  			struct device_node *node)
>  {
> @@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(struct device_node *dev,
>  {
>  	return NULL;
>  }
> +static inline int of_pci_range_to_resource(struct of_pci_range *range,
> +					   struct device_node *np,
> +					   struct resource *res)
> +{
> +	return -ENOSYS;
> +}
>  #endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
>  
>  #endif /* __OF_ADDRESS_H */
> 

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Liviu Dudau]

On Tue, Sep 30, 2014 at 10:01:39AM -0600, Bjorn Helgaas wrote:
> On Tue, Sep 30, 2014 at 03:19:05PM +0200, Arnd Bergmann wrote:
> > The pci_register_io_range() and pci_pio_to_address() were recently
> > introduced to generalize the handling of memory mapped PCI I/O space,
> > but they are only valid when CONFIG_OF is set, leading to a possible
> > build error:
> > 
> > drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
> > drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
> >    res_start = pci_pio_to_address(res->start);
> >    ^
> > drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
> > drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
> >    err = of_pci_range_to_resource(&range, pdev->dev.of_node,
> >    ^
> > cc1: some warnings being treated as errors
> > 
> > This provides inline dummy implementations for the case that
> > CONFIG_OF is disabled, to allow better build testing.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")
> 
> I'm rebuilding the pci/host-generic branch to pick up the other v13 fixes,
> so I folded this fix directly into the "of/pci: Add pci_register_io_range()
> and pci_pio_to_address()" patch.  Thanks for finding this; the config
> dependencies are a bit of a mess.

Bjorn,

I suggest splitting this patch into 2 parts, move the one that adds pci_pio_to_address() in patch 2
and the other in patch 5.

Best regards,
Liviu

> 
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 7ebb877b07c2..851097aab115 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -71,6 +71,11 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
> >  	return NULL;
> >  }
> >  
> > +static inline phys_addr_t pci_pio_to_address(unsigned long pio)
> > +{
> > +	return 0;
> > +}
> > +
> >  static inline int of_pci_range_parser_init(struct of_pci_range_parser *parser,
> >  			struct device_node *node)
> >  {
> > @@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(struct device_node *dev,
> >  {
> >  	return NULL;
> >  }
> > +static inline int of_pci_range_to_resource(struct of_pci_range *range,
> > +					   struct device_node *np,
> > +					   struct resource *res)
> > +{
> > +	return -ENOSYS;
> > +}
> >  #endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
> >  
> >  #endif /* __OF_ADDRESS_H */
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Bjorn Helgaas]

On Tue, Sep 30, 2014 at 11:10 AM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> On Tue, Sep 30, 2014 at 10:01:39AM -0600, Bjorn Helgaas wrote:
>> On Tue, Sep 30, 2014 at 03:19:05PM +0200, Arnd Bergmann wrote:
>> > The pci_register_io_range() and pci_pio_to_address() were recently
>> > introduced to generalize the handling of memory mapped PCI I/O space,
>> > but they are only valid when CONFIG_OF is set, leading to a possible
>> > build error:
>> >
>> > drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
>> > drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
>> >    res_start = pci_pio_to_address(res->start);
>> >    ^
>> > drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
>> > drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
>> >    err = of_pci_range_to_resource(&range, pdev->dev.of_node,
>> >    ^
>> > cc1: some warnings being treated as errors
>> >
>> > This provides inline dummy implementations for the case that
>> > CONFIG_OF is disabled, to allow better build testing.
>> >
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")
>>
>> I'm rebuilding the pci/host-generic branch to pick up the other v13 fixes,
>> so I folded this fix directly into the "of/pci: Add pci_register_io_range()
>> and pci_pio_to_address()" patch.  Thanks for finding this; the config
>> dependencies are a bit of a mess.
>
> Bjorn,
>
> I suggest splitting this patch into 2 parts, move the one that adds pci_pio_to_address() in patch 2
> and the other in patch 5.

Yeah, I screwed that up.  I had just applied Arnd's patch completely
to 279c5dd046, which doesn't quite make sense.

I tried again like this:

  - In "of/pci: Add pci_register_io_range() and pci_pio_to_address()",
I added the dummy pci_pio_to_address(), so there's an extern and a
definition in address.c when CONFIG_OF_ADDRESS=y, and an inline dummy
otherwise.

  - I dropped "of/pci: Define of_pci_range_to_resource() only when
CONFIG_PCI=y".

  - In "of/pci: Move of_pci_range_to_resource() to of/address.c", I
added the dummy of_pci_range_to_resource(), so there's an extern an a
definition in address.c when CONFIG_OF_ADDRESS=y and CONFIG_PCI=y, and
an inline dummy otherwise.

I'm not really set up to build test this, and my head is swimming a
bit, so I hope this is it.  I pushed the pci/host-generic branch
again, so you can look at it yourself.

Bjorn

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Liviu Dudau]

On Tue, Sep 30, 2014 at 01:53:06PM -0600, Bjorn Helgaas wrote:
> On Tue, Sep 30, 2014 at 11:10 AM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> > On Tue, Sep 30, 2014 at 10:01:39AM -0600, Bjorn Helgaas wrote:
> >> On Tue, Sep 30, 2014 at 03:19:05PM +0200, Arnd Bergmann wrote:
> >> > The pci_register_io_range() and pci_pio_to_address() were recently
> >> > introduced to generalize the handling of memory mapped PCI I/O space,
> >> > but they are only valid when CONFIG_OF is set, leading to a possible
> >> > build error:
> >> >
> >> > drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
> >> > drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
> >> >    res_start = pci_pio_to_address(res->start);
> >> >    ^
> >> > drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
> >> > drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
> >> >    err = of_pci_range_to_resource(&range, pdev->dev.of_node,
> >> >    ^
> >> > cc1: some warnings being treated as errors
> >> >
> >> > This provides inline dummy implementations for the case that
> >> > CONFIG_OF is disabled, to allow better build testing.
> >> >
> >> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> > Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")
> >>
> >> I'm rebuilding the pci/host-generic branch to pick up the other v13 fixes,
> >> so I folded this fix directly into the "of/pci: Add pci_register_io_range()
> >> and pci_pio_to_address()" patch.  Thanks for finding this; the config
> >> dependencies are a bit of a mess.
> >
> > Bjorn,
> >
> > I suggest splitting this patch into 2 parts, move the one that adds pci_pio_to_address() in patch 2
> > and the other in patch 5.
> 
> Yeah, I screwed that up.  I had just applied Arnd's patch completely
> to 279c5dd046, which doesn't quite make sense.
> 
> I tried again like this:
> 
>   - In "of/pci: Add pci_register_io_range() and pci_pio_to_address()",
> I added the dummy pci_pio_to_address(), so there's an extern and a
> definition in address.c when CONFIG_OF_ADDRESS=y, and an inline dummy
> otherwise.
> 
>   - I dropped "of/pci: Define of_pci_range_to_resource() only when
> CONFIG_PCI=y".
> 
>   - In "of/pci: Move of_pci_range_to_resource() to of/address.c", I
> added the dummy of_pci_range_to_resource(), so there's an extern an a
> definition in address.c when CONFIG_OF_ADDRESS=y and CONFIG_PCI=y, and
> an inline dummy otherwise.
> 
> I'm not really set up to build test this, and my head is swimming a
> bit, so I hope this is it.  I pushed the pci/host-generic branch
> again, so you can look at it yourself.

Hi Bjorn,

It looks good to me. Functionality-wise I can only test it tomorrow UK time,
but from the diffs I've done I thinks we're good.

Will wait for more reports from the build bot.

Take care,
Liviu

> 
> Bjorn
> 

Re: [PATCH] of/pci: add pci_pio_to_address dummy for !CONFIG_OF

[Liviu Dudau]

On Tue, Sep 30, 2014 at 02:19:05PM +0100, Arnd Bergmann wrote:
> The pci_register_io_range() and pci_pio_to_address() were recently
> introduced to generalize the handling of memory mapped PCI I/O space,
> but they are only valid when CONFIG_OF is set, leading to a possible
> build error:
> 
> drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_setup_window':
> drivers/pci/host/pcie-rcar.c:340:3: error: implicit declaration of function 'pci_pio_to_address' [-Werror=implicit-function-declaration]
>    res_start = pci_pio_to_address(res->start);
>    ^
> drivers/pci/host/pcie-rcar.c: In function 'rcar_pcie_probe':
> drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of function 'of_pci_range_to_resource' [-Werror=implicit-function-declaration]
>    err = of_pci_range_to_resource(&range, pdev->dev.of_node,
>    ^
> cc1: some warnings being treated as errors
> 
> This provides inline dummy implementations for the case that
> CONFIG_OF is disabled, to allow better build testing.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_to_address()")
> 
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 7ebb877b07c2..851097aab115 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -71,6 +71,11 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
>  	return NULL;
>  }
>  
> +static inline phys_addr_t pci_pio_to_address(unsigned long pio)
> +{
> +	return 0;
> +}
> +
>  static inline int of_pci_range_parser_init(struct of_pci_range_parser *parser,
>  			struct device_node *node)
>  {
> @@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(struct device_node *dev,
>  {
>  	return NULL;
>  }
> +static inline int of_pci_range_to_resource(struct of_pci_range *range,
> +					   struct device_node *np,
> +					   struct resource *res)
> +{
> +	return -ENOSYS;
> +}
>  #endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
>  
>  #endif /* __OF_ADDRESS_H */
> 

Arnd,

I have a more general question that is related only vaguely to this patch but it was prompted by it:
given that this pcie-rcar.c driver is so dependent on the OF functionality, why the fix(es) (in
general) tend to prefer creating these dummy functions for !CONFIG_OF rather than making
CONFIG_PCI_RCAR_GEN2_PCIE dependent on CONFIG_OF? We are basically compiling a driver that we can
guarantee it will fail when used without OF support and contribute to the overall size of the kernel.
Given the presentation you have done at Linaro Connect last month on this topic, I started to wonder
if there is a deeper API style agreement that is not apparent to me?

Thanks for your time,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯