[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range

[Gabriele Paoloni]

> -----Original Message-----
> From: Kishon Vijay Abraham I [mailto:kishon at ti.com]
> Sent: Friday, July 31, 2015 3:57 PM
> To: Gabriele Paoloni; Rob Herring
> Cc: Bjorn Helgaas; arnd at arndb.de; lorenzo.pieralisi at arm.com; Wangzhou
> (B); robh+dt at kernel.org; james.morse at arm.com; Liviu.Dudau at arm.com;
> linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> devicetree at vger.kernel.org; Yuanzhichang; Zhudacai; zhangjukuo;
> qiuzhenfa; Liguozhu (Kenneth); Jingoo Han; Pratyush Anand; Arnd
> Bergmann; Arnd Bergmann
> Subject: Re: [PATCH v6] PCI: Store PCIe bus address in struct
> of_pci_range
> 
> +Arnd
> 
> Hi,
> 
> On Friday 31 July 2015 07:55 PM, Gabriele Paoloni wrote:
> > [+cc Kishon]
> >
> >> -----Original Message-----
> >> From: linux-pci-owner at vger.kernel.org [mailto:linux-pci-
> >> owner at vger.kernel.org] On Behalf Of Rob Herring
> >> Sent: Thursday, July 30, 2015 9:42 PM
> >> To: Gabriele Paoloni
> >> Cc: Bjorn Helgaas; arnd at arndb.de; lorenzo.pieralisi at arm.com;
> Wangzhou
> >> (B); robh+dt at kernel.org; james.morse at arm.com; Liviu.Dudau at arm.com;
> >> linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> >> devicetree at 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 at huawei.com> wrote:
> >>>> -----Original Message-----
> >>>> From: Bjorn Helgaas [mailto:bhelgaas at 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 at vger.kernel.org [mailto:linux-pci-
> >>>>>> owner at 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


In this patch you use the mask into designware....instead the approach 
proposed by Rob means to have the mask declared in the dra7 driver and
you modified the pp members in dra7xx_pcie_host_init by masking them...

BTW good to have Arnd opinion too..
> 
> Thanks
> Kishon