2023-07-05 11:18:57

by Alistair Francis

[permalink] [raw]
Subject: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

pci-epc.h doesn't define the members of the pci_epf_header struct, so
trying to access them results in errors like this:

error: invalid use of undefined type 'struct pci_epf_header'
167 | val = hdr->vendorid;

Instead let's include pci-epf.h which not only defines the
pci_epf_header but also includes pci-epc.h.

Signed-off-by: Alistair Francis <[email protected]>
---
drivers/pci/controller/pcie-rcar-ep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rcar-ep.c b/drivers/pci/controller/pcie-rcar-ep.c
index f9682df1da61..5afc91d11eef 100644
--- a/drivers/pci/controller/pcie-rcar-ep.c
+++ b/drivers/pci/controller/pcie-rcar-ep.c
@@ -10,7 +10,7 @@
#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/pci.h>
-#include <linux/pci-epc.h>
+#include <linux/pci-epf.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>

--
2.40.1



2023-07-05 11:40:04

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

On 7/5/23 19:48, Alistair Francis wrote:
> pci-epc.h doesn't define the members of the pci_epf_header struct, so
> trying to access them results in errors like this:
>
> error: invalid use of undefined type 'struct pci_epf_header'
> 167 | val = hdr->vendorid;
>
> Instead let's include pci-epf.h which not only defines the
> pci_epf_header but also includes pci-epc.h.
>
> Signed-off-by: Alistair Francis <[email protected]>

It is odd that the the build bot did not detect this...

Looks good.

Reviewed-by: Damien Le Moal <[email protected]>

> ---
> drivers/pci/controller/pcie-rcar-ep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-rcar-ep.c b/drivers/pci/controller/pcie-rcar-ep.c
> index f9682df1da61..5afc91d11eef 100644
> --- a/drivers/pci/controller/pcie-rcar-ep.c
> +++ b/drivers/pci/controller/pcie-rcar-ep.c
> @@ -10,7 +10,7 @@
> #include <linux/of_address.h>
> #include <linux/of_platform.h>
> #include <linux/pci.h>
> -#include <linux/pci-epc.h>
> +#include <linux/pci-epf.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
>

--
Damien Le Moal
Western Digital Research


2023-07-05 11:59:10

by Krzysztof Wilczyński

[permalink] [raw]
Subject: Re: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

Hello,

> > pci-epc.h doesn't define the members of the pci_epf_header struct, so
> > trying to access them results in errors like this:
> >
> > error: invalid use of undefined type 'struct pci_epf_header'
> > 167 | val = hdr->vendorid;
> >
> > Instead let's include pci-epf.h which not only defines the
> > pci_epf_header but also includes pci-epc.h.

[...]
> It is odd that the the build bot did not detect this...

This is a bit of a surprise to me too, especially since none of the usual
bots pick this up, and I can't seem to find such a failure in the nightly
CI logs either.

Alistair, how did you stumble into this issue? Also, which version or
a tree would that be?

Thank you!

Krzysztof

2023-07-05 12:13:04

by Alistair Francis

[permalink] [raw]
Subject: Re: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

On Wed, Jul 5, 2023 at 9:44 PM Krzysztof Wilczyński <[email protected]> wrote:
>
> Hello,
>
> > > pci-epc.h doesn't define the members of the pci_epf_header struct, so
> > > trying to access them results in errors like this:
> > >
> > > error: invalid use of undefined type 'struct pci_epf_header'
> > > 167 | val = hdr->vendorid;
> > >
> > > Instead let's include pci-epf.h which not only defines the
> > > pci_epf_header but also includes pci-epc.h.
>
> [...]
> > It is odd that the the build bot did not detect this...
>
> This is a bit of a surprise to me too, especially since none of the usual
> bots pick this up, and I can't seem to find such a failure in the nightly
> CI logs either.
>
> Alistair, how did you stumble into this issue? Also, which version or
> a tree would that be?

I was building the kernel with this defconfig [1] inside OpenEmbedded.
It was the 6.4-rc7 kernel, specifically this one [2].

1: https://github.com/damien-lemoal/buildroot/blob/rockpro64_ep_v23/board/pine64/rockpro64_ep/linux.config
2: https://github.com/damien-lemoal/linux/tree/rockpro64_ep_v23

Alistair

>
> Thank you!
>
> Krzysztof

2023-07-05 13:41:42

by Krzysztof Wilczyński

[permalink] [raw]
Subject: Re: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

Hello,

> > > > pci-epc.h doesn't define the members of the pci_epf_header struct, so
> > > > trying to access them results in errors like this:
> > > >
> > > > error: invalid use of undefined type 'struct pci_epf_header'
> > > > 167 | val = hdr->vendorid;
> > > >
> > > > Instead let's include pci-epf.h which not only defines the
> > > > pci_epf_header but also includes pci-epc.h.
> >
> > [...]
> > > It is odd that the the build bot did not detect this...
> >
> > This is a bit of a surprise to me too, especially since none of the usual
> > bots pick this up, and I can't seem to find such a failure in the nightly
> > CI logs either.
> >
> > Alistair, how did you stumble into this issue? Also, which version or
> > a tree would that be?
>
> I was building the kernel with this defconfig [1] inside OpenEmbedded.
> It was the 6.4-rc7 kernel, specifically this one [2].
>
> 1: https://github.com/damien-lemoal/buildroot/blob/rockpro64_ep_v23/board/pine64/rockpro64_ep/linux.config
> 2: https://github.com/damien-lemoal/linux/tree/rockpro64_ep_v23

