On 12.02.2019 12:33, Lorenzo Pieralisi wrote: > On Tue, Feb 12, 2019 at 09:54:54AM +0100, Lucas Stach wrote: >> Hi Bjorn, >> >> Am Montag, den 11.02.2019, 15:39 -0600 schrieb Bjorn Helgaas: >> > On Wed, Feb 06, 2019 at 10:57:32AM +0100, Stefan Agner wrote: >> > > Define the length of the DBI registers. This makes sure that >> > > the kernel does not access registers beyond that point, avoiding >> > > the following abort on a i.MX 6Quad: >> > > # cat /sys/devices/soc0/soc/1ffc000.pcie/pci0000\:00/0000\:00\:00.0/config >> > > [ 100.021433] Unhandled fault: imprecise external abort (0x1406) at 0xb6ea7000 >> > > ... >> > > [ 100.056423] PC is at dw_pcie_read+0x50/0x84 >> > > [ 100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48 >> > > ... >> > >> > I assume this problem happens when using the pci_read_config() path or >> > something similar? >> > >> > Could this be solved using pci_dev.cfg_size instead of building a new >> > dwc-specific mechanism? There are some quirks that set dev->cfg_size >> > to keep from reading past certain points in config space, e.g., >> > quirk_citrine(), quirk_nfp6000(). >> > >> > I'm not necessarily opposed to doing it in dwc, but maybe there's some >> > advantage in reducing the number of ways of doing the same thing. >> >> This actually started out as a quirk changing the cfg size. But the >> valid config space size seems to be different between root ports that >> share the same (broken) device ID (Synopsys abcd), so I doubt that this >> would be easier and/or any cleaner to implement as a quirk. For reference, this was the initial patch using DECLARE_PCI_FIXUP_HEADER: https://lore.kernel.org/lkml/20181019111350.6170-1-stefan@xxxxxxxx/T/#u > > There are two things here: matching the root port and setting > the cfg size limit. > > I agree with Bjorn that the cfg size limit, given that it is > implemented in core code should be leveraged instead of reinventing > the wheel to solve the same problem in driver specific code. Seems sensible yes. > > In the quirk code I do not think it is that complicated to retrieve > the IMX variant to apply the quirk accordingly on the pci_dev. It seems that drivers/pci/controller/pcie-iproc.c uses FIXUP functions which access driver specific structs. I think we can get from (struct pci_host_bridge *)->sysdata to struct pcie_port * and from there to struct dw_pci. I will give this a try next week. > > Please let me know if that's feasible so that I can drop the > patches from the branch and update it with a new version. > Fine for me. -- Stefan
