| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7d1b5897-8f17-4213-b893-d99d96afef94@oss.qualcomm.com>
Date: Tue, 3 Feb 2026 15:43:58 +0530
From: Vijayanand Jitta <vijayanand.jitta@....qualcomm.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Vikash Garodia <vikash.garodia@....qualcomm.com>,
Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Bryan O'Donoghue <bod@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Saravana Kannan <saravanak@...nel.org>, Joerg Roedel <joro@...tes.org>,
Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
Stefan Schmidt <stefan.schmidt@...aro.org>,
Hans Verkuil <hverkuil@...nel.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Vishnu Reddy <busanna.reddy@....qualcomm.com>,
Hans Verkuil <hverkuil+cisco@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
iommu@...ts.linux.dev,
Charan Teja Kalla <charan.kalla@....qualcomm.com>
Subject: Re: [PATCH 2/7] of: factor out of_map_id() code
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@....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@....qualcomm.com>
>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@....qualcomm.com>
>> Signed-off-by: Vikash Garodia <vikash.garodia@....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);
>>
>
Powered by blists - more mailing lists