| Message ID | 20241024-pcie_ep_range-v4-1-08f8dcd4e481@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: ep: dwc/imx6: Add bus address support for PCI endpoint devices | expand |
On Thu, Oct 24, 2024 at 04:41:43PM -0400, Frank Li wrote: > 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 └───────┘ └─────────┘ > > Add `bus_addr_base` to configure the outbound window address for CPU write. > The bus fabric generally passes the same address to the PCIe EP controller, > but some bus fabrics convert the address before sending it to the PCIe EP > controller. > > Above diagram, CPU write data to outbound windows address 0x7000_0000, > Bus fabric convert it to 0x8000_0000. ATU should use bus address > 0x8000_0000 as input address and convert to PCI address 0xA000_0000. Thanks for the diagram and description. I don't think the top half is relevant to *this* patch, is it? I think this patch is only concerned with the address translations between the CPU in the endpoint and the PCI bus address. In this case it happens in two steps: the bus fabric applies one offset, and the ATU applies a second offset. Unless the top half is relevant, I would omit it and simply use something like this: Endpoint ┌───────────────────────────────────────────────┐ │ pcie-ep@5f010000 │ │ ┌────────────────┐│ │ │ Endpoint ││ │ │ PCIe ││ │ │ Controller ││ │ bus@5f000000 │ ││ │ ┌──────────┐ │ ││ │ │ │ Outbound Transfer ││ │┌─────┐ │ Bus ┼─────►│ ATU ──────────┬┬─────► ││ │ │ Fabric │Bus │ ││PCI Addr ││ CPU ├───►│ │Addr │ ││0xA000_0000 ││ │CPU │ │0x8000_0000 ││ │└─────┘Addr└──────────┘ │ ││ │ 0x7000_0000 └────────────────┘│ └───────────────────────────────────────────────┘ If you don't want a big "Endpoint" box including the CPU and bus fabric, that's OK with me, too. I added it because everything on the PCI side only sees TLPs that contain PCI bus addresses, and can't tell anything about the internal implementation of the Endpoint. > Previously, `cpu_addr_fixup()` was used to handle address conversion. Now, > the device tree provides this information, preferring a common method. > > bus@5f000000 { > compatible = "simple-bus"; > ranges = <0x80000000 0x0 0x70000000 0x10000000>; > > pcie-ep@5f010000 { > reg = <0x5f010000 0x00010000>, > <0x80000000 0x10000000>; > reg-names = "dbi", "addr_space"; > ... > }; > ... > }; I guess bus@5f000000 includes a "ranges" property because that translation from 0x7000_0000 -> 0x8000_0000 is fixed or at least not touched by Linux? And the pcie-ep@5f010000 address translation from 0x8000_0000 to 0xA000_0000 *is* programmed by Linux and therefore can't be described by a DT? But I guess Linux only programs the *PCI* side, and the parent side (0x8000_0000) is fixed? AFAICT, the "reg = <0x5f010000 0x00010000>" part is not relevant here. I guess this implementation assumes there's only a single aperture through the Bus Fabric, right? And also a single ATU aperture through the endpoint PCIe controller? And also that there's only one layer of Bus Fabric address translation? The fact that only a few DWC controllers have this translation suggests that this part of the picture might be external to the DWC IP and there could be more variation. But I guess there's no point in adding code for topologies that don't exist; we can deal with that if the need ever arises. > 'ranges' in bus@5f000000 descript how address convert from CPU address > to bus address. > > Use `of_property_read_reg()` to obtain the bus address and set it to the > ATU correctly, eliminating the need for vendor-specific cpu_addr_fixup(). > > Add 'using_dtbus_info' to indicate device tree reflect correctly bus > address translation in case break compatibility. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > Change from v3 to v4 > - change bus_addr_base to u64 to fix 32bit build error > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202410230328.BTHareG1-lkp@intel.com/ > > Change from v2 to v3 > - Add using_dtbus_info to control if use device tree bus ranges > information. > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 +++++++++++++- > drivers/pci/controller/dwc/pcie-designware.h | 9 +++++++++ > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 43ba5c6738df1..81b4057befa62 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -9,6 +9,7 @@ > #include <linux/align.h> > #include <linux/bitfield.h> > #include <linux/of.h> > +#include <linux/of_address.h> > #include <linux/platform_device.h> > > #include "pcie-designware.h" > @@ -294,7 +295,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > > atu.func_no = func_no; > atu.type = PCIE_ATU_TYPE_MEM; > - atu.cpu_addr = addr; > + atu.cpu_addr = addr - ep->phys_base + ep->bus_addr_base; Tangent: Maybe dw_pcie_ob_atu_cfg.cpu_addr isn't exactly the right name, since it now contains an address that is not a CPU physical address. Not a question for *this* patch though. > atu.pci_addr = pci_addr; > atu.size = size; > ret = dw_pcie_ep_outbound_atu(ep, &atu); > @@ -861,6 +862,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > struct device *dev = pci->dev; > struct platform_device *pdev = to_platform_device(dev); > struct device_node *np = dev->of_node; > + int index; > > INIT_LIST_HEAD(&ep->func_list); > > @@ -873,6 +875,16 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > return -EINVAL; > > ep->phys_base = res->start; > + ep->bus_addr_base = ep->phys_base; > + > + if (pci->using_dtbus_info) { > + index = of_property_match_string(np, "reg-names", "addr_space"); > + if (index < 0) > + return -EINVAL; > + > + of_property_read_reg(np, index, &ep->bus_addr_base, NULL); > + } If this translation were fixed, I suppose we'd extract something from a "ranges" property that contains (child-bus-address, parent-bus-address) information. So I suppose "addr_space" contains a fixed parent-bus-address, and is setting the child (PCI) bus address, right? If so, I might add a comment here for other readers who come this way. (And me, because I won't remember the next time I read it :) > ep->addr_size = resource_size(res); > > if (ep->ops->pre_init) > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 347ab74ac35aa..f10b533b04f77 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -410,6 +410,7 @@ struct dw_pcie_ep { > struct list_head func_list; > const struct dw_pcie_ep_ops *ops; > phys_addr_t phys_base; > + u64 bus_addr_base; > size_t addr_size; > size_t page_size; > u8 bar_to_atu[PCI_STD_NUM_BARS]; > @@ -463,6 +464,14 @@ struct dw_pcie { > struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS]; > struct gpio_desc *pe_rst; > bool suspended; > + /* > + * Use device tree 'ranges' property of bus node instead using > + * cpu_addr_fixup(). Some old platform dts 'ranges' in bus node may not > + * reflect real hardware's behavior. In case break these platform back > + * compatibility, add below flags. Set it true if dts already correct > + * indicate bus fabric address convert. > + */ > + bool using_dtbus_info; > }; > > #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp) > > -- > 2.34.1 >
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 43ba5c6738df1..81b4057befa62 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -9,6 +9,7 @@ #include <linux/align.h> #include <linux/bitfield.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/platform_device.h> #include "pcie-designware.h" @@ -294,7 +295,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, atu.func_no = func_no; atu.type = PCIE_ATU_TYPE_MEM; - atu.cpu_addr = addr; + atu.cpu_addr = addr - ep->phys_base + ep->bus_addr_base; atu.pci_addr = pci_addr; atu.size = size; ret = dw_pcie_ep_outbound_atu(ep, &atu); @@ -861,6 +862,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) struct device *dev = pci->dev; struct platform_device *pdev = to_platform_device(dev); struct device_node *np = dev->of_node; + int index; INIT_LIST_HEAD(&ep->func_list); @@ -873,6 +875,16 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) return -EINVAL; ep->phys_base = res->start; + ep->bus_addr_base = ep->phys_base; + + if (pci->using_dtbus_info) { + index = of_property_match_string(np, "reg-names", "addr_space"); + if (index < 0) + return -EINVAL; + + of_property_read_reg(np, index, &ep->bus_addr_base, NULL); + } + ep->addr_size = resource_size(res); if (ep->ops->pre_init) diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 347ab74ac35aa..f10b533b04f77 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -410,6 +410,7 @@ struct dw_pcie_ep { struct list_head func_list; const struct dw_pcie_ep_ops *ops; phys_addr_t phys_base; + u64 bus_addr_base; size_t addr_size; size_t page_size; u8 bar_to_atu[PCI_STD_NUM_BARS]; @@ -463,6 +464,14 @@ struct dw_pcie { struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS]; struct gpio_desc *pe_rst; bool suspended; + /* + * Use device tree 'ranges' property of bus node instead using + * cpu_addr_fixup(). Some old platform dts 'ranges' in bus node may not + * reflect real hardware's behavior. In case break these platform back + * compatibility, add below flags. Set it true if dts already correct + * indicate bus fabric address convert. + */ + bool using_dtbus_info; }; #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
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 └───────┘ └─────────┘ Add `bus_addr_base` to configure the outbound window address for CPU write. The bus fabric generally passes the same address to the PCIe EP controller, but some bus fabrics convert the address before sending it to the PCIe EP controller. Above diagram, CPU write data to outbound windows address 0x7000_0000, Bus fabric convert it to 0x8000_0000. ATU should use bus address 0x8000_0000 as input address and convert 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@5f000000 { compatible = "simple-bus"; ranges = <0x80000000 0x0 0x70000000 0x10000000>; pcie-ep@5f010000 { reg = <0x5f010000 0x00010000>, <0x80000000 0x10000000>; reg-names = "dbi", "addr_space"; ... }; ... }; 'ranges' in bus@5f000000 descript how address convert from CPU address to bus address. Use `of_property_read_reg()` to obtain the bus address and set it to the ATU correctly, eliminating the need for vendor-specific cpu_addr_fixup(). Add 'using_dtbus_info' to indicate device tree reflect correctly bus address translation in case break compatibility. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Change from v3 to v4 - change bus_addr_base to u64 to fix 32bit build error | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410230328.BTHareG1-lkp@intel.com/ Change from v2 to v3 - Add using_dtbus_info to control if use device tree bus ranges information. --- drivers/pci/controller/dwc/pcie-designware-ep.c | 14 +++++++++++++- drivers/pci/controller/dwc/pcie-designware.h | 9 +++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)