Introduce field 'cpu_untranslate_addr' in of_pci_range to retrieve
untranslated CPU address information. This is required for hardware like
i.MX8QXP to configure the PCIe controller ATU and eliminate the need for
workaround address fixups in drivers. Currently, many drivers use
hardcoded CPU addresses for fixups, but this information is already
described in the Device Tree. With correct hardware descriptions, such
fixups can be removed.
┌─────────┐ ┌────────────┐
┌─────┐ │ │ IA: 0x8ff0_0000 │ │
│ CPU ├───►│ BUS ├─────────────────┐ │ PCI │
└─────┘ │ │ IA: 0x8ff8_0000 │ │ │
CPU Addr │ Fabric ├─────────────┐ │ │ Controller │
0x7000_0000 │ │ │ │ │ │
│ │ │ │ │ │ PCI Addr
│ │ │ └──► CfgSpace ─┼────────────►
│ ├─────────┐ │ │ │ 0
│ │ │ │ │ │
└─────────┘ │ └──────► IOSpace ─┼────────────►
│ │ │ 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>;
pcieb: 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>;
...
};
};
'cpu_untranslate_addr' in of_pci_range can indicate above diagram IA
address information.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v1 to v2
- add cpu_untranslate_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..f4cb82f5313cf 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->cpu_untranslate_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..0683ce0c07f68 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 cpu_untranslate_addr;
u64 size;
u32 flags;
};
--
2.34.1
On Thu, Sep 26, 2024 at 12:47:13PM -0400, Frank Li wrote:
> Introduce field 'cpu_untranslate_addr' in of_pci_range to retrieve
> untranslated CPU address information. This is required for hardware like
> i.MX8QXP to configure the PCIe controller ATU and eliminate the need for
> workaround address fixups in drivers. Currently, many drivers use
> hardcoded CPU addresses for fixups, but this information is already
> described in the Device Tree. With correct hardware descriptions, such
> fixups can be removed.
Instead of saying "required for hardware like i.MX8QXP", can we say
something specific about what this kind of hardware *does* that
requires this?
I *think* the point is that there's some address translation being
done between the primary and secondary sides of some bridge.
I think "many drivers use hardcoded CPU addresses for fixups"
basically means the .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.
> ┌─────────┐ ┌────────────┐
> ┌─────┐ │ │ IA: 0x8ff0_0000 │ │
> │ CPU ├───►│ BUS ├─────────────────┐ │ PCI │
> └─────┘ │ │ IA: 0x8ff8_0000 │ │ │
> CPU Addr │ Fabric ├─────────────┐ │ │ Controller │
> 0x7000_0000 │ │ │ │ │ │
> │ │ │ │ │ │ PCI Addr
> │ │ │ └──► CfgSpace ─┼────────────►
> │ ├─────────┐ │ │ │ 0
> │ │ │ │ │ │
> └─────────┘ │ └──────► IOSpace ─┼────────────►
> │ │ │ 0
> │ │ │
> └──────────► MemSpace ─┼────────────►
> IA: 0x8000_0000 │ │ 0x8000_0000
> └────────────┘
What does "IA" stand for?
I don't quite understand the mapping done by the "BUS Fabric" block.
It looks like you're saying the CPU Addr 0x7000_0000 is translated to
all three of IA 0x8ff0_0000, IA 0x8ff8_0000, and IA 0x8000_0000, but
that doesn't seem right.
> bus@5f000000 {
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
> <0x80000000 0x0 0x70000000 0x10000000>;
>
> pcieb: 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>;
> ...
> };
> };
>
> 'cpu_untranslate_addr' in of_pci_range can indicate above diagram IA
> address information.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - add cpu_untranslate_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..f4cb82f5313cf 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->cpu_untranslate_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..0683ce0c07f68 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 cpu_untranslate_addr;
> u64 size;
> u32 flags;
> };
>
> --
> 2.34.1
>
On Fri, Sep 27, 2024 at 06:51:17PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 26, 2024 at 12:47:13PM -0400, Frank Li wrote:
> > Introduce field 'cpu_untranslate_addr' in of_pci_range to retrieve
> > untranslated CPU address information. This is required for hardware like
> > i.MX8QXP to configure the PCIe controller ATU and eliminate the need for
> > workaround address fixups in drivers. Currently, many drivers use
> > hardcoded CPU addresses for fixups, but this information is already
> > described in the Device Tree. With correct hardware descriptions, such
> > fixups can be removed.
>
> Instead of saying "required for hardware like i.MX8QXP", can we say
> something specific about what this kind of hardware *does* that
> requires this?
>
> I *think* the point is that there's some address translation being
> done between the primary and secondary sides of some bridge.
>
> I think "many drivers use hardcoded CPU addresses for fixups"
> basically means the .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.
>
> > ┌─────────┐ ┌────────────┐
> > ┌─────┐ │ │ IA: 0x8ff0_0000 │ │
> > │ CPU ├───►│ BUS ├─────────────────┐ │ PCI │
> > └─────┘ │ │ IA: 0x8ff8_0000 │ │ │
> > CPU Addr │ Fabric ├─────────────┐ │ │ Controller │
> > 0x7000_0000 │ │ │ │ │ │
> > │ │ │ │ │ │ PCI Addr
> > │ │ │ └──► CfgSpace ─┼────────────►
> > │ ├─────────┐ │ │ │ 0
> > │ │ │ │ │ │
> > └─────────┘ │ └──────► IOSpace ─┼────────────►
> > │ │ │ 0
> > │ │ │
> > └──────────► MemSpace ─┼────────────►
> > IA: 0x8000_0000 │ │ 0x8000_0000
> > └────────────┘
>
> What does "IA" stand for?
internal address
>
> I don't quite understand the mapping done by the "BUS Fabric" block.
> It looks like you're saying the CPU Addr 0x7000_0000 is translated to
> all three of IA 0x8ff0_0000, IA 0x8ff8_0000, and IA 0x8000_0000, but
> that doesn't seem right.
0x7000_0000 --> 0x8000_0000
0x7ff0_0000 --> 0x8ff0_0000
0x7ff8_0000 --> 0x8ff8_0000
I can update diagram at next version
>
> > bus@5f000000 {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > #size-cells = <1>;
> > ranges = <0x5f000000 0x0 0x5f000000 0x21000000>,
> > <0x80000000 0x0 0x70000000 0x10000000>;
> >
> > pcieb: 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>;
> > ...
> > };
> > };
> >
> > 'cpu_untranslate_addr' in of_pci_range can indicate above diagram IA
> > address information.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Change from v1 to v2
> > - add cpu_untranslate_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..f4cb82f5313cf 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->cpu_untranslate_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..0683ce0c07f68 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 cpu_untranslate_addr;
> > u64 size;
> > u32 flags;
> > };
> >
> > --
> > 2.34.1
> >
On Thu, Sep 26, 2024 at 12:47:13PM -0400, Frank Li wrote:
> Introduce field 'cpu_untranslate_addr' in of_pci_range to retrieve
> untranslated CPU address information. This is required for hardware like
> i.MX8QXP to configure the PCIe controller ATU and eliminate the need for
> workaround address fixups in drivers. Currently, many drivers use
> hardcoded CPU addresses for fixups, but this information is already
> described in the Device Tree. With correct hardware descriptions, such
> fixups can be removed.
>
> ┌─────────┐ ┌────────────┐
> ┌─────┐ │ │ IA: 0x8ff0_0000 │ │
> │ CPU ├───►│ BUS ├─────────────────┐ │ PCI │
> └─────┘ │ │ IA: 0x8ff8_0000 │ │ │
> CPU Addr │ Fabric ├─────────────┐ │ │ Controller │
> 0x7000_0000 │ │ │ │ │ │
> │ │ │ │ │ │ PCI Addr
> │ │ │ └──► CfgSpace ─┼────────────►
> │ ├─────────┐ │ │ │ 0
> │ │ │ │ │ │
> └─────────┘ │ └──────► IOSpace ─┼────────────►
> │ │ │ 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>;
>
> pcieb: 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>;
> ...
> };
> };
>
> 'cpu_untranslate_addr' in of_pci_range can indicate above diagram IA
> address information.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - add cpu_untranslate_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..f4cb82f5313cf 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->cpu_untranslate_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..0683ce0c07f68 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 cpu_untranslate_addr;
Let's call it "parent_bus_addr" as it's not really the "cpu" address any
more. With that,
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Rob
© 2016 - 2024 Red Hat, Inc.