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

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

[Alistair Francis]

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 <alistair@alistair23.me>
---
 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

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

[Damien Le Moal]

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 <alistair@alistair23.me>

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

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

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

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

[Krzysztof Wilczyński]

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

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

[Alistair Francis]

On Wed, Jul 5, 2023 at 9:44 PM Krzysztof Wilczyński <kw@linux.com> 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

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

[Krzysztof Wilczyński]

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

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

[Damien Le Moal]

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

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

[Alistair Francis]

On Wed, Jul 5, 2023 at 11:15 PM Krzysztof Wilczyński <kw@linux.com> 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