2023-09-15 10:28:14

by Minda Chen

[permalink] [raw]
Subject: [PATCH v6 07/19] PCI: plda: Move the setup functions to pcie-plda-host.c

Move setup functions to common pcie-plda-host.c.

Signed-off-by: Minda Chen <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
drivers/pci/controller/plda/Kconfig | 4 +
drivers/pci/controller/plda/Makefile | 1 +
.../pci/controller/plda/pcie-microchip-host.c | 59 -------------
drivers/pci/controller/plda/pcie-plda-host.c | 82 +++++++++++++++++++
drivers/pci/controller/plda/pcie-plda.h | 6 ++
5 files changed, 93 insertions(+), 59 deletions(-)
create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c

diff --git a/drivers/pci/controller/plda/Kconfig b/drivers/pci/controller/plda/Kconfig
index 5cb3be4fc98c..e54a82ee94f5 100644
--- a/drivers/pci/controller/plda/Kconfig
+++ b/drivers/pci/controller/plda/Kconfig
@@ -3,10 +3,14 @@
menu "PLDA-based PCIe controllers"
depends on PCI

+config PCIE_PLDA_HOST
+ bool
+
config PCIE_MICROCHIP_HOST
tristate "Microchip AXI PCIe controller"
depends on PCI_MSI && OF
select PCI_HOST_COMMON
+ select PCIE_PLDA_HOST
help
Say Y here if you want kernel to support the Microchip AXI PCIe
Host Bridge driver.
diff --git a/drivers/pci/controller/plda/Makefile b/drivers/pci/controller/plda/Makefile
index e1a265cbf91c..4340ab007f44 100644
--- a/drivers/pci/controller/plda/Makefile
+++ b/drivers/pci/controller/plda/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PCIE_PLDA_HOST) += pcie-plda-host.o
obj-$(CONFIG_PCIE_MICROCHIP_HOST) += pcie-microchip-host.o
diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c
index 1d253acd6bc2..ac7126b0bacf 100644
--- a/drivers/pci/controller/plda/pcie-microchip-host.c
+++ b/drivers/pci/controller/plda/pcie-microchip-host.c
@@ -837,65 +837,6 @@ static int mc_pcie_init_irq_domains(struct plda_pcie_rp *port)
return mc_allocate_msi_domains(port);
}

-static void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
- phys_addr_t axi_addr, phys_addr_t pci_addr,
- size_t size)
-{
- u32 atr_sz = ilog2(size) - 1;
- u32 val;
-
- if (index == 0)
- val = PCIE_CONFIG_INTERFACE;
- else
- val = PCIE_TX_RX_INTERFACE;
-
- 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;
- writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
- ATR0_AXI4_SLV0_SRCADDR_PARAM);
-
- val = upper_32_bits(axi_addr);
- writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
- ATR0_AXI4_SLV0_SRC_ADDR);
-
- val = lower_32_bits(pci_addr);
- writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
- ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
-
- 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 plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
- struct plda_pcie_rp *port)
-{
- void __iomem *bridge_base_addr = port->bridge_addr;
- struct resource_entry *entry;
- u64 pci_addr;
- u32 index = 1;
-
- resource_list_for_each_entry(entry, &bridge->windows) {
- if (resource_type(entry->res) == IORESOURCE_MEM) {
- pci_addr = entry->res->start - entry->offset;
- plda_pcie_setup_window(bridge_base_addr, index,
- entry->res->start, pci_addr,
- resource_size(entry->res));
- index++;
- }
- }
-
- return 0;
-}
-
static inline void mc_clear_secs(struct mc_pcie *port)
{
void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
new file mode 100644
index 000000000000..f0c7636f1f64
--- /dev/null
+++ b/drivers/pci/controller/plda/pcie-plda-host.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PLDA PCIe XpressRich host controller driver
+ *
+ * Copyright (C) 2023 Microchip Co. Ltd
+ * StarFive Co. Ltd.
+ *
+ * Author: Daire McNamara <[email protected]>
+ * Author: Minda Chen <[email protected]>
+ */
+
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/msi.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/pci_regs.h>
+#include <linux/pci-ecam.h>
+#include <linux/platform_device.h>
+
+#include "pcie-plda.h"
+
+void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
+ phys_addr_t axi_addr, phys_addr_t pci_addr,
+ size_t size)
+{
+ u32 atr_sz = ilog2(size) - 1;
+ u32 val;
+
+ if (index == 0)
+ val = PCIE_CONFIG_INTERFACE;
+ else
+ val = PCIE_TX_RX_INTERFACE;
+
+ 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;
+ writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
+ ATR0_AXI4_SLV0_SRCADDR_PARAM);
+
+ val = upper_32_bits(axi_addr);
+ writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
+ ATR0_AXI4_SLV0_SRC_ADDR);
+
+ val = lower_32_bits(pci_addr);
+ writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
+ ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
+
+ 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);
+}
+EXPORT_SYMBOL_GPL(plda_pcie_setup_window);
+
+int plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
+ struct plda_pcie_rp *port)
+{
+ void __iomem *bridge_base_addr = port->bridge_addr;
+ struct resource_entry *entry;
+ u64 pci_addr;
+ u32 index = 1;
+
+ resource_list_for_each_entry(entry, &bridge->windows) {
+ if (resource_type(entry->res) == IORESOURCE_MEM) {
+ pci_addr = entry->res->start - entry->offset;
+ plda_pcie_setup_window(bridge_base_addr, index,
+ entry->res->start, pci_addr,
+ resource_size(entry->res));
+ index++;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(plda_pcie_setup_iomems);
diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h
index d04a571404b9..3deefd35fa5a 100644
--- a/drivers/pci/controller/plda/pcie-plda.h
+++ b/drivers/pci/controller/plda/pcie-plda.h
@@ -119,4 +119,10 @@ struct plda_pcie_rp {
struct plda_msi msi;
void __iomem *bridge_addr;
};
+
+void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
+ phys_addr_t axi_addr, phys_addr_t pci_addr,
+ size_t size);
+int plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
+ struct plda_pcie_rp *port);
#endif
--
2.17.1


2023-09-18 03:46:41

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v6 07/19] PCI: plda: Move the setup functions to pcie-plda-host.c

Minda Chen wrote:
> Move setup functions to common pcie-plda-host.c.
>
> Signed-off-by: Minda Chen <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> ---
> drivers/pci/controller/plda/Kconfig | 4 +
> drivers/pci/controller/plda/Makefile | 1 +
> .../pci/controller/plda/pcie-microchip-host.c | 59 -------------
> drivers/pci/controller/plda/pcie-plda-host.c | 82 +++++++++++++++++++
> drivers/pci/controller/plda/pcie-plda.h | 6 ++
> 5 files changed, 93 insertions(+), 59 deletions(-)
> create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c
>
> diff --git a/drivers/pci/controller/plda/Kconfig b/drivers/pci/controller/plda/Kconfig
> index 5cb3be4fc98c..e54a82ee94f5 100644
> --- a/drivers/pci/controller/plda/Kconfig
> +++ b/drivers/pci/controller/plda/Kconfig
> @@ -3,10 +3,14 @@
> menu "PLDA-based PCIe controllers"
> depends on PCI
>
> +config PCIE_PLDA_HOST
> + bool
> +
> config PCIE_MICROCHIP_HOST
> tristate "Microchip AXI PCIe controller"
> depends on PCI_MSI && OF
> select PCI_HOST_COMMON
> + select PCIE_PLDA_HOST
> help
> Say Y here if you want kernel to support the Microchip AXI PCIe
> Host Bridge driver.
> diff --git a/drivers/pci/controller/plda/Makefile b/drivers/pci/controller/plda/Makefile
> index e1a265cbf91c..4340ab007f44 100644
> --- a/drivers/pci/controller/plda/Makefile
> +++ b/drivers/pci/controller/plda/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PCIE_PLDA_HOST) += pcie-plda-host.o
> obj-$(CONFIG_PCIE_MICROCHIP_HOST) += pcie-microchip-host.o
> diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c
> index 1d253acd6bc2..ac7126b0bacf 100644
> --- a/drivers/pci/controller/plda/pcie-microchip-host.c
> +++ b/drivers/pci/controller/plda/pcie-microchip-host.c
> @@ -837,65 +837,6 @@ static int mc_pcie_init_irq_domains(struct plda_pcie_rp *port)
> return mc_allocate_msi_domains(port);
> }
>
> -static void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> - phys_addr_t axi_addr, phys_addr_t pci_addr,
> - size_t size)
> -{
> - u32 atr_sz = ilog2(size) - 1;
> - u32 val;
> -
> - if (index == 0)
> - val = PCIE_CONFIG_INTERFACE;
> - else
> - val = PCIE_TX_RX_INTERFACE;
> -
> - 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;
> - writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> - ATR0_AXI4_SLV0_SRCADDR_PARAM);
> -
> - val = upper_32_bits(axi_addr);
> - writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> - ATR0_AXI4_SLV0_SRC_ADDR);
> -
> - val = lower_32_bits(pci_addr);
> - writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> - ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
> -
> - 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 plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
> - struct plda_pcie_rp *port)
> -{
> - void __iomem *bridge_base_addr = port->bridge_addr;
> - struct resource_entry *entry;
> - u64 pci_addr;
> - u32 index = 1;
> -
> - resource_list_for_each_entry(entry, &bridge->windows) {
> - if (resource_type(entry->res) == IORESOURCE_MEM) {
> - pci_addr = entry->res->start - entry->offset;
> - plda_pcie_setup_window(bridge_base_addr, index,
> - entry->res->start, pci_addr,
> - resource_size(entry->res));
> - index++;
> - }
> - }
> -
> - return 0;
> -}
> -
> static inline void mc_clear_secs(struct mc_pcie *port)
> {
> void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
> diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
> new file mode 100644
> index 000000000000..f0c7636f1f64
> --- /dev/null
> +++ b/drivers/pci/controller/plda/pcie-plda-host.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PLDA PCIe XpressRich host controller driver
> + *
> + * Copyright (C) 2023 Microchip Co. Ltd
> + * StarFive Co. Ltd.
> + *
> + * Author: Daire McNamara <[email protected]>
> + * Author: Minda Chen <[email protected]>
> + */
> +
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/msi.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci_regs.h>
> +#include <linux/pci-ecam.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-plda.h"
> +
> +void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> + phys_addr_t axi_addr, phys_addr_t pci_addr,
> + size_t size)
> +{
> + u32 atr_sz = ilog2(size) - 1;
> + u32 val;
> +
> + if (index == 0)
> + val = PCIE_CONFIG_INTERFACE;
> + else
> + val = PCIE_TX_RX_INTERFACE;
> +
> + 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;
> + writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> + ATR0_AXI4_SLV0_SRCADDR_PARAM);
> +
> + val = upper_32_bits(axi_addr);
> + writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> + ATR0_AXI4_SLV0_SRC_ADDR);
> +
> + val = lower_32_bits(pci_addr);
> + writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
> + ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
> +
> + 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);

I know this code is just a straight copy, but for future cleanups are we
guaranteed that this field is always zero?

Otherwise it looks a little suspicious to do read-modify-write, but just
set the (0x25 << 1) bits without clearing the field first.

> + writel(val, bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
> + writel(0, bridge_base_addr + ATR0_PCIE_WIN0_SRC_ADDR);
> +}
> +EXPORT_SYMBOL_GPL(plda_pcie_setup_window);
> +
> +int plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
> + struct plda_pcie_rp *port)
> +{
> + void __iomem *bridge_base_addr = port->bridge_addr;
> + struct resource_entry *entry;
> + u64 pci_addr;
> + u32 index = 1;
> +
> + resource_list_for_each_entry(entry, &bridge->windows) {
> + if (resource_type(entry->res) == IORESOURCE_MEM) {
> + pci_addr = entry->res->start - entry->offset;
> + plda_pcie_setup_window(bridge_base_addr, index,
> + entry->res->start, pci_addr,
> + resource_size(entry->res));
> + index++;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(plda_pcie_setup_iomems);
> diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h
> index d04a571404b9..3deefd35fa5a 100644
> --- a/drivers/pci/controller/plda/pcie-plda.h
> +++ b/drivers/pci/controller/plda/pcie-plda.h
> @@ -119,4 +119,10 @@ struct plda_pcie_rp {
> struct plda_msi msi;
> void __iomem *bridge_addr;
> };
> +
> +void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
> + phys_addr_t axi_addr, phys_addr_t pci_addr,
> + size_t size);
> +int plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
> + struct plda_pcie_rp *port);
> #endif
> --
> 2.17.1

2023-09-22 11:12:36

by Minda Chen

[permalink] [raw]
Subject: Re: [PATCH v6 07/19] PCI: plda: Move the setup functions to pcie-plda-host.c



On 2023/9/18 6:45, Emil Renner Berthing wrote:
> Minda Chen wrote:
>> Move setup functions to common pcie-plda-host.c.
>>
>> Signed-off-by: Minda Chen <[email protected]>
>> Reviewed-by: Conor Dooley <[email protected]>
>> ---
>> drivers/pci/controller/plda/Kconfig | 4 +
>> drivers/pci/controller/plda/Makefile | 1 +
>> .../pci/controller/plda/pcie-microchip-host.c | 59 -------------
>> drivers/pci/controller/plda/pcie-plda-host.c | 82 +++++++++++++++++++
>> drivers/pci/controller/plda/pcie-plda.h | 6 ++
>> 5 files changed, 93 insertions(+), 59 deletions(-)
>> create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c
>>
>> diff --git a/drivers/pci/controller/plda/Kconfig b/drivers/pci/controller/plda/Kconfig
>> index 5cb3be4fc98c..e54a82ee94f5 100644
>> --- a/drivers/pci/controller/plda/Kconfig
>> +++ b/drivers/pci/controller/plda/Kconfig
>> @@ -3,10 +3,14 @@
>> menu "PLDA-based PCIe controllers"
>> depends on PCI
>>
>> +config PCIE_PLDA_HOST
>> + bool
>> +
>> config PCIE_MICROCHIP_HOST
>> tristate "Microchip AXI PCIe controller"
>> depends on PCI_MSI && OF
>> select PCI_HOST_COMMON
>> + select PCIE_PLDA_HOST
>> help
>> Say Y here if you want kernel to support the Microchip AXI PCIe
>> Host Bridge driver.
>> diff --git a/drivers/pci/controller/plda/Makefile b/drivers/pci/controller/plda/Makefile
>> index e1a265cbf91c..4340ab007f44 100644
>> --- a/drivers/pci/controller/plda/Makefile
>> +++ b/drivers/pci/controller/plda/Makefile
>> @@ -1,2 +1,3 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_PCIE_PLDA_HOST) += pcie-plda-host.o
>> obj-$(CONFIG_PCIE_MICROCHIP_HOST) += pcie-microchip-host.o
>> diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c
>> index 1d253acd6bc2..ac7126b0bacf 100644
>> --- a/drivers/pci/controller/plda/pcie-microchip-host.c
>> +++ b/drivers/pci/controller/plda/pcie-microchip-host.c
>> @@ -837,65 +837,6 @@ static int mc_pcie_init_irq_domains(struct plda_pcie_rp *port)
>> return mc_allocate_msi_domains(port);
>> }
>>
>> -static void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
>> - phys_addr_t axi_addr, phys_addr_t pci_addr,
>> - size_t size)
>> -{
>> - u32 atr_sz = ilog2(size) - 1;
>> - u32 val;
>> -
>> - if (index == 0)
>> - val = PCIE_CONFIG_INTERFACE;
>> - else
>> - val = PCIE_TX_RX_INTERFACE;
>> -
>> - 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;
>> - writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
>> - ATR0_AXI4_SLV0_SRCADDR_PARAM);
>> -
>> - val = upper_32_bits(axi_addr);
>> - writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
>> - ATR0_AXI4_SLV0_SRC_ADDR);
>> -
>> - val = lower_32_bits(pci_addr);
>> - writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
>> - ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
>> -
>> - 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 plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
>> - struct plda_pcie_rp *port)
>> -{
>> - void __iomem *bridge_base_addr = port->bridge_addr;
>> - struct resource_entry *entry;
>> - u64 pci_addr;
>> - u32 index = 1;
>> -
>> - resource_list_for_each_entry(entry, &bridge->windows) {
>> - if (resource_type(entry->res) == IORESOURCE_MEM) {
>> - pci_addr = entry->res->start - entry->offset;
>> - plda_pcie_setup_window(bridge_base_addr, index,
>> - entry->res->start, pci_addr,
>> - resource_size(entry->res));
>> - index++;
>> - }
>> - }
>> -
>> - return 0;
>> -}
>> -
>> static inline void mc_clear_secs(struct mc_pcie *port)
>> {
>> void __iomem *ctrl_base_addr = port->axi_base_addr + MC_PCIE_CTRL_ADDR;
>> diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
>> new file mode 100644
>> index 000000000000..f0c7636f1f64
>> --- /dev/null
>> +++ b/drivers/pci/controller/plda/pcie-plda-host.c
>> @@ -0,0 +1,82 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PLDA PCIe XpressRich host controller driver
>> + *
>> + * Copyright (C) 2023 Microchip Co. Ltd
>> + * StarFive Co. Ltd.
>> + *
>> + * Author: Daire McNamara <[email protected]>
>> + * Author: Minda Chen <[email protected]>
>> + */
>> +
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/msi.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/pci_regs.h>
>> +#include <linux/pci-ecam.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pcie-plda.h"
>> +
>> +void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
>> + phys_addr_t axi_addr, phys_addr_t pci_addr,
>> + size_t size)
>> +{
>> + u32 atr_sz = ilog2(size) - 1;
>> + u32 val;
>> +
>> + if (index == 0)
>> + val = PCIE_CONFIG_INTERFACE;
>> + else
>> + val = PCIE_TX_RX_INTERFACE;
>> +
>> + 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;
>> + writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
>> + ATR0_AXI4_SLV0_SRCADDR_PARAM);
>> +
>> + val = upper_32_bits(axi_addr);
>> + writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
>> + ATR0_AXI4_SLV0_SRC_ADDR);
>> +
>> + val = lower_32_bits(pci_addr);
>> + writel(val, bridge_base_addr + (index * ATR_ENTRY_SIZE) +
>> + ATR0_AXI4_SLV0_TRSL_ADDR_LSB);
>> +
>> + 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);
>
> I know this code is just a straight copy, but for future cleanups are we
> guaranteed that this field is always zero?
>
> Otherwise it looks a little suspicious to do read-modify-write, but just
> set the (0x25 << 1) bits without clearing the field first.
>
The reset value is zero. And this is called once.
Maybe I can add a patch to fix this.
>> + writel(val, bridge_base_addr + ATR0_PCIE_WIN0_SRCADDR_PARAM);
>> + writel(0, bridge_base_addr + ATR0_PCIE_WIN0_SRC_ADDR);
>> +}
>> +EXPORT_SYMBOL_GPL(plda_pcie_setup_window);
>> +
>> +int plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
>> + struct plda_pcie_rp *port)
>> +{
>> + void __iomem *bridge_base_addr = port->bridge_addr;
>> + struct resource_entry *entry;
>> + u64 pci_addr;
>> + u32 index = 1;
>> +
>> + resource_list_for_each_entry(entry, &bridge->windows) {
>> + if (resource_type(entry->res) == IORESOURCE_MEM) {
>> + pci_addr = entry->res->start - entry->offset;
>> + plda_pcie_setup_window(bridge_base_addr, index,
>> + entry->res->start, pci_addr,
>> + resource_size(entry->res));
>> + index++;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(plda_pcie_setup_iomems);
>> diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h
>> index d04a571404b9..3deefd35fa5a 100644
>> --- a/drivers/pci/controller/plda/pcie-plda.h
>> +++ b/drivers/pci/controller/plda/pcie-plda.h
>> @@ -119,4 +119,10 @@ struct plda_pcie_rp {
>> struct plda_msi msi;
>> void __iomem *bridge_addr;
>> };
>> +
>> +void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index,
>> + phys_addr_t axi_addr, phys_addr_t pci_addr,
>> + size_t size);
>> +int plda_pcie_setup_iomems(struct pci_host_bridge *bridge,
>> + struct plda_pcie_rp *port);
>> #endif
>> --
>> 2.17.1