Re: [v2] PCI: cadence: Add configuration space capability search API

[Hans Zhang]

On 2025/3/9 18:02, Siddharth Vadapalli wrote:
> > Hi Siddharth,
> >
> > Prior to this patch, I don't think hard-coded is that reasonable. Because
> > the SOC design of each company does not guarantee that the offset of each
> > capability is the same. This parameter can be configured when selecting PCIe
> > configuration options. The current code that just happens to hit the offset
> > address is the same.
>
> 1. You are modifying the driver for the Cadence PCIe Controller IP and
> not the one for your SoC (a.k.a the application/glue/wrapper driver).
> 2. The offsets are tied to the Controller IP and not to your SoC. Any
> differences that arise due to IP Integration into your SoC should be
> handled in the Glue driver (the one which you haven't upstreamed yet).
> 3. If the offsets in the Controller IP itself have changed, this
> indicates a different IP version which has nothing to do with the SoC
> that you are using.
>
> Please clarify whether you are facing problems with the offsets due to a
> difference in the IP Controller Version, or due to the way in which the IP
> was integrated into your SoC.

Hi Siddharth,

I have consulted several PCIe RTL designers in the past two days. They 
told me that the controller IP of Synopsys or Cadence can be configured 
with the offset address of the capability. I don't think it has anything 
to do with SOC, it's just a feature of PCIe controller IP. In 
particular, the number of extended capability is relatively large. When 
RTL is generated, one more configuration may cause the offset addresses 
of extended capability to be different. Therefore, it is unreasonable to 
assign all the offset addresses in advance.

Here, I want to make Cadence PCIe common driver more general. When we 
keep developing new SoCs, the capability or extended capability offset 
address may eventually be inconsistent.

Thank you very much Siddharth for discussing this patch with me. I would 
like to know what other maintainers have to say about this.

> > You can refer to the pci_find_*capability() or dw_pcie_find_*capability API.
> > The meaning of their appearance is the same as what I said, and the design
> > of each company may be different.
>
> These APIs exits since there are multiple users with different Controllers
> (in the case of pci_find_*capability()) or different versions of the
> Controller (in the case of dw_pcie_find_*capability) due to which we cannot
> hard-code offsets. In the case of the Cadence PCIe Controller, I see only
> one user in Upstream Linux at the moment which happens to be the
> "pci-j721e.c" driver. Maybe there are other users, but the offsets seem
> to be compatible with their SoCs in that case.

Yes. Cadence definitely has a lot of customers, and from what I 
understand, not every customer wants to go upstream on their drive. Most 
of these customers are just local modifications, probably mimicking the 
API that the dwc driver already uses. We are, for example.

Best regards,
Hans