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

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>]

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?

>       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).

>       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)?

>   *
>   * 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,

-- 
With best wishes
Dmitry