2023-06-09 18:38:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH] bus: fsl-mc: Use common ranges functions

Since commit 3d5089c4263d ("of/address: Add support for 3 address cell
bus"), the DT address functions can handle translating buses with 3
address cells. Replace the custom code with the for_each_of_range()
iterator.

The original code had fallbacks to get "#address-cells"/"#size-cells"
from the bus parent node if they are missing. This is non-standard
behavior, and AFAICT the upstream .dts files never relied on that.

Signed-off-by: Rob Herring <[email protected]>
---
drivers/bus/fsl-mc/fsl-mc-bus.c | 90 +++++----------------------------
1 file changed, 13 insertions(+), 77 deletions(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 653e2d4c116f..5062d73b13e2 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -999,75 +999,18 @@ struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev,
}
EXPORT_SYMBOL_GPL(fsl_mc_get_endpoint);

-static int parse_mc_ranges(struct device *dev,
- int *paddr_cells,
- int *mc_addr_cells,
- int *mc_size_cells,
- const __be32 **ranges_start)
-{
- const __be32 *prop;
- int range_tuple_cell_count;
- int ranges_len;
- int tuple_len;
- struct device_node *mc_node = dev->of_node;
-
- *ranges_start = of_get_property(mc_node, "ranges", &ranges_len);
- if (!(*ranges_start) || !ranges_len) {
- dev_warn(dev,
- "missing or empty ranges property for device tree node '%pOFn'\n",
- mc_node);
- return 0;
- }
-
- *paddr_cells = of_n_addr_cells(mc_node);
-
- prop = of_get_property(mc_node, "#address-cells", NULL);
- if (prop)
- *mc_addr_cells = be32_to_cpup(prop);
- else
- *mc_addr_cells = *paddr_cells;
-
- prop = of_get_property(mc_node, "#size-cells", NULL);
- if (prop)
- *mc_size_cells = be32_to_cpup(prop);
- else
- *mc_size_cells = of_n_size_cells(mc_node);
-
- range_tuple_cell_count = *paddr_cells + *mc_addr_cells +
- *mc_size_cells;
-
- tuple_len = range_tuple_cell_count * sizeof(__be32);
- if (ranges_len % tuple_len != 0) {
- dev_err(dev, "malformed ranges property '%pOFn'\n", mc_node);
- return -EINVAL;
- }
-
- return ranges_len / tuple_len;
-}
-
static int get_mc_addr_translation_ranges(struct device *dev,
struct fsl_mc_addr_translation_range
**ranges,
u8 *num_ranges)
{
- int ret;
- int paddr_cells;
- int mc_addr_cells;
- int mc_size_cells;
- int i;
- const __be32 *ranges_start;
- const __be32 *cell;
-
- ret = parse_mc_ranges(dev,
- &paddr_cells,
- &mc_addr_cells,
- &mc_size_cells,
- &ranges_start);
- if (ret < 0)
- return ret;
+ struct fsl_mc_addr_translation_range *r;
+ struct of_range_parser parser;
+ struct of_range range;

- *num_ranges = ret;
- if (!ret) {
+ of_range_parser_init(&parser, dev->of_node);
+ *num_ranges = of_range_count(&parser);
+ if (!*num_ranges) {
/*
* Missing or empty ranges property ("ranges;") for the
* 'fsl,qoriq-mc' node. In this case, identity mapping
@@ -1083,20 +1026,13 @@ static int get_mc_addr_translation_ranges(struct device *dev,
if (!(*ranges))
return -ENOMEM;

- cell = ranges_start;
- for (i = 0; i < *num_ranges; ++i) {
- struct fsl_mc_addr_translation_range *range = &(*ranges)[i];
-
- range->mc_region_type = of_read_number(cell, 1);
- range->start_mc_offset = of_read_number(cell + 1,
- mc_addr_cells - 1);
- cell += mc_addr_cells;
- range->start_phys_addr = of_read_number(cell, paddr_cells);
- cell += paddr_cells;
- range->end_mc_offset = range->start_mc_offset +
- of_read_number(cell, mc_size_cells);
-
- cell += mc_size_cells;
+ r = *ranges;
+ for_each_of_range(&parser, &range) {
+ r->mc_region_type = range.flags;
+ r->start_mc_offset = range.bus_addr;
+ r->end_mc_offset = range.bus_addr + range.size;
+ r->start_phys_addr = range.cpu_addr;
+ r++;
}

return 0;
--
2.39.2



2023-06-21 16:39:59

by Laurentiu Tudor

[permalink] [raw]
Subject: Re: [PATCH] bus: fsl-mc: Use common ranges functions



On 6/9/2023 9:32 PM, Rob Herring wrote:
> Since commit 3d5089c4263d ("of/address: Add support for 3 address cell
> bus"), the DT address functions can handle translating buses with 3
> address cells. Replace the custom code with the for_each_of_range()
> iterator.
>
> The original code had fallbacks to get "#address-cells"/"#size-cells"
> from the bus parent node if they are missing. This is non-standard
> behavior, and AFAICT the upstream .dts files never relied on that.
>
> Signed-off-by: Rob Herring <[email protected]>

Thanks, Rob!

Reviewed-by: Laurentiu Tudor <[email protected]>
Tested-by: Laurentiu Tudor <[email protected]>

---
Best Regards, Laurentiu

> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 90 +++++----------------------------
> 1 file changed, 13 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 653e2d4c116f..5062d73b13e2 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -999,75 +999,18 @@ struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev,
> }
> EXPORT_SYMBOL_GPL(fsl_mc_get_endpoint);
>
> -static int parse_mc_ranges(struct device *dev,
> - int *paddr_cells,
> - int *mc_addr_cells,
> - int *mc_size_cells,
> - const __be32 **ranges_start)
> -{
> - const __be32 *prop;
> - int range_tuple_cell_count;
> - int ranges_len;
> - int tuple_len;
> - struct device_node *mc_node = dev->of_node;
> -
> - *ranges_start = of_get_property(mc_node, "ranges", &ranges_len);
> - if (!(*ranges_start) || !ranges_len) {
> - dev_warn(dev,
> - "missing or empty ranges property for device tree node '%pOFn'\n",
> - mc_node);
> - return 0;
> - }
> -
> - *paddr_cells = of_n_addr_cells(mc_node);
> -
> - prop = of_get_property(mc_node, "#address-cells", NULL);
> - if (prop)
> - *mc_addr_cells = be32_to_cpup(prop);
> - else
> - *mc_addr_cells = *paddr_cells;
> -
> - prop = of_get_property(mc_node, "#size-cells", NULL);
> - if (prop)
> - *mc_size_cells = be32_to_cpup(prop);
> - else
> - *mc_size_cells = of_n_size_cells(mc_node);
> -
> - range_tuple_cell_count = *paddr_cells + *mc_addr_cells +
> - *mc_size_cells;
> -
> - tuple_len = range_tuple_cell_count * sizeof(__be32);
> - if (ranges_len % tuple_len != 0) {
> - dev_err(dev, "malformed ranges property '%pOFn'\n", mc_node);
> - return -EINVAL;
> - }
> -
> - return ranges_len / tuple_len;
> -}
> -
> static int get_mc_addr_translation_ranges(struct device *dev,
> struct fsl_mc_addr_translation_range
> **ranges,
> u8 *num_ranges)
> {
> - int ret;
> - int paddr_cells;
> - int mc_addr_cells;
> - int mc_size_cells;
> - int i;
> - const __be32 *ranges_start;
> - const __be32 *cell;
> -
> - ret = parse_mc_ranges(dev,
> - &paddr_cells,
> - &mc_addr_cells,
> - &mc_size_cells,
> - &ranges_start);
> - if (ret < 0)
> - return ret;
> + struct fsl_mc_addr_translation_range *r;
> + struct of_range_parser parser;
> + struct of_range range;
>
> - *num_ranges = ret;
> - if (!ret) {
> + of_range_parser_init(&parser, dev->of_node);
> + *num_ranges = of_range_count(&parser);
> + if (!*num_ranges) {
> /*
> * Missing or empty ranges property ("ranges;") for the
> * 'fsl,qoriq-mc' node. In this case, identity mapping
> @@ -1083,20 +1026,13 @@ static int get_mc_addr_translation_ranges(struct device *dev,
> if (!(*ranges))
> return -ENOMEM;
>
> - cell = ranges_start;
> - for (i = 0; i < *num_ranges; ++i) {
> - struct fsl_mc_addr_translation_range *range = &(*ranges)[i];
> -
> - range->mc_region_type = of_read_number(cell, 1);
> - range->start_mc_offset = of_read_number(cell + 1,
> - mc_addr_cells - 1);
> - cell += mc_addr_cells;
> - range->start_phys_addr = of_read_number(cell, paddr_cells);
> - cell += paddr_cells;
> - range->end_mc_offset = range->start_mc_offset +
> - of_read_number(cell, mc_size_cells);
> -
> - cell += mc_size_cells;
> + r = *ranges;
> + for_each_of_range(&parser, &range) {
> + r->mc_region_type = range.flags;
> + r->start_mc_offset = range.bus_addr;
> + r->end_mc_offset = range.bus_addr + range.size;
> + r->start_phys_addr = range.cpu_addr;
> + r++;
> }
>
> return 0;