* [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
@ 2024-06-21 11:29 ` daire.mcnamara
0 siblings, 0 replies; 17+ messages in thread
From: daire.mcnamara @ 2024-06-21 11:29 UTC (permalink / raw)
To: linux-pci, devicetree
Cc: conor.dooley, lpieralisi, kw, robh, bhelgaas, linux-kernel,
linux-riscv, krzk+dt, conor+dt, daire.mcnamara, ilpo.jarvinen
From: Daire McNamara <daire.mcnamara@microchip.com>
On Microchip PolarFire SoC the PCIe Root Port can be behind one of three
general purpose Fabric Interface Controller (FIC) buses that encapsulates
an AXI-S bus. Depending on which FIC(s) the Root Port is connected
through to CPU space, and what address translation is done by that FIC,
the Root Port driver's inbound address translation may vary.
For all current supported designs and all future expected designs,
inbound address translation done by a FIC on PolarFire SoC varies
depending on whether PolarFire SoC in operating in dma-coherent mode or
dma-noncoherent mode.
The setup of the outbound address translation tables in the root port
driver only needs to handle these two cases.
Setup the inbound address translation tables to one of two address
translations, depending on whether the rootport is marked as dma-coherent or
dma-noncoherent.
Fixes: 6f15a9c9f941 ("PCI: microchip: Add Microchip Polarfire PCIe controller driver")
Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
---
drivers/pci/controller/pcie-microchip-host.c | 102 +++++++++++++++++--
1 file changed, 93 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
index 853adce24492..d0489bd42bef 100644
--- a/drivers/pci/controller/pcie-microchip-host.c
+++ b/drivers/pci/controller/pcie-microchip-host.c
@@ -30,6 +30,9 @@
#define MC_PCIE_BRIDGE_ADDR (MC_PCIE1_BRIDGE_ADDR)
#define MC_PCIE_CTRL_ADDR (MC_PCIE1_CTRL_ADDR)
+#define MC_MAX_NUM_INBOUND_WINDOWS 8
+#define MPFS_NC_BOUNCE_ADDR 0x80000000
+
/* PCIe Bridge Phy Regs */
#define PCIE_PCI_IRQ_DW0 0xa8
#define MSIX_CAP_MASK BIT(31)
@@ -97,14 +100,15 @@
/* PCIe AXI slave table init defines */
#define ATR0_AXI4_SLV0_SRCADDR_PARAM 0x800u
-#define ATR_SIZE_SHIFT 1
-#define ATR_IMPL_ENABLE 1
+#define ATR_SIZE_MASK GENMASK(6, 1)
+#define ATR_IMPL_ENABLE_MASK 1
#define ATR0_AXI4_SLV0_SRC_ADDR 0x804u
#define ATR0_AXI4_SLV0_TRSL_ADDR_LSB 0x808u
#define ATR0_AXI4_SLV0_TRSL_ADDR_UDW 0x80cu
#define ATR0_AXI4_SLV0_TRSL_PARAM 0x810u
#define PCIE_TX_RX_INTERFACE 0x00000000u
#define PCIE_CONFIG_INTERFACE 0x00000001u
+#define TRSL_ID_AXI4_MASTER_0 0x00000004u
#define ATR_ENTRY_SIZE 32
@@ -931,6 +935,86 @@ static int mc_pcie_init_irq_domains(struct mc_pcie *port)
return mc_allocate_msi_domains(port);
}
+static void mc_pcie_setup_inbound_atr(int window_index, u64 axi_addr, u64 pcie_addr, size_t size)
+{
+ void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
+ u32 table_offset = window_index * ATR_ENTRY_SIZE;
+ void __iomem *table_addr = bridge_base_addr + table_offset;
+ u32 atr_sz;
+ u32 val;
+
+ atr_sz = ilog2(size) - 1;
+
+ val = ALIGN_DOWN(lower_32_bits(pcie_addr), SZ_4K);
+ val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
+ val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
+
+ writel(val, table_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
+
+ writel(upper_32_bits(pcie_addr), table_addr + ATR0_PCIE_WIN0_SRC_ADDR);
+
+ writel(lower_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_LSB);
+ writel(upper_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_UDW);
+
+ writel(TRSL_ID_AXI4_MASTER_0, table_addr + ATR0_PCIE_WIN0_TRSL_PARAM);
+}
+
+static int mc_pcie_setup_inbound_ranges(struct platform_device *pdev, struct mc_pcie *port)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *dn = dev->of_node;
+ struct of_range_parser parser;
+ struct of_range range;
+ int atr_index = 0;
+
+ /*
+ * MPFS PCIe root port is 32-bit only, behind a Fabric Interface
+ * Controller FPGA logic block which contains the AXI-S interface.
+ *
+ * From the point of view of the PCIe root port, There are only
+ * two supported Root Port configurations
+ *
+ * Configuration 1: for use with fully coherent designs; supports a
+ * window from 0x0 (CPU space) to specified PCIe space.
+ *
+ * Configuration 2: for use with non-coherent designs; supports two
+ * 1 Gb wide windows to CPU space; one mapping cpu space 0 to pcie
+ * space 0x80000000 and mapping cpu space 0x40000000 to pcie
+ * space 0xc0000000. This cfg needs two windows because of how
+ * the MSI space is allocated in the AXI-S range on MPFS.
+ *
+ * The FIC interface outside the PCIe block *must* complete the inbound
+ * address translation as per MCHP MPFS FPGA design guidelines.
+ */
+ if (device_property_read_bool(dev, "dma-noncoherent")) {
+ /*
+ * Always need same two tables in this case. Need two tables
+ * due to hardware interactions between address and size.
+ */
+ mc_pcie_setup_inbound_atr(0, 0, MPFS_NC_BOUNCE_ADDR, SZ_1G);
+ mc_pcie_setup_inbound_atr(1, SZ_1G, MPFS_NC_BOUNCE_ADDR + SZ_1G, SZ_1G);
+ } else {
+ /* Find any dma-ranges */
+ if (of_pci_dma_range_parser_init(&parser, dn)) {
+ /* No dma-range property - setup default */
+ mc_pcie_setup_inbound_atr(0, 0, 0, SZ_4G);
+ return 0;
+ }
+
+ for_each_of_range(&parser, &range) {
+ if (atr_index >= MC_MAX_NUM_INBOUND_WINDOWS) {
+ dev_err(dev, "too many inbound ranges; %d available tables\n",
+ MC_MAX_NUM_INBOUND_WINDOWS);
+ return -EINVAL;
+ }
+ mc_pcie_setup_inbound_atr(atr_index, 0, range.pci_addr, range.size);
+ atr_index++;
+ }
+ }
+
+ return 0;
+}
+
static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
phys_addr_t axi_addr, phys_addr_t pci_addr,
u64 size)
@@ -946,8 +1030,9 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
ATR0_AXI4_SLV0_TRSL_PARAM);
- val = lower_32_bits(axi_addr) | (atr_sz << ATR_SIZE_SHIFT) |
- ATR_IMPL_ENABLE;
+ val = ALIGN_DOWN(lower_32_bits(axi_addr), SZ_4K);
+ val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
+ val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
ATR0_AXI4_SLV0_SRCADDR_PARAM);
@@ -962,11 +1047,6 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
val = upper_32_bits(pci_addr);
writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
ATR0_AXI4_SLV0_TRSL_ADDR_UDW);
-
- val = readl(bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
- val |= (ATR0_PCIE_ATR_SIZE << ATR0_PCIE_ATR_SIZE_SHIFT);
- writel(val, bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
- writel(0, bridge_base_addr + ATR0_PCIE_WIN0_SRC_ADDR);
}
static int mc_pcie_setup_windows(struct platform_device *pdev,
@@ -1129,6 +1209,10 @@ static int mc_platform_init(struct pci_config_window *cfg)
if (ret)
return ret;
+ ret = mc_pcie_setup_inbound_ranges(pdev, port);
+ if (ret)
+ return ret;
+
/* Address translation is up; safe to enable interrupts */
ret = mc_init_interrupts(pdev, port);
if (ret)
--
2.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
2024-06-21 11:29 ` daire.mcnamara
@ 2024-06-23 17:19 ` Ilpo Järvinen
-1 siblings, 0 replies; 17+ messages in thread
From: Ilpo Järvinen @ 2024-06-23 17:19 UTC (permalink / raw)
To: daire.mcnamara
Cc: linux-pci, devicetree, conor.dooley, lpieralisi, kw, robh,
bhelgaas, LKML, linux-riscv, krzk+dt, conor+dt, ilpo.jarvinen
On Fri, 21 Jun 2024, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> On Microchip PolarFire SoC the PCIe Root Port can be behind one of three
> general purpose Fabric Interface Controller (FIC) buses that encapsulates
> an AXI-S bus. Depending on which FIC(s) the Root Port is connected
> through to CPU space, and what address translation is done by that FIC,
> the Root Port driver's inbound address translation may vary.
>
> For all current supported designs and all future expected designs,
> inbound address translation done by a FIC on PolarFire SoC varies
> depending on whether PolarFire SoC in operating in dma-coherent mode or
> dma-noncoherent mode.
>
> The setup of the outbound address translation tables in the root port
> driver only needs to handle these two cases.
>
> Setup the inbound address translation tables to one of two address
> translations, depending on whether the rootport is marked as dma-coherent or
> dma-noncoherent.
>
> Fixes: 6f15a9c9f941 ("PCI: microchip: Add Microchip Polarfire PCIe controller driver")
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Hi,
As mentioned against v3 1/3, don't leave empty lines between tags.
> ---
> drivers/pci/controller/pcie-microchip-host.c | 102 +++++++++++++++++--
> 1 file changed, 93 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
> index 853adce24492..d0489bd42bef 100644
> --- a/drivers/pci/controller/pcie-microchip-host.c
> +++ b/drivers/pci/controller/pcie-microchip-host.c
> @@ -30,6 +30,9 @@
> #define MC_PCIE_BRIDGE_ADDR (MC_PCIE1_BRIDGE_ADDR)
> #define MC_PCIE_CTRL_ADDR (MC_PCIE1_CTRL_ADDR)
>
> +#define MC_MAX_NUM_INBOUND_WINDOWS 8
> +#define MPFS_NC_BOUNCE_ADDR 0x80000000
> +
> /* PCIe Bridge Phy Regs */
> #define PCIE_PCI_IRQ_DW0 0xa8
> #define MSIX_CAP_MASK BIT(31)
> @@ -97,14 +100,15 @@
>
> /* PCIe AXI slave table init defines */
> #define ATR0_AXI4_SLV0_SRCADDR_PARAM 0x800u
> -#define ATR_SIZE_SHIFT 1
> -#define ATR_IMPL_ENABLE 1
> +#define ATR_SIZE_MASK GENMASK(6, 1)
#include <linux/bits.h>
> +#define ATR_IMPL_ENABLE_MASK 1
This would be BIT(0), I think. IMO, you don't need to add _MASK postfix
for it, since it's just 1-bit wide field.
> #define ATR0_AXI4_SLV0_SRC_ADDR 0x804u
> #define ATR0_AXI4_SLV0_TRSL_ADDR_LSB 0x808u
> #define ATR0_AXI4_SLV0_TRSL_ADDR_UDW 0x80cu
> #define ATR0_AXI4_SLV0_TRSL_PARAM 0x810u
> #define PCIE_TX_RX_INTERFACE 0x00000000u
> #define PCIE_CONFIG_INTERFACE 0x00000001u
> +#define TRSL_ID_AXI4_MASTER_0 0x00000004u
>
> #define ATR_ENTRY_SIZE 32
>
> @@ -931,6 +935,86 @@ static int mc_pcie_init_irq_domains(struct mc_pcie *port)
> return mc_allocate_msi_domains(port);
> }
>
> +static void mc_pcie_setup_inbound_atr(int window_index, u64 axi_addr, u64 pcie_addr, size_t size)
> +{
> + void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
> + u32 table_offset = window_index * ATR_ENTRY_SIZE;
> + void __iomem *table_addr = bridge_base_addr + table_offset;
> + u32 atr_sz;
> + u32 val;
> +
> + atr_sz = ilog2(size) - 1;
You should add explicit includes you use:
#include <linux/log2.h>
> +
> + val = ALIGN_DOWN(lower_32_bits(pcie_addr), SZ_4K);
#include <linux/align.h>
> + val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
> + val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
> +
> + writel(val, table_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> +
> + writel(upper_32_bits(pcie_addr), table_addr + ATR0_PCIE_WIN0_SRC_ADDR);
#include <linux/wordpart.h>
> + writel(lower_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_LSB);
> + writel(upper_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_UDW);
> +
> + writel(TRSL_ID_AXI4_MASTER_0, table_addr + ATR0_PCIE_WIN0_TRSL_PARAM);
> +}
> +
> +static int mc_pcie_setup_inbound_ranges(struct platform_device *pdev, struct mc_pcie *port)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *dn = dev->of_node;
> + struct of_range_parser parser;
> + struct of_range range;
> + int atr_index = 0;
> +
> + /*
> + * MPFS PCIe root port is 32-bit only, behind a Fabric Interface
> + * Controller FPGA logic block which contains the AXI-S interface.
> + *
> + * From the point of view of the PCIe root port, There are only
> + * two supported Root Port configurations
> + *
> + * Configuration 1: for use with fully coherent designs; supports a
> + * window from 0x0 (CPU space) to specified PCIe space.
> + *
> + * Configuration 2: for use with non-coherent designs; supports two
> + * 1 Gb wide windows to CPU space; one mapping cpu space 0 to pcie
> + * space 0x80000000 and mapping cpu space 0x40000000 to pcie
> + * space 0xc0000000. This cfg needs two windows because of how
> + * the MSI space is allocated in the AXI-S range on MPFS.
> + *
> + * The FIC interface outside the PCIe block *must* complete the inbound
> + * address translation as per MCHP MPFS FPGA design guidelines.
> + */
> + if (device_property_read_bool(dev, "dma-noncoherent")) {
> + /*
> + * Always need same two tables in this case. Need two tables
> + * due to hardware interactions between address and size.
> + */
> + mc_pcie_setup_inbound_atr(0, 0, MPFS_NC_BOUNCE_ADDR, SZ_1G);
> + mc_pcie_setup_inbound_atr(1, SZ_1G, MPFS_NC_BOUNCE_ADDR + SZ_1G, SZ_1G);
> + } else {
> + /* Find any dma-ranges */
> + if (of_pci_dma_range_parser_init(&parser, dn)) {
> + /* No dma-range property - setup default */
> + mc_pcie_setup_inbound_atr(0, 0, 0, SZ_4G);
> + return 0;
> + }
> +
> + for_each_of_range(&parser, &range) {
> + if (atr_index >= MC_MAX_NUM_INBOUND_WINDOWS) {
> + dev_err(dev, "too many inbound ranges; %d available tables\n",
> + MC_MAX_NUM_INBOUND_WINDOWS);
> + return -EINVAL;
> + }
> + mc_pcie_setup_inbound_atr(atr_index, 0, range.pci_addr, range.size);
> + atr_index++;
> + }
> + }
> +
> + return 0;
> +}
> +
> static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> phys_addr_t axi_addr, phys_addr_t pci_addr,
> u64 size)
> @@ -946,8 +1030,9 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_TRSL_PARAM);
>
> - val = lower_32_bits(axi_addr) | (atr_sz << ATR_SIZE_SHIFT) |
> - ATR_IMPL_ENABLE;
> + val = ALIGN_DOWN(lower_32_bits(axi_addr), SZ_4K);
> + val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
> + val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
This can be just val |= ATR_IMPL_ENABLE when you don't have _MASK
there which is easier to read (IMO).
It would be nice to put the GENMASK()/FIELD_PREP() refactor into a
preparatory patch on top of which you'd make the actual fix to keep the
fix change itself simpler.
Nonetheless, this was already much better than the previous version.
--
i.
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_SRCADDR_PARAM);
>
> @@ -962,11 +1047,6 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> val = upper_32_bits(pci_addr);
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_TRSL_ADDR_UDW);
> -
> - val = readl(bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> - val |= (ATR0_PCIE_ATR_SIZE << ATR0_PCIE_ATR_SIZE_SHIFT);
> - writel(val, bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> - writel(0, bridge_base_addr + ATR0_PCIE_WIN0_SRC_ADDR);
> }
>
> static int mc_pcie_setup_windows(struct platform_device *pdev,
> @@ -1129,6 +1209,10 @@ static int mc_platform_init(struct pci_config_window *cfg)
> if (ret)
> return ret;
>
> + ret = mc_pcie_setup_inbound_ranges(pdev, port);
> + if (ret)
> + return ret;
> +
> /* Address translation is up; safe to enable interrupts */
> ret = mc_init_interrupts(pdev, port);
> if (ret)
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
@ 2024-06-23 17:19 ` Ilpo Järvinen
0 siblings, 0 replies; 17+ messages in thread
From: Ilpo Järvinen @ 2024-06-23 17:19 UTC (permalink / raw)
To: daire.mcnamara
Cc: linux-pci, devicetree, conor.dooley, lpieralisi, kw, robh,
bhelgaas, LKML, linux-riscv, krzk+dt, conor+dt, ilpo.jarvinen
On Fri, 21 Jun 2024, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> On Microchip PolarFire SoC the PCIe Root Port can be behind one of three
> general purpose Fabric Interface Controller (FIC) buses that encapsulates
> an AXI-S bus. Depending on which FIC(s) the Root Port is connected
> through to CPU space, and what address translation is done by that FIC,
> the Root Port driver's inbound address translation may vary.
>
> For all current supported designs and all future expected designs,
> inbound address translation done by a FIC on PolarFire SoC varies
> depending on whether PolarFire SoC in operating in dma-coherent mode or
> dma-noncoherent mode.
>
> The setup of the outbound address translation tables in the root port
> driver only needs to handle these two cases.
>
> Setup the inbound address translation tables to one of two address
> translations, depending on whether the rootport is marked as dma-coherent or
> dma-noncoherent.
>
> Fixes: 6f15a9c9f941 ("PCI: microchip: Add Microchip Polarfire PCIe controller driver")
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Hi,
As mentioned against v3 1/3, don't leave empty lines between tags.
> ---
> drivers/pci/controller/pcie-microchip-host.c | 102 +++++++++++++++++--
> 1 file changed, 93 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
> index 853adce24492..d0489bd42bef 100644
> --- a/drivers/pci/controller/pcie-microchip-host.c
> +++ b/drivers/pci/controller/pcie-microchip-host.c
> @@ -30,6 +30,9 @@
> #define MC_PCIE_BRIDGE_ADDR (MC_PCIE1_BRIDGE_ADDR)
> #define MC_PCIE_CTRL_ADDR (MC_PCIE1_CTRL_ADDR)
>
> +#define MC_MAX_NUM_INBOUND_WINDOWS 8
> +#define MPFS_NC_BOUNCE_ADDR 0x80000000
> +
> /* PCIe Bridge Phy Regs */
> #define PCIE_PCI_IRQ_DW0 0xa8
> #define MSIX_CAP_MASK BIT(31)
> @@ -97,14 +100,15 @@
>
> /* PCIe AXI slave table init defines */
> #define ATR0_AXI4_SLV0_SRCADDR_PARAM 0x800u
> -#define ATR_SIZE_SHIFT 1
> -#define ATR_IMPL_ENABLE 1
> +#define ATR_SIZE_MASK GENMASK(6, 1)
#include <linux/bits.h>
> +#define ATR_IMPL_ENABLE_MASK 1
This would be BIT(0), I think. IMO, you don't need to add _MASK postfix
for it, since it's just 1-bit wide field.
> #define ATR0_AXI4_SLV0_SRC_ADDR 0x804u
> #define ATR0_AXI4_SLV0_TRSL_ADDR_LSB 0x808u
> #define ATR0_AXI4_SLV0_TRSL_ADDR_UDW 0x80cu
> #define ATR0_AXI4_SLV0_TRSL_PARAM 0x810u
> #define PCIE_TX_RX_INTERFACE 0x00000000u
> #define PCIE_CONFIG_INTERFACE 0x00000001u
> +#define TRSL_ID_AXI4_MASTER_0 0x00000004u
>
> #define ATR_ENTRY_SIZE 32
>
> @@ -931,6 +935,86 @@ static int mc_pcie_init_irq_domains(struct mc_pcie *port)
> return mc_allocate_msi_domains(port);
> }
>
> +static void mc_pcie_setup_inbound_atr(int window_index, u64 axi_addr, u64 pcie_addr, size_t size)
> +{
> + void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
> + u32 table_offset = window_index * ATR_ENTRY_SIZE;
> + void __iomem *table_addr = bridge_base_addr + table_offset;
> + u32 atr_sz;
> + u32 val;
> +
> + atr_sz = ilog2(size) - 1;
You should add explicit includes you use:
#include <linux/log2.h>
> +
> + val = ALIGN_DOWN(lower_32_bits(pcie_addr), SZ_4K);
#include <linux/align.h>
> + val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
> + val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
> +
> + writel(val, table_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> +
> + writel(upper_32_bits(pcie_addr), table_addr + ATR0_PCIE_WIN0_SRC_ADDR);
#include <linux/wordpart.h>
> + writel(lower_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_LSB);
> + writel(upper_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_UDW);
> +
> + writel(TRSL_ID_AXI4_MASTER_0, table_addr + ATR0_PCIE_WIN0_TRSL_PARAM);
> +}
> +
> +static int mc_pcie_setup_inbound_ranges(struct platform_device *pdev, struct mc_pcie *port)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *dn = dev->of_node;
> + struct of_range_parser parser;
> + struct of_range range;
> + int atr_index = 0;
> +
> + /*
> + * MPFS PCIe root port is 32-bit only, behind a Fabric Interface
> + * Controller FPGA logic block which contains the AXI-S interface.
> + *
> + * From the point of view of the PCIe root port, There are only
> + * two supported Root Port configurations
> + *
> + * Configuration 1: for use with fully coherent designs; supports a
> + * window from 0x0 (CPU space) to specified PCIe space.
> + *
> + * Configuration 2: for use with non-coherent designs; supports two
> + * 1 Gb wide windows to CPU space; one mapping cpu space 0 to pcie
> + * space 0x80000000 and mapping cpu space 0x40000000 to pcie
> + * space 0xc0000000. This cfg needs two windows because of how
> + * the MSI space is allocated in the AXI-S range on MPFS.
> + *
> + * The FIC interface outside the PCIe block *must* complete the inbound
> + * address translation as per MCHP MPFS FPGA design guidelines.
> + */
> + if (device_property_read_bool(dev, "dma-noncoherent")) {
> + /*
> + * Always need same two tables in this case. Need two tables
> + * due to hardware interactions between address and size.
> + */
> + mc_pcie_setup_inbound_atr(0, 0, MPFS_NC_BOUNCE_ADDR, SZ_1G);
> + mc_pcie_setup_inbound_atr(1, SZ_1G, MPFS_NC_BOUNCE_ADDR + SZ_1G, SZ_1G);
> + } else {
> + /* Find any dma-ranges */
> + if (of_pci_dma_range_parser_init(&parser, dn)) {
> + /* No dma-range property - setup default */
> + mc_pcie_setup_inbound_atr(0, 0, 0, SZ_4G);
> + return 0;
> + }
> +
> + for_each_of_range(&parser, &range) {
> + if (atr_index >= MC_MAX_NUM_INBOUND_WINDOWS) {
> + dev_err(dev, "too many inbound ranges; %d available tables\n",
> + MC_MAX_NUM_INBOUND_WINDOWS);
> + return -EINVAL;
> + }
> + mc_pcie_setup_inbound_atr(atr_index, 0, range.pci_addr, range.size);
> + atr_index++;
> + }
> + }
> +
> + return 0;
> +}
> +
> static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> phys_addr_t axi_addr, phys_addr_t pci_addr,
> u64 size)
> @@ -946,8 +1030,9 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_TRSL_PARAM);
>
> - val = lower_32_bits(axi_addr) | (atr_sz << ATR_SIZE_SHIFT) |
> - ATR_IMPL_ENABLE;
> + val = ALIGN_DOWN(lower_32_bits(axi_addr), SZ_4K);
> + val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
> + val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
This can be just val |= ATR_IMPL_ENABLE when you don't have _MASK
there which is easier to read (IMO).
It would be nice to put the GENMASK()/FIELD_PREP() refactor into a
preparatory patch on top of which you'd make the actual fix to keep the
fix change itself simpler.
Nonetheless, this was already much better than the previous version.
--
i.
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_SRCADDR_PARAM);
>
> @@ -962,11 +1047,6 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> val = upper_32_bits(pci_addr);
> writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> ATR0_AXI4_SLV0_TRSL_ADDR_UDW);
> -
> - val = readl(bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> - val |= (ATR0_PCIE_ATR_SIZE << ATR0_PCIE_ATR_SIZE_SHIFT);
> - writel(val, bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> - writel(0, bridge_base_addr + ATR0_PCIE_WIN0_SRC_ADDR);
> }
>
> static int mc_pcie_setup_windows(struct platform_device *pdev,
> @@ -1129,6 +1209,10 @@ static int mc_platform_init(struct pci_config_window *cfg)
> if (ret)
> return ret;
>
> + ret = mc_pcie_setup_inbound_ranges(pdev, port);
> + if (ret)
> + return ret;
> +
> /* Address translation is up; safe to enable interrupts */
> ret = mc_init_interrupts(pdev, port);
> if (ret)
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
2024-06-21 11:29 ` daire.mcnamara
(?)
(?)
@ 2024-06-25 21:56 ` kernel test robot
-1 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-06-25 21:56 UTC (permalink / raw)
To: daire.mcnamara; +Cc: llvm, oe-kbuild-all
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6]
url: https://github.com/intel-lab-lkp/linux/commits/daire-mcnamara-microchip-com/PCI-microchip-Fix-outbound-address-translation-tables/20240625-161731
base: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
patch link: https://lore.kernel.org/r/20240621112915.3434402-3-daire.mcnamara%40microchip.com
patch subject: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
config: i386-buildonly-randconfig-001-20240626 (https://download.01.org/0day-ci/archive/20240626/202406260553.oh9Ai6yy-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406260553.oh9Ai6yy-lkp@intel.com/reproduce)
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/202406260553.oh9Ai6yy-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/pci/controller/pcie-microchip-host.c:1000:39: warning: implicit conversion from 'unsigned long long' to 'size_t' (aka 'unsigned int') changes value from 4294967296 to 0 [-Wconstant-conversion]
1000 | mc_pcie_setup_inbound_atr(0, 0, 0, SZ_4G);
| ~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~
include/linux/sizes.h:46:18: note: expanded from macro 'SZ_4G'
46 | #define SZ_4G _AC(0x100000000, ULL)
| ^~~~~~~~~~~~~~~~~~~~~
include/uapi/linux/const.h:21:18: note: expanded from macro '_AC'
21 | #define _AC(X,Y) __AC(X,Y)
| ^~~~~~~~~
include/uapi/linux/const.h:20:20: note: expanded from macro '__AC'
20 | #define __AC(X,Y) (X##Y)
| ^~~~
<scratch space>:27:1: note: expanded from here
27 | 0x100000000ULL
| ^~~~~~~~~~~~~~
>> drivers/pci/controller/pcie-microchip-host.c:949:9: error: call to '__compiletime_assert_227' declared with 'error' attribute: FIELD_PREP: value too large for the field
949 | val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
| ^
include/linux/bitfield.h:115:3: note: expanded from macro 'FIELD_PREP'
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^
include/linux/bitfield.h:68:3: note: expanded from macro '__BF_FIELD_CHECK'
68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
| ^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler_types.h:448:2: note: expanded from macro '_compiletime_assert'
448 | __compiletime_assert(condition, msg, prefix, suffix)
| ^
include/linux/compiler_types.h:441:4: note: expanded from macro '__compiletime_assert'
441 | prefix ## suffix(); \
| ^
<scratch space>:3:1: note: expanded from here
3 | __compiletime_assert_227
| ^
1 warning and 1 error generated.
vim +949 drivers/pci/controller/pcie-microchip-host.c
937
938 static void mc_pcie_setup_inbound_atr(int window_index, u64 axi_addr, u64 pcie_addr, size_t size)
939 {
940 void __iomem *bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR;
941 u32 table_offset = window_index * ATR_ENTRY_SIZE;
942 void __iomem *table_addr = bridge_base_addr + table_offset;
943 u32 atr_sz;
944 u32 val;
945
946 atr_sz = ilog2(size) - 1;
947
948 val = ALIGN_DOWN(lower_32_bits(pcie_addr), SZ_4K);
> 949 val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
950 val |= FIELD_PREP(ATR_IMPL_ENABLE_MASK, 1);
951
952 writel(val, table_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
953
954 writel(upper_32_bits(pcie_addr), table_addr + ATR0_PCIE_WIN0_SRC_ADDR);
955
956 writel(lower_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_LSB);
957 writel(upper_32_bits(axi_addr), table_addr + ATR0_PCIE_WIN0_TRSL_ADDR_UDW);
958
959 writel(TRSL_ID_AXI4_MASTER_0, table_addr + ATR0_PCIE_WIN0_TRSL_PARAM);
960 }
961
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
2024-06-21 11:29 ` daire.mcnamara
` (2 preceding siblings ...)
(?)
@ 2024-06-26 0:19 ` kernel test robot
-1 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-06-26 0:19 UTC (permalink / raw)
To: daire.mcnamara; +Cc: oe-kbuild-all
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6]
url: https://github.com/intel-lab-lkp/linux/commits/daire-mcnamara-microchip-com/PCI-microchip-Fix-outbound-address-translation-tables/20240625-161731
base: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
patch link: https://lore.kernel.org/r/20240621112915.3434402-3-daire.mcnamara%40microchip.com
patch subject: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
config: i386-buildonly-randconfig-004-20240626 (https://download.01.org/0day-ci/archive/20240626/202406260846.ybHvD2P1-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406260846.ybHvD2P1-lkp@intel.com/reproduce)
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/202406260846.ybHvD2P1-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/vdso/const.h:5,
from include/linux/const.h:4,
from include/linux/align.h:5,
from include/linux/kernel.h:15,
from include/linux/clk.h:13,
from drivers/pci/controller/pcie-microchip-host.c:11:
drivers/pci/controller/pcie-microchip-host.c: In function 'mc_pcie_setup_inbound_ranges':
include/uapi/linux/const.h:20:25: warning: conversion from 'long long unsigned int' to 'size_t' {aka 'unsigned int'} changes value from '4294967296' to '0' [-Woverflow]
20 | #define __AC(X,Y) (X##Y)
| ^~~~~~
include/uapi/linux/const.h:21:25: note: in expansion of macro '__AC'
21 | #define _AC(X,Y) __AC(X,Y)
| ^~~~
include/linux/sizes.h:46:41: note: in expansion of macro '_AC'
46 | #define SZ_4G _AC(0x100000000, ULL)
| ^~~
drivers/pci/controller/pcie-microchip-host.c:1000:60: note: in expansion of macro 'SZ_4G'
1000 | mc_pcie_setup_inbound_atr(0, 0, 0, SZ_4G);
| ^~~~~
In file included from <command-line>:
In function 'mc_pcie_setup_inbound_atr',
inlined from 'mc_pcie_setup_inbound_ranges' at drivers/pci/controller/pcie-microchip-host.c:1000:4,
inlined from 'mc_platform_init' at drivers/pci/controller/pcie-microchip-host.c:1212:8:
>> include/linux/compiler_types.h:460:45: error: call to '__compiletime_assert_441' declared with attribute error: FIELD_PREP: value too large for the field
460 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:441:25: note: in definition of macro '__compiletime_assert'
441 | prefix ## suffix(); \
| ^~~~~~
include/linux/compiler_types.h:460:9: note: in expansion of macro '_compiletime_assert'
460 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/bitfield.h:68:17: note: in expansion of macro 'BUILD_BUG_ON_MSG'
68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
| ^~~~~~~~~~~~~~~~
include/linux/bitfield.h:115:17: note: in expansion of macro '__BF_FIELD_CHECK'
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~
drivers/pci/controller/pcie-microchip-host.c:949:16: note: in expansion of macro 'FIELD_PREP'
949 | val |= FIELD_PREP(ATR_SIZE_MASK, atr_sz);
| ^~~~~~~~~~
vim +/__compiletime_assert_441 +460 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 446
eb5c2d4b45e3d2 Will Deacon 2020-07-21 447 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 448 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 449
eb5c2d4b45e3d2 Will Deacon 2020-07-21 450 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 451 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 452 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 453 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 454 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 455 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 456 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 457 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 458 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 459 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @460 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 461
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
2024-06-21 11:29 ` daire.mcnamara
` (3 preceding siblings ...)
(?)
@ 2024-06-26 21:45 ` kernel test robot
-1 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-06-26 21:45 UTC (permalink / raw)
To: daire.mcnamara; +Cc: oe-kbuild-all
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6]
url: https://github.com/intel-lab-lkp/linux/commits/daire-mcnamara-microchip-com/PCI-microchip-Fix-outbound-address-translation-tables/20240625-161731
base: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
patch link: https://lore.kernel.org/r/20240621112915.3434402-3-daire.mcnamara%40microchip.com
patch subject: [PATCH v4 2/3] PCI: microchip: Fix inbound address translation tables
config: mips-randconfig-r123-20240626 (https://download.01.org/0day-ci/archive/20240627/202406270503.ZMuH0eTS-lkp@intel.com/config)
compiler: mipsel-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240627/202406270503.ZMuH0eTS-lkp@intel.com/reproduce)
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/202406270503.ZMuH0eTS-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/pcie-microchip-host.c:1000:60: sparse: sparse: cast truncates bits from constant value (100000000 becomes 0)
vim +1000 drivers/pci/controller/pcie-microchip-host.c
961
962 static int mc_pcie_setup_inbound_ranges(struct platform_device *pdev, struct mc_pcie *port)
963 {
964 struct device *dev = &pdev->dev;
965 struct device_node *dn = dev->of_node;
966 struct of_range_parser parser;
967 struct of_range range;
968 int atr_index = 0;
969
970 /*
971 * MPFS PCIe root port is 32-bit only, behind a Fabric Interface
972 * Controller FPGA logic block which contains the AXI-S interface.
973 *
974 * From the point of view of the PCIe root port, There are only
975 * two supported Root Port configurations
976 *
977 * Configuration 1: for use with fully coherent designs; supports a
978 * window from 0x0 (CPU space) to specified PCIe space.
979 *
980 * Configuration 2: for use with non-coherent designs; supports two
981 * 1 Gb wide windows to CPU space; one mapping cpu space 0 to pcie
982 * space 0x80000000 and mapping cpu space 0x40000000 to pcie
983 * space 0xc0000000. This cfg needs two windows because of how
984 * the MSI space is allocated in the AXI-S range on MPFS.
985 *
986 * The FIC interface outside the PCIe block *must* complete the inbound
987 * address translation as per MCHP MPFS FPGA design guidelines.
988 */
989 if (device_property_read_bool(dev, "dma-noncoherent")) {
990 /*
991 * Always need same two tables in this case. Need two tables
992 * due to hardware interactions between address and size.
993 */
994 mc_pcie_setup_inbound_atr(0, 0, MPFS_NC_BOUNCE_ADDR, SZ_1G);
995 mc_pcie_setup_inbound_atr(1, SZ_1G, MPFS_NC_BOUNCE_ADDR + SZ_1G, SZ_1G);
996 } else {
997 /* Find any dma-ranges */
998 if (of_pci_dma_range_parser_init(&parser, dn)) {
999 /* No dma-range property - setup default */
> 1000 mc_pcie_setup_inbound_atr(0, 0, 0, SZ_4G);
1001 return 0;
1002 }
1003
1004 for_each_of_range(&parser, &range) {
1005 if (atr_index >= MC_MAX_NUM_INBOUND_WINDOWS) {
1006 dev_err(dev, "too many inbound ranges; %d available tables\n",
1007 MC_MAX_NUM_INBOUND_WINDOWS);
1008 return -EINVAL;
1009 }
1010 mc_pcie_setup_inbound_atr(atr_index, 0, range.pci_addr, range.size);
1011 atr_index++;
1012 }
1013 }
1014
1015 return 0;
1016 }
1017
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread