* [PATCH v3 0/2] PCI: microchip: support using either instance 1 or 2
@ 2024-07-01 10:22 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-01 10:22 UTC (permalink / raw)
To: linux-pci
Cc: conor, Conor Dooley, Daire McNamara, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
linux-riscv
From: Conor Dooley <conor.dooley@microchip.com>
The current driver and binding for PolarFire SoC's PCI controller assume
that the root port instance in use is instance 1. The second reg
property constitutes the region encompassing both "control" and "bridge"
registers for both instances. In the driver, a fixed offset is applied to
find the base addresses for instance 1's "control" and "bridge"
registers. The BeagleV Fire uses root port instance 2, so something must
be done so that software can differentiate. This series splits the
second reg property in two, with dedicated "control" and "bridge"
entries so that either instance can be used.
Cheers,
Conor.
v3:
- rename a variable in probe s/axi/apb/
v2:
- try the new reg format before the old one to avoid warnings in the
good case
- reword $subject for 2/2
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Lorenzo Pieralisi <lpieralisi@kernel.org>
CC: Krzysztof Wilczyński <kw@linux.com>
CC: Rob Herring <robh@kernel.org>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: Conor Dooley <conor+dt@kernel.org>
CC: linux-pci@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-riscv@lists.infradead.org
Conor Dooley (2):
dt-bindings: PCI: microchip,pcie-host: fix reg properties
PCI: microchip: rework reg region handing to support using either
instance 1 or 2
.../bindings/pci/microchip,pcie-host.yaml | 10 +-
drivers/pci/controller/pcie-microchip-host.c | 155 +++++++++---------
2 files changed, 79 insertions(+), 86 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/2] PCI: microchip: support using either instance 1 or 2
@ 2024-07-01 10:22 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-01 10:22 UTC (permalink / raw)
To: linux-pci
Cc: conor, Conor Dooley, Daire McNamara, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
linux-riscv
From: Conor Dooley <conor.dooley@microchip.com>
The current driver and binding for PolarFire SoC's PCI controller assume
that the root port instance in use is instance 1. The second reg
property constitutes the region encompassing both "control" and "bridge"
registers for both instances. In the driver, a fixed offset is applied to
find the base addresses for instance 1's "control" and "bridge"
registers. The BeagleV Fire uses root port instance 2, so something must
be done so that software can differentiate. This series splits the
second reg property in two, with dedicated "control" and "bridge"
entries so that either instance can be used.
Cheers,
Conor.
v3:
- rename a variable in probe s/axi/apb/
v2:
- try the new reg format before the old one to avoid warnings in the
good case
- reword $subject for 2/2
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Lorenzo Pieralisi <lpieralisi@kernel.org>
CC: Krzysztof Wilczyński <kw@linux.com>
CC: Rob Herring <robh@kernel.org>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: Conor Dooley <conor+dt@kernel.org>
CC: linux-pci@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-riscv@lists.infradead.org
Conor Dooley (2):
dt-bindings: PCI: microchip,pcie-host: fix reg properties
PCI: microchip: rework reg region handing to support using either
instance 1 or 2
.../bindings/pci/microchip,pcie-host.yaml | 10 +-
drivers/pci/controller/pcie-microchip-host.c | 155 +++++++++---------
2 files changed, 79 insertions(+), 86 deletions(-)
--
2.43.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] dt-bindings: PCI: microchip,pcie-host: fix reg properties
2024-07-01 10:22 ` Conor Dooley
@ 2024-07-01 10:22 ` Conor Dooley
-1 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-01 10:22 UTC (permalink / raw)
To: linux-pci
Cc: conor, Conor Dooley, Daire McNamara, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
linux-riscv, Krzysztof Kozlowski
From: Conor Dooley <conor.dooley@microchip.com>
The PCI host controller on PolarFire SoC has multiple "instances", each
with their own bridge and ctrl address spaces. The original binding has
an "apb" register region, and it is expected to be set to the base
address of the host controllers register space. Some defines in the
Linux driver were used to compute the addresses of the bridge and ctrl
address ranges corresponding to instance1. Some customers want to use
instance2 however and that requires changing the defines in the driver,
which is clearly not a portable solution.
Remove this "apb" register region from the binding and add "bridge" &
"ctrl" regions instead, that will directly communicate the address of
these regions
Fixes: 6ee6c89aac35 ("dt-bindings: PCI: microchip: Add Microchip PolarFire host binding")
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../devicetree/bindings/pci/microchip,pcie-host.yaml | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
index 5d7aec5f54e7..45c14b6e4aa4 100644
--- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
@@ -18,12 +18,13 @@ properties:
const: microchip,pcie-host-1.0 # PolarFire
reg:
- maxItems: 2
+ maxItems: 3
reg-names:
items:
- const: cfg
- - const: apb
+ - const: bridge
+ - const: ctrl
clocks:
description:
@@ -115,8 +116,9 @@ examples:
pcie0: pcie@2030000000 {
compatible = "microchip,pcie-host-1.0";
reg = <0x0 0x70000000 0x0 0x08000000>,
- <0x0 0x43000000 0x0 0x00010000>;
- reg-names = "cfg", "apb";
+ <0x0 0x43008000 0x0 0x00002000>,
+ <0x0 0x4300a000 0x0 0x00002000>;
+ reg-names = "cfg", "bridge", "ctrl";
device_type = "pci";
#address-cells = <3>;
#size-cells = <2>;
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] dt-bindings: PCI: microchip,pcie-host: fix reg properties
@ 2024-07-01 10:22 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-01 10:22 UTC (permalink / raw)
To: linux-pci
Cc: conor, Conor Dooley, Daire McNamara, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
linux-riscv, Krzysztof Kozlowski
From: Conor Dooley <conor.dooley@microchip.com>
The PCI host controller on PolarFire SoC has multiple "instances", each
with their own bridge and ctrl address spaces. The original binding has
an "apb" register region, and it is expected to be set to the base
address of the host controllers register space. Some defines in the
Linux driver were used to compute the addresses of the bridge and ctrl
address ranges corresponding to instance1. Some customers want to use
instance2 however and that requires changing the defines in the driver,
which is clearly not a portable solution.
Remove this "apb" register region from the binding and add "bridge" &
"ctrl" regions instead, that will directly communicate the address of
these regions
Fixes: 6ee6c89aac35 ("dt-bindings: PCI: microchip: Add Microchip PolarFire host binding")
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../devicetree/bindings/pci/microchip,pcie-host.yaml | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
index 5d7aec5f54e7..45c14b6e4aa4 100644
--- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
@@ -18,12 +18,13 @@ properties:
const: microchip,pcie-host-1.0 # PolarFire
reg:
- maxItems: 2
+ maxItems: 3
reg-names:
items:
- const: cfg
- - const: apb
+ - const: bridge
+ - const: ctrl
clocks:
description:
@@ -115,8 +116,9 @@ examples:
pcie0: pcie@2030000000 {
compatible = "microchip,pcie-host-1.0";
reg = <0x0 0x70000000 0x0 0x08000000>,
- <0x0 0x43000000 0x0 0x00010000>;
- reg-names = "cfg", "apb";
+ <0x0 0x43008000 0x0 0x00002000>,
+ <0x0 0x4300a000 0x0 0x00002000>;
+ reg-names = "cfg", "bridge", "ctrl";
device_type = "pci";
#address-cells = <3>;
#size-cells = <2>;
--
2.43.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
2024-07-01 10:22 ` Conor Dooley
@ 2024-07-01 10:22 ` Conor Dooley
-1 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-01 10:22 UTC (permalink / raw)
To: linux-pci
Cc: conor, Conor Dooley, Daire McNamara, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
linux-riscv
From: Conor Dooley <conor.dooley@microchip.com>
The PCI host controller on PolarFire SoC has multiple "instances", each
with their own bridge and ctrl address spaces. The original binding has
an "apb" register region, and it is expected to be set to the base
address of the host controllers register space. Defines in the driver
were used to compute the addresses of the bridge and ctrl address ranges
corresponding to instance1. Some customers want to use instance0 however
and that requires changing the defines in the driver, which is clearly
not a portable solution.
The binding has been changed from a single register region to a pair,
corresponding to the bridge and ctrl regions respectively, so modify the
driver to read these regions directly from the devicetree rather than
compute them from the base address of the abp region.
To maintain backwards compatibility with the existing binding, the
driver retains code to handle the "abp" reg and computes the base
address of the bridge and ctrl regions using the defines if it is
present. reg-names has always been a required property, so this is
safe to do.
Acked-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/pci/controller/pcie-microchip-host.c | 155 +++++++++----------
1 file changed, 73 insertions(+), 82 deletions(-)
diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
index 137fb8570ba2..c05259224d36 100644
--- a/drivers/pci/controller/pcie-microchip-host.c
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -27,9 +27,6 @@
#define MC_PCIE1_BRIDGE_ADDR 0x00008000u
#define MC_PCIE1_CTRL_ADDR 0x0000a000u
-#define MC_PCIE_BRIDGE_ADDR (MC_PCIE1_BRIDGE_ADDR)
-#define MC_PCIE_CTRL_ADDR (MC_PCIE1_CTRL_ADDR)
-
/* PCIe Bridge Phy Regs */
#define PCIE_PCI_IRQ_DW0 0xa8
#define MSIX_CAP_MASK BIT(31)
@@ -207,7 +204,6 @@
[EVENT_LOCAL_ ## x] = { __stringify(x), s }
#define PCIE_EVENT(x) \
- .base = MC_PCIE_CTRL_ADDR, \
.offset = PCIE_EVENT_INT, \
.mask_offset = PCIE_EVENT_INT, \
.mask_high = 1, \
@@ -215,7 +211,6 @@
.enb_mask = PCIE_EVENT_INT_ENB_MASK
#define SEC_EVENT(x) \
- .base = MC_PCIE_CTRL_ADDR, \
.offset = SEC_ERROR_INT, \
.mask_offset = SEC_ERROR_INT_MASK, \
.mask = SEC_ERROR_INT_ ## x ## _INT, \
@@ -223,7 +218,6 @@
.enb_mask = 0
#define DED_EVENT(x) \
- .base = MC_PCIE_CTRL_ADDR, \
.offset = DED_ERROR_INT, \
.mask_offset = DED_ERROR_INT_MASK, \
.mask_high = 1, \
@@ -231,7 +225,6 @@
.enb_mask = 0
#define LOCAL_EVENT(x) \
- .base = MC_PCIE_BRIDGE_ADDR, \
.offset = ISTATUS_LOCAL, \
.mask_offset = IMASK_LOCAL, \
.mask_high = 0, \
@@ -265,7 +258,8 @@ struct mc_msi {
};
struct mc_pcie {
- void __iomem *axi_base_addr;
+ void __iomem *bridge_base_addr;
+ void __iomem *ctrl_base_addr;
struct device *dev;
struct irq_domain *intx_domain;
struct irq_domain *event_domain;
@@ -344,7 +338,6 @@ static struct event_map local_status_to_event[] = {
};
static struct {
- u32 base;
u32 offset;
u32 mask;
u32 shift;
@@ -415,18 +408,17 @@ static void mc_handle_msi(struct irq_desc *desc)
struct irq_chip *chip = irq_desc_get_chip(desc);
struct device *dev = port->dev;
struct mc_msi *msi = &port->msi;
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long status;
u32 bit;
int ret;
chained_irq_enter(chip, desc);
- status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
+ status = readl_relaxed(port->bridge_base_addr + ISTATUS_LOCAL);
if (status & PM_MSI_INT_MSI_MASK) {
- writel_relaxed(status & PM_MSI_INT_MSI_MASK, bridge_base_addr + ISTATUS_LOCAL);
- status = readl_relaxed(bridge_base_addr + ISTATUS_MSI);
+ writel_relaxed(status & PM_MSI_INT_MSI_MASK,
+ port->bridge_base_addr + ISTATUS_LOCAL);
+ status = readl_relaxed(port->bridge_base_addr + ISTATUS_MSI);
for_each_set_bit(bit, &status, msi->num_vectors) {
ret = generic_handle_domain_irq(msi->dev_domain, bit);
if (ret)
@@ -441,11 +433,9 @@ static void mc_handle_msi(struct irq_desc *desc)
static void mc_msi_bottom_irq_ack(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
u32 bitpos = data->hwirq;
- writel_relaxed(BIT(bitpos), bridge_base_addr + ISTATUS_MSI);
+ writel_relaxed(BIT(bitpos), port->bridge_base_addr + ISTATUS_MSI);
}
static void mc_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
@@ -564,15 +554,13 @@ static void mc_handle_intx(struct irq_desc *desc)
struct mc_pcie *port = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
struct device *dev = port->dev;
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long status;
u32 bit;
int ret;
chained_irq_enter(chip, desc);
- status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
+ status = readl_relaxed(port->bridge_base_addr + ISTATUS_LOCAL);
if (status & PM_MSI_INT_INTX_MASK) {
status &= PM_MSI_INT_INTX_MASK;
status >>= PM_MSI_INT_INTX_SHIFT;
@@ -590,42 +578,36 @@ static void mc_handle_intx(struct irq_desc *desc)
static void mc_ack_intx_irq(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT);
- writel_relaxed(mask, bridge_base_addr + ISTATUS_LOCAL);
+ writel_relaxed(mask, port->bridge_base_addr + ISTATUS_LOCAL);
}
static void mc_mask_intx_irq(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long flags;
u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT);
u32 val;
raw_spin_lock_irqsave(&port->lock, flags);
- val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
+ val = readl_relaxed(port->bridge_base_addr + IMASK_LOCAL);
val &= ~mask;
- writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
+ writel_relaxed(val, port->bridge_base_addr + IMASK_LOCAL);
raw_spin_unlock_irqrestore(&port->lock, flags);
}
static void mc_unmask_intx_irq(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long flags;
u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT);
u32 val;
raw_spin_lock_irqsave(&port->lock, flags);
- val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
+ val = readl_relaxed(port->bridge_base_addr + IMASK_LOCAL);
val |= mask;
- writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
+ writel_relaxed(val, port->bridge_base_addr + IMASK_LOCAL);
raw_spin_unlock_irqrestore(&port->lock, flags);
}
@@ -656,8 +638,7 @@ static inline u32 reg_to_event(u32 reg, struct event_map field)
static u32 pcie_events(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
- u32 reg = readl_relaxed(ctrl_base_addr + PCIE_EVENT_INT);
+ u32 reg = readl_relaxed(port->ctrl_base_addr + PCIE_EVENT_INT);
u32 val = 0;
int i;
@@ -669,8 +650,7 @@ static u32 pcie_events(struct mc_pcie *port)
static u32 sec_errors(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
- u32 reg = readl_relaxed(ctrl_base_addr + SEC_ERROR_INT);
+ u32 reg = readl_relaxed(port->ctrl_base_addr + SEC_ERROR_INT);
u32 val = 0;
int i;
@@ -682,8 +662,7 @@ static u32 sec_errors(struct mc_pcie *port)
static u32 ded_errors(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
- u32 reg = readl_relaxed(ctrl_base_addr + DED_ERROR_INT);
+ u32 reg = readl_relaxed(port->ctrl_base_addr + DED_ERROR_INT);
u32 val = 0;
int i;
@@ -695,8 +674,7 @@ static u32 ded_errors(struct mc_pcie *port)
static u32 local_events(struct mc_pcie *port)
{
- void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
- u32 reg = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
+ u32 reg = readl_relaxed(port->bridge_base_addr + ISTATUS_LOCAL);
u32 val = 0;
int i;
@@ -758,8 +736,12 @@ static void mc_ack_event_irq(struct irq_data *data)
void __iomem *addr;
u32 mask;
- addr = port->axi_base_addr + event_descs[event].base +
- event_descs[event].offset;
+ if (event_descs[event].offset == ISTATUS_LOCAL)
+ addr = port->bridge_base_addr;
+ else
+ addr = port->ctrl_base_addr;
+
+ addr += event_descs[event].offset;
mask = event_descs[event].mask;
mask |= event_descs[event].enb_mask;
@@ -774,8 +756,12 @@ static void mc_mask_event_irq(struct irq_data *data)
u32 mask;
u32 val;
- addr = port->axi_base_addr + event_descs[event].base +
- event_descs[event].mask_offset;
+ if (event_descs[event].offset == ISTATUS_LOCAL)
+ addr = port->bridge_base_addr;
+ else
+ addr = port->ctrl_base_addr;
+
+ addr += event_descs[event].mask_offset;
mask = event_descs[event].mask;
if (event_descs[event].enb_mask) {
mask <<= PCIE_EVENT_INT_ENB_SHIFT;
@@ -804,8 +790,12 @@ static void mc_unmask_event_irq(struct irq_data *data)
u32 mask;
u32 val;
- addr = port->axi_base_addr + event_descs[event].base +
- event_descs[event].mask_offset;
+ if (event_descs[event].offset == ISTATUS_LOCAL)
+ addr = port->bridge_base_addr;
+ else
+ addr = port->ctrl_base_addr;
+
+ addr += event_descs[event].mask_offset;
mask = event_descs[event].mask;
if (event_descs[event].enb_mask)
@@ -972,8 +962,6 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
static int mc_pcie_setup_windows(struct platform_device *pdev,
struct mc_pcie *port)
{
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
struct pci_host_bridge *bridge = platform_get_drvdata(pdev);
struct resource_entry *entry;
u64 pci_addr;
@@ -982,7 +970,7 @@ static int mc_pcie_setup_windows(struct platform_device *pdev,
resource_list_for_each_entry(entry, &bridge->windows) {
if (resource_type(entry->res) == IORESOURCE_MEM) {
pci_addr = entry->res->start - entry->offset;
- mc_pcie_setup_window(bridge_base_addr, index,
+ mc_pcie_setup_window(port->bridge_base_addr, index,
entry->res->start, pci_addr,
resource_size(entry->res));
index++;
@@ -994,26 +982,20 @@ static int mc_pcie_setup_windows(struct platform_device *pdev,
static inline void mc_clear_secs(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
-
- writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, ctrl_base_addr +
+ writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, port->ctrl_base_addr +
SEC_ERROR_INT);
- writel_relaxed(0, ctrl_base_addr + SEC_ERROR_EVENT_CNT);
+ writel_relaxed(0, port->ctrl_base_addr + SEC_ERROR_EVENT_CNT);
}
static inline void mc_clear_deds(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
-
- writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, ctrl_base_addr +
+ writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, port->ctrl_base_addr +
DED_ERROR_INT);
- writel_relaxed(0, ctrl_base_addr + DED_ERROR_EVENT_CNT);
+ writel_relaxed(0, port->ctrl_base_addr + DED_ERROR_EVENT_CNT);
}
static void mc_disable_interrupts(struct mc_pcie *port)
{
- void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
u32 val;
/* Ensure ECC bypass is enabled */
@@ -1021,22 +1003,22 @@ static void mc_disable_interrupts(struct mc_pcie *port)
ECC_CONTROL_RX_RAM_ECC_BYPASS |
ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS |
ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS;
- writel_relaxed(val, ctrl_base_addr + ECC_CONTROL);
+ writel_relaxed(val, port->ctrl_base_addr + ECC_CONTROL);
/* Disable SEC errors and clear any outstanding */
- writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, ctrl_base_addr +
+ writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, port->ctrl_base_addr +
SEC_ERROR_INT_MASK);
mc_clear_secs(port);
/* Disable DED errors and clear any outstanding */
- writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, ctrl_base_addr +
+ writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, port->ctrl_base_addr +
DED_ERROR_INT_MASK);
mc_clear_deds(port);
/* Disable local interrupts and clear any outstanding */
- writel_relaxed(0, bridge_base_addr + IMASK_LOCAL);
- writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_LOCAL);
- writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_MSI);
+ writel_relaxed(0, port->bridge_base_addr + IMASK_LOCAL);
+ writel_relaxed(GENMASK(31, 0), port->bridge_base_addr + ISTATUS_LOCAL);
+ writel_relaxed(GENMASK(31, 0), port->bridge_base_addr + ISTATUS_MSI);
/* Disable PCIe events and clear any outstanding */
val = PCIE_EVENT_INT_L2_EXIT_INT |
@@ -1045,11 +1027,11 @@ static void mc_disable_interrupts(struct mc_pcie *port)
PCIE_EVENT_INT_L2_EXIT_INT_MASK |
PCIE_EVENT_INT_HOTRST_EXIT_INT_MASK |
PCIE_EVENT_INT_DLUP_EXIT_INT_MASK;
- writel_relaxed(val, ctrl_base_addr + PCIE_EVENT_INT);
+ writel_relaxed(val, port->ctrl_base_addr + PCIE_EVENT_INT);
/* Disable host interrupts and clear any outstanding */
- writel_relaxed(0, bridge_base_addr + IMASK_HOST);
- writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST);
+ writel_relaxed(0, port->bridge_base_addr + IMASK_HOST);
+ writel_relaxed(GENMASK(31, 0), port->bridge_base_addr + ISTATUS_HOST);
}
static int mc_init_interrupts(struct platform_device *pdev, struct mc_pcie *port)
@@ -1112,14 +1094,11 @@ static int mc_platform_init(struct pci_config_window *cfg)
{
struct device *dev = cfg->parent;
struct platform_device *pdev = to_platform_device(dev);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
int ret;
/* Configure address translation table 0 for PCIe config space */
- mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start,
- cfg->res.start,
- resource_size(&cfg->res));
+ mc_pcie_setup_window(port->bridge_base_addr, 0, cfg->res.start,
+ cfg->res.start, resource_size(&cfg->res));
/* Need some fixups in config space */
mc_pcie_enable_msi(port, cfg->win);
@@ -1140,7 +1119,7 @@ static int mc_platform_init(struct pci_config_window *cfg)
static int mc_host_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- void __iomem *bridge_base_addr;
+ void __iomem *apb_base_addr;
int ret;
u32 val;
@@ -1150,28 +1129,40 @@ static int mc_host_probe(struct platform_device *pdev)
port->dev = dev;
- port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
- if (IS_ERR(port->axi_base_addr))
- return PTR_ERR(port->axi_base_addr);
+ port->bridge_base_addr = devm_platform_ioremap_resource_byname(pdev, "bridge");
+ port->ctrl_base_addr = devm_platform_ioremap_resource_byname(pdev, "ctrl");
+ if(!IS_ERR(port->bridge_base_addr) && !IS_ERR(port->ctrl_base_addr))
+ goto addrs_set;
+ /*
+ * The original, incorrect, binding that lumped the control and
+ * bridge addresses together still needs to be handled by the driver.
+ */
+ apb_base_addr = devm_platform_ioremap_resource_byname(pdev, "apb");
+ if (IS_ERR(apb_base_addr))
+ return dev_err_probe(dev, PTR_ERR(port->bridge_base_addr),
+ "both legacy apb register and ctrl/bridge regions missing");
+
+ port->bridge_base_addr = apb_base_addr + MC_PCIE1_BRIDGE_ADDR;
+ port->ctrl_base_addr = apb_base_addr + MC_PCIE1_CTRL_ADDR;
+
+addrs_set:
mc_disable_interrupts(port);
- bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
-
/* Allow enabling MSI by disabling MSI-X */
- val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
+ val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
val &= ~MSIX_CAP_MASK;
- writel(val, bridge_base_addr + PCIE_PCI_IRQ_DW0);
+ writel(val, port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
/* Pick num vectors from bitfile programmed onto FPGA fabric */
- val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
+ val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
val &= NUM_MSI_MSGS_MASK;
val >>= NUM_MSI_MSGS_SHIFT;
port->msi.num_vectors = 1 << val;
/* Pick vector address from design */
- port->msi.vector_phy = readl_relaxed(bridge_base_addr + IMSI_ADDR);
+ port->msi.vector_phy = readl_relaxed(port->bridge_base_addr + IMSI_ADDR);
ret = mc_pcie_init_clks(dev);
if (ret) {
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
@ 2024-07-01 10:22 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-01 10:22 UTC (permalink / raw)
To: linux-pci
Cc: conor, Conor Dooley, Daire McNamara, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
linux-riscv
From: Conor Dooley <conor.dooley@microchip.com>
The PCI host controller on PolarFire SoC has multiple "instances", each
with their own bridge and ctrl address spaces. The original binding has
an "apb" register region, and it is expected to be set to the base
address of the host controllers register space. Defines in the driver
were used to compute the addresses of the bridge and ctrl address ranges
corresponding to instance1. Some customers want to use instance0 however
and that requires changing the defines in the driver, which is clearly
not a portable solution.
The binding has been changed from a single register region to a pair,
corresponding to the bridge and ctrl regions respectively, so modify the
driver to read these regions directly from the devicetree rather than
compute them from the base address of the abp region.
To maintain backwards compatibility with the existing binding, the
driver retains code to handle the "abp" reg and computes the base
address of the bridge and ctrl regions using the defines if it is
present. reg-names has always been a required property, so this is
safe to do.
Acked-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/pci/controller/pcie-microchip-host.c | 155 +++++++++----------
1 file changed, 73 insertions(+), 82 deletions(-)
diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
index 137fb8570ba2..c05259224d36 100644
--- a/drivers/pci/controller/pcie-microchip-host.c
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -27,9 +27,6 @@
#define MC_PCIE1_BRIDGE_ADDR 0x00008000u
#define MC_PCIE1_CTRL_ADDR 0x0000a000u
-#define MC_PCIE_BRIDGE_ADDR (MC_PCIE1_BRIDGE_ADDR)
-#define MC_PCIE_CTRL_ADDR (MC_PCIE1_CTRL_ADDR)
-
/* PCIe Bridge Phy Regs */
#define PCIE_PCI_IRQ_DW0 0xa8
#define MSIX_CAP_MASK BIT(31)
@@ -207,7 +204,6 @@
[EVENT_LOCAL_ ## x] = { __stringify(x), s }
#define PCIE_EVENT(x) \
- .base = MC_PCIE_CTRL_ADDR, \
.offset = PCIE_EVENT_INT, \
.mask_offset = PCIE_EVENT_INT, \
.mask_high = 1, \
@@ -215,7 +211,6 @@
.enb_mask = PCIE_EVENT_INT_ENB_MASK
#define SEC_EVENT(x) \
- .base = MC_PCIE_CTRL_ADDR, \
.offset = SEC_ERROR_INT, \
.mask_offset = SEC_ERROR_INT_MASK, \
.mask = SEC_ERROR_INT_ ## x ## _INT, \
@@ -223,7 +218,6 @@
.enb_mask = 0
#define DED_EVENT(x) \
- .base = MC_PCIE_CTRL_ADDR, \
.offset = DED_ERROR_INT, \
.mask_offset = DED_ERROR_INT_MASK, \
.mask_high = 1, \
@@ -231,7 +225,6 @@
.enb_mask = 0
#define LOCAL_EVENT(x) \
- .base = MC_PCIE_BRIDGE_ADDR, \
.offset = ISTATUS_LOCAL, \
.mask_offset = IMASK_LOCAL, \
.mask_high = 0, \
@@ -265,7 +258,8 @@ struct mc_msi {
};
struct mc_pcie {
- void __iomem *axi_base_addr;
+ void __iomem *bridge_base_addr;
+ void __iomem *ctrl_base_addr;
struct device *dev;
struct irq_domain *intx_domain;
struct irq_domain *event_domain;
@@ -344,7 +338,6 @@ static struct event_map local_status_to_event[] = {
};
static struct {
- u32 base;
u32 offset;
u32 mask;
u32 shift;
@@ -415,18 +408,17 @@ static void mc_handle_msi(struct irq_desc *desc)
struct irq_chip *chip = irq_desc_get_chip(desc);
struct device *dev = port->dev;
struct mc_msi *msi = &port->msi;
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long status;
u32 bit;
int ret;
chained_irq_enter(chip, desc);
- status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
+ status = readl_relaxed(port->bridge_base_addr + ISTATUS_LOCAL);
if (status & PM_MSI_INT_MSI_MASK) {
- writel_relaxed(status & PM_MSI_INT_MSI_MASK, bridge_base_addr + ISTATUS_LOCAL);
- status = readl_relaxed(bridge_base_addr + ISTATUS_MSI);
+ writel_relaxed(status & PM_MSI_INT_MSI_MASK,
+ port->bridge_base_addr + ISTATUS_LOCAL);
+ status = readl_relaxed(port->bridge_base_addr + ISTATUS_MSI);
for_each_set_bit(bit, &status, msi->num_vectors) {
ret = generic_handle_domain_irq(msi->dev_domain, bit);
if (ret)
@@ -441,11 +433,9 @@ static void mc_handle_msi(struct irq_desc *desc)
static void mc_msi_bottom_irq_ack(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
u32 bitpos = data->hwirq;
- writel_relaxed(BIT(bitpos), bridge_base_addr + ISTATUS_MSI);
+ writel_relaxed(BIT(bitpos), port->bridge_base_addr + ISTATUS_MSI);
}
static void mc_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
@@ -564,15 +554,13 @@ static void mc_handle_intx(struct irq_desc *desc)
struct mc_pcie *port = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
struct device *dev = port->dev;
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long status;
u32 bit;
int ret;
chained_irq_enter(chip, desc);
- status = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
+ status = readl_relaxed(port->bridge_base_addr + ISTATUS_LOCAL);
if (status & PM_MSI_INT_INTX_MASK) {
status &= PM_MSI_INT_INTX_MASK;
status >>= PM_MSI_INT_INTX_SHIFT;
@@ -590,42 +578,36 @@ static void mc_handle_intx(struct irq_desc *desc)
static void mc_ack_intx_irq(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT);
- writel_relaxed(mask, bridge_base_addr + ISTATUS_LOCAL);
+ writel_relaxed(mask, port->bridge_base_addr + ISTATUS_LOCAL);
}
static void mc_mask_intx_irq(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long flags;
u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT);
u32 val;
raw_spin_lock_irqsave(&port->lock, flags);
- val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
+ val = readl_relaxed(port->bridge_base_addr + IMASK_LOCAL);
val &= ~mask;
- writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
+ writel_relaxed(val, port->bridge_base_addr + IMASK_LOCAL);
raw_spin_unlock_irqrestore(&port->lock, flags);
}
static void mc_unmask_intx_irq(struct irq_data *data)
{
struct mc_pcie *port = irq_data_get_irq_chip_data(data);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
unsigned long flags;
u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT);
u32 val;
raw_spin_lock_irqsave(&port->lock, flags);
- val = readl_relaxed(bridge_base_addr + IMASK_LOCAL);
+ val = readl_relaxed(port->bridge_base_addr + IMASK_LOCAL);
val |= mask;
- writel_relaxed(val, bridge_base_addr + IMASK_LOCAL);
+ writel_relaxed(val, port->bridge_base_addr + IMASK_LOCAL);
raw_spin_unlock_irqrestore(&port->lock, flags);
}
@@ -656,8 +638,7 @@ static inline u32 reg_to_event(u32 reg, struct event_map field)
static u32 pcie_events(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
- u32 reg = readl_relaxed(ctrl_base_addr + PCIE_EVENT_INT);
+ u32 reg = readl_relaxed(port->ctrl_base_addr + PCIE_EVENT_INT);
u32 val = 0;
int i;
@@ -669,8 +650,7 @@ static u32 pcie_events(struct mc_pcie *port)
static u32 sec_errors(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
- u32 reg = readl_relaxed(ctrl_base_addr + SEC_ERROR_INT);
+ u32 reg = readl_relaxed(port->ctrl_base_addr + SEC_ERROR_INT);
u32 val = 0;
int i;
@@ -682,8 +662,7 @@ static u32 sec_errors(struct mc_pcie *port)
static u32 ded_errors(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
- u32 reg = readl_relaxed(ctrl_base_addr + DED_ERROR_INT);
+ u32 reg = readl_relaxed(port->ctrl_base_addr + DED_ERROR_INT);
u32 val = 0;
int i;
@@ -695,8 +674,7 @@ static u32 ded_errors(struct mc_pcie *port)
static u32 local_events(struct mc_pcie *port)
{
- void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
- u32 reg = readl_relaxed(bridge_base_addr + ISTATUS_LOCAL);
+ u32 reg = readl_relaxed(port->bridge_base_addr + ISTATUS_LOCAL);
u32 val = 0;
int i;
@@ -758,8 +736,12 @@ static void mc_ack_event_irq(struct irq_data *data)
void __iomem *addr;
u32 mask;
- addr = port->axi_base_addr + event_descs[event].base +
- event_descs[event].offset;
+ if (event_descs[event].offset == ISTATUS_LOCAL)
+ addr = port->bridge_base_addr;
+ else
+ addr = port->ctrl_base_addr;
+
+ addr += event_descs[event].offset;
mask = event_descs[event].mask;
mask |= event_descs[event].enb_mask;
@@ -774,8 +756,12 @@ static void mc_mask_event_irq(struct irq_data *data)
u32 mask;
u32 val;
- addr = port->axi_base_addr + event_descs[event].base +
- event_descs[event].mask_offset;
+ if (event_descs[event].offset == ISTATUS_LOCAL)
+ addr = port->bridge_base_addr;
+ else
+ addr = port->ctrl_base_addr;
+
+ addr += event_descs[event].mask_offset;
mask = event_descs[event].mask;
if (event_descs[event].enb_mask) {
mask <<= PCIE_EVENT_INT_ENB_SHIFT;
@@ -804,8 +790,12 @@ static void mc_unmask_event_irq(struct irq_data *data)
u32 mask;
u32 val;
- addr = port->axi_base_addr + event_descs[event].base +
- event_descs[event].mask_offset;
+ if (event_descs[event].offset == ISTATUS_LOCAL)
+ addr = port->bridge_base_addr;
+ else
+ addr = port->ctrl_base_addr;
+
+ addr += event_descs[event].mask_offset;
mask = event_descs[event].mask;
if (event_descs[event].enb_mask)
@@ -972,8 +962,6 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
static int mc_pcie_setup_windows(struct platform_device *pdev,
struct mc_pcie *port)
{
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
struct pci_host_bridge *bridge = platform_get_drvdata(pdev);
struct resource_entry *entry;
u64 pci_addr;
@@ -982,7 +970,7 @@ static int mc_pcie_setup_windows(struct platform_device *pdev,
resource_list_for_each_entry(entry, &bridge->windows) {
if (resource_type(entry->res) == IORESOURCE_MEM) {
pci_addr = entry->res->start - entry->offset;
- mc_pcie_setup_window(bridge_base_addr, index,
+ mc_pcie_setup_window(port->bridge_base_addr, index,
entry->res->start, pci_addr,
resource_size(entry->res));
index++;
@@ -994,26 +982,20 @@ static int mc_pcie_setup_windows(struct platform_device *pdev,
static inline void mc_clear_secs(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
-
- writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, ctrl_base_addr +
+ writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, port->ctrl_base_addr +
SEC_ERROR_INT);
- writel_relaxed(0, ctrl_base_addr + SEC_ERROR_EVENT_CNT);
+ writel_relaxed(0, port->ctrl_base_addr + SEC_ERROR_EVENT_CNT);
}
static inline void mc_clear_deds(struct mc_pcie *port)
{
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
-
- writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, ctrl_base_addr +
+ writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, port->ctrl_base_addr +
DED_ERROR_INT);
- writel_relaxed(0, ctrl_base_addr + DED_ERROR_EVENT_CNT);
+ writel_relaxed(0, port->ctrl_base_addr + DED_ERROR_EVENT_CNT);
}
static void mc_disable_interrupts(struct mc_pcie *port)
{
- void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
- void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
u32 val;
/* Ensure ECC bypass is enabled */
@@ -1021,22 +1003,22 @@ static void mc_disable_interrupts(struct mc_pcie *port)
ECC_CONTROL_RX_RAM_ECC_BYPASS |
ECC_CONTROL_PCIE2AXI_RAM_ECC_BYPASS |
ECC_CONTROL_AXI2PCIE_RAM_ECC_BYPASS;
- writel_relaxed(val, ctrl_base_addr + ECC_CONTROL);
+ writel_relaxed(val, port->ctrl_base_addr + ECC_CONTROL);
/* Disable SEC errors and clear any outstanding */
- writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, ctrl_base_addr +
+ writel_relaxed(SEC_ERROR_INT_ALL_RAM_SEC_ERR_INT, port->ctrl_base_addr +
SEC_ERROR_INT_MASK);
mc_clear_secs(port);
/* Disable DED errors and clear any outstanding */
- writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, ctrl_base_addr +
+ writel_relaxed(DED_ERROR_INT_ALL_RAM_DED_ERR_INT, port->ctrl_base_addr +
DED_ERROR_INT_MASK);
mc_clear_deds(port);
/* Disable local interrupts and clear any outstanding */
- writel_relaxed(0, bridge_base_addr + IMASK_LOCAL);
- writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_LOCAL);
- writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_MSI);
+ writel_relaxed(0, port->bridge_base_addr + IMASK_LOCAL);
+ writel_relaxed(GENMASK(31, 0), port->bridge_base_addr + ISTATUS_LOCAL);
+ writel_relaxed(GENMASK(31, 0), port->bridge_base_addr + ISTATUS_MSI);
/* Disable PCIe events and clear any outstanding */
val = PCIE_EVENT_INT_L2_EXIT_INT |
@@ -1045,11 +1027,11 @@ static void mc_disable_interrupts(struct mc_pcie *port)
PCIE_EVENT_INT_L2_EXIT_INT_MASK |
PCIE_EVENT_INT_HOTRST_EXIT_INT_MASK |
PCIE_EVENT_INT_DLUP_EXIT_INT_MASK;
- writel_relaxed(val, ctrl_base_addr + PCIE_EVENT_INT);
+ writel_relaxed(val, port->ctrl_base_addr + PCIE_EVENT_INT);
/* Disable host interrupts and clear any outstanding */
- writel_relaxed(0, bridge_base_addr + IMASK_HOST);
- writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST);
+ writel_relaxed(0, port->bridge_base_addr + IMASK_HOST);
+ writel_relaxed(GENMASK(31, 0), port->bridge_base_addr + ISTATUS_HOST);
}
static int mc_init_interrupts(struct platform_device *pdev, struct mc_pcie *port)
@@ -1112,14 +1094,11 @@ static int mc_platform_init(struct pci_config_window *cfg)
{
struct device *dev = cfg->parent;
struct platform_device *pdev = to_platform_device(dev);
- void __iomem *bridge_base_addr =
- port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
int ret;
/* Configure address translation table 0 for PCIe config space */
- mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start,
- cfg->res.start,
- resource_size(&cfg->res));
+ mc_pcie_setup_window(port->bridge_base_addr, 0, cfg->res.start,
+ cfg->res.start, resource_size(&cfg->res));
/* Need some fixups in config space */
mc_pcie_enable_msi(port, cfg->win);
@@ -1140,7 +1119,7 @@ static int mc_platform_init(struct pci_config_window *cfg)
static int mc_host_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- void __iomem *bridge_base_addr;
+ void __iomem *apb_base_addr;
int ret;
u32 val;
@@ -1150,28 +1129,40 @@ static int mc_host_probe(struct platform_device *pdev)
port->dev = dev;
- port->axi_base_addr = devm_platform_ioremap_resource(pdev, 1);
- if (IS_ERR(port->axi_base_addr))
- return PTR_ERR(port->axi_base_addr);
+ port->bridge_base_addr = devm_platform_ioremap_resource_byname(pdev, "bridge");
+ port->ctrl_base_addr = devm_platform_ioremap_resource_byname(pdev, "ctrl");
+ if(!IS_ERR(port->bridge_base_addr) && !IS_ERR(port->ctrl_base_addr))
+ goto addrs_set;
+ /*
+ * The original, incorrect, binding that lumped the control and
+ * bridge addresses together still needs to be handled by the driver.
+ */
+ apb_base_addr = devm_platform_ioremap_resource_byname(pdev, "apb");
+ if (IS_ERR(apb_base_addr))
+ return dev_err_probe(dev, PTR_ERR(port->bridge_base_addr),
+ "both legacy apb register and ctrl/bridge regions missing");
+
+ port->bridge_base_addr = apb_base_addr + MC_PCIE1_BRIDGE_ADDR;
+ port->ctrl_base_addr = apb_base_addr + MC_PCIE1_CTRL_ADDR;
+
+addrs_set:
mc_disable_interrupts(port);
- bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
-
/* Allow enabling MSI by disabling MSI-X */
- val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
+ val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
val &= ~MSIX_CAP_MASK;
- writel(val, bridge_base_addr + PCIE_PCI_IRQ_DW0);
+ writel(val, port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
/* Pick num vectors from bitfile programmed onto FPGA fabric */
- val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0);
+ val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
val &= NUM_MSI_MSGS_MASK;
val >>= NUM_MSI_MSGS_SHIFT;
port->msi.num_vectors = 1 << val;
/* Pick vector address from design */
- port->msi.vector_phy = readl_relaxed(bridge_base_addr + IMSI_ADDR);
+ port->msi.vector_phy = readl_relaxed(port->bridge_base_addr + IMSI_ADDR);
ret = mc_pcie_init_clks(dev);
if (ret) {
--
2.43.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
2024-07-01 10:22 ` Conor Dooley
(?)
@ 2024-07-03 20:01 ` kernel test robot
2024-07-03 20:14 ` Conor Dooley
-1 siblings, 1 reply; 8+ messages in thread
From: kernel test robot @ 2024-07-03 20:01 UTC (permalink / raw)
To: Conor Dooley; +Cc: oe-kbuild-all
Hi Conor,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/for-linus]
[also build test WARNING on robh/for-next linus/master v6.10-rc6]
[cannot apply to pci/next next-20240703]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Conor-Dooley/dt-bindings-PCI-microchip-pcie-host-fix-reg-properties/20240701-183154
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
patch link: https://lore.kernel.org/r/20240701-charger-recognize-fd8a65026a89%40spud
patch subject: [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
config: x86_64-randconfig-102-20240703 (https://download.01.org/0day-ci/archive/20240704/202407040339.i1nFFozS-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407040339.i1nFFozS-lkp@intel.com/
cocci warnings: (new ones prefixed by >>)
>> drivers/pci/controller/pcie-microchip-host.c:1142:5-11: inconsistent IS_ERR and PTR_ERR on line 1143.
vim +1142 drivers/pci/controller/pcie-microchip-host.c
1118
1119 static int mc_host_probe(struct platform_device *pdev)
1120 {
1121 struct device *dev = &pdev->dev;
1122 void __iomem *apb_base_addr;
1123 int ret;
1124 u32 val;
1125
1126 port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
1127 if (!port)
1128 return -ENOMEM;
1129
1130 port->dev = dev;
1131
1132 port->bridge_base_addr = devm_platform_ioremap_resource_byname(pdev, "bridge");
1133 port->ctrl_base_addr = devm_platform_ioremap_resource_byname(pdev, "ctrl");
1134 if(!IS_ERR(port->bridge_base_addr) && !IS_ERR(port->ctrl_base_addr))
1135 goto addrs_set;
1136
1137 /*
1138 * The original, incorrect, binding that lumped the control and
1139 * bridge addresses together still needs to be handled by the driver.
1140 */
1141 apb_base_addr = devm_platform_ioremap_resource_byname(pdev, "apb");
> 1142 if (IS_ERR(apb_base_addr))
> 1143 return dev_err_probe(dev, PTR_ERR(port->bridge_base_addr),
1144 "both legacy apb register and ctrl/bridge regions missing");
1145
1146 port->bridge_base_addr = apb_base_addr + MC_PCIE1_BRIDGE_ADDR;
1147 port->ctrl_base_addr = apb_base_addr + MC_PCIE1_CTRL_ADDR;
1148
1149 addrs_set:
1150 mc_disable_interrupts(port);
1151
1152 /* Allow enabling MSI by disabling MSI-X */
1153 val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
1154 val &= ~MSIX_CAP_MASK;
1155 writel(val, port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
1156
1157 /* Pick num vectors from bitfile programmed onto FPGA fabric */
1158 val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
1159 val &= NUM_MSI_MSGS_MASK;
1160 val >>= NUM_MSI_MSGS_SHIFT;
1161
1162 port->msi.num_vectors = 1 << val;
1163
1164 /* Pick vector address from design */
1165 port->msi.vector_phy = readl_relaxed(port->bridge_base_addr + IMSI_ADDR);
1166
1167 ret = mc_pcie_init_clks(dev);
1168 if (ret) {
1169 dev_err(dev, "failed to get clock resources, error %d\n", ret);
1170 return -ENODEV;
1171 }
1172
1173 return pci_host_common_probe(pdev);
1174 }
1175
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
2024-07-03 20:01 ` kernel test robot
@ 2024-07-03 20:14 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2024-07-03 20:14 UTC (permalink / raw)
To: kernel test robot, Julia Lawall; +Cc: oe-kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4693 bytes --]
On Thu, Jul 04, 2024 at 04:01:42AM +0800, kernel test robot wrote:
> Hi Conor,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on pci/for-linus]
> [also build test WARNING on robh/for-next linus/master v6.10-rc6]
> [cannot apply to pci/next next-20240703]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Conor-Dooley/dt-bindings-PCI-microchip-pcie-host-fix-reg-properties/20240701-183154
> base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
> patch link: https://lore.kernel.org/r/20240701-charger-recognize-fd8a65026a89%40spud
> patch subject: [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
> config: x86_64-randconfig-102-20240703 (https://download.01.org/0day-ci/archive/20240704/202407040339.i1nFFozS-lkp@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202407040339.i1nFFozS-lkp@intel.com/
>
> cocci warnings: (new ones prefixed by >>)
> >> drivers/pci/controller/pcie-microchip-host.c:1142:5-11: inconsistent IS_ERR and PTR_ERR on line 1143.
This is a valid warning and was reported against my patch submission
rather than my kernel.org tree where I had put the patch overnight
before submitting - any reason why it only CCed me and the build issue
mailing list? Seems worth sending to the maintainers etc since it is a
Real Bug^tm.
Also, Julia - any chance of an improved error message here? I was
scratching my head saying wtf cos IS_ERR() and PTR_ERR() are the right
things to use here - missing that the inconsistency was the variable
passed to it. Would something like "inconsistent argument passed to IS_ERR()
and PTR_ERR() on line $" be an improvement here?
Cheers,
Conor.
>
> vim +1142 drivers/pci/controller/pcie-microchip-host.c
>
> 1118
> 1119 static int mc_host_probe(struct platform_device *pdev)
> 1120 {
> 1121 struct device *dev = &pdev->dev;
> 1122 void __iomem *apb_base_addr;
> 1123 int ret;
> 1124 u32 val;
> 1125
> 1126 port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> 1127 if (!port)
> 1128 return -ENOMEM;
> 1129
> 1130 port->dev = dev;
> 1131
> 1132 port->bridge_base_addr = devm_platform_ioremap_resource_byname(pdev, "bridge");
> 1133 port->ctrl_base_addr = devm_platform_ioremap_resource_byname(pdev, "ctrl");
> 1134 if(!IS_ERR(port->bridge_base_addr) && !IS_ERR(port->ctrl_base_addr))
> 1135 goto addrs_set;
> 1136
> 1137 /*
> 1138 * The original, incorrect, binding that lumped the control and
> 1139 * bridge addresses together still needs to be handled by the driver.
> 1140 */
> 1141 apb_base_addr = devm_platform_ioremap_resource_byname(pdev, "apb");
> > 1142 if (IS_ERR(apb_base_addr))
> > 1143 return dev_err_probe(dev, PTR_ERR(port->bridge_base_addr),
> 1144 "both legacy apb register and ctrl/bridge regions missing");
> 1145
> 1146 port->bridge_base_addr = apb_base_addr + MC_PCIE1_BRIDGE_ADDR;
> 1147 port->ctrl_base_addr = apb_base_addr + MC_PCIE1_CTRL_ADDR;
> 1148
> 1149 addrs_set:
> 1150 mc_disable_interrupts(port);
> 1151
> 1152 /* Allow enabling MSI by disabling MSI-X */
> 1153 val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
> 1154 val &= ~MSIX_CAP_MASK;
> 1155 writel(val, port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
> 1156
> 1157 /* Pick num vectors from bitfile programmed onto FPGA fabric */
> 1158 val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
> 1159 val &= NUM_MSI_MSGS_MASK;
> 1160 val >>= NUM_MSI_MSGS_SHIFT;
> 1161
> 1162 port->msi.num_vectors = 1 << val;
> 1163
> 1164 /* Pick vector address from design */
> 1165 port->msi.vector_phy = readl_relaxed(port->bridge_base_addr + IMSI_ADDR);
> 1166
> 1167 ret = mc_pcie_init_clks(dev);
> 1168 if (ret) {
> 1169 dev_err(dev, "failed to get clock resources, error %d\n", ret);
> 1170 return -ENODEV;
> 1171 }
> 1172
> 1173 return pci_host_common_probe(pdev);
> 1174 }
> 1175
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-07-03 20:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-01 10:22 [PATCH v3 0/2] PCI: microchip: support using either instance 1 or 2 Conor Dooley
2024-07-01 10:22 ` Conor Dooley
2024-07-01 10:22 ` [PATCH v3 1/2] dt-bindings: PCI: microchip,pcie-host: fix reg properties Conor Dooley
2024-07-01 10:22 ` Conor Dooley
2024-07-01 10:22 ` [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2 Conor Dooley
2024-07-01 10:22 ` Conor Dooley
2024-07-03 20:01 ` kernel test robot
2024-07-03 20:14 ` Conor Dooley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.