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

Discussion:

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

Arnd Bergmann

2014-09-30 13:19:05 UTC

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 <***@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 */

Liviu Dudau

2014-09-30 14:45:06 UTC

Permalink

nction 'pci_pio_to_address' [-Werror=3Dimplicit-function-declaration]

Post by Arnd Bergmann
res_start =3D pci_pio_to_address(res->start);
^
drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of fu=

nction 'of_pci_range_to_resource' [-Werror=3Dimplicit-function-declarat=
ion]

Post by Arnd Bergmann
err =3D of_pci_range_to_resource(&range, pdev->dev.of_node,
^
cc1: some warnings being treated as errors
=20
This provides inline dummy implementations for the case that
CONFIG_OF is disabled, to allow better build testing.
=20
Fixes: 279c5dd046 ("of/pci: Add pci_register_io_range() and pci_pio_t=

o_address()")

Acked-by: Liviu Dudau <Liviu.Dudau-***@public.gmane.org>

Is that triggered by shmobile_defconfig?

Best regards,
Liviu

Post by Arnd Bergmann
=20
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,

Post by Arnd Bergmann
return NULL;
}
=20
+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_parse=

r *parser,

Post by Arnd Bergmann
struct device_node *node)
{
@@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(s=

truct device_node *dev,

Post by Arnd Bergmann
{
return NULL;
}
+static inline int of_pci_range_to_resource(struct of_pci_range *rang=

Post by Arnd Bergmann
+ struct device_node *np,
+ struct resource *res)
+{
+ return -ENOSYS;
+}
#endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
=20
#endif /* __OF_ADDRESS_H */
=20
=20
=20

--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF

--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Arnd Bergmann

2014-09-30 15:15:19 UTC

Permalink

Post by Liviu Dudau
Is that triggered by shmobile_defconfig?

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

Arnd

Liviu Dudau

2014-09-30 15:42:02 UTC

Permalink

Post by Liviu Dudau
=20
=20
Is that triggered by shmobile_defconfig?
=20
=20

=20
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 on=
ly
present in shmobile_defconfig, but that one does not select any OF opti=
ons (other
than CONFIG_ARM_APPENDED_DTB). So there is no way of triggering this ot=
her
than by randconfig?

Best regards,
Liviu

=20
Arnd
=20
=20

Arnd Bergmann

2014-09-30 15:54:16 UTC

Permalink

Post by Arnd Bergmann

Post by Liviu Dudau
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

Bjorn Helgaas

2014-09-30 16:01:39 UTC

Permalink

Post by 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
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: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.
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.

Post by Arnd Bergmann
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 */

Liviu Dudau

2014-09-30 17:10:03 UTC

Permalink

Post by Bjorn Helgaas

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

Post by Bjorn Helgaas

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

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

One small step
for me ...

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Bjorn Helgaas

2014-09-30 19:53:06 UTC

Permalink

Post by Liviu Dudau

Post by Bjorn Helgaas

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

Liviu Dudau

2014-09-30 21:28:55 UTC

Permalink

Post by Bjorn Helgaas

Post by Liviu Dudau

Post by Bjorn Helgaas

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.
- 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

Post by Bjorn Helgaas
Bjorn

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Liviu Dudau

2014-10-01 08:54:36 UTC

Permalink

nction 'pci_pio_to_address' [-Werror=3Dimplicit-function-declaration]

Post by Arnd Bergmann
res_start =3D pci_pio_to_address(res->start);
^
drivers/pci/host/pcie-rcar.c:945:3: error: implicit declaration of fu=

nction 'of_pci_range_to_resource' [-Werror=3Dimplicit-function-declarat=
ion]

o_address()")

device_node *dev, int index,

Post by Arnd Bergmann
return NULL;
}
=20
+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_parse=

r *parser,

Post by Arnd Bergmann
struct device_node *node)
{
@@ -144,6 +149,12 @@ static inline const __be32 *of_get_pci_address(s=

truct device_node *dev,

Post by Arnd Bergmann
{
return NULL;
}
+static inline int of_pci_range_to_resource(struct of_pci_range *rang=

Post by Arnd Bergmann
+ struct device_node *np,
+ struct resource *res)
+{
+ return -ENOSYS;
+}
#endif /* CONFIG_OF_ADDRESS && CONFIG_PCI */
=20
#endif /* __OF_ADDRESS_H */
=20

Arnd,

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

Thanks for your time,
Liviu

--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF