On Tue, Sep 29, 2015 at 9:47 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
> + several affected driver maintainers
>
> On Mon, Sep 28, 2015 at 6:05 PM, Russell King - ARM Linux
> <linux@xxxxxxxxxxxxxxxx> wrote:
>> On Mon, Sep 28, 2015 at 05:42:20PM -0500, Rob Herring wrote:
>>> On Mon, Sep 28, 2015 at 5:08 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>>> > Hi Rob,
>>> >
>>> > Russell pointed out a problem with 1f94a94f67e1 ("PCI: Add generic config
>>> > accessors"). pci_generic_config_write32() does a read/modify/write if the
>>> > size is less than 32 bits, so I think we have problem if this is used with
>>> > 8- or 16-bit registers that contain RW1C bits. Any thoughts on how we can
>>> > fix this?
>>>
>>> My series didn't change access sizes unless I made a made a mistake
>>> somewhere, so the problem should have existed before.
>>>
>>> Is it known addresses we need to deal with? We could do special case
>>> handling of certain addresses to mask out RW1C bits. This could be in
>>> the generic functions or in wrappers around the generic functions.
>>> There's already some similar examples of special address handling
>>> IIRC.
>>
>> I think this originally came into being because Intel decided that their
>> IOP platforms wouldn't support anything except 32-bit accesses, and
>> cargo cult programming took over. I complained about it on the Intel IOP
>> platforms, but obviously if the hardware doesn't support anything else
>> then your stuck with it.
>
> What about SA1100 nanoengine?
>
> Others are Tegra, XGene, iproc, and maybe rcar. These are all
> relatively active platforms, so hopefully some testing can be done.
On X-Gene, so far I don't see anything strange after switching to use
pci_generic_config_write32 with all the cards that I can test.
But looks like it's still safer to use pci_generic_config_write
instead as this will make sure no unexpected read-modify-write will
happen?
>
> Rob
>
>> However, what's utterly wrong is that the generic PCI layer should give
>> credence to this crap - it should _not_ provide a generic helper for
>> performing only 32-bit accesses which is non-conformant to the PCI specs,
>> at least without a big fat warning about it.
>>
>> The problem is not limited to what the PCI specs say - remember, vendors
>> are free to add their own PCI configuration registers: you don't know
>> where these registers are in the PCI config address space. What's even
>> worse is that you don't know what use vendors have put the neighbouring
>> bytes of PCI config space to in the case of an 8-bit or 16-bit vendor
>> register.
>>
>> For the standard PCI config registers, some addresses are well known.
>> For example, the PCI status register is at fixed address 6, sharing the
>> PCI command register at address 4.
>>
>> Others depend on the function, and whether there's capabilities present.
>> For example, the PCIe device status register shares the same double-word
>> as the PCI device control register, and the status register contains RW1C
>> bits.
>>
>> The same is true for the PCIe AER capability.
>>
>> --
>> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
>> according to speedtest.net.
> --
> 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
Regards,
Duc Dang.
--
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
