Introduce `for_each_of_range_untranslate()` to retrieve untranslated CPU
address information, similar to `of_property_read_reg()`. 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>;
...
};
};
Currently all function related 'range' return CPU address. THe new help
function for_each_of_range_untranslate() can get above diagram IA address
informaiton.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/of/address.c | 33 +++++++++++++++++++++++----------
include/linux/of_address.h | 9 ++++++++-
2 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 286f0c161e332..09c73936e573f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -787,8 +787,9 @@ int of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
EXPORT_SYMBOL_GPL(of_pci_dma_range_parser_init);
#define of_dma_range_parser_init of_pci_dma_range_parser_init
-struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
- struct of_pci_range *range)
+struct of_pci_range *of_pci_range_parser_one_common(struct of_pci_range_parser *parser,
+ struct of_pci_range *range,
+ bool translate)
{
int na = parser->na;
int ns = parser->ns;
@@ -806,11 +807,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
range->bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
if (parser->dma)
- range->cpu_addr = of_translate_dma_address(parser->node,
- parser->range + na);
+ range->cpu_addr = translate ? of_translate_dma_address(parser->node,
+ parser->range + na) :
+ of_read_number(parser->range + na, parser->pna);
else
- range->cpu_addr = of_translate_address(parser->node,
- parser->range + na);
+ range->cpu_addr = translate ? of_translate_address(parser->node,
+ parser->range + na) :
+ of_read_number(parser->range + na, parser->pna);
range->size = of_read_number(parser->range + parser->pna + na, ns);
parser->range += np;
@@ -823,11 +826,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
flags = parser->bus->get_flags(parser->range);
bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
if (parser->dma)
- cpu_addr = of_translate_dma_address(parser->node,
- parser->range + na);
+ cpu_addr = translate ? of_translate_dma_address(parser->node,
+ parser->range + na) :
+ of_read_number(parser->range + np, np);
else
- cpu_addr = of_translate_address(parser->node,
- parser->range + na);
+ cpu_addr = translate ? of_translate_address(parser->node,
+ parser->range + na) :
+ of_read_number(parser->range + np, np);
size = of_read_number(parser->range + parser->pna + na, ns);
if (flags != range->flags)
@@ -842,6 +847,14 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
return range;
}
+EXPORT_SYMBOL_GPL(of_pci_range_parser_one_common);
+
+struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
+ struct of_pci_range *range)
+{
+ return of_pci_range_parser_one_common(parser, range, true);
+}
+
EXPORT_SYMBOL_GPL(of_pci_range_parser_one);
static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 26a19daf0d092..692aae853217a 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -32,8 +32,11 @@ struct of_pci_range {
#define of_range of_pci_range
#define for_each_of_pci_range(parser, range) \
- for (; of_pci_range_parser_one(parser, range);)
+ for (; of_pci_range_parser_one_common(parser, range, true);)
+#define for_each_of_pci_range_untranslate(parser, range) \
+ for (; of_pci_range_parser_one_common(parser, range, false);)
#define for_each_of_range for_each_of_pci_range
+#define for_each_of_range_untranslate for_each_of_pci_range_untranslate
/*
* of_range_count - Get the number of "ranges" or "dma-ranges" entries
@@ -78,6 +81,10 @@ extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
struct device_node *node);
extern int of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
struct device_node *node);
+extern struct of_pci_range *of_pci_range_parser_one_common(
+ struct of_pci_range_parser *parser,
+ struct of_pci_range *range,
+ bool translate);
extern struct of_pci_range *of_pci_range_parser_one(
struct of_pci_range_parser *parser,
struct of_pci_range *range);
--
2.34.1
On Tue, Sep 24, 2024 at 05:54:19PM -0400, Frank Li wrote:
> Introduce `for_each_of_range_untranslate()` to retrieve untranslated CPU
> address information, similar to `of_property_read_reg()`. 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.
Just to be clear, you don't have to change the DT to add the
intermediate bus? If you do, then that's an ABI issue.
>
> ┌─────────┐ ┌────────────┐
> ┌─────┐ │ │ 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>;
> ...
> };
> };
>
> Currently all function related 'range' return CPU address. THe new help
> function for_each_of_range_untranslate() can get above diagram IA address
> informaiton.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/of/address.c | 33 +++++++++++++++++++++++----------
> include/linux/of_address.h | 9 ++++++++-
> 2 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 286f0c161e332..09c73936e573f 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -787,8 +787,9 @@ int of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
> EXPORT_SYMBOL_GPL(of_pci_dma_range_parser_init);
> #define of_dma_range_parser_init of_pci_dma_range_parser_init
>
> -struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> - struct of_pci_range *range)
> +struct of_pci_range *of_pci_range_parser_one_common(struct of_pci_range_parser *parser,
> + struct of_pci_range *range,
> + bool translate)
> {
> int na = parser->na;
> int ns = parser->ns;
> @@ -806,11 +807,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> range->bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
>
> if (parser->dma)
> - range->cpu_addr = of_translate_dma_address(parser->node,
> - parser->range + na);
> + range->cpu_addr = translate ? of_translate_dma_address(parser->node,
> + parser->range + na) :
> + of_read_number(parser->range + na, parser->pna);
> else
> - range->cpu_addr = of_translate_address(parser->node,
> - parser->range + na);
> + range->cpu_addr = translate ? of_translate_address(parser->node,
> + parser->range + na) :
> + of_read_number(parser->range + na, parser->pna);
> range->size = of_read_number(parser->range + parser->pna + na, ns);
>
> parser->range += np;
> @@ -823,11 +826,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> flags = parser->bus->get_flags(parser->range);
> bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
> if (parser->dma)
> - cpu_addr = of_translate_dma_address(parser->node,
> - parser->range + na);
> + cpu_addr = translate ? of_translate_dma_address(parser->node,
> + parser->range + na) :
> + of_read_number(parser->range + np, np);
> else
> - cpu_addr = of_translate_address(parser->node,
> - parser->range + na);
> + cpu_addr = translate ? of_translate_address(parser->node,
> + parser->range + na) :
> + of_read_number(parser->range + np, np);
> size = of_read_number(parser->range + parser->pna + na, ns);
>
> if (flags != range->flags)
> @@ -842,6 +847,14 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
>
> return range;
> }
> +EXPORT_SYMBOL_GPL(of_pci_range_parser_one_common);
> +
> +struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> + struct of_pci_range *range)
> +{
> + return of_pci_range_parser_one_common(parser, range, true);
> +}
> +
> EXPORT_SYMBOL_GPL(of_pci_range_parser_one);
>
> static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 26a19daf0d092..692aae853217a 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -32,8 +32,11 @@ struct of_pci_range {
> #define of_range of_pci_range
>
> #define for_each_of_pci_range(parser, range) \
> - for (; of_pci_range_parser_one(parser, range);)
> + for (; of_pci_range_parser_one_common(parser, range, true);)
> +#define for_each_of_pci_range_untranslate(parser, range) \
> + for (; of_pci_range_parser_one_common(parser, range, false);)
> #define for_each_of_range for_each_of_pci_range
> +#define for_each_of_range_untranslate for_each_of_pci_range_untranslate
You may want both the translated and untranslated address, so I would
just add the untranslated address to the of_pci_range struct and return
both with the existing iterator.
Rob
On Wed, Sep 25, 2024 at 09:26:10PM -0500, Rob Herring wrote:
> On Tue, Sep 24, 2024 at 05:54:19PM -0400, Frank Li wrote:
> > Introduce `for_each_of_range_untranslate()` to retrieve untranslated CPU
> > address information, similar to `of_property_read_reg()`. 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.
>
> Just to be clear, you don't have to change the DT to add the
> intermediate bus? If you do, then that's an ABI issue.
imx8q's pci dts is still not upstreamed. We are working on it. I think use
this way is more reasonable.
>
> >
> > ┌─────────┐ ┌────────────┐
> > ┌─────┐ │ │ 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>;
> > ...
> > };
> > };
> >
> > Currently all function related 'range' return CPU address. THe new help
> > function for_each_of_range_untranslate() can get above diagram IA address
> > informaiton.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > drivers/of/address.c | 33 +++++++++++++++++++++++----------
> > include/linux/of_address.h | 9 ++++++++-
> > 2 files changed, 31 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 286f0c161e332..09c73936e573f 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -787,8 +787,9 @@ int of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
> > EXPORT_SYMBOL_GPL(of_pci_dma_range_parser_init);
> > #define of_dma_range_parser_init of_pci_dma_range_parser_init
> >
> > -struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> > - struct of_pci_range *range)
> > +struct of_pci_range *of_pci_range_parser_one_common(struct of_pci_range_parser *parser,
> > + struct of_pci_range *range,
> > + bool translate)
> > {
> > int na = parser->na;
> > int ns = parser->ns;
> > @@ -806,11 +807,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> > range->bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
> >
> > if (parser->dma)
> > - range->cpu_addr = of_translate_dma_address(parser->node,
> > - parser->range + na);
> > + range->cpu_addr = translate ? of_translate_dma_address(parser->node,
> > + parser->range + na) :
> > + of_read_number(parser->range + na, parser->pna);
> > else
> > - range->cpu_addr = of_translate_address(parser->node,
> > - parser->range + na);
> > + range->cpu_addr = translate ? of_translate_address(parser->node,
> > + parser->range + na) :
> > + of_read_number(parser->range + na, parser->pna);
> > range->size = of_read_number(parser->range + parser->pna + na, ns);
> >
> > parser->range += np;
> > @@ -823,11 +826,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> > flags = parser->bus->get_flags(parser->range);
> > bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
> > if (parser->dma)
> > - cpu_addr = of_translate_dma_address(parser->node,
> > - parser->range + na);
> > + cpu_addr = translate ? of_translate_dma_address(parser->node,
> > + parser->range + na) :
> > + of_read_number(parser->range + np, np);
> > else
> > - cpu_addr = of_translate_address(parser->node,
> > - parser->range + na);
> > + cpu_addr = translate ? of_translate_address(parser->node,
> > + parser->range + na) :
> > + of_read_number(parser->range + np, np);
> > size = of_read_number(parser->range + parser->pna + na, ns);
> >
> > if (flags != range->flags)
> > @@ -842,6 +847,14 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> >
> > return range;
> > }
> > +EXPORT_SYMBOL_GPL(of_pci_range_parser_one_common);
> > +
> > +struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
> > + struct of_pci_range *range)
> > +{
> > + return of_pci_range_parser_one_common(parser, range, true);
> > +}
> > +
> > EXPORT_SYMBOL_GPL(of_pci_range_parser_one);
> >
> > static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 26a19daf0d092..692aae853217a 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -32,8 +32,11 @@ struct of_pci_range {
> > #define of_range of_pci_range
> >
> > #define for_each_of_pci_range(parser, range) \
> > - for (; of_pci_range_parser_one(parser, range);)
> > + for (; of_pci_range_parser_one_common(parser, range, true);)
> > +#define for_each_of_pci_range_untranslate(parser, range) \
> > + for (; of_pci_range_parser_one_common(parser, range, false);)
> > #define for_each_of_range for_each_of_pci_range
> > +#define for_each_of_range_untranslate for_each_of_pci_range_untranslate
>
> You may want both the translated and untranslated address, so I would
> just add the untranslated address to the of_pci_range struct and return
> both with the existing iterator.
Yes, let me try it.
Frank
>
> Rob
© 2016 - 2024 Red Hat, Inc.