Re: [PATCH v10 2/3] of: factor arguments passed to of_map_id() into a struct

[Vijayanand Jitta <vijayanand.jitta@xxxxxxxxxxxxxxxx>]

On 3/10/2026 2:53 AM, Dmitry Baryshkov wrote:
> On Mon, Mar 09, 2026 at 01:03:38AM +0530, Vijayanand Jitta wrote:
>> From: Charan Teja Kalla <charan.kalla@xxxxxxxxxxxxxxxx>
>>
>> Change of_map_id() to take a pointer to struct of_phandle_args
>> instead of passing target device node and translated IDs separately.
>> Update all callers accordingly.
>>
>> Subsequent patch will make use of the args_count field in
>> struct of_phandle_args.
>>
>> Suggested-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
>> Signed-off-by: Charan Teja Kalla <charan.kalla@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@xxxxxxxxxxxxxxxx>
>> ---
>>  drivers/cdx/cdx_msi.c                    |  5 ++-
>>  drivers/iommu/of_iommu.c                 |  2 +-
>>  drivers/irqchip/irq-gic-its-msi-parent.c | 11 ++++--
>>  drivers/of/base.c                        | 63 
>> ++++++++++++++++----------------
>>  drivers/of/irq.c                         |  8 +++-
>>  drivers/pci/controller/dwc/pci-imx6.c    | 16 ++++++--
>>  drivers/pci/controller/pcie-apple.c      |  5 ++-
>>  drivers/xen/grant-dma-ops.c              |  4 +-
>>  include/linux/of.h                       | 12 +++---
>>  9 files changed, 76 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/cdx/cdx_msi.c b/drivers/cdx/cdx_msi.c
>> index 63b3544ec997..03232b5ffbca 100644
>> --- a/drivers/cdx/cdx_msi.c
>> +++ b/drivers/cdx/cdx_msi.c
>> @@ -121,6 +121,7 @@ static int cdx_msi_prepare(struct irq_domain *msi_domain,
>>                         struct device *dev,
>>                         int nvec, msi_alloc_info_t *info)
>>  {
>> +    struct of_phandle_args msi_spec = { .np = NULL };
> 
> Why do you need to set it? Parse functions ignore passed args, don't
> they?
> 

The parse function does check arg->np on input — if it is non-NULL,
it is used to match only entries targeting that specific node.

Also, there is this existing path in drivers/pci/msi/irqdomain.c which
pre-sets the node and passes it as input to of_msi_xlate(), so the node
pointer is not ignored by the parse code:

u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
{
        struct device_node *of_node;
        u32 rid = pci_dev_id(pdev);

        pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);

        of_node = irq_domain_get_of_node(domain);
        rid = of_node ? of_msi_xlate(&pdev->dev, &of_node, rid) :
                        iort_msi_map_id(&pdev->dev, rid);

        return rid;
}

