* [PATCH v3 1/3] of: address: Add parent_bus_addr to struct of_pci_range
2024-09-30 18:44 [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr() Frank Li
@ 2024-09-30 18:44 ` Frank Li
2024-10-03 5:38 ` Manivannan Sadhasivam
2024-09-30 18:44 ` [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup() Frank Li
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Frank Li @ 2024-09-30 18:44 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan, Jingoo Han, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: devicetree, linux-kernel, linux-pci, linux-arm-kernel, imx,
Frank Li
Introduce field 'parent_bus_addr' in of_pci_range to retrieve untranslated
CPU address information.
Refer to the diagram below to understand that the bus fabric in some
systems (like i.MX8QXP) does not use a 1:1 address map between input and
output.
Currently, many drivers use .cpu_addr_fixup() callback hardcodes that
translation in the code, e.g., "cpu_addr & CDNS_PLAT_CPU_TO_BUS_ADDR",
"cpu_addr + BUS_IATU_OFFSET", etc, even though those translations *should*
be described via DT.
The .cpu_addr_fixup() can be eliminated if DT correct reflect hardware
behavior and driver use 'parent_bus_addr' in of_pci_range.
┌─────────┐ ┌────────────┐
┌─────┐ │ │ IA: 0x8ff0_0000 │ │
│ CPU ├───►│ ┌────►├─────────────────┐ │ PCI │
└─────┘ │ │ │ IA: 0x8ff8_0000 │ │ │
CPU Addr │ │ ┌─►├─────────────┐ │ │ Controller │
0x7ff0_0000─┼───┘ │ │ │ │ │ │
│ │ │ │ │ │ │ PCI Addr
0x7ff8_0000─┼──────┘ │ │ └──► CfgSpace ─┼────────────►
│ │ │ │ │ 0
0x7000_0000─┼────────►├─────────┐ │ │ │
└─────────┘ │ └──────► IOSpace ─┼────────────►
BUS Fabric │ │ │ 0
│ │ │
└──────────► MemSpace ─┼────────────►
IA: 0x8000_0000 │ │ 0x8000_0000
└────────────┘
bus@5f000000 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
<0x80000000 0x0 0x70000000 0x10000000>;
pcie@5f010000 {
compatible = "fsl,imx8q-pcie";
reg = <0x5f010000 0x10000>, <0x8ff00000 0x80000>;
reg-names = "dbi", "config";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
bus-range = <0x00 0xff>;
ranges = <0x81000000 0 0x00000000 0x8ff80000 0 0x00010000>,
<0x82000000 0 0x80000000 0x80000000 0 0x0ff00000>;
...
};
};
'parent_bus_addr' in of_pci_range can indicate above diagram internal
address (IA) address information.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3
- cpu_untranslate_addr -> parent_bus_addr
- Add Rob's review tag
I changed commit message base on Bjorn, if you have concern about review
added tag, let me know.
Change from v1 to v2
- add parent_bus_addr in of_pci_range, instead adding new API.
---
drivers/of/address.c | 2 ++
include/linux/of_address.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 286f0c161e332..1a0229ee4e0b2 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -811,6 +811,8 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
else
range->cpu_addr = of_translate_address(parser->node,
parser->range + na);
+
+ range->parent_bus_addr = of_read_number(parser->range + na, parser->pna);
range->size = of_read_number(parser->range + parser->pna + na, ns);
parser->range += np;
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 26a19daf0d092..13dd79186d02c 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -26,6 +26,7 @@ struct of_pci_range {
u64 bus_addr;
};
u64 cpu_addr;
+ u64 parent_bus_addr;
u64 size;
u32 flags;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 1/3] of: address: Add parent_bus_addr to struct of_pci_range
2024-09-30 18:44 ` [PATCH v3 1/3] of: address: Add parent_bus_addr to struct of_pci_range Frank Li
@ 2024-10-03 5:38 ` Manivannan Sadhasivam
0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2024-10-03 5:38 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Saravana Kannan, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Richard Zhu,
Lucas Stach, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, devicetree, linux-kernel, linux-pci,
linux-arm-kernel, imx
On Mon, Sep 30, 2024 at 02:44:53PM -0400, Frank Li wrote:
> Introduce field 'parent_bus_addr' in of_pci_range to retrieve untranslated
> CPU address information.
>
> Refer to the diagram below to understand that the bus fabric in some
> systems (like i.MX8QXP) does not use a 1:1 address map between input and
> output.
>
> Currently, many drivers use .cpu_addr_fixup() callback hardcodes that
s/drivers/controller drivers/
> translation in the code, e.g., "cpu_addr & CDNS_PLAT_CPU_TO_BUS_ADDR",
> "cpu_addr + BUS_IATU_OFFSET", etc, even though those translations *should*
> be described via DT.
>
It'd be useful to mention which driver has the mentioned fixup as an example.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup()
2024-09-30 18:44 [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr() Frank Li
2024-09-30 18:44 ` [PATCH v3 1/3] of: address: Add parent_bus_addr to struct of_pci_range Frank Li
@ 2024-09-30 18:44 ` Frank Li
2024-10-03 5:51 ` Manivannan Sadhasivam
2024-09-30 18:44 ` [PATCH v3 3/3] PCI: imx6: Remove cpu_addr_fixup() Frank Li
2024-10-03 5:34 ` [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr() Manivannan Sadhasivam
3 siblings, 1 reply; 8+ messages in thread
From: Frank Li @ 2024-09-30 18:44 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan, Jingoo Han, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: devicetree, linux-kernel, linux-pci, linux-arm-kernel, imx,
Frank Li
parent_bus_addr in struct of_range can indicate address information just
ahead of PCIe controller. Most system's bus fabric use 1:1 map between
input and output address. but some hardware like i.MX8QXP doesn't use 1:1
map. See below diagram:
┌─────────┐ ┌────────────┐
┌─────┐ │ │ IA: 0x8ff0_0000 │ │
│ CPU ├───►│ ┌────►├─────────────────┐ │ PCI │
└─────┘ │ │ │ IA: 0x8ff8_0000 │ │ │
CPU Addr │ │ ┌─►├─────────────┐ │ │ Controller │
0x7ff0_0000─┼───┘ │ │ │ │ │ │
│ │ │ │ │ │ │ PCI Addr
0x7ff8_0000─┼──────┘ │ │ └──► CfgSpace ─┼────────────►
│ │ │ │ │ 0
0x7000_0000─┼────────►├─────────┐ │ │ │
└─────────┘ │ └──────► IOSpace ─┼────────────►
BUS Fabric │ │ │ 0
│ │ │
└──────────► MemSpace ─┼────────────►
IA: 0x8000_0000 │ │ 0x8000_0000
└────────────┘
bus@5f000000 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
<0x80000000 0x0 0x70000000 0x10000000>;
pcie@5f010000 {
compatible = "fsl,imx8q-pcie";
reg = <0x5f010000 0x10000>, <0x8ff00000 0x80000>;
reg-names = "dbi", "config";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
bus-range = <0x00 0xff>;
ranges = <0x81000000 0 0x00000000 0x8ff80000 0 0x00010000>,
<0x82000000 0 0x80000000 0x80000000 0 0x0ff00000>;
...
};
};
Term internal address (IA) here means the address just before PCIe
controller. After ATU use this IA instead CPU address, cpu_addr_fixup() can
be removed.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3
- %s/cpu_untranslate_addr/parent_bus_addr/g
- update diagram.
- improve commit message.
Change from v1 to v2
- update because patch1 change get untranslate address method.
- add using_dtbus_info in case break back compatibility for exited platform.
---
drivers/pci/controller/dwc/pcie-designware-host.c | 42 +++++++++++++++++++++++
drivers/pci/controller/dwc/pcie-designware.h | 8 +++++
2 files changed, 50 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3e41865c72904..823ff42c2e2c9 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -418,6 +418,34 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
}
}
+static int dw_pcie_get_untranslate_addr(struct dw_pcie *pci, resource_size_t pci_addr,
+ resource_size_t *i_addr)
+{
+ struct device *dev = pci->dev;
+ struct device_node *np = dev->of_node;
+ struct of_range_parser parser;
+ struct of_range range;
+ int ret;
+
+ if (!pci->using_dtbus_info) {
+ *i_addr = pci_addr;
+ return 0;
+ }
+
+ ret = of_range_parser_init(&parser, np);
+ if (ret)
+ return ret;
+
+ for_each_of_pci_range(&parser, &range) {
+ if (pci_addr == range.bus_addr) {
+ *i_addr = range.parent_bus_addr;
+ break;
+ }
+ }
+
+ return 0;
+}
+
int dw_pcie_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -427,6 +455,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
struct resource_entry *win;
struct pci_host_bridge *bridge;
struct resource *res;
+ int index;
int ret;
raw_spin_lock_init(&pp->lock);
@@ -440,6 +469,13 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
pp->cfg0_size = resource_size(res);
pp->cfg0_base = res->start;
+ if (pci->using_dtbus_info) {
+ index = of_property_match_string(np, "reg-names", "config");
+ if (index < 0)
+ return -EINVAL;
+ of_property_read_reg(np, index, &pp->cfg0_base, NULL);
+ }
+
pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
if (IS_ERR(pp->va_cfg0_base))
return PTR_ERR(pp->va_cfg0_base);
@@ -462,6 +498,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
pp->io_base = pci_pio_to_address(win->res->start);
}
+ if (dw_pcie_get_untranslate_addr(pci, pp->io_bus_addr, &pp->io_base))
+ return -ENODEV;
+
/* Set default bus ops */
bridge->ops = &dw_pcie_ops;
bridge->child_ops = &dw_child_pcie_ops;
@@ -733,6 +772,9 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
atu.cpu_addr = entry->res->start;
atu.pci_addr = entry->res->start - entry->offset;
+ if (dw_pcie_get_untranslate_addr(pci, atu.pci_addr, &atu.cpu_addr))
+ return -EINVAL;
+
/* Adjust iATU size if MSG TLP region was allocated before */
if (pp->msg_res && pp->msg_res->parent == entry->res)
atu.size = resource_size(entry->res) -
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index c189781524fb8..e22d32b5a5f19 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -464,6 +464,14 @@ struct dw_pcie {
struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS];
struct gpio_desc *pe_rst;
bool suspended;
+ /*
+ * Use device tree 'ranges' property of bus node instead using
+ * cpu_addr_fixup(). Some old platform dts 'ranges' in bus node may not
+ * reflect real hardware's behavior. In case break these platform back
+ * compatibility, add below flags. Set it true if dts already correct
+ * indicate bus fabric address convert.
+ */
+ bool using_dtbus_info;
};
#define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup()
2024-09-30 18:44 ` [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup() Frank Li
@ 2024-10-03 5:51 ` Manivannan Sadhasivam
2024-10-03 19:30 ` [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup()y Frank Li
0 siblings, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2024-10-03 5:51 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Saravana Kannan, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Richard Zhu,
Lucas Stach, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, devicetree, linux-kernel, linux-pci,
linux-arm-kernel, imx
On Mon, Sep 30, 2024 at 02:44:54PM -0400, Frank Li wrote:
> parent_bus_addr in struct of_range can indicate address information just
> ahead of PCIe controller. Most system's bus fabric use 1:1 map between
> input and output address. but some hardware like i.MX8QXP doesn't use 1:1
> map. See below diagram:
>
> ┌─────────┐ ┌────────────┐
> ┌─────┐ │ │ IA: 0x8ff0_0000 │ │
> │ CPU ├───►│ ┌────►├─────────────────┐ │ PCI │
> └─────┘ │ │ │ IA: 0x8ff8_0000 │ │ │
> CPU Addr │ │ ┌─►├─────────────┐ │ │ Controller │
> 0x7ff0_0000─┼───┘ │ │ │ │ │ │
> │ │ │ │ │ │ │ PCI Addr
> 0x7ff8_0000─┼──────┘ │ │ └──► CfgSpace ─┼────────────►
> │ │ │ │ │ 0
> 0x7000_0000─┼────────►├─────────┐ │ │ │
> └─────────┘ │ └──────► IOSpace ─┼────────────►
> BUS Fabric │ │ │ 0
> │ │ │
> └──────────► MemSpace ─┼────────────►
> IA: 0x8000_0000 │ │ 0x8000_0000
> └────────────┘
>
> bus@5f000000 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
> <0x80000000 0x0 0x70000000 0x10000000>;
Does this address translation apply to all peripherals in the bus or just PCIe?
If it is just PCIe, why can't you encode the mapping in the below PCIe node
'ranges' property itself?
- Mani
>
> pcie@5f010000 {
> compatible = "fsl,imx8q-pcie";
> reg = <0x5f010000 0x10000>, <0x8ff00000 0x80000>;
> reg-names = "dbi", "config";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> bus-range = <0x00 0xff>;
> ranges = <0x81000000 0 0x00000000 0x8ff80000 0 0x00010000>,
> <0x82000000 0 0x80000000 0x80000000 0 0x0ff00000>;
> ...
> };
> };
>
> Term internal address (IA) here means the address just before PCIe
> controller. After ATU use this IA instead CPU address, cpu_addr_fixup() can
> be removed.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v2 to v3
> - %s/cpu_untranslate_addr/parent_bus_addr/g
> - update diagram.
> - improve commit message.
>
> Change from v1 to v2
> - update because patch1 change get untranslate address method.
> - add using_dtbus_info in case break back compatibility for exited platform.
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 42 +++++++++++++++++++++++
> drivers/pci/controller/dwc/pcie-designware.h | 8 +++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 3e41865c72904..823ff42c2e2c9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -418,6 +418,34 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> }
> }
>
> +static int dw_pcie_get_untranslate_addr(struct dw_pcie *pci, resource_size_t pci_addr,
> + resource_size_t *i_addr)
> +{
> + struct device *dev = pci->dev;
> + struct device_node *np = dev->of_node;
> + struct of_range_parser parser;
> + struct of_range range;
> + int ret;
> +
> + if (!pci->using_dtbus_info) {
> + *i_addr = pci_addr;
> + return 0;
> + }
> +
> + ret = of_range_parser_init(&parser, np);
> + if (ret)
> + return ret;
> +
> + for_each_of_pci_range(&parser, &range) {
> + if (pci_addr == range.bus_addr) {
> + *i_addr = range.parent_bus_addr;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> int dw_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -427,6 +455,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> struct resource_entry *win;
> struct pci_host_bridge *bridge;
> struct resource *res;
> + int index;
> int ret;
>
> raw_spin_lock_init(&pp->lock);
> @@ -440,6 +469,13 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> pp->cfg0_size = resource_size(res);
> pp->cfg0_base = res->start;
>
> + if (pci->using_dtbus_info) {
> + index = of_property_match_string(np, "reg-names", "config");
> + if (index < 0)
> + return -EINVAL;
> + of_property_read_reg(np, index, &pp->cfg0_base, NULL);
> + }
> +
> pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
> if (IS_ERR(pp->va_cfg0_base))
> return PTR_ERR(pp->va_cfg0_base);
> @@ -462,6 +498,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> pp->io_base = pci_pio_to_address(win->res->start);
> }
>
> + if (dw_pcie_get_untranslate_addr(pci, pp->io_bus_addr, &pp->io_base))
> + return -ENODEV;
> +
> /* Set default bus ops */
> bridge->ops = &dw_pcie_ops;
> bridge->child_ops = &dw_child_pcie_ops;
> @@ -733,6 +772,9 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> atu.cpu_addr = entry->res->start;
> atu.pci_addr = entry->res->start - entry->offset;
>
> + if (dw_pcie_get_untranslate_addr(pci, atu.pci_addr, &atu.cpu_addr))
> + return -EINVAL;
> +
> /* Adjust iATU size if MSG TLP region was allocated before */
> if (pp->msg_res && pp->msg_res->parent == entry->res)
> atu.size = resource_size(entry->res) -
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index c189781524fb8..e22d32b5a5f19 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -464,6 +464,14 @@ struct dw_pcie {
> struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS];
> struct gpio_desc *pe_rst;
> bool suspended;
> + /*
> + * Use device tree 'ranges' property of bus node instead using
> + * cpu_addr_fixup(). Some old platform dts 'ranges' in bus node may not
> + * reflect real hardware's behavior. In case break these platform back
> + * compatibility, add below flags. Set it true if dts already correct
> + * indicate bus fabric address convert.
> + */
> + bool using_dtbus_info;
> };
>
> #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
>
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup()y
2024-10-03 5:51 ` Manivannan Sadhasivam
@ 2024-10-03 19:30 ` Frank Li
0 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2024-10-03 19:30 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Rob Herring, Saravana Kannan, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Richard Zhu,
Lucas Stach, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, devicetree, linux-kernel, linux-pci,
linux-arm-kernel, imx
On Thu, Oct 03, 2024 at 11:21:06AM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 30, 2024 at 02:44:54PM -0400, Frank Li wrote:
> > parent_bus_addr in struct of_range can indicate address information just
> > ahead of PCIe controller. Most system's bus fabric use 1:1 map between
> > input and output address. but some hardware like i.MX8QXP doesn't use 1:1
> > map. See below diagram:
> >
> > ┌─────────┐ ┌────────────┐
> > ┌─────┐ │ │ IA: 0x8ff0_0000 │ │
> > │ CPU ├───►│ ┌────►├─────────────────┐ │ PCI │
> > └─────┘ │ │ │ IA: 0x8ff8_0000 │ │ │
> > CPU Addr │ │ ┌─►├─────────────┐ │ │ Controller │
> > 0x7ff0_0000─┼───┘ │ │ │ │ │ │
> > │ │ │ │ │ │ │ PCI Addr
> > 0x7ff8_0000─┼──────┘ │ │ └──► CfgSpace ─┼────────────►
> > │ │ │ │ │ 0
> > 0x7000_0000─┼────────►├─────────┐ │ │ │
> > └─────────┘ │ └──────► IOSpace ─┼────────────►
> > BUS Fabric │ │ │ 0
> > │ │ │
> > └──────────► MemSpace ─┼────────────►
> > IA: 0x8000_0000 │ │ 0x8000_0000
> > └────────────┘
> >
> > bus@5f000000 {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > #size-cells = <1>;
> > ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
> > <0x80000000 0x0 0x70000000 0x10000000>;
>
> Does this address translation apply to all peripherals in the bus or just PCIe?
No difference. Actually only PCIe under this bus fabric.
> If it is just PCIe, why can't you encode the mapping in the below PCIe node
> 'ranges' property itself?
Even it change by bus fabric, you still treat it as by PCIe. But it can't
remove ugly .cpu_fixes_up() function, becuase IO range can't know actual
address input to PCI controller.
If dt correct descript hardware behavior, we will not need .cpu_fixed_up()
at all.
>
> - Mani
>
> >
> > pcie@5f010000 {
> > compatible = "fsl,imx8q-pcie";
> > reg = <0x5f010000 0x10000>, <0x8ff00000 0x80000>;
> > reg-names = "dbi", "config";
> > #address-cells = <3>;
> > #size-cells = <2>;
> > device_type = "pci";
> > bus-range = <0x00 0xff>;
> > ranges = <0x81000000 0 0x00000000 0x8ff80000 0 0x00010000>,
> > <0x82000000 0 0x80000000 0x80000000 0 0x0ff00000>;
> > ...
> > };
> > };
> >
> > Term internal address (IA) here means the address just before PCIe
> > controller. After ATU use this IA instead CPU address, cpu_addr_fixup() can
> > be removed.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Change from v2 to v3
> > - %s/cpu_untranslate_addr/parent_bus_addr/g
> > - update diagram.
> > - improve commit message.
> >
> > Change from v1 to v2
> > - update because patch1 change get untranslate address method.
> > - add using_dtbus_info in case break back compatibility for exited platform.
> > ---
> > drivers/pci/controller/dwc/pcie-designware-host.c | 42 +++++++++++++++++++++++
> > drivers/pci/controller/dwc/pcie-designware.h | 8 +++++
> > 2 files changed, 50 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 3e41865c72904..823ff42c2e2c9 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -418,6 +418,34 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> > }
> > }
> >
> > +static int dw_pcie_get_untranslate_addr(struct dw_pcie *pci, resource_size_t pci_addr,
> > + resource_size_t *i_addr)
> > +{
> > + struct device *dev = pci->dev;
> > + struct device_node *np = dev->of_node;
> > + struct of_range_parser parser;
> > + struct of_range range;
> > + int ret;
> > +
> > + if (!pci->using_dtbus_info) {
> > + *i_addr = pci_addr;
> > + return 0;
> > + }
> > +
> > + ret = of_range_parser_init(&parser, np);
> > + if (ret)
> > + return ret;
> > +
> > + for_each_of_pci_range(&parser, &range) {
> > + if (pci_addr == range.bus_addr) {
> > + *i_addr = range.parent_bus_addr;
> > + break;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -427,6 +455,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > struct resource_entry *win;
> > struct pci_host_bridge *bridge;
> > struct resource *res;
> > + int index;
> > int ret;
> >
> > raw_spin_lock_init(&pp->lock);
> > @@ -440,6 +469,13 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > pp->cfg0_size = resource_size(res);
> > pp->cfg0_base = res->start;
> >
> > + if (pci->using_dtbus_info) {
> > + index = of_property_match_string(np, "reg-names", "config");
> > + if (index < 0)
> > + return -EINVAL;
> > + of_property_read_reg(np, index, &pp->cfg0_base, NULL);
> > + }
> > +
> > pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
> > if (IS_ERR(pp->va_cfg0_base))
> > return PTR_ERR(pp->va_cfg0_base);
> > @@ -462,6 +498,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > pp->io_base = pci_pio_to_address(win->res->start);
> > }
> >
> > + if (dw_pcie_get_untranslate_addr(pci, pp->io_bus_addr, &pp->io_base))
> > + return -ENODEV;
> > +
> > /* Set default bus ops */
> > bridge->ops = &dw_pcie_ops;
> > bridge->child_ops = &dw_child_pcie_ops;
> > @@ -733,6 +772,9 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> > atu.cpu_addr = entry->res->start;
> > atu.pci_addr = entry->res->start - entry->offset;
> >
> > + if (dw_pcie_get_untranslate_addr(pci, atu.pci_addr, &atu.cpu_addr))
> > + return -EINVAL;
> > +
> > /* Adjust iATU size if MSG TLP region was allocated before */
> > if (pp->msg_res && pp->msg_res->parent == entry->res)
> > atu.size = resource_size(entry->res) -
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index c189781524fb8..e22d32b5a5f19 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -464,6 +464,14 @@ struct dw_pcie {
> > struct reset_control_bulk_data core_rsts[DW_PCIE_NUM_CORE_RSTS];
> > struct gpio_desc *pe_rst;
> > bool suspended;
> > + /*
> > + * Use device tree 'ranges' property of bus node instead using
> > + * cpu_addr_fixup(). Some old platform dts 'ranges' in bus node may not
> > + * reflect real hardware's behavior. In case break these platform back
> > + * compatibility, add below flags. Set it true if dts already correct
> > + * indicate bus fabric address convert.
> > + */
> > + bool using_dtbus_info;
> > };
> >
> > #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
> >
> > --
> > 2.34.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] PCI: imx6: Remove cpu_addr_fixup()
2024-09-30 18:44 [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr() Frank Li
2024-09-30 18:44 ` [PATCH v3 1/3] of: address: Add parent_bus_addr to struct of_pci_range Frank Li
2024-09-30 18:44 ` [PATCH v3 2/3] PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup() Frank Li
@ 2024-09-30 18:44 ` Frank Li
2024-10-03 5:34 ` [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr() Manivannan Sadhasivam
3 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2024-09-30 18:44 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan, Jingoo Han, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: devicetree, linux-kernel, linux-pci, linux-arm-kernel, imx,
Frank Li
Remove cpu_addr_fixup() because dwc common driver already handle address
translate.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3
- none
Change from v1 to v2
- set using_dtbus_info true
---
drivers/pci/controller/dwc/pci-imx6.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 1e58c24137e7f..94f3411352bf0 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -82,7 +82,6 @@ enum imx_pcie_variants {
#define IMX_PCIE_FLAG_HAS_PHY_RESET BIT(5)
#define IMX_PCIE_FLAG_HAS_SERDES BIT(6)
#define IMX_PCIE_FLAG_SUPPORT_64BIT BIT(7)
-#define IMX_PCIE_FLAG_CPU_ADDR_FIXUP BIT(8)
#define imx_check_flag(pci, val) (pci->drvdata->flags & val)
@@ -1015,22 +1014,6 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
regulator_disable(imx_pcie->vpcie);
}
-static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
-{
- struct imx_pcie *imx_pcie = to_imx_pcie(pcie);
- struct dw_pcie_rp *pp = &pcie->pp;
- struct resource_entry *entry;
-
- if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_CPU_ADDR_FIXUP))
- return cpu_addr;
-
- entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
- if (!entry)
- return cpu_addr;
-
- return cpu_addr - entry->offset;
-}
-
static const struct dw_pcie_host_ops imx_pcie_host_ops = {
.init = imx_pcie_host_init,
.deinit = imx_pcie_host_exit,
@@ -1039,7 +1022,6 @@ static const struct dw_pcie_host_ops imx_pcie_host_ops = {
static const struct dw_pcie_ops dw_pcie_ops = {
.start_link = imx_pcie_start_link,
.stop_link = imx_pcie_stop_link,
- .cpu_addr_fixup = imx_pcie_cpu_addr_fixup,
};
static void imx_pcie_ep_init(struct dw_pcie_ep *ep)
@@ -1459,6 +1441,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
if (ret)
return ret;
+ pci->using_dtbus_info = true;
if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
ret = imx_add_pcie_ep(imx_pcie, pdev);
if (ret < 0)
@@ -1598,8 +1581,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
},
[IMX8Q] = {
.variant = IMX8Q,
- .flags = IMX_PCIE_FLAG_HAS_PHYDRV |
- IMX_PCIE_FLAG_CPU_ADDR_FIXUP,
+ .flags = IMX_PCIE_FLAG_HAS_PHYDRV,
.clk_names = imx8q_clks,
.clks_cnt = ARRAY_SIZE(imx8q_clks),
},
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr()
2024-09-30 18:44 [PATCH v3 0/3] PCI: dwc: opitimaze RC host pci_fixup_addr() Frank Li
` (2 preceding siblings ...)
2024-09-30 18:44 ` [PATCH v3 3/3] PCI: imx6: Remove cpu_addr_fixup() Frank Li
@ 2024-10-03 5:34 ` Manivannan Sadhasivam
3 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2024-10-03 5:34 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Saravana Kannan, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Richard Zhu,
Lucas Stach, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, devicetree, linux-kernel, linux-pci,
linux-arm-kernel, imx
On Mon, Sep 30, 2024 at 02:44:52PM -0400, Frank Li wrote:
> ┌─────────┐ ┌────────────┐
> ┌─────┐ │ │ IA: 0x8ff0_0000 │ │
> │ CPU ├───►│ ┌────►├─────────────────┐ │ PCI │
> └─────┘ │ │ │ IA: 0x8ff8_0000 │ │ │
> CPU Addr │ │ ┌─►├─────────────┐ │ │ Controller │
> 0x7ff0_0000─┼───┘ │ │ │ │ │ │
> │ │ │ │ │ │ │ PCI Addr
> 0x7ff8_0000─┼──────┘ │ │ └──► CfgSpace ─┼────────────►
> │ │ │ │ │ 0
> 0x7000_0000─┼────────►├─────────┐ │ │ │
> └─────────┘ │ └──────► IOSpace ─┼────────────►
> BUS Fabric │ │ │ 0
> │ │ │
> └──────────► MemSpace ─┼────────────►
> IA: 0x8000_0000 │ │ 0x8000_0000
> └────────────┘
>
> Current dwc implimemnt, pci_fixup_addr() call back is needed when bus
> fabric convert cpu address before send to PCIe controller.
>
> bus@5f000000 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
> <0x80000000 0x0 0x70000000 0x10000000>;
>
> pcie@5f010000 {
> compatible = "fsl,imx8q-pcie";
> reg = <0x5f010000 0x10000>, <0x8ff00000 0x80000>;
> reg-names = "dbi", "config";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> bus-range = <0x00 0xff>;
> ranges = <0x81000000 0 0x00000000 0x8ff80000 0 0x00010000>,
> <0x82000000 0 0x80000000 0x80000000 0 0x0ff00000>;
> ...
> };
> };
>
> Device tree already can descript all address translate. Some hardware
> driver implement fixup function by mask some bits of cpu address. Last
> pci-imx6.c are little bit better by fetch memory resource's offset to do
> fixup.
>
> static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
> {
> ...
> entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> return cpu_addr - entry->offset;
> }
>
> But it is not good by using IORESOURCE_MEM to fix up io/cfg address map
> although address translate is the same as IORESOURCE_MEM.
>
> This patches to fetch untranslate range information for PCIe controller
> (pcie@5f010000: ranges). So current config ATU without cpu_fixup_addr().
>
> EP side patch:
> https://lore.kernel.org/linux-pci/20240923-pcie_ep_range-v2-0-78d2ea434d9f@nxp.com/T/#mfc73ca113a69ad2c0294a2e629ecee3105b72973
>
> The both pave the road to eliminate ugle cpu_fixup_addr() callback function.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Changes in v3:
> - see each patch
You should make the life easier for the reviewer, not hard. Please summarise
what has changed since the last version.
- Mani
> - Link to v2: https://lore.kernel.org/r/20240926-pci_fixup_addr-v2-0-e4524541edf4@nxp.com
>
> Changes in v2:
> - see each patch
> - Link to v1: https://lore.kernel.org/r/20240924-pci_fixup_addr-v1-0-57d14a91ec4f@nxp.com
>
> ---
> Frank Li (3):
> of: address: Add parent_bus_addr to struct of_pci_range
> PCI: dwc: Using parent_bus_addr in of_range to eliminate cpu_addr_fixup()
> PCI: imx6: Remove cpu_addr_fixup()
>
> drivers/of/address.c | 2 ++
> drivers/pci/controller/dwc/pci-imx6.c | 22 ++----------
> drivers/pci/controller/dwc/pcie-designware-host.c | 42 +++++++++++++++++++++++
> drivers/pci/controller/dwc/pcie-designware.h | 8 +++++
> include/linux/of_address.h | 1 +
> 5 files changed, 55 insertions(+), 20 deletions(-)
> ---
> base-commit: 69940764dc1c429010d37cded159fadf1347d318
> change-id: 20240924-pci_fixup_addr-a8568f9bbb34
>
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread