Re: [PATCH 3/7] of/iommu: add multi-map support

[Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>]

On 2/2/2026 8:27 PM, Bryan O'Donoghue wrote:
> On 26/01/2026 12:25, Vikash Garodia wrote:
>> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>>
>> When multiple mappings are present for an input id, linux matches just
>> the first one. There is a usecase[1] where all the mappings are to be
>> maintained in parallel for an iommu-map entry of a same input id.
>>
>> Whether multi-map is needed is reported by the callers through the
>> callback function passed, which is called for every input id match.
>>
>> Since the requirement in the usecase[1] is for platform devices, not
>> sure if it is really clean to maintain this decision on the bus type at
>> the of_iommu layer or further to be from the respective
>> iommu_driver->impl_ops().
>>
>> [1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
>>
>> 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/iommu/of_iommu.c | 36 ++++++++++++++++++++++++++++--------
>>   drivers/of/base.c        | 38 ++++++++++++++++++++++++++++----------
>>   include/linux/of.h       |  6 ++++++
>>   3 files changed, 62 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 768eaddf927b0700b2497b08ea21611b1a1b5688..067bb2298973671e1eaf01bb2ea52df3d2a52a44 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/pci.h>
>>   #include <linux/slab.h>
>>   #include <linux/fsl/mc.h>
>> +#include <linux/platform_device.h>
>>     #include "iommu-priv.h"
>>   @@ -41,22 +42,41 @@ static int of_iommu_xlate(struct device *dev,
>>       return ret;
>>   }
>>   +/*
>> + * Callback to be called from of_map_id(), that tells if
>> + * all the mappings for an input id to be maintained in
>> + * parallel. Should this decission be from further layers,
>> + * iommu_driver->impl_ops?
>> + */
>> +static int of_iommu_configure_cb(struct of_map_id_arg *arg)
>> +{
>> +    struct of_phandle_args *iommu_spec = &arg->map_args;
>> +    struct device *dev = arg->dev;
>> +    int err;
>> +
>> +    err = of_iommu_xlate(dev, iommu_spec);
>> +    of_node_put(iommu_spec->np);
>> +
>> +    /* !iommu_spec->np may be from the bypassed translations */
>> +    if (!err)
>> +        err = (!arg->multi_map || !iommu_spec->np) ? 0 : -EAGAIN;
>> +
>> +    return err;
>> +}
>> +
>>   static int of_iommu_configure_dev_id(struct device_node *master_np,
>>                        struct device *dev,
>>                        const u32 *id)
>>   {
>>       struct of_map_id_arg arg = {
>>           .map_args = {},
>> +        .cb = of_iommu_configure_cb,
>> +        .dev = dev,
>> +        /* Should this be pushed to iommu_driver->impl_ops? */
>> +        .multi_map = dev_is_platform(dev),
>>       };
>> -    int err;
>> -
>> -    err = of_map_iommu_id(master_np, *id, &arg);
>> -    if (err)
>> -        return err;
>>   -    err = of_iommu_xlate(dev, &arg.map_args);
>> -    of_node_put(arg.map_args.np);
>> -    return err;
>> +    return of_map_iommu_id(master_np, *id, &arg);
>>   }
>>     static int of_iommu_configure_dev(struct device_node *master_np,
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 606bef4f90e7d13bae4f7b0c45acd1755ad89826..a1c3c5954ec7e8eb3753c8fd782a1570f9eb9c17 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2122,14 +2122,21 @@ 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)
>> +/*
>> + * Fill the id_out and target for the of_map_id() caller. Also
>> + * call the callback passed to the of_map_id() as part of the arg
>> + * that decides if to continue further search.
>> + */
>> +static int of_map_id_fill_arg(struct of_map_id_arg *arg,
>> +                  struct device_node *phandle_node, u32 id_or_offset,
>> +                  const __be32 *out_base, u32 cells,
>> +                  bool bypass, bool *multi_id_map)
>>   {
>> +    int ret;
>> +
>>       if (bypass) {
>>           arg->map_args.args[0] = id_or_offset;
>> -        return 0;
>> +        goto output;
>>       }
>>         if (arg->map_args.np)
>> @@ -2145,7 +2152,14 @@ static int of_map_id_fill_output(struct of_map_id_arg *arg,
>>         arg->map_args.args_count = cells;
>>   -    return 0;
>> +output:
>> +    /* pass the output for the callback, callers may further decide */
>> +    ret =  arg->cb ? arg->cb(arg) : 0;
>> +
>> +    if (multi_id_map && ret == -EAGAIN)
>> +        *multi_id_map = true;
>> +
>> +    return ret;
>>   }
>>     /**
>> @@ -2179,6 +2193,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>>       int map_bytes, map_len, offset = 0;
>>       bool bad_map = false;
>>       const __be32 *map = NULL;
>> +    bool multi_id_map = false;
>>         if (!np || !map_name || !arg)
>>           return -EINVAL;
>> @@ -2264,23 +2279,26 @@ 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;
>>   -        ret = of_map_id_fill_output(arg, phandle_node, id_off, out_base, cells, false);
>> +        ret = of_map_id_fill_arg(arg, phandle_node, id_off, out_base,
>> +                     cells, false, &multi_id_map);
>>           if (ret == -EAGAIN)
>>               continue;
>>             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));
>> -        return 0;
>> +        return ret;
>>       }
>>   +    if (multi_id_map)
>> +        return 0;
>> +
>>       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 */
>> -    return of_map_id_fill_output(arg, NULL, id, 0, 0, true);
>> -
>> +    return of_map_id_fill_arg(arg, NULL, id, 0, 0, true, NULL);
>>   err_map_len:
>>       pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
>>       return -EINVAL;
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 9efa6f93712c6024f05476f9fd39f3294f942ec1..abab73a76682351f5635c1127a6c899917525050 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -25,6 +25,9 @@
>>   typedef u32 phandle;
>>   typedef u32 ihandle;
>>   +struct of_map_id_arg;
>> +typedef int (*of_map_id_cb)(struct of_map_id_arg *arg);
>> +
>>   struct property {
>>       char    *name;
>>       int    length;
>> @@ -76,6 +79,9 @@ struct of_phandle_args {
>>     struct of_map_id_arg {
>>       struct of_phandle_args map_args;
>> +    of_map_id_cb cb;
>> +    struct device *dev;
>> +    bool multi_map;
>>   };
>>     struct of_phandle_iterator {
>>
> 
> I think at a minimum this and the previous patch should be separated into its/their own series ∵ you really require this to be applied before proceeding on with the rest of the submission.
> 
> Get these two patches through iommu@lists.linux.dev in isolation and then submit the driver changes to consume.
> 
> ---
> bod

Sure, I’ll split the first two patches into a seperate series and submit them.

Thanks,
Vijay