Message-ID: <ZvGT4+UaPOmnGh4M@lizhi-Precision-Tower-5810>
Date: Mon, 23 Sep 2024 12:14:27 -0400
From: Frank Li <Frank.li@....com>
To: Rob Herring <robh@...nel.org>
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Abraham I <kishon@...nel.org>,
	Saravana Kannan <saravanak@...gle.com>,
	Jingoo Han <jingoohan1@...il.com>,
	Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Richard Zhu <hongxing.zhu@....com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, linux-pci@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...s.com, linux-arm-kernel@...ts.infradead.org,
	imx@...ts.linux.dev,
	Krzysztof Wilczyński <kwilczynski@...nel.org>
Subject: Re: [PATCH 0/9] PCI-EP: Add 'ranges' support for PCI endpoint devices

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@....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@...10000 {
> > >                 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

Powered by blists - more mailing lists