Thank you! Much appreciated.

So, the vanilla kernel does not have headers arranged like this custom tree
that Damien maintains for his own needs, per:

- https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/pci-epc.h
- https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/pci-epf.h

I suppose, there are some changes that break it for you, for example:

https://github.com/damien-lemoal/linux/commit/c7aa8ddd76a141b975a097532050a76c6a58c436

So, I cannot take this patch as it would break vanilla kernel for us. :)

Try building using vanilla kernel, and see if that helps.

Krzysztof

2023-07-05 22:26:45

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

On 7/5/23 22:15, Krzysztof Wilczyński wrote:
> Hello,
>
>>>>> pci-epc.h doesn't define the members of the pci_epf_header struct, so
>>>>> trying to access them results in errors like this:
>>>>>
>>>>> error: invalid use of undefined type 'struct pci_epf_header'
>>>>> 167 | val = hdr->vendorid;
>>>>>
>>>>> Instead let's include pci-epf.h which not only defines the
>>>>> pci_epf_header but also includes pci-epc.h.
>>>
>>> [...]
>>>> It is odd that the the build bot did not detect this...
>>>
>>> This is a bit of a surprise to me too, especially since none of the usual
>>> bots pick this up, and I can't seem to find such a failure in the nightly
>>> CI logs either.
>>>
>>> Alistair, how did you stumble into this issue? Also, which version or
>>> a tree would that be?
>>
>> I was building the kernel with this defconfig [1] inside OpenEmbedded.
>> It was the 6.4-rc7 kernel, specifically this one [2].
>>
>> 1: https://github.com/damien-lemoal/buildroot/blob/rockpro64_ep_v23/board/pine64/rockpro64_ep/linux.config
>> 2: https://github.com/damien-lemoal/linux/tree/rockpro64_ep_v23
>
> Thank you! Much appreciated.
>
> So, the vanilla kernel does not have headers arranged like this custom tree
> that Damien maintains for his own needs, per:
>
> - https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/pci-epc.h
> - https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/pci-epf.h
>
> I suppose, there are some changes that break it for you, for example:
>
> https://github.com/damien-lemoal/linux/commit/c7aa8ddd76a141b975a097532050a76c6a58c436

Arg ! Yes, my bad ! The patches in this tree are a work-in-progress and I am
waiting for 6.5-rc1 to rebase everything and re-testing before posting them.
This is all tested using the rockchip-ep controller and I did not compile test
other controllers. I will make sure to do that before posting :)

Apologies for the noise.

>
> So, I cannot take this patch as it would break vanilla kernel for us. :)
>
> Try building using vanilla kernel, and see if that helps.
>
> Krzysztof

--
Damien Le Moal
Western Digital Research


2023-07-06 02:06:17

by Alistair Francis

[permalink] [raw]
Subject: Re: [PATCH] PCI: rcar-ep: Include linux/pci-epf.h instead of linux/pci-epc.h

On Wed, Jul 5, 2023 at 11:15 PM Krzysztof Wilczyński <[email protected]> wrote:
>
> Hello,
>
> > > > > pci-epc.h doesn't define the members of the pci_epf_header struct, so
> > > > > trying to access them results in errors like this:
> > > > >
> > > > > error: invalid use of undefined type 'struct pci_epf_header'
> > > > > 167 | val = hdr->vendorid;
> > > > >
> > > > > Instead let's include pci-epf.h which not only defines the
> > > > > pci_epf_header but also includes pci-epc.h.
> > >
> > > [...]
> > > > It is odd that the the build bot did not detect this...
> > >
> > > This is a bit of a surprise to me too, especially since none of the usual
> > > bots pick this up, and I can't seem to find such a failure in the nightly
> > > CI logs either.
> > >
> > > Alistair, how did you stumble into this issue? Also, which version or
> > > a tree would that be?
> >
> > I was building the kernel with this defconfig [1] inside OpenEmbedded.
> > It was the 6.4-rc7 kernel, specifically this one [2].
> >
> > 1: https://github.com/damien-lemoal/buildroot/blob/rockpro64_ep_v23/board/pine64/rockpro64_ep/linux.config
> > 2: https://github.com/damien-lemoal/linux/tree/rockpro64_ep_v23
>
> Thank you! Much appreciated.
>
> So, the vanilla kernel does not have headers arranged like this custom tree
> that Damien maintains for his own needs, per:
>
> - https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/pci-epc.h
> - https://elixir.bootlin.com/linux/v6.4-rc7/source/include/linux/pci-epf.h
>
> I suppose, there are some changes that break it for you, for example:
>
> https://github.com/damien-lemoal/linux/commit/c7aa8ddd76a141b975a097532050a76c6a58c436
>
> So, I cannot take this patch as it would break vanilla kernel for us. :)

Urgh! Sorry about that. I should have double checked if it was an
issue upstream. Thanks for looking into it though

Alistair

>
> Try building using vanilla kernel, and see if that helps.
>
> Krzysztof