[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44010837-65c6-437e-aede-7b45f69b6758@arm.com>
Date: Wed, 5 Nov 2025 17:35:03 +0000
From: Robin Murphy <robin.murphy@....com>
To: Charan Teja Kalla <charan.kalla@....qualcomm.com>, will@...nel.org,
joro@...tes.org, robh@...nel.org, dmitry.baryshkov@....qualcomm.com,
konrad.dybcio@....qualcomm.com, bjorn.andersson@....qualcomm.com,
bod@...nel.org, conor+dt@...nel.org, krzk+dt@...nel.org,
saravanak@...gle.com, prakash.gupta@....qualcomm.com,
vikash.garodia@....qualcomm.com
Cc: iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH 2/6] of: introduce wrapper function to query the cell
count
On 2025-11-04 8:51 am, Charan Teja Kalla wrote:
> Introduce the wrapper function, of_map_id_cell_count(), to query the
> actual cell count that need to be considered by the of_map_id() when
> used for translating the <property>-map entries. Accordingly make sure
> of_map_id_or_funcid() operates on this returned cell count.
>
> This wrapper function returns cell count as 1 thus no functional
> changes.
>
> The subsequent patches improve the logic in wrapper function to detect
> the proper cell count.
>
> Signed-off-by: Charan Teja Kalla <charan.kalla@....qualcomm.com>
> ---
> drivers/of/base.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ac6b726cd5e3..5e76abcc7940 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2045,6 +2045,12 @@ int of_find_last_cache_level(unsigned int cpu)
> return cache_level;
> }
>
> +static int of_map_id_cell_count(const __be32 *map, const char *map_name,
> + int map_len)
> +{
> + return 1;
> +}
> +
> /*
> * Look at the documentation of of_map_id.
> */
> @@ -2053,6 +2059,7 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> struct device_node **target, u32 *id_out)
> {
> u32 map_mask, masked_id;
> + u32 cell_count, total_cells;
> int map_len;
> const __be32 *map = NULL;
>
> @@ -2068,7 +2075,13 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> return 0;
> }
>
> - if (!map_len || map_len % (4 * sizeof(*map))) {
> + cell_count = of_map_id_cell_count(map, map_name, map_len);
This doesn't work. The output cell count for any given entry depends on
whatever phandle *that* entry maps to - it can't be assumed to be
constant for the whole map.
Thanks,
Robin.
> + if (!cell_count)
> + return -EINVAL;
> +
> + total_cells = 2 + cell_count + 1;
> +
> + if (!map_len || map_len % (total_cells * sizeof(*map))) {
> pr_err("%pOF: Error: Bad %s length: %d\n", np,
> map_name, map_len);
> return -EINVAL;
> @@ -2085,12 +2098,12 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> of_property_read_u32(np, map_mask_name, &map_mask);
>
> masked_id = map_mask & id;
> - for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> + for ( ; map_len > 0; map_len -= total_cells * sizeof(*map), map += total_cells) {
> struct device_node *phandle_node;
> u32 id_base = be32_to_cpup(map + 0);
> u32 phandle = be32_to_cpup(map + 1);
> u32 out_base = be32_to_cpup(map + 2);
> - u32 id_len = be32_to_cpup(map + 3);
> + u32 id_len = be32_to_cpup(map + total_cells - 1);
>
> if (id_base & ~map_mask) {
> pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
Powered by blists - more mailing lists