Re: Re: [PATCH v2 1/2] dt-bindings: PCI: eic7700: Add Eswin eic7700 PCIe host controller

[Bjorn Helgaas]

On Thu, Sep 18, 2025 at 01:35:40PM +0800, zhangsenchuan wrote:
> > -----Original Messages-----
> > From: "Bjorn Helgaas" <helgaas@xxxxxxxxxx>
> > On Fri, Aug 29, 2025 at 04:22:37PM +0800, zhangsenchuan@xxxxxxxxxxxxxxxxxx wrote:
> > > From: Senchuan Zhang <zhangsenchuan@xxxxxxxxxxxxxxxxxx>
> > > 
> > > Add Device Tree binding documentation for the ESWIN EIC7700
> > > PCIe controller module,the PCIe controller enables the core
> > > to correctly initialize and manage the PCIe bus and connected
> > > devices.

> > > +            resets = <&reset 8 (1 << 0)>,
> > > +                     <&reset 8 (1 << 1)>,
> > > +                     <&reset 8 (1 << 2)>;
> > > +            reset-names = "cfg", "powerup", "pwren";
> > > +            interrupts = <220>, <179>, <180>, <181>, <182>, <183>, <184>, <185>, <186>;
> > > +            interrupt-names = "msi", "inta", "intb", "intc", "intd",
> > > +                              "inte", "intf", "intg", "inth";
> > > +            interrupt-parent = <&plic>;
> > > +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> > > +            interrupt-map = <0x0 0x0 0x0 0x1 &plic 179>,
> > > +                            <0x0 0x0 0x0 0x2 &plic 180>,
> > > +                            <0x0 0x0 0x0 0x3 &plic 181>,
> > > +                            <0x0 0x0 0x0 0x4 &plic 182>;
> > > +            device_type = "pci";
> > > +            num-lanes = <0x4>;
> > 
> > num-lanes and perst are per-Root Port items.  Please put anything
> > related specifically to the Root Port in its own stanza to make it
> > easier to support multiple Root Ports in future versions of the
> > hardware.
> > 
> > See
> > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/
> > for examples of how to do this.
> 
> Thank you very much for your review.
> I think the suggestions you put forward are very good，I placed
> perst in the root port as per your suggestion.
> 
> I'm a bit confused about the "num-lanes" attribute.  The "num-lanes"
> attribute will be parsed in the "pcie-designware.c" file. In the
> "pcie-designware-host.c" file, When our driver calls the
> dw_pcie_host_init function for initialization, the attribute
> "num_lanes" will be judged. If the attribute is available, use the
> value parsed from the device tree. If the attribute cannot be
> obtained from the node, the lanes supported by the hardware default
> will be obtained by reading the register.Can I avoid reparsing the
> num-lanes attribute?
> 
> I saw vendors based on Synopsys implementation. They separated the
> root port node and did not place "num-lanes" in the root port node.
> For examples:
> hisilicon,kirin-pcie.yaml
> qcom,pcie-sc7280.yaml
> qcom,pcie-sa8255p.yaml

This is currently a problem because the DWC core doesn't know to look
for "num-lanes" in a Root Port node.  Similar situation in the NXP
driver: https://lore.kernel.org/r/20250917212833.GA1873293@bhelgaas

Would it work for you to add a Root Port parser in eic7700, similar to
mvebu_pcie_parse_port() or qcom_pcie_parse_port() that would get
"num-lanes"?

It looks like that would keep the DWC core from setting num-lanes.

Eventually the DWC core should look first for a Root Port node before
falling back to the current behavior of looking in the host bridge
node.  If/when that happens, we should be able to remove the num-lanes
parsing in eic7700 and similar drivers.

I'd like to separate the per-Root Port things in the devicetree from
the beginning because once devicetrees are out in the world, we
basically have to support their structure forever.

Bjorn