On 20.11.2018 20:13, Trent Piepho wrote: > On Tue, 2018-11-20 at 18:19 +0000, Leonard Crestez wrote: >> On Tue, 2018-11-20 at 17:56 +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 don't know exactly where this limitation comes from, I can indeed >> reproduce a stack dump when dumping pci config from /sys/ >> >> Unfortunately this seems to block access to registers used for >> functionality like interrupts. For example dw_handle_msi_irq does: >> >> dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + >> (i * MSI_REG_CTRL_BLOCK_SIZE), >> 4, &val); >> >> where PCI_MSI_INTR0_STATUS is 0x830. There are more accesses like this. >> >> Testing on 6dl-sabreauto (dts change required) with an ath9k pcie card >> with your series I sometimes get "irq 295: nobody cared" on boot. Maybe >> I'm missing something? > > On IMX7d, there are significant blocks of 00s in the config space, and > all 0xff at 0xb50 on up. > > I.e., significant portions are empty, in the middle of the config > space, not just at the end. > > But they can be read without problem. > > Perhaps imx6q aborts on a read of an unimplemented address instead of > returning zeros like imx7d. In that case it really needs something > more complex to prevent abort than just a length. Yeah it seems those SoCs behave differently. Describing a register set with holes will get complicated, I guess it would ask for a regmap... > > It also seems to me that this doesn't need to be in the internal pci > config access functions. The driver shouldn't be reading registers > that don't exist anyway. It's really about trying to fix sysfs access > to registers that don't exist. So maybe it should be done there. That was my first approach, see: https://lkml.org/lkml/2018/11/14/716 -- Stefan