Message-ID: <266CE37A-A50B-498F-8BE7-5D6449F72E5B@linaro.org>
Date: Fri, 17 Jan 2025 20:05:16 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Bjorn Helgaas <helgaas@...nel.org>, Frank Li <Frank.li@....com>
CC: Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
 Jingoo Han <jingoohan1@...il.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof Wilczyński <kw@...ux.com>,
 Bjorn Helgaas <bhelgaas@...gle.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>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, imx@...ts.linux.dev
Subject: Re: [PATCH v8 3/7] PCI: dwc: ep: Add bus_addr_base for outbound window



On January 17, 2025 4:19:02 AM GMT+05:30, Bjorn Helgaas <helgaas@...nel.org> wrote:
>On Thu, Jan 16, 2025 at 03:02:44PM -0500, Frank Li wrote:
>> On Thu, Jan 16, 2025 at 01:45:58PM -0600, Bjorn Helgaas wrote:
>> > On Thu, Jan 16, 2025 at 01:04:16PM -0500, Frank Li wrote:
>> > > On Thu, Jan 16, 2025 at 09:32:39AM -0600, Bjorn Helgaas wrote:
>> > > > On Tue, Nov 19, 2024 at 02:44:21PM -0500, Frank Li wrote:
>> > > > >                    Endpoint
>> > > > >   ┌───────────────────────────────────────────────┐
>> > > > >   │                             pcie-ep@...10000  │
>> > > > >   │                             ┌────────────────┐│
>> > > > >   │                             │   Endpoint     ││
>> > > > >   │                             │   PCIe         ││
>> > > > >   │                             │   Controller   ││
>> > > > >   │           bus@...00000      │                ││
>> > > > >   │           ┌──────────┐      │                ││
>> > > > >   │           │          │ Outbound Transfer     ││
>> > > > >   │┌─────┐    │  Bus     ┼─────►│ ATU  ──────────┬┬─────►
>> > > > >   ││     │    │  Fabric  │Bus   │                ││PCI Addr
>> > > > >   ││ CPU ├───►│          │Addr  │                ││0xA000_0000
>> > > > >   ││     │CPU │          │0x8000_0000            ││
>> > > > >   │└─────┘Addr└──────────┘      │                ││
>> > > > >   │       0x7000_0000           └────────────────┘│
>> > > > >   └───────────────────────────────────────────────┘
>> > > > >
>> > > > > Use 'ranges' property in DT to configure the iATU outbound window address.
>> > > > > The bus fabric generally passes the same address to the PCIe EP controller,
>> > > > > but some bus fabrics map the address before sending it to the PCIe EP
>> > > > > controller.
>> > > > >
>> > > > > Above diagram, CPU write data to outbound windows address 0x7000_0000, Bus
>> > > > > fabric map it to 0x8000_0000. ATU should use bus address 0x8000_0000 as
>> > > > > input address and map to PCI address 0xA000_0000.
>> > > > >
>> > > > > Previously, 'cpu_addr_fixup()' was used to handle address conversion. Now,
>> > > > > the device tree provides this information, preferring a common method.
>> > > > >
>> > > > > bus@...00000 {
>> > > > > 	compatible = "simple-bus";
>> > > > > 	ranges = <0x80000000 0x0 0x70000000 0x10000000>;
>> > > > >
>> > > > > 	pcie-ep@...10000 {
>> > > > > 		reg = <0x80000000 0x10000000>;
>> > > > > 		reg-names ="addr_space";
>> > > > > 		...
>> > > > > 	};
>> > > > > 	...
>> > > > > };
>> > > > >
>> > > > > 'ranges' in bus@...00000 descript how address map from CPU address to bus
>> > > > > address.
>> > > >
>> > > > Shouldn't there also be a pcie-ep@...10000 'ranges' property to
>> > > > describe the translation for the window from bus addr 0x8000_0000 to
>> > > > PCI addr 0xA000_0000?
>> > >
>> > > Needn't 'ranges' under pcie-ep@...10000 because history reason. DWC use
>> > > reg-names "addr_space" descript outbound windows space.
>> >
>> > If reg-name "addr_space" is used instead of 'ranges' for some
>> > historical reason, we should mention that in the commit log so people
>> > don't assume that this difference is the way it's *supposed* to be
>> > done.
>> 
>> How about add comments after
>> 
>> reg-names ="addr_space"; // Indicate EP outbound windows space instead use
>> ranges by histortical reason.
>
>OK, that seems reasonable.
>

Unfortunately not. Please see below.

>Where does the 0xA000_0000 PCI address come from?  I assume that's in
>DT somewhere too?
>

No, PCI address is from host address space, hence it cannot be described in DT. That's the reason why 'addr_space' reg property is used to define outbound window region. iATU will map the host PCI address to endpoint outbound window region dynamically based on usecase (like mapping the buffer in host address space).

The translation between CPU:BUS address space is a hardware property, hence using 'ranges' to describe them makes sense.

But the same cannot be applied for BUS:PCI address space. Maybe the diagram had misled you thinking that PCI address is also static.

- Mani

>Is there a binding in the tree that would take advantage of this patch
>that I can look at?  arch/arm64/boot/dts/freescale/imx8-ss-hsio.dtsi 
>has bus@...00000 that does this translation, but I don't see any
>endpoint mode that uses it.
>
>> > > All regs need call of_property_read_reg() to get untranslated address.
>> > > ranges:  use "parent_bus_addr" in [1].
>> >
>> > I think we should at least use the same name ("parent_bus_addr", not
>> > "bus_addr_base") and probably also figure out a wrapper or similar way
>> > to use 'ranges' for future endpoint drivers and fall back to
>> > "addr_space" for DWC.
>> 
>> Okay for name parent_bus_addr.
>> Do you need me to respin it? Or you change it by yourself?
>
>I can do that.
>
>Bjorn
>
>> > > > [1] https://lore.kernel.org/r/20241119-pci_fixup_addr-v8-1-c4bfa5193288@nxp.com

மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists