PCI: aardvark controller fixes BATCH 2
Subsystems (3)
v2 cover PCI: aardvark controller fixes BATCH 2
Lorenzo,
this is v2 of the second batch of aardvark changes.
As requested, I have removed patches 4-10, which will be rebased and sent in the next batch.
Also as requested I have removed my Reviewed-by tags, since there are my Signed-off-by tags.
Marek
Marek Behún (3):
PCI: pci-bridge-emul: Fix emulation of W1C bits
PCI: aardvark: Fix return value of MSI domain .alloc() method
PCI: aardvark: Read all 16-bits from PCIE_MSI_PAYLOAD_REG
Pali Rohár (4):
PCI: aardvark: Fix support for bus mastering and PCI_COMMAND on
emulated bridge
PCI: aardvark: Set PCI Bridge Class Code to PCI Bridge
PCI: aardvark: Fix support for PCI_BRIDGE_CTL_BUS_RESET on emulated
bridge
PCI: aardvark: Fix support for PCI_ROM_ADDRESS1 on emulated bridge
drivers/pci/controller/pci-aardvark.c | 119 ++++++++++++++++++++++---- drivers/pci/pci-bridge-emul.c | 13 +++ 2 files changed, 114 insertions(+), 18 deletions(-)
Signature · 1 line
2.32.0
PATCH v2 1/7 PCI: pci-bridge-emul: Fix emulation of W1C bits
The pci_bridge_emul_conf_write() function correctly clears W1C bits in cfgspace cache, but it does not inform the underlying implementation about the clear request: the .write_op() method is given the value with these bits cleared.
This is wrong if the .write_op() needs to know which bits were requested to be cleared.
Fix the value to be passed into the .write_op() method to have requested W1C bits set, so that it can clear them.
Both pci-bridge-emul users (mvebu and aardvark) are compatible with this change.
--- drivers/pci/pci-bridge-emul.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/pci/pci-bridge-emul.c +13 -0
@@ -431,8 +431,21 @@ int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,/* Clear the W1C bits */ new &= ~((value << shift) & (behavior[reg /4 ].w1c & mask)); +/* Save the new value with the cleared W1C bits into the cfgspace */ cfgspace[reg /4 ] =cpu_to_le32 (new); +/* +* Clear the W1C bits not specified by the write mask, so that the +* write_op() does not clear them. +*/ + new &= ~(behavior[reg /4 ].w1c & ~mask); + +/* +* Set the W1C bits specified by the write mask, so that write_op() +* knows about that they are to be cleared. +*/ + new |= (value << shift) & (behavior[reg /4 ].w1c & mask); +if (write_op)write_op (bridge, reg, old, new, mask);
Signature · 1 line
2.32.0
PATCH v2 2/7 PCI: aardvark: Fix return value of MSI domain .alloc() method
In reply to Marek BehúnMSI domain callback .alloc() (implemented by advk_msi_irq_domain_alloc() function) should return zero on success, since non-zero value indicates failure.
When the driver was converted to generic MSI API in commit f21a8b1b6837 ("PCI: aardvark: Move to MSI handling using generic MSI support"), it was converted so that it returns hwirq number.
Fix this.
--- drivers/pci/controller/pci-aardvark.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/controller/pci-aardvark.c +1 -1
@@ -1138,7 +1138,7 @@ static int advk_msi_irq_domain_alloc(struct irq_domain *domain, domain->host_data, handle_simple_irq, NULL, NULL); -return hwirq; +return 0 ; } static void advk_msi_irq_domain_free(struct irq_domain *domain,
Signature · 1 line
2.32.0
PATCH v2 3/7 PCI: aardvark: Read all 16-bits from PCIE_MSI_PAYLOAD_REG
In reply to Marek BehúnThe PCIE_MSI_PAYLOAD_REG contains 16-bit MSI number, not only lower 8 bits. Fix reading content of this register and add a comment describing the access to this register.
--- drivers/pci/controller/pci-aardvark.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/pci/controller/pci-aardvark.c +6 -1
@@ -119,6 +119,7 @@#define PCIE_MSI_STATUS_REG (CONTROL_BASE_ADDR + 0x58)#define PCIE_MSI_MASK_REG (CONTROL_BASE_ADDR + 0x5C)#define PCIE_MSI_PAYLOAD_REG (CONTROL_BASE_ADDR + 0x9C) +#define PCIE_MSI_DATA_MASK GENMASK(15, 0)/* PCIe window configuration */ #define OB_WIN_BASE_ADDR 0x4c00
@@ -1319,8 +1320,12 @@ static void advk_pcie_handle_msi(struct advk_pcie *pcie)if (!(BIT (msi_idx) & msi_status))continue ; +/* +* msi_idx contains bits [4:0] of the msi_data and msi_data +* contains 16bit MSI interrupt number +*/ advk_writel (pcie,BIT (msi_idx),PCIE_MSI_STATUS_REG ); - msi_data =advk_readl (pcie,PCIE_MSI_PAYLOAD_REG ) &0xFF ; + msi_data =advk_readl (pcie,PCIE_MSI_PAYLOAD_REG ) &PCIE_MSI_DATA_MASK ;generic_handle_irq (msi_data); }
Signature · 1 line
2.32.0
PATCH v2 4/7 PCI: aardvark: Fix support for bus mastering and PCI_COMMAND on emulated bridge
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
From very vague, ambiguous and incomplete information from Marvell we deduced that the 32-bit Aardvark register at address 0x4 (PCIE_CORE_CMD_STATUS_REG), which is not documented for Root Complex mode in the Functional Specification (only for Endpoint mode), controls two 16-bit PCIe registers: Command Register and Status Registers of PCIe Root Port.
This means that bit 2 controls bus mastering and forwarding of memory and I/O requests in the upstream direction. According to PCI specifications bits [0:2] of Command Register, this should be by default disabled on reset. So explicitly disable these bits at early setup of the Aardvark driver.
Remove code which unconditionally enables all 3 bits and let kernel code (via pci_set_master() function) to handle bus mastering of Root PCIe Bridge via emulated PCI_COMMAND on emulated bridge.
--- drivers/pci/controller/pci-aardvark.c | 54 +++++++++++++++++++-------- 1 file changed, 38 insertions(+), 16 deletions(-)
drivers/pci/controller/pci-aardvark.c +38 -16
@@ -31,9 +31,6 @@/* PCIe core registers */ #define PCIE_CORE_DEV_ID_REG 0x0#define PCIE_CORE_CMD_STATUS_REG 0x4 -#define PCIE_CORE_CMD_IO_ACCESS_EN BIT(0) -#define PCIE_CORE_CMD_MEM_ACCESS_EN BIT(1) -#define PCIE_CORE_CMD_MEM_IO_REQ_EN BIT(2)#define PCIE_CORE_DEV_REV_REG 0x8#define PCIE_CORE_PCIEXP_CAP 0xc0#define PCIE_CORE_ERR_CAPCTL_REG 0x118
@@ -514,6 +511,11 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg = (PCI_VENDOR_ID_MARVELL <<16 ) |PCI_VENDOR_ID_MARVELL ;advk_writel (pcie, reg,VENDOR_ID_REG ); +/* Disable Root Bridge I/O space, memory space and bus mastering */ + reg =advk_readl (pcie,PCIE_CORE_CMD_STATUS_REG ); + reg &= ~(PCI_COMMAND_IO |PCI_COMMAND_MEMORY |PCI_COMMAND_MASTER ); +advk_writel (pcie, reg,PCIE_CORE_CMD_STATUS_REG ); +/* Set Advanced Error Capabilities and Control PF0 register */ reg =PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX | PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN |
@@ -612,19 +614,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)advk_pcie_disable_ob_win (pcie, i);advk_pcie_train_link (pcie); - -/* -* FIXME: The following register update is suspicious. This register is -* applicable only when the PCI controller is configured for Endpoint -* mode, not as a Root Complex. But apparently when this code is -* removed, some cards stop working. This should be investigated and -* a comment explaining this should be put here. -*/ - reg =advk_readl (pcie,PCIE_CORE_CMD_STATUS_REG ); - reg |=PCIE_CORE_CMD_MEM_ACCESS_EN | -PCIE_CORE_CMD_IO_ACCESS_EN | -PCIE_CORE_CMD_MEM_IO_REQ_EN ; -advk_writel (pcie, reg,PCIE_CORE_CMD_STATUS_REG ); }static int advk_pcie_check_pio_status (struct advk_pcie *pcie,bool allow_crs,u32 *val)
@@ -753,6 +742,37 @@ static int advk_pcie_wait_pio(struct advk_pcie *pcie)return -ETIMEDOUT ; } +static pci_bridge_emul_read_status_t +advk_pci_bridge_emul_base_conf_read (struct pci_bridge_emul *bridge, +int reg,u32 *value) +{ +struct advk_pcie *pcie = bridge->data; + +switch (reg) { +case PCI_COMMAND : + *value =advk_readl (pcie,PCIE_CORE_CMD_STATUS_REG ); +return PCI_BRIDGE_EMUL_HANDLED ; + +default : +return PCI_BRIDGE_EMUL_NOT_HANDLED ; + } +} + +static void +advk_pci_bridge_emul_base_conf_write (struct pci_bridge_emul *bridge, +int reg,u32 old,u32 new,u32 mask) +{ +struct advk_pcie *pcie = bridge->data; + +switch (reg) { +case PCI_COMMAND : +advk_writel (pcie, new,PCIE_CORE_CMD_STATUS_REG ); +break ; + +default : +break ; + } +} static pci_bridge_emul_read_status_t advk_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
@@ -854,6 +874,8 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge, }static struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = { + .read_base = advk_pci_bridge_emul_base_conf_read, + .write_base = advk_pci_bridge_emul_base_conf_write, .read_pcie = advk_pci_bridge_emul_pcie_conf_read, .write_pcie = advk_pci_bridge_emul_pcie_conf_write, };
Signature · 1 line
2.32.0
PATCH v2 5/7 PCI: aardvark: Set PCI Bridge Class Code to PCI Bridge
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
Aardvark controller has something like config space of a Root Port available at offset 0x0 of internal registers - these registers are used for implementation of the emulated bridge.
The default value of Class Code of this bridge corresponds to a RAID Mass storage controller, though. (This is probably intended for when the controller is used as Endpoint.)
Change the Class Code to correspond to a PCI Bridge.
Add comment explaining this change.
--- drivers/pci/controller/pci-aardvark.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
drivers/pci/controller/pci-aardvark.c +20 -0
@@ -511,6 +511,26 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg = (PCI_VENDOR_ID_MARVELL <<16 ) |PCI_VENDOR_ID_MARVELL ;advk_writel (pcie, reg,VENDOR_ID_REG ); +/* +* Change Class Code of PCI Bridge device to PCI Bridge (0x600400), +* because the default value is Mass storage controller (0x010400). +* +* Note that this Aardvark PCI Bridge does not have compliant Type 1 +* Configuration Space and it even cannot be accessed via Aardvark's +* PCI config space access method. Something like config space is +* available in internal Aardvark registers starting at offset 0x0 +* and is reported as Type 0. In range 0x10 - 0x34 it has totally +* different registers. +* +* Therefore driver uses emulation of PCI Bridge which emulates +* access to configuration space via internal Aardvark registers or +* emulated configuration buffer. +*/ + reg =advk_readl (pcie,PCIE_CORE_DEV_REV_REG ); + reg &= ~0xffffff00 ; + reg |= (PCI_CLASS_BRIDGE_PCI <<8 ) <<8 ; +advk_writel (pcie, reg,PCIE_CORE_DEV_REV_REG ); +/* Disable Root Bridge I/O space, memory space and bus mastering */ reg =advk_readl (pcie,PCIE_CORE_CMD_STATUS_REG ); reg &= ~(PCI_COMMAND_IO |PCI_COMMAND_MEMORY |PCI_COMMAND_MASTER );
Signature · 1 line
2.32.0
PATCH v2 6/7 PCI: aardvark: Fix support for PCI_BRIDGE_CTL_BUS_RESET on emulated bridge
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
Aardvark supports PCIe Hot Reset via PCIE_CORE_CTRL1_REG.
Use it for implementing PCI_BRIDGE_CTL_BUS_RESET bit of PCI_BRIDGE_CONTROL register on emulated bridge.
With this, the function pci_reset_secondary_bus() starts working and can reset connected PCIe card. Custom userspace script [1] which uses setpci can trigger PCIe Hot Reset and reset the card manually.
[1] https://alexforencich.com/wiki/en/pcie/hot-reset-linux
--- drivers/pci/controller/pci-aardvark.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
drivers/pci/controller/pci-aardvark.c +27 -0
@@ -773,6 +773,22 @@ 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_INTERRUPT_LINE : { +/* +* From the whole 32bit register we support reading from HW only +* one bit: PCI_BRIDGE_CTL_BUS_RESET. +* Other bits are retrieved only from emulated config buffer. +*/ +__le32 *cfgspace = (__le32 *)&bridge->conf; +u32 val =le32_to_cpu (cfgspace[PCI_INTERRUPT_LINE /4 ]); +if (advk_readl (pcie,PCIE_CORE_CTRL1_REG ) &HOT_RESET_GEN ) + val |=PCI_BRIDGE_CTL_BUS_RESET <<16 ; +else + val &= ~(PCI_BRIDGE_CTL_BUS_RESET <<16 ); + *value = val; +return PCI_BRIDGE_EMUL_HANDLED ; + } +default :return PCI_BRIDGE_EMUL_NOT_HANDLED ; }
@@ -789,6 +805,17 @@ advk_pci_bridge_emul_base_conf_write(struct pci_bridge_emul *bridge,advk_writel (pcie, new,PCIE_CORE_CMD_STATUS_REG );break ; +case PCI_INTERRUPT_LINE : +if (mask & (PCI_BRIDGE_CTL_BUS_RESET <<16 )) { +u32 val =advk_readl (pcie,PCIE_CORE_CTRL1_REG ); +if (new & (PCI_BRIDGE_CTL_BUS_RESET <<16 )) + val |=HOT_RESET_GEN ; +else + val &= ~HOT_RESET_GEN ; +advk_writel (pcie, val,PCIE_CORE_CTRL1_REG ); + } +break ; +default :break ; }
Signature · 1 line
2.32.0
PATCH v2 7/7 PCI: aardvark: Fix support for PCI_ROM_ADDRESS1 on emulated bridge
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
This register is exported at address offset 0x30.
--- drivers/pci/controller/pci-aardvark.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/pci/controller/pci-aardvark.c +9 -0
@@ -32,6 +32,7 @@#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)
@@ -773,6 +774,10 @@ 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
@@ -805,6 +810,10 @@ 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 );
Signature · 1 line
2.32.0
Lorenzo Pieralisi
AppliedQuoted 8 lines · Marek Behún
Lorenzo,
this is v2 of the second batch of aardvark changes.
As requested, I have removed patches 4-10, which will be rebased and sent in the next batch.
[...]
Applied to pci/aardvark, thanks!
[1/7] PCI: pci-bridge-emul: Fix emulation of W1C bits
https://git.kernel.org/lpieralisi/pci/c/7a41ae80bd [2/7] PCI: aardvark: Fix return value of MSI domain .alloc() method https://git.kernel.org/lpieralisi/pci/c/e4313be159 [3/7] PCI: aardvark: Read all 16-bits from PCIE_MSI_PAYLOAD_REG https://git.kernel.org/lpieralisi/pci/c/95997723b6 [4/7] PCI: aardvark: Fix support for bus mastering and PCI_COMMAND on emulated bridge https://git.kernel.org/lpieralisi/pci/c/771153fc88 [5/7] PCI: aardvark: Set PCI Bridge Class Code to PCI Bridge https://git.kernel.org/lpieralisi/pci/c/84e1b4045d [6/7] PCI: aardvark: Fix support for PCI_BRIDGE_CTL_BUS_RESET on emulated bridge https://git.kernel.org/lpieralisi/pci/c/bc4fac42e5 [7/7] PCI: aardvark: Fix support for PCI_ROM_ADDRESS1 on emulated bridge https://git.kernel.org/lpieralisi/pci/c/239edf686c
Thanks,
Lorenzo