[+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? > In looking at this, I'm curious why only 2 ATU viewports are used by > default as this causes switching on config accesses. Probably some > configuration only has 2 viewports. This would not even work on SMP > systems! Memory and i/o accesses do not have any lock. A config access > on one core will temporarily disable the i/o or memory viewport which > will cause an abort, dropped, or garbage data on an i/o or memory > access from another core. You would have to be accessing cfg space on > a bus other than the root bus, so maybe no one is doing that. > > >> > > >> > Speaking in general terms so far I've always seen linear mappings > >> > that differ by bitmask offset, however linear does not mean that > you > >> > cannot affect the lower bits: e.g. <0x0> to <0x0 + size> can map > to > >> > <0x0000cafe to 0x0000cafe + size>, but I guess that for HW design > >> reasons > >> > it is much easier to remap directly using a bitmask...but I was > not > >> sure > >> > and I didn't think about the problems that can arise with ACPI. > > For higher speed buses, the h/w designs are not going to be doing > complicated translations in order to meet timing requirements. At the > top level only the highest order bits are going to be looked at. For > downstream segments, the high order bits may get dropped to simplify > routing. If an IP block has full address bits in this case, they could > either be tied to 0 or tied off to the correct address. Either is > reasonable, so I'm sure there is h/w doing both. That doesn't mean h/w > designers haven't done something crazy, too. > > >> As I said, it wouldn't make sense for the bits that comprise the > >> offset into the window to change, so the mapping only has to be > linear > >> inside the window. > >> > >> For the bits outside the window offset, I don't know enough to say > >> whether masking is sufficient or safe. > >> > >> > If you think the bitmask is Ok then I can directly define it in > >> > designware and we can drop this patch. > >> > >> It's OK by me, but I know nothing about the actual hardware involved. > >> For the DesignWare question, I think you would just have to convince > >> Jingoo and Pratyush (cc'd). > > > > Yes actually looking at > > http://lxr.free- > electrons.com/source/arch/arm/boot/dts/spear1310.dtsi#L99 > > I can see that pci_addr=0 is mapped to bus_addr 0x80020000, so > clearing > > the top 4 bits would alter the behaviour of the current designware > > SW framework...now I don't know if DW needs only the low 28b of that > > value or not.... > > Given that most cases have same cpu and bus address, masking is not > the right solution in general. > > There is also a bug here BTW. pcie0 and pcie2 have the same CPU > address for the memory range. > > Rob > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
