On Mon, Dec 09, 2013 at 01:15:52PM -0700, Bjorn Helgaas wrote: > Is there actually some spec requirement about the access sizes that > must be supported by the hardware? If there is something, I'll gladly > keep the 32-bit access, but if it's only a tegra/mvebu-specific > restriction, then I think it needs to be handled in the PCI config > accessors for that hardware. The host register interface is not specified by any standard. I looked a for a bit and found other drivers with the same problem: arch/arm/mach-cns3xxx/pcie.c arch/arm/mach-iop13xx/pci.c arch/arm/mach-ks8695/pci.c arch/arm/mach-sa1100/pci-nanoengine.c [..] I stopped looking after that point.. > This spec statement (PCI (not PCIe) r3.0, sec 3.2.2.3.4) makes it > sound like tegra/mvebu is non-conforming and should deal with this > specially: > > "A function must not restrict the size of the access it supports in > Configuration Space. The configuration commands, like other commands, > allow data to be accessed using any combination of bytes (including a > byte, word, DWORD, or non-contiguous bytes) and multiple data phases > in a burst. The target is required to handle any combination of byte > enables." This verbage is talking about the target/responder, not the host. Any target must respond to all possible sizes for all possible config registers, including combinations that even x86 can't issue (like a BE pattern of b0110) > I think the tegra/mvebu accessors should be able to use the address > and access size to figure out what we're trying to do. If we're doing > a WORD write to PCI_IO_BASE, it can turn that into: 32-bit read, clear > the upper 16 bits (secondary status), insert the PCI_IO_BASE part, > 32-bit write. If we're doing a 16-bit write to PCI_SEC_STATUS, it can > turn that into: 32-bit read, insert upper 16 bits (secondary status), > leave lower 16 bits alone, 32-bit write. Seems reasonable, but this obviously shouldn't be open coded in every driver.. Maybe pci_ops should gain a read32/write32 and HW that can only do 32 bit access should not define read/write. The core code could provide read/write functions to do the bit mangling for all possibilities? That would be very clear what is going on. Jason -- 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