On Thu, Nov 25, 2021 at 05:01:48PM +0100, Marek Behún wrote:
> This reverts commit 239edf686c14a9ff926dec2f350289ed7adfefe2.
>
> PCI Bridge which represents aardvark's PCIe Root Port has Expansion ROM
> Base Address register at offset 0x30, but its meaning is different than
> PCI's Expansion ROM BAR register, although the layout is the same.
> (This is why we thought it does the same thing.)
>
> First: there is no ROM (or part of BootROM) in the A3720 SOC dedicated
> for PCIe Root Port (or controller in RC mode) containing executable code
> that would initialize the Root Port, suitable for execution in
> bootloader (this is how Expansion ROM BAR is used on x86).
>
> Second: in A3720 spec the register (address D0070030) is not documented
> at all for Root Complex mode, but similar to other BAR registers, it has
> an "entangled partner" in register D0075920, which does address
> translation for the BAR in D0070030:
> - the BAR register sets the address from the view of PCIe bus
> - the translation register sets the address from the view of the CPU
>
> The other BAR registers also have this entangled partner, and they
> can be used to:
> - in RC mode: address-checking on the receive side of the RC (they
> can define address ranges for memory accesses from remote Endpoints
> to the RC)
> - in Endpoint mode: allow the remote CPU to access memory on A3720
>
> The Expansion ROM BAR has only the Endpoint part documented, but from
> the similarities we think that it can also be used in RC mode in that
> way.
>
> So either Expansion ROM BAR has different meaning (if the hypothesis
> above is true), or we don't know it's meaning (since it is not
> documented for RC mode).
>
> Remove the register from the emulated bridge accessing functions.
>
> Fixes: 239edf686c14 ("PCI: aardvark: Fix support for PCI_ROM_ADDRESS1 on emulated bridge")
> Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> ---
> drivers/pci/controller/pci-aardvark.c | 9 ---------
> 1 file changed, 9 deletions(-)
Bjorn,
this reverts a commit we merged the last merge window so it is
a candidate for one of the upcoming -rcX.
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index baa62cdcaab4..e3001b3b3293 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -32,7 +32,6 @@
> #define PCIE_CORE_DEV_ID_REG 0x0
> #define PCIE_CORE_CMD_STATUS_REG 0x4
> #define PCIE_CORE_DEV_REV_REG 0x8
> -#define PCIE_CORE_EXP_ROM_BAR_REG 0x30
> #define PCIE_CORE_PCIEXP_CAP 0xc0
> #define PCIE_CORE_ERR_CAPCTL_REG 0x118
> #define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX BIT(5)
> @@ -774,10 +773,6 @@ advk_pci_bridge_emul_base_conf_read(struct pci_bridge_emul *bridge,
> *value = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
> return PCI_BRIDGE_EMUL_HANDLED;
>
> - case PCI_ROM_ADDRESS1:
> - *value = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
> - return PCI_BRIDGE_EMUL_HANDLED;
> -
> case PCI_INTERRUPT_LINE: {
> /*
> * From the whole 32bit register we support reading from HW only
> @@ -810,10 +805,6 @@ advk_pci_bridge_emul_base_conf_write(struct pci_bridge_emul *bridge,
> advk_writel(pcie, new, PCIE_CORE_CMD_STATUS_REG);
> break;
>
> - case PCI_ROM_ADDRESS1:
> - advk_writel(pcie, new, PCIE_CORE_EXP_ROM_BAR_REG);
> - break;
> -
> case PCI_INTERRUPT_LINE:
> if (mask & (PCI_BRIDGE_CTL_BUS_RESET << 16)) {
> u32 val = advk_readl(pcie, PCIE_CORE_CTRL1_REG);
> --
> 2.32.0
>
