On Fri, Jul 31, 2015 at 9:57 AM, Kishon Vijay Abraham I <kishon@xxxxxx> wrote: > +Arnd > > Hi, > > On Friday 31 July 2015 07:55 PM, Gabriele Paoloni wrote: >> [+cc Kishon] >> >>> -----Original Message----- >>> From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci- >>> owner@xxxxxxxxxxxxxxx] On Behalf Of Rob Herring >>> Sent: Thursday, July 30, 2015 9:42 PM >>> To: Gabriele Paoloni >>> Cc: Bjorn Helgaas; arnd@xxxxxxxx; lorenzo.pieralisi@xxxxxxx; Wangzhou >>> (B); robh+dt@xxxxxxxxxx; james.morse@xxxxxxx; Liviu.Dudau@xxxxxxx; >>> linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; >>> devicetree@xxxxxxxxxxxxxxx; 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@xxxxxxxxxx> wrote: >>>>> -----Original Message----- >>>>> From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx] >>>>> 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@xxxxxxxxxxxxxxx [mailto:linux-pci- >>>>>>> owner@xxxxxxxxxxxxxxx] 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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