>>      struct cdx_device *cdx_dev = to_cdx_device(dev);
>>      struct device *parent = cdx_dev->cdx->dev;
>>      struct msi_domain_info *msi_info;
>> @@ -128,11 +129,13 @@ static int cdx_msi_prepare(struct irq_domain 
>> *msi_domain,
>>      int ret;
>>  
>>      /* Retrieve device ID from requestor ID using parent device */
>> -    ret = of_map_msi_id(parent->of_node, cdx_dev->msi_dev_id, NULL, 
>> &dev_id);
>> +    ret = of_map_msi_id(parent->of_node, cdx_dev->msi_dev_id, &msi_spec);
>>      if (ret) {
>>              dev_err(dev, "of_map_id failed for MSI: %d\n", ret);
>>              return ret;
>>      }
>> +    of_node_put(msi_spec.np);
>> +    dev_id = msi_spec.args[0];
>>  
>>  #ifdef GENERIC_MSI_DOMAIN_OPS
>>      /* Set the device Id to be passed to the GIC-ITS */
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index a511ecf21fcd..d255d0f58e8c 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -48,7 +48,7 @@ static int of_iommu_configure_dev_id(struct device_node 
>> *master_np,
>>      struct of_phandle_args iommu_spec = { .args_count = 1 };
> 
> .args_count = 1 should be set by of_map_iommu_id now (and dropped here).
> 

Sure, will move it.

>>      int err;
>>  
>> -    err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
>> +    err = of_map_iommu_id(master_np, *id, &iommu_spec);
>>      if (err)
>>              return err;
>>  
>> diff --git a/drivers/irqchip/irq-gic-its-msi-parent.c 
>> b/drivers/irqchip/irq-gic-its-msi-parent.c
>> index b63343a227a9..729fa2f9f758 100644
>> --- a/drivers/irqchip/irq-gic-its-msi-parent.c
>> +++ b/drivers/irqchip/irq-gic-its-msi-parent.c
>> @@ -152,6 +152,8 @@ static int its_v5_pci_msi_prepare(struct irq_domain 
>> *domain, struct device *dev,
>>  static int of_pmsi_get_msi_info(struct irq_domain *domain, struct device 
>> *dev, u32 *dev_id,
>>                              phys_addr_t *pa)
>>  {
>> +    struct device_node *msi_ctrl __free(device_node) = NULL;
>> +    struct of_phandle_args msi_spec = { .np = NULL };
>>      struct of_phandle_iterator it;
>>      int ret;
>>  
>> @@ -178,9 +180,12 @@ static int of_pmsi_get_msi_info(struct irq_domain 
>> *domain, struct device *dev, u
>>              }
>>      }
>>  
>> -    struct device_node *msi_ctrl __free(device_node) = NULL;
>> -
>> -    return of_map_msi_id(dev->of_node, dev->id, &msi_ctrl, dev_id);
>> +    ret = of_map_msi_id(dev->of_node, dev->id, &msi_spec);
>> +    if (!ret) {
>> +            msi_ctrl = msi_spec.np;
>> +            *dev_id = msi_spec.args[0];
>> +    }
>> +    return ret;
>>  }
>>  
>>  static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 959305a84748..b6e07c5fe715 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2102,36 +2102,37 @@ int of_find_last_cache_level(unsigned int cpu)
>>   * @id: device ID to map.
>>   * @map_name: property name of the map to use.
>>   * @map_mask_name: optional property name of the mask to use.
>> - * @target: optional pointer to a target device node.
>> - * @id_out: optional pointer to receive the translated ID.
>> + * @arg: pointer to a &struct of_phandle_args. On input, @arg->np may be
>> + *  set to a target device node to match, or NULL to match any. On
>> + *  success, @arg->np will be set to the matched target node (with a
>> + *  reference held) and @arg->args[0] will contain the translated ID.
> 
> Is this part being actually used (and useful)?
> 

Please refer the above comment. I see there is existing path in
drivers/pci/msi/irqdomain.c which sets np.

Thanks,
Vijay

>>   *
>>   * Given a device ID, look up the appropriate implementation-defined
>>   * platform ID and/or the target device which receives transactions on that
>> - * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
>> - * @id_out may be NULL if only the other is required. If @target points to
>> - * a non-NULL device node pointer, only entries targeting that node will be
>> - * matched; if it points to a NULL value, it will receive the device node of
>> - * the first matching target phandle, with a reference held.
>> + * ID, as per the "iommu-map" and "msi-map" bindings. If @arg->np points to
>> + * a non-NULL device node, only entries targeting that node will be matched;
>> + * if it is NULL, it will receive the device node of the first matching
>> + * target phandle, with a reference held.
>>   *
>>   * Return: 0 on success or a standard error code on failure.
>>   */
>>  int of_map_id(const struct device_node *np, u32 id,
>>             const char *map_name, const char *map_mask_name,
>> -           struct device_node **target, u32 *id_out)
>> +           struct of_phandle_args *arg)
>>  {
>>      u32 map_mask, masked_id;
>>      int map_len;
>>      const __be32 *map = NULL;
>>  
>> -    if (!np || !map_name || (!target && !id_out))
>> +    if (!np || !map_name || !arg)
>>              return -EINVAL;
>>  
>>      map = of_get_property(np, map_name, &map_len);
>>      if (!map) {
>> -            if (target)
>> +            if (arg->np)
>>                      return -ENODEV;
>>              /* Otherwise, no map implies no translation */
>> -            *id_out = id;
>> +            arg->args[0] = id;
>>              return 0;
>>      }
>>  
>> @@ -2173,18 +2174,15 @@ int of_map_id(const struct device_node *np, u32 id,
>>              if (!phandle_node)
>>                      return -ENODEV;
>>  
>> -            if (target) {
>> -                    if (*target)
>> -                            of_node_put(phandle_node);
>> -                    else
>> -                            *target = phandle_node;
>> +            if (arg->np)
>> +                    of_node_put(phandle_node);
>> +            else
>> +                    arg->np = phandle_node;
>>  
>> -                    if (*target != phandle_node)
>> -                            continue;
>> -            }
>> +            if (arg->np != phandle_node)
>> +                    continue;
>>  
>> -            if (id_out)
>> -                    *id_out = masked_id - id_base + out_base;
>> +            arg->args[0] = masked_id - id_base + out_base;
>>  
>>              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, out_base,
>