Re: [PATCH v6] PCI: Store PCIe bus address in struct of_pci_range
From: Jingoo Han <jingoohan1@gmail.com>
Date: 2015-08-04 04:19:50
Also in:
linux-arm-kernel, linux-pci
Possibly related (same subject, not in this thread)
- 2015-07-30 · Re: [PATCH v6] PCI: Store PCIe bus address in struct of_pci_range · Bjorn Helgaas <bhelgaas@google.com>
- 2015-07-30 · Re: [PATCH v6] PCI: Store PCIe bus address in struct of_pci_range · Rob Herring <hidden>
- 2015-07-30 · Re: [PATCH v6] PCI: Store PCIe bus address in struct of_pci_range · Liviu Dudau <Liviu.Dudau@arm.com>
- 2015-07-30 · RE: [PATCH v6] PCI: Store PCIe bus address in struct of_pci_range · Gabriele Paoloni <hidden>
- 2015-07-30 · Re: [PATCH v6] PCI: Store PCIe bus address in struct of_pci_range · Zhou Wang <wangzhou1@hisilicon.com>
On 2015. 8. 3., at PM 8:18, Gabriele Paoloni [off-list ref] wrote:
Rob, Kishon what about the following solution?.... --- drivers/pci/host/pci-dra7xx.c | 12 ++++++++++++ drivers/pci/host/pcie-designware.c | 9 +++------
Hi Paoloni, OK, it looks good. I want you to revert the following patch. commit "f4c55c5a3f7f (PCI: designware: Program ATU with untranslated address)" Would you remove all '*_mode_base's? Best regards, Jingoo Han
quoted hunk ↗ jump to hunk
2 files changed, 15 insertions(+), 6 deletions(-)diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c index 80db09e..bb2635f 100644 --- a/drivers/pci/host/pci-dra7xx.c +++ b/drivers/pci/host/pci-dra7xx.c@@ -61,6 +61,7 @@#define PCIECTRL_DRA7XX_CONF_PHY_CS 0x010C #define LINK_UP BIT(16) +#define CPU_TO_BUS_ADDR 0x0FFFFFFF struct dra7xx_pcie { void __iomem *base;@@ -138,6 +139,17 @@ static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)static void dra7xx_pcie_host_init(struct pcie_port *pp) { + if (pp->io_mod_base) + pp->io_mod_base &= CPU_TO_BUS_ADDR; + + if (pp->mem_mod_base) + pp->mem_mod_base &= CPU_TO_BUS_ADDR; + + if (pp->cfg0_mod_base) { + pp->cfg0_mod_base &= CPU_TO_BUS_ADDR; + pp->cfg1_mod_base &= CPU_TO_BUS_ADDR; + } + dw_pcie_setup_rc(pp); dra7xx_pcie_establish_link(pp); if (IS_ENABLED(CONFIG_PCI_MSI))diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 69486be..06c682b 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c@@ -416,8 +416,7 @@ int dw_pcie_host_init(struct pcie_port *pp) pp->io_base = range.cpu_addr; /* Find the untranslated IO space address */ - pp->io_mod_base = of_read_number(parser.range - - parser.np + na, ns); + pp->io_mod_base = range.cpu_addr;; } if (restype == IORESOURCE_MEM) { of_pci_range_to_resource(&range, np, &pp->mem);@@ -426,8 +425,7 @@ int dw_pcie_host_init(struct pcie_port *pp) pp->mem_bus_addr = range.pci_addr; /* Find the untranslated MEM space address */ - pp->mem_mod_base = of_read_number(parser.range - - parser.np + na, ns); + pp->mem_mod_base = range.cpu_addr; } if (restype == 0) { of_pci_range_to_resource(&range, np, &pp->cfg);@@ -437,8 +435,7 @@ int dw_pcie_host_init(struct pcie_port *pp) pp->cfg1_base = pp->cfg.start + pp->cfg0_size; /* Find the untranslated configuration space address */ - pp->cfg0_mod_base = of_read_number(parser.range - - parser.np + na, ns); + pp->cfg0_mod_base = range.cpu_addr; pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size; }-- 1.9.1 > -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- > owner@vger.kernel.org] On Behalf Of Rob Herring > Sent: Friday, July 31, 2015 5:53 PM > To: Kishon Vijay Abraham I > Cc: Gabriele Paoloni; Bjorn Helgaas; arnd@arndb.de; > lorenzo.pieralisi@arm.com; Wangzhou (B); robh+dt@kernel.org; > james.morse@arm.com; Liviu.Dudau@arm.com; linux-pci@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; > Yuanzhichang; Zhudacai; zhangjukuo; qiuzhenfa; Liguozhu (Kenneth); > Jingoo Han; Pratyush Anand; Arnd Bergmann > Subject: Re: [PATCH v6] PCI: Store PCIe bus address in struct > of_pci_range > > On Fri, Jul 31, 2015 at 9:57 AM, Kishon Vijay Abraham I <kishon@ti.com> > wrote: >> +Arnd >> >> Hi, >> >>> On Friday 31 July 2015 07:55 PM, Gabriele Paoloni wrote: >>> [+cc Kishon] >>> >>>> -----Original Message----- >>>> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- >>>> owner@vger.kernel.org] On Behalf Of Rob Herring >>>> Sent: Thursday, July 30, 2015 9:42 PM >>>> To: Gabriele Paoloni >>>> Cc: Bjorn Helgaas; arnd@arndb.de; lorenzo.pieralisi@arm.com; > Wangzhou >>>> (B); robh+dt@kernel.org; james.morse@arm.com; Liviu.Dudau@arm.com; >>>> linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >>>> devicetree@vger.kernel.org; Yuanzhichang; Zhudacai; zhangjukuo; >>>> qiuzhenfa; Liguozhu (Kenneth); Jingoo Han; Pratyush Anand >>>> Subject: Re: [PATCH v6] PCI: Store PCIe bus address in struct >>>> of_pci_range >>>> >>>> On Thu, Jul 30, 2015 at 12:34 PM, Gabriele Paoloni >>>> <gabriele.paoloni@huawei.com> wrote: >>>>>> -----Original Message----- >>>>>> From: Bjorn Helgaas [mailto:bhelgaas@google.com] >>>>>> Sent: 30 July 2015 18:15 >>>>>> On Thu, Jul 30, 2015 at 04:50:55PM +0000, Gabriele Paoloni wrote: >>>>>>>> -----Original Message----- >>>>>>>> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- >>>>>>>> owner@vger.kernel.org] On Behalf Of Bjorn Helgaas >>>>>>>> Sent: Thursday, July 30, 2015 5:15 PM >>>>>>>> On Thu, Jul 30, 2015 at 01:52:13PM +0000, Gabriele Paoloni > wrote: >>>> >>>> [...] >>>> >>>>>>>>> I don’t think we should rely on [CPU] addresses...what if the >>>>>>>> intermediate >>>>>>>>> translation layer changes the lower significant bits of the >>>> "bus >>>>>>>> address" >>>>>>>>> to translate into a cpu address? >>>>>>>> >>>>>>>> Is it really a possiblity that the lower bits could be changed? >>>>>>> >>>>>>> I've checked all the current deignware users DTs except "pci- >>>>>> layerscape" >>>>>>> that I could not find: >>>>>>> spear1310.dtsi >>>>>>> spear1340.dtsi >>>>>>> dra7.dtsi >>>>>>> imx6qdl.dtsi >>>>>>> imx6sx.dtsi >>>>>>> keystone.dtsi >>>>>>> exynos5440.dtsi >>>>>>> >>>>>>> None of them modifies the lower bits. To be more precise the > only >>>> guy >>>>>>> that provides another translation layer is "dra7.dtsi": >>>>>>> axi0 >>>>>>> http://lxr.free- >>>> electrons.com/source/arch/arm/boot/dts/dra7.dtsi#L207 >>>>>>> >>>>>>> axi1 >>>>>>> http://lxr.free- >>>> electrons.com/source/arch/arm/boot/dts/dra7.dtsi#L241 >>>>>>> >>>>>>> For this case masking the top 4bits (bits28 to 31) should make > the >>>> job. >>>> >>>> IMO, we should just fix this case. After further study, I don't > think >>>> this is a DW issue, but rather an SOC integration issue. >>>> >>>> I believe you can just fixup the address in the pp->ops->host_init > hook. >>> >>> Yes I guess that I could just assign pp->(*)_mod_base to the CPU > address >>> in DW and mask it out in dra7xx_pcie_host_init()... >>> >>> Kishon, would you be ok with that? >> >> Initially I was using *base-mask* property from dt. Me and Arnd > (cc'ed) had >> this discussion [1] before we decided the current approach. It'll be > good to >> check with Arnd too. >> >> [1] -> http://lists.infradead.org/pipermail/linux-arm-kernel/2014- > May/253528.html > > The problem I have here is the use of ranges does not necessarily mean > fewer address bits are available. It can be used just for convenience > of not putting the full address into every node's reg property. And > vice versa, there are probably plenty of cases where we have the full > address in the nodes, but really only some of the address bits are > decoded at the IP block. Whether the address bits are present is > rarely cared about or known by s/w folks until you hit a problem like > this. Given this is an isolated case ATM, I would fix it in an > isolated way. It does not affect the binding and could be changed in > the kernel later if this becomes a common need. > > Rob > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html