Re: [PATCH 0/9] PCI-EP: Add 'ranges' support for PCI endpoint devices

Frank Li
2024-09-23T16:16:25Z

On Sat, Sep 21, 2024 at 03:18:50PM -0400, Frank Li wrote:
> On Sat, Sep 21, 2024 at 09:43:17AM -0500, Rob Herring wrote:
> > On Thu, Sep 19, 2024 at 5:03 PM Frank Li <Frank.Li@nxp.com> wrote:
> > >
> > > The PCI bus device tree supports 'ranges' properties that indicate
> > > how to convert PCI addresses to CPU addresses. Many PCI controllers
> > > are dual-role controllers, supporting both Root Complex (RC) and
> > > Endpoint (EP) modes. The EP side also needs similar information for
> > > proper address translation.
> > >
> > > This commit introduces several changes to add 'ranges' support for
> > > PCI endpoint devices:
> > >
> > > 1. **Modify of_address.c**: Add support for the new `device_type`
> > > "pci-ep", enabling it to parse 'ranges' using the same functions
> > > as for PCI devices.
> > >
> > > 2. **Update DesignWare PCIe EP driver**: Enhance the driver to
> > > support 'ranges' when 'addr_space' is missing, maintaining
> > > compatibility with existing drivers.
> > >
> > > 3. **Update binding documentation**: Modify the device tree bindings
> > > to include 'ranges' support and make 'addr_space' an optional
> > > entry in 'reg-names'.
> > >
> > > 4. **Add i.MX8QXP EP support**: Incorporate support for the
> > > i.MX8QXP PCIe EP in the driver.
> > >
> > > i.MX8QXP PCIe dts is upstreaming. Below is pcie-ep part.
> > >
> > > pcieb_ep: pcie-ep@5f010000 {
> > > compatible = "fsl,imx8q-pcie-ep";
> > > reg = <0x5f010000 0x00010000>;
> > > reg-names = "dbi";
> > > #address-cells = <3>;
> > > #size-cells = <2>;
> > > device_type = "pci-ep";
> > > ranges = <0x82000000 0 0x80000000 0x70000000 0 0x10000000>;
> >
> > How does a PCI endpoint set PCI addresses? Those get assigned by the
> > PCI host system. They can't be static in DT.
>
> PCI address is set by other channel, such as RC write some place in bar0.
>
> It indicates EP side outbound windows mapping. See below detail.
>
>
> Endpoint Root complex
> ┌───────┐ ┌─────────┐
> ┌─────┐ │ EP │ │ │ ┌─────┐
> │ │ │ Ctrl │ │ │ │ CPU │
> │ DDR │ │ │ │ ┌────┐ │ └──┬──┘
> │ │◄──────┼─ATU ◄─┼────────┼─┤BarN│◄─┼─────────┘
> │ │ │ │ │ └────┘ │ Outbound Transfer
> └─────┘ │ │ │ │
> │ │ │ │
> │ │ │ │
> │ │ │ │ Inbound Transfer
> │ │ │ │ ┌──▼──┐
> ┌───────┐ │ │ │ ┌───────┼─────►│DDR │
> │ │ outbound Transfer* │ │ │ └─────┘
> ┌─────┐ │ Bus ┼─────►│ ATU ─┬────────┼─┘ │
> │ │ │ Fabric│Bus │ │ PCI Addr │
> │ CPU ├───►│ │Addr │ │ 0xA000_0000 │
> │ │CPU │ │0x8000_0000 │ │ │
> └─────┘Addr└───────┘ │ │ │ │
> 0x7000_0000 └───────┘ └─────────┘
>
>
> This ranges descript above diagram Endpoint outbound Transfer*'s
> information. There are address space (previous use addr_space in reg-name)
> indicate such informaiton, such as [0x7000_00000, 0xB000_0000] as PCI EP
> outbound windows. when cpu write 0x7000_0000, data will write to EP ctrl,
> the ATU in EP ctrl convert to PCI address such 0xA000,0000, then write
> to RC's DDR>
>
> The PCI Addr 0xA000_0000 information was sent to EP driver by use other
> channel, such as RC write it some place in Bar0.
>
> The 'range' here indicated EP side's outbound windows information. Most
> system CPU address is the same as bus address. but in imx8q, it is
> difference. Bus fabric convert 0x7000_0000 to 0x8000_00000.
>
> So need range indicate such address convertion.
>
> >
> > If you need the PCI address, just read your BAR registers.
> >
> > In general, why do you need this when none of the other PCI endpoint
> > drivers have needed this?
>
> Most system, the address is the same. Some system convert is simple, just
> cut some high address bit, so their driver hardcode it. Maybe imx8QM have
> first one, they have more than one controller and address map is not
> such simple.
>
> We use customer dt property in downstream kernel, but I think common
> solution should be better, other drivers can remove their hardcode in
> future. And it will be more symmetry with PCI host side's property.

I found a more simple the method, will post v2 soon.

Frank

>
> Frank
> >
> > Rob