Commit 4f2e2280862a ("RFC: arm: pci: Add PCI cam support to PCI-E ecam driver")

Pali Rohár pali at kernel.org
Thu Jan 13 13:34:32 CET 2022


Hello!

Now I see that you have merged commit 4f2e2280862a ("RFC: arm: pci: Add
PCI cam support to PCI-E ecam driver"). It adds some "PCI cam support"
with generic DT binding "pci-host-cam-generic".

I have tried to find some information about it, but in PCIe
specification there is nothing like PCI CAM. And neither in old PCI
local bus 2.x or 3.x specs.

This access looks like a mix of "PCI Configuration Mechanism #1" and
"PCI Configuration Mechanism #2" from PCI Local Bus Specification
(rev 2.1, sections 3.7.4.1 and 3.7.4.2) and incompatible with both of
them. It has layout similar to Mechanism #1 and access similar to #2.

PCI Configuration Mechanism #1 uses two registers, one which select
config address and second for accessing config space (selected address).
But that U-Boot "PCI CAM" is implemented as memory mapped address space,
something similar to PCI Configuration Mechanism #2 but with different
layout. Also that "PCI CAM" does not set "enable" bit which is per PCI
Configuration Mechanism #1 required to access PCI config space.

Recently I converted all PCI drivers in U-Boot which uses PCI
Configuration Mechanism #1 to use PCI_CONF1_ADDRESS() macro for
accessing PCI config space. Basically every HW which uses PCI
Configuration Mechanism #1 requires to set "enable" bit like it is
described in PCI local bus spec. There is only one exception pci_msc01.c
which requires to have "enable" bit unset. And I'm not sure if this is
not rather bug in U-Boot driver (but it is in U-Boot in this state for a
long time).

Do you have some references to this "PCI CAM" specification? Because for
me it looks like some vendor/proprietary undocumented API and
incompatible with everything which I saw.

Therefore I would suggest to not call it "pci-host-cam-generic" or
TYPE_PCI as it is not generic for sure (like PCIe ECAM which is
documented in PCIe base spec) and also because it is not PCI type (does
not match neither PCI Mechanism #1 nor Mechanism #2).

Anyway, I would like to know, which hardware uses this unusual PCI
config space access?


Btw, that commit probably does not work. It uses construction:

  (PCI_FUNC(bdf) << 8) | offset

offset passed by U-Boot is number between 0..4095 and therefore it
overlaps with PCI function number. Either shift by 8 is wrong and it
should be shift by 12 or offset needs to be limited just to 0..255. But
then there would be no access to PCIe extended space (256..4095), only
PCI and I doubt that somebody in 2022 is still doing new development for
Conventional PCI local bus hardware.

Also in my opinion as this "PCI CAM" does not set "enable" bit, there is
needed some other code which sets it via vendor-specific API.


More information about the U-Boot mailing list