From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
Linux interprets multiple mappings for the same input ID as a set of
equivalent choices to pick one. There exists usecases where these set
must be maintained in parallel, ex: on ARM, a dynamically created child
device(s) is referencing multiple input id's in parent iommu-map.
Factor out the code where multiple mappings needs to be maintained in
parallel can be achieved through callback from this factored out code.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
drivers/of/base.c | 47 ++++++++++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 0825f3dc93f2472e9947af09acdde72031ab85bc..606bef4f90e7d13bae4f7b0c45acd1755ad89826 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2122,6 +2122,32 @@ static bool of_check_bad_map(const __be32 *map, int len)
return true;
}
+static int of_map_id_fill_output(struct of_map_id_arg *arg,
+ struct device_node *phandle_node, u32 id_or_offset,
+ const __be32 *out_base, u32 cells,
+ bool bypass)
+{
+ if (bypass) {
+ arg->map_args.args[0] = id_or_offset;
+ return 0;
+ }
+
+ if (arg->map_args.np)
+ of_node_put(phandle_node);
+ else
+ arg->map_args.np = phandle_node;
+
+ if (arg->map_args.np != phandle_node)
+ return -EAGAIN;
+
+ for (int i = 0; i < cells; i++)
+ arg->map_args.args[i] = (id_or_offset + be32_to_cpu(out_base[i]));
+
+ arg->map_args.args_count = cells;
+
+ return 0;
+}
+
/**
* of_map_id - Translate an ID through a downstream mapping.
* @np: root complex device node.
@@ -2162,8 +2188,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
if (arg->map_args.np)
return -ENODEV;
/* Otherwise, no map implies no translation */
- arg->map_args.args[0] = id;
- return 0;
+ goto bypass_translation;
}
if (map_bytes % sizeof(*map))
@@ -2185,6 +2210,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
struct device_node *phandle_node;
u32 id_base, phandle, id_len, id_off, cells = 0;
const __be32 *out_base;
+ int ret;
if (map_len - offset < 2)
goto err_map_len;
@@ -2238,19 +2264,10 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
if (masked_id < id_base || id_off >= id_len)
continue;
- if (arg->map_args.np)
- of_node_put(phandle_node);
- else
- arg->map_args.np = phandle_node;
-
- if (arg->map_args.np != phandle_node)
+ ret = of_map_id_fill_output(arg, phandle_node, id_off, out_base, cells, false);
+ if (ret == -EAGAIN)
continue;
- for (int i = 0; i < cells; i++)
- arg->map_args.args[i] = (id_off + be32_to_cpu(out_base[i]));
-
- arg->map_args.args_count = cells;
-
pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
np, map_name, map_mask, id_base, be32_to_cpup(out_base),
id_len, id, id_off + be32_to_cpup(out_base));
@@ -2260,9 +2277,9 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
id, arg->map_args.np ? arg->map_args.np : NULL);
+bypass_translation:
/* Bypasses translation */
- arg->map_args.args[0] = id;
- return 0;
+ return of_map_id_fill_output(arg, NULL, id, 0, 0, true);
err_map_len:
pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
--
2.34.1On 26/01/2026 12:25, Vikash Garodia wrote:
> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
This commit message is confusing and inaccurate.
First up, you're not factoring _out_ of_map_id() - factor out
of_map_id() means to remove of_map_id() - you are refactoring of_map_id().
Your patch title should be something like "refactor of_map_id() to
prepare for mapping of multiple IDs to a single device"
> Linux interprets multiple mappings for the same input ID as a set of
> equivalent choices to pick one. There exists usecases where these set
> must be maintained in parallel, ex: on ARM, a dynamically created child
> device(s) is referencing multiple input id's in parent iommu-map.
>
> Factor out the code where multiple mappings needs to be maintained in
> parallel can be achieved through callback from this factored out code.
Which callback ? There is no ->function(pointer, here...); ?!
Just make some plain and straightforward statements about what you are
doing and why. There's no need to resort to dissertation-speak.
> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
> drivers/of/base.c | 47 ++++++++++++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 0825f3dc93f2472e9947af09acdde72031ab85bc..606bef4f90e7d13bae4f7b0c45acd1755ad89826 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2122,6 +2122,32 @@ static bool of_check_bad_map(const __be32 *map, int len)
> return true;
> }
>
> +static int of_map_id_fill_output(struct of_map_id_arg *arg,
> + struct device_node *phandle_node, u32 id_or_offset,
> + const __be32 *out_base, u32 cells,
> + bool bypass)
> +{
> + if (bypass) {
> + arg->map_args.args[0] = id_or_offset;
> + return 0;
> + }
> +
> + if (arg->map_args.np)
> + of_node_put(phandle_node);
> + else
> + arg->map_args.np = phandle_node;
> +
> + if (arg->map_args.np != phandle_node)
> + return -EAGAIN;
> +
> + for (int i = 0; i < cells; i++)
> + arg->map_args.args[i] = (id_or_offset + be32_to_cpu(out_base[i]));
> +
> + arg->map_args.args_count = cells;
> +
> + return 0;
> +}
> +
> /**
> * of_map_id - Translate an ID through a downstream mapping.
> * @np: root complex device node.
> @@ -2162,8 +2188,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> if (arg->map_args.np)
> return -ENODEV;
> /* Otherwise, no map implies no translation */
> - arg->map_args.args[0] = id;
> - return 0;
> + goto bypass_translation;
> }
>
> if (map_bytes % sizeof(*map))
> @@ -2185,6 +2210,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> struct device_node *phandle_node;
> u32 id_base, phandle, id_len, id_off, cells = 0;
> const __be32 *out_base;
> + int ret;
>
> if (map_len - offset < 2)
> goto err_map_len;
> @@ -2238,19 +2264,10 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> if (masked_id < id_base || id_off >= id_len)
> continue;
>
> - if (arg->map_args.np)
> - of_node_put(phandle_node);
> - else
> - arg->map_args.np = phandle_node;
> -
> - if (arg->map_args.np != phandle_node)
> + ret = of_map_id_fill_output(arg, phandle_node, id_off, out_base, cells, false);
> + if (ret == -EAGAIN)
> continue;
>
> - for (int i = 0; i < cells; i++)
> - arg->map_args.args[i] = (id_off + be32_to_cpu(out_base[i]));
> -
> - arg->map_args.args_count = cells;
> -
> pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
> np, map_name, map_mask, id_base, be32_to_cpup(out_base),
> id_len, id, id_off + be32_to_cpup(out_base));
> @@ -2260,9 +2277,9 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
> id, arg->map_args.np ? arg->map_args.np : NULL);
>
> +bypass_translation:
> /* Bypasses translation */
> - arg->map_args.args[0] = id;
> - return 0;
> + return of_map_id_fill_output(arg, NULL, id, 0, 0, true);
>
> err_map_len:
> pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
>
On 2/2/2026 8:22 PM, Bryan O'Donoghue wrote:
> On 26/01/2026 12:25, Vikash Garodia wrote:
>> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>
> This commit message is confusing and inaccurate.
>
> First up, you're not factoring _out_ of_map_id() - factor out of_map_id() means to remove of_map_id() - you are refactoring of_map_id().
>
> Your patch title should be something like "refactor of_map_id() to prepare for mapping of multiple IDs to a single device"
>
Sure, will update the commit.
>> Linux interprets multiple mappings for the same input ID as a set of
>> equivalent choices to pick one. There exists usecases where these set
>> must be maintained in parallel, ex: on ARM, a dynamically created child
>> device(s) is referencing multiple input id's in parent iommu-map.
>>
>> Factor out the code where multiple mappings needs to be maintained in
>> parallel can be achieved through callback from this factored out code.
>
> Which callback ? There is no ->function(pointer, here...); ?!
>
> Just make some plain and straightforward statements about what you are doing and why. There's no need to resort to dissertation-speak.
>
The callback in introduced in patch 2 of this series. will update the commit descripition as suggested.
Thanks,
Vijay
>> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>> ---
>> drivers/of/base.c | 47 ++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 32 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 0825f3dc93f2472e9947af09acdde72031ab85bc..606bef4f90e7d13bae4f7b0c45acd1755ad89826 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2122,6 +2122,32 @@ static bool of_check_bad_map(const __be32 *map, int len)
>> return true;
>> }
>> +static int of_map_id_fill_output(struct of_map_id_arg *arg,
>> + struct device_node *phandle_node, u32 id_or_offset,
>> + const __be32 *out_base, u32 cells,
>> + bool bypass)
>> +{
>> + if (bypass) {
>> + arg->map_args.args[0] = id_or_offset;
>> + return 0;
>> + }
>> +
>> + if (arg->map_args.np)
>> + of_node_put(phandle_node);
>> + else
>> + arg->map_args.np = phandle_node;
>> +
>> + if (arg->map_args.np != phandle_node)
>> + return -EAGAIN;
>> +
>> + for (int i = 0; i < cells; i++)
>> + arg->map_args.args[i] = (id_or_offset + be32_to_cpu(out_base[i]));
>> +
>> + arg->map_args.args_count = cells;
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * of_map_id - Translate an ID through a downstream mapping.
>> * @np: root complex device node.
>> @@ -2162,8 +2188,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> if (arg->map_args.np)
>> return -ENODEV;
>> /* Otherwise, no map implies no translation */
>> - arg->map_args.args[0] = id;
>> - return 0;
>> + goto bypass_translation;
>> }
>> if (map_bytes % sizeof(*map))
>> @@ -2185,6 +2210,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> struct device_node *phandle_node;
>> u32 id_base, phandle, id_len, id_off, cells = 0;
>> const __be32 *out_base;
>> + int ret;
>> if (map_len - offset < 2)
>> goto err_map_len;
>> @@ -2238,19 +2264,10 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> if (masked_id < id_base || id_off >= id_len)
>> continue;
>> - if (arg->map_args.np)
>> - of_node_put(phandle_node);
>> - else
>> - arg->map_args.np = phandle_node;
>> -
>> - if (arg->map_args.np != phandle_node)
>> + ret = of_map_id_fill_output(arg, phandle_node, id_off, out_base, cells, false);
>> + if (ret == -EAGAIN)
>> continue;
>> - for (int i = 0; i < cells; i++)
>> - arg->map_args.args[i] = (id_off + be32_to_cpu(out_base[i]));
>> -
>> - arg->map_args.args_count = cells;
>> -
>> pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
>> np, map_name, map_mask, id_base, be32_to_cpup(out_base),
>> id_len, id, id_off + be32_to_cpup(out_base));
>> @@ -2260,9 +2277,9 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
>> id, arg->map_args.np ? arg->map_args.np : NULL);
>> +bypass_translation:
>> /* Bypasses translation */
>> - arg->map_args.args[0] = id;
>> - return 0;
>> + return of_map_id_fill_output(arg, NULL, id, 0, 0, true);
>> err_map_len:
>> pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
>>
>
On Tue, Feb 03, 2026 at 03:43:58PM +0530, Vijayanand Jitta wrote: > > > On 2/2/2026 8:22 PM, Bryan O'Donoghue wrote: > > On 26/01/2026 12:25, Vikash Garodia wrote: > >> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com> > > > > This commit message is confusing and inaccurate. > > > > First up, you're not factoring _out_ of_map_id() - factor out of_map_id() means to remove of_map_id() - you are refactoring of_map_id(). > > > > Your patch title should be something like "refactor of_map_id() to prepare for mapping of multiple IDs to a single device" > > > > Sure, will update the commit. > > >> Linux interprets multiple mappings for the same input ID as a set of > >> equivalent choices to pick one. There exists usecases where these set > >> must be maintained in parallel, ex: on ARM, a dynamically created child > >> device(s) is referencing multiple input id's in parent iommu-map. > >> > >> Factor out the code where multiple mappings needs to be maintained in > >> parallel can be achieved through callback from this factored out code. > > > > Which callback ? There is no ->function(pointer, here...); ?! > > > > Just make some plain and straightforward statements about what you are doing and why. There's no need to resort to dissertation-speak. > > > > The callback in introduced in patch 2 of this series. will update the commit descripition as suggested. I think, the callback was NAKed already. -- With best wishes Dmitry
On 2/4/2026 6:41 AM, Dmitry Baryshkov wrote: > On Tue, Feb 03, 2026 at 03:43:58PM +0530, Vijayanand Jitta wrote: >> >> >> On 2/2/2026 8:22 PM, Bryan O'Donoghue wrote: >>> On 26/01/2026 12:25, Vikash Garodia wrote: >>>> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com> >>> >>> This commit message is confusing and inaccurate. >>> >>> First up, you're not factoring _out_ of_map_id() - factor out of_map_id() means to remove of_map_id() - you are refactoring of_map_id(). >>> >>> Your patch title should be something like "refactor of_map_id() to prepare for mapping of multiple IDs to a single device" >>> >> >> Sure, will update the commit. >> >>>> Linux interprets multiple mappings for the same input ID as a set of >>>> equivalent choices to pick one. There exists usecases where these set >>>> must be maintained in parallel, ex: on ARM, a dynamically created child >>>> device(s) is referencing multiple input id's in parent iommu-map. >>>> >>>> Factor out the code where multiple mappings needs to be maintained in >>>> parallel can be achieved through callback from this factored out code. >>> >>> Which callback ? There is no ->function(pointer, here...); ?! >>> >>> Just make some plain and straightforward statements about what you are doing and why. There's no need to resort to dissertation-speak. >>> >> >> The callback in introduced in patch 2 of this series. will update the commit descripition as suggested. > > I think, the callback was NAKed already. > > I'll remove the callback and update change such that all entries of iommu-map are always scanned. This would handle the video usecase ( i.e; same input id's mapping to different SIDs ) and in other cases it would result in few additional scans in iommu-map compared to existing implementation (where it just returns after first input id match) , does this look fine ? Thanks, Vijay
On Thu, Feb 05, 2026 at 01:39:54PM +0530, Vijayanand Jitta wrote: > > > On 2/4/2026 6:41 AM, Dmitry Baryshkov wrote: > > On Tue, Feb 03, 2026 at 03:43:58PM +0530, Vijayanand Jitta wrote: > >> > >> > >> On 2/2/2026 8:22 PM, Bryan O'Donoghue wrote: > >>> On 26/01/2026 12:25, Vikash Garodia wrote: > >>>> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com> > >>> > >>> This commit message is confusing and inaccurate. > >>> > >>> First up, you're not factoring _out_ of_map_id() - factor out of_map_id() means to remove of_map_id() - you are refactoring of_map_id(). > >>> > >>> Your patch title should be something like "refactor of_map_id() to prepare for mapping of multiple IDs to a single device" > >>> > >> > >> Sure, will update the commit. > >> > >>>> Linux interprets multiple mappings for the same input ID as a set of > >>>> equivalent choices to pick one. There exists usecases where these set > >>>> must be maintained in parallel, ex: on ARM, a dynamically created child > >>>> device(s) is referencing multiple input id's in parent iommu-map. > >>>> > >>>> Factor out the code where multiple mappings needs to be maintained in > >>>> parallel can be achieved through callback from this factored out code. > >>> > >>> Which callback ? There is no ->function(pointer, here...); ?! > >>> > >>> Just make some plain and straightforward statements about what you are doing and why. There's no need to resort to dissertation-speak. > >>> > >> > >> The callback in introduced in patch 2 of this series. will update the commit descripition as suggested. > > > > I think, the callback was NAKed already. > > > > > > I'll remove the callback and update change such that all entries of iommu-map are always scanned. > This would handle the video usecase ( i.e; same input id's mapping to different SIDs ) and in other > cases it would result in few additional scans in iommu-map compared to existing implementation (where > it just returns after first input id match) , does this look fine ? This probably means that we can also drop of_map_args. -- With best wishes Dmitry
© 2016 - 2026 Red Hat, Inc.