[RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range

[Peter Robinson]

On Wed, 5 Nov 2025 at 16:44, Torsten Duwe <duwe at lst.de> wrote:
>
> of_get_dma_range uses (of_bus *)->count_cells to look up the
> #cells for addresses and sizes. It tries to do so for the current
> device node and for the parent tree. count_cells defaults to
> of_bus_default_count_cells, which _starts_ looking at the parent
> node already, and then upwards. The correct way is to use
> of_simple_*_cells for the current node, and count_cells(dev) for
> the parent tree.
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev at epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk at epam.com>
> Signed-off-by: Torsten Duwe <duwe at suse.de>
> Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev at epam.com>

There can't be the same person sign off and review, were the reviews
on the public list?

> ---
>
> Here is the original, very detailed analysis and explanation

It would likely be useful to make this more succinct and put the most
useful bits of it in the commit, in documentation or something
similar.

> [edited] :
>
> According to the Section 2.3.9 of the Device Tree specification[0]
> the format of the value of the dma-ranges property is an arbitrary
> number of triplets of (child-bus-address,
> parent-bus-address, length).
> Where:
> * The child-bus-address is a physical address within the child bus’
>   address space. The number of cells to represent the address depends on
>   the bus and can be determined from the #address-cells of this node
>   (the node in which the dma-ranges property appears).
> * The parent-bus-address is a physical address within the parent bus’
>   address space. The number of cells to represent the parent address
>   is bus dependent and can be determined from the #address-cells
>   property of the node that defines the parent’s address space.
> * The length specifies the size of the range in the child’s address
>   space. The number of cells to represent the size can be determined
>   from the #size-cells of this node (the node in which the dma-ranges
>   property appears)
>
> This description matches comments in the of_addr source code. But the
> actual implementation does not match the specification.
> 'of_match_bus' returns address-cells and size-cells of the parent
> device. That's why the following calls were used to receive addr_cells
> and size_cells of the current device:
> - of_simple_addr_cells
> - of_simple_size_cells
>
> This fix was tested on RPI5 board with both device types: directly
> connected (e.g Serial) and connected over PCIe (e.g ethernet).
>
> For example when dma_ranges are parsing the following device tree
> structure:
> / {
>         pcie2: {
>                 #address-cells = <3>;
>                 #size-cells = <2>;
>                 // 64GB system RAM space at PCIe 10_00000000
>                 dma-ranges =
>                            <0x02000000 0x00 0x00000000
>                             0x1f 0x00000000
>                             0x00 0x00400000>,
>                            <0x43000000 0x10 0x00000000
>                             0x00 0x00000000
>                             0x10 0x00000000>;
>                 rp1: {
>                         #address-cells = <2>;
>                         #size-cells = <2>;
>                         dma-ranges =
>                         // inbound RP1 1x_xxxxxxxx -> PCIe 1x_xxxxxxxx
>                           <0x10 0x00000000
>                           0x43000000 0x10 0x00000000
>                           0x10 0x00000000>,
>
>                         // inbound RP1 c0_40xxxxxx -> PCIe 00_00xxxxxx
>                           <0xc0 0x40000000
>                           0x02000000 0x0 0x00000000
>                           0x0 0x00400000>,
>
>                         // inbound RP1 0x_xxxxxxxx -> PCIe 1x_xxxxxxxx
>                           <0x00 0x00000000
>                           0x02000000 0x10 0x00000000
>                           0x10 0x00000000>;
>                 };
>         };
> };
>
> So when we parse ranges on rp1, #address-cells of the rp1 node should
> be taken into account.
> In the provided example we have the following format:
> pcie2:
>    #addres-cells = <0x3>;
>    #size-cells = <0x2>;
>    dma-ranges = < addr_hi addr_mid addr_lo
>                   trans_addr_hi trans_addr_lo
>                   size_hi size_lo>;
>
> rp1:
>    #addres-cells = <0x2>;
>    #size-cells = <0x2>;
>    dma-ranges = < addr_hi addr_lo
>                   trans_addr_hi trans_addr_mid trans_addr_lo
>                   size_hi size_lo>;
>
> of_get_dma_range searches for the first dev with dma_ranges
> and then executes of_match_bus which follows by count_cells call
> for both device and parent.
>
> count_cells returns #address-cells for the parent device to
> the caller.
> So in the provided example address cells should be 2 for rp1,
> but we get 3 when using count_cells.
> This breaks address translation and is fixed in the provided patch
> by using the of_simple_addr_cells call, which will return correct
> address-cells for the node. This matches the requirements of the
> device-tree specification.
>
> [0] https://readthedocs.org/projects/devicetree-specification/downloads/pdf/latest/
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev at epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk at epam.com>

There's more sign offs?

> ---
>  drivers/core/of_addr.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
> index 250dd175b55..823a4d70a6b 100644
> --- a/drivers/core/of_addr.c
> +++ b/drivers/core/of_addr.c
> @@ -374,16 +374,16 @@ int of_get_dma_range(const struct device_node *dev, phys_addr_t *cpu,
>         }
>
>         /* Get the address sizes both for the bus and its parent */
> -       bus_node = of_match_bus((struct device_node*)dev);
> -       bus_node->count_cells(dev, &na, &ns);
> +       na = of_simple_addr_cells(dev);
> +       ns = of_simple_size_cells(dev);
>         if (!OF_CHECK_COUNTS(na, ns)) {
>                 printf("Bad cell count for %s\n", of_node_full_name(dev));
>                 ret = -EINVAL;
>                 goto out_parent;
>         }
>
> -       bus_node = of_match_bus(parent);
> -       bus_node->count_cells(parent, &pna, &pns);
> +       bus_node = of_match_bus((struct device_node *)dev);
> +       bus_node->count_cells(dev, &pna, &pns);
>         if (!OF_CHECK_COUNTS(pna, pns)) {
>                 printf("Bad cell count for %s\n", of_node_full_name(parent));
>                 ret = -EINVAL;
> --
> 2.51.0
>