[PATCH v2 0/3] of: parsing of multi #{iommu,msi}-cells in maps

[PATCH v2 0/3] of: parsing of multi #{iommu,msi}-cells in maps

[Vijayanand Jitta]

So far our parsing of {iommu,msi}-map properites has always blindly
assumed that the output specifiers will always have exactly 1 cell.
This typically does happen to be the case, but is not actually enforced
(and the PCI msi-map binding even explicitly states support for 0 or 1
cells) - as a result we've now ended up with dodgy DTs out in the field
which depend on this behaviour to map a 1-cell specifier for a 2-cell
provider, despite that being bogus per the bindings themselves.

Since there is some potential use[1] in being able to map at least
single input IDs to multi-cell output specifiers (and properly support
0-cell outputs as well), add support for properly parsing and using the
target nodes' #cells values, albeit with the unfortunate complication of
still having to work around expectations of the old behaviour too.
							-- Robin.

Unlike single #{}-cell, it is complex to establish a linear relation
between input 'id' and output specifier for multi-cell properties, thus
it is always expected that len never going to be > 1. 

The motivation for this patchset originates from the multi-map use case
described in V1, which will be addressed in a subsequent series once the
current fixes for iommu-cell handling are concluded.

These changes have been tested on QEMU for the arm64 architecture. We
plan to perform more extensive testing based on community feedback and
are sending this series out early to gather review comments.

[1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/

V2:
  1) Incorporated the patches from Robin that does the clean implementation.
  2) Dropped the patches the were adding multi-map support from this series
  as suggested.

V1:
 https://lore.kernel.org/all/cover.1762235099.git.charan.kalla@oss.qualcomm.com/

RFC:
 https://lore.kernel.org/all/20250928171718.436440-1-charan.kalla@oss.qualcomm.com/#r

Charan Teja Kalla (1):
  of: factor arguments passed to of_map_id() into a struct

Robin Murphy (2):
  of: Add convenience wrappers for of_map_id()
  of: Respect #{iommu,msi}-cells in maps

 drivers/cdx/cdx_msi.c                    |   3 +-
 drivers/iommu/of_iommu.c                 |  11 +-
 drivers/irqchip/irq-gic-its-msi-parent.c |   4 +-
 drivers/of/base.c                        | 132 ++++++++++++++++-------
 drivers/of/irq.c                         |   3 +-
 drivers/pci/controller/dwc/pci-imx6.c    |  10 +-
 drivers/pci/controller/pcie-apple.c      |   6 +-
 drivers/xen/grant-dma-ops.c              |   6 +-
 include/linux/of.h                       |  36 +++++--
 9 files changed, 151 insertions(+), 60 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] of: Add convenience wrappers for of_map_id()

[Vijayanand Jitta]

From: Robin Murphy <robin.murphy@arm.com>

Since we now have quite a few users parsing "iommu-map" and "msi-map"
properties, give them some wrappers to conveniently encapsulate the
appropriate sets of property names. This will also make it easier to
then change of_map_id() to correctly account for specifier cells.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
 drivers/cdx/cdx_msi.c                    |  3 +--
 drivers/iommu/of_iommu.c                 |  4 +---
 drivers/irqchip/irq-gic-its-msi-parent.c |  4 ++--
 drivers/of/irq.c                         |  3 +--
 drivers/pci/controller/dwc/pci-imx6.c    |  6 ++----
 drivers/pci/controller/pcie-apple.c      |  3 +--
 drivers/xen/grant-dma-ops.c              |  3 +--
 include/linux/of.h                       | 14 ++++++++++++++
 8 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/cdx/cdx_msi.c b/drivers/cdx/cdx_msi.c
index 91b95422b263..63b3544ec997 100644
--- a/drivers/cdx/cdx_msi.c
+++ b/drivers/cdx/cdx_msi.c
@@ -128,8 +128,7 @@ static int cdx_msi_prepare(struct irq_domain *msi_domain,
 	int ret;
 
 	/* Retrieve device ID from requestor ID using parent device */
-	ret = of_map_id(parent->of_node, cdx_dev->msi_dev_id, "msi-map", "msi-map-mask",
-			NULL, &dev_id);
+	ret = of_map_msi_id(parent->of_node, cdx_dev->msi_dev_id, NULL, &dev_id);
 	if (ret) {
 		dev_err(dev, "of_map_id failed for MSI: %d\n", ret);
 		return ret;
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 6b989a62def2..a511ecf21fcd 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -48,9 +48,7 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
 	struct of_phandle_args iommu_spec = { .args_count = 1 };
 	int err;
 
-	err = of_map_id(master_np, *id, "iommu-map",
-			 "iommu-map-mask", &iommu_spec.np,
-			 iommu_spec.args);
+	err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
 	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 eb1473f1448a..0884c4cbd245 100644
--- a/drivers/irqchip/irq-gic-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-its-msi-parent.c
@@ -166,7 +166,7 @@ static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev,
 	if (ret) {
 		struct device_node *np = NULL;
 
-		ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id);
+		ret = of_map_msi_id(dev->of_node, dev->id, &np, dev_id);
 		if (np)
 			of_node_put(np);
 	}
@@ -211,7 +211,7 @@ static int of_v5_pmsi_get_msi_info(struct irq_domain *domain, struct device *dev
 	if (ret) {
 		struct device_node *np = NULL;
 
-		ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id);
+		ret = of_map_msi_id(dev->of_node, dev->id, &np, dev_id);
 		if (np) {
 			ret = its_translate_frame_address(np, pa);
 			of_node_put(np);
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 1cd93549d093..9549dda8f9d6 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -725,8 +725,7 @@ u32 of_msi_xlate(struct device *dev, struct device_node **msi_np, u32 id_in)
 	 * "msi-map" or an "msi-parent" property.
 	 */
 	for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
-		if (!of_map_id(parent_dev->of_node, id_in, "msi-map",
-				"msi-map-mask", msi_np, &id_out))
+		if (!of_map_msi_id(parent_dev->of_node, id_in, msi_np, &id_out))
 			break;
 		if (!of_check_msi_parent(parent_dev->of_node, msi_np))
 			break;
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 4668fc9648bf..c8da2e88e9c6 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1106,8 +1106,7 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
 	u32 sid = 0;
 
 	target = NULL;
-	err_i = of_map_id(dev->of_node, rid, "iommu-map", "iommu-map-mask",
-			  &target, &sid_i);
+	err_i = of_map_iommu_id(dev->of_node, rid, &target, &sid_i);
 	if (target) {
 		of_node_put(target);
 	} else {
@@ -1120,8 +1119,7 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
 	}
 
 	target = NULL;
-	err_m = of_map_id(dev->of_node, rid, "msi-map", "msi-map-mask",
-			  &target, &sid_m);
+	err_m = of_map_msi_id(dev->of_node, rid, &target, &sid_m);
 
 	/*
 	 *   err_m      target
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 0380d300adca..ce21728d6e51 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -791,8 +791,7 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d
 	dev_dbg(&pdev->dev, "added to bus %s, index %d\n",
 		pci_name(pdev->bus->self), port->idx);
 
-	err = of_map_id(port->pcie->dev->of_node, rid, "iommu-map",
-			"iommu-map-mask", NULL, &sid);
+	err = of_map_iommu_id(port->pcie->dev->of_node, rid, NULL, &sid);
 	if (err)
 		return err;
 
diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index 29257d2639db..b661f9c1f4fe 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -321,8 +321,7 @@ static int xen_dt_grant_init_backend_domid(struct device *dev,
 		struct pci_dev *pdev = to_pci_dev(dev);
 		u32 rid = PCI_DEVID(pdev->bus->number, pdev->devfn);
 
-		if (of_map_id(np, rid, "iommu-map", "iommu-map-mask", &iommu_spec.np,
-				iommu_spec.args)) {
+		if (of_map_iommu_id(np, rid, &iommu_spec.np, iommu_spec.args)) {
 			dev_dbg(dev, "Cannot translate ID\n");
 			return -ESRCH;
 		}
diff --git a/include/linux/of.h b/include/linux/of.h
index 121a288ca92d..8cd486d89da2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1435,6 +1435,20 @@ static inline int of_property_read_s32(const struct device_node *np,
 	return of_property_read_u32(np, propname, (u32*) out_value);
 }
 
+static inline int of_map_iommu_id(const struct device_node *np, u32 id,
+				  struct device_node **target, u32 *id_out)
+{
+	return of_map_id(np, id, "iommu-map", "iommu-map-mask",
+			 target, id_out);
+}
+
+static inline int of_map_msi_id(const struct device_node *np, u32 id,
+				struct device_node **target, u32 *id_out)
+{
+	return of_map_id(np, id, "msi-map", "msi-map-mask",
+			 target, id_out);
+}
+
 #define of_for_each_phandle(it, err, np, ln, cn, cc)			\
 	for (of_phandle_iterator_init((it), (np), (ln), (cn), (cc)),	\
 	     err = of_phandle_iterator_next(it);			\
-- 
2.34.1

Re: [PATCH v2 1/3] of: Add convenience wrappers for of_map_id()

[Rob Herring (Arm)]

On Thu, 04 Dec 2025 15:25:28 +0530, Vijayanand Jitta wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> 
> Since we now have quite a few users parsing "iommu-map" and "msi-map"
> properties, give them some wrappers to conveniently encapsulate the
> appropriate sets of property names. This will also make it easier to
> then change of_map_id() to correctly account for specifier cells.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> ---
>  drivers/cdx/cdx_msi.c                    |  3 +--
>  drivers/iommu/of_iommu.c                 |  4 +---
>  drivers/irqchip/irq-gic-its-msi-parent.c |  4 ++--
>  drivers/of/irq.c                         |  3 +--
>  drivers/pci/controller/dwc/pci-imx6.c    |  6 ++----
>  drivers/pci/controller/pcie-apple.c      |  3 +--
>  drivers/xen/grant-dma-ops.c              |  3 +--
>  include/linux/of.h                       | 14 ++++++++++++++
>  8 files changed, 23 insertions(+), 17 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

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

[Vijayanand Jitta]

From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>

Introduce a new struct type where the optional arguments passed to
of_map_id() are in struct. Subsequent patches add additional arguments
to the struct that the caller expects to be filled of_map_id().

Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
 drivers/iommu/of_iommu.c              |  6 +++++-
 drivers/of/base.c                     | 31 ++++++++++++++-------------
 drivers/pci/controller/dwc/pci-imx6.c |  6 +++++-
 drivers/pci/controller/pcie-apple.c   |  5 ++++-
 drivers/xen/grant-dma-ops.c           |  5 ++++-
 include/linux/of.h                    | 23 ++++++++++++++------
 6 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a511ecf21fcd..eac62bc441c5 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -46,9 +46,13 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
 				     const u32 *id)
 {
 	struct of_phandle_args iommu_spec = { .args_count = 1 };
+	struct of_map_id_arg arg = {
+		.target = &iommu_spec.np,
+		.id_out = iommu_spec.args,
+	};
 	int err;
 
-	err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
+	err = of_map_iommu_id(master_np, *id, &arg);
 	if (err)
 		return err;
 
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7043acd971a0..b8f78a9e6a09 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2051,8 +2051,9 @@ 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: contains the optional params, @target which is a pointer
+ *	to the target device node and id_out which is a pointer
+ *	to receive the translated ID.
  *
  * Given a device ID, look up the appropriate implementation-defined
  * platform ID and/or the target device which receives transactions on that
@@ -2066,21 +2067,21 @@ int of_find_last_cache_level(unsigned int cpu)
  */
 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_map_id_arg *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 || (!arg->target && !arg->id_out))
 		return -EINVAL;
 
 	map = of_get_property(np, map_name, &map_len);
 	if (!map) {
-		if (target)
+		if (arg->target)
 			return -ENODEV;
 		/* Otherwise, no map implies no translation */
-		*id_out = id;
+		*arg->id_out = id;
 		return 0;
 	}
 
@@ -2122,18 +2123,18 @@ int of_map_id(const struct device_node *np, u32 id,
 		if (!phandle_node)
 			return -ENODEV;
 
-		if (target) {
-			if (*target)
+		if (arg->target) {
+			if (*arg->target)
 				of_node_put(phandle_node);
 			else
-				*target = phandle_node;
+				*arg->target = phandle_node;
 
-			if (*target != phandle_node)
+			if (*arg->target != phandle_node)
 				continue;
 		}
 
-		if (id_out)
-			*id_out = masked_id - id_base + out_base;
+		if (arg->id_out)
+			*arg->id_out = 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,
@@ -2142,11 +2143,11 @@ int of_map_id(const struct device_node *np, u32 id,
 	}
 
 	pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
-		id, target && *target ? *target : NULL);
+		id, arg->target && *arg->target ? *arg->target : NULL);
 
 	/* Bypasses translation */
-	if (id_out)
-		*id_out = id;
+	if (arg->id_out)
+		*arg->id_out = id;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_map_id);
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index c8da2e88e9c6..7b54295e553b 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1101,12 +1101,16 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
 {
 	struct device *dev = imx_pcie->pci->dev;
 	struct device_node *target;
+	struct of_map_id_arg arg = {};
 	u32 sid_i, sid_m;
 	int err_i, err_m;
 	u32 sid = 0;
 
 	target = NULL;
-	err_i = of_map_iommu_id(dev->of_node, rid, &target, &sid_i);
+
+	arg.target = &target;
+	arg.id_out = &sid_i;
+	err_i = of_map_iommu_id(dev->of_node, rid, &arg);
 	if (target) {
 		of_node_put(target);
 	} else {
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index ce21728d6e51..965f65ce8ad3 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -782,6 +782,7 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d
 {
 	u32 sid, rid = pci_dev_id(pdev);
 	struct apple_pcie_port *port;
+	struct of_map_id_arg arg = {};
 	int idx, err;
 
 	port = apple_pcie_get_port(pdev);
@@ -791,7 +792,9 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d
 	dev_dbg(&pdev->dev, "added to bus %s, index %d\n",
 		pci_name(pdev->bus->self), port->idx);
 
-	err = of_map_iommu_id(port->pcie->dev->of_node, rid, NULL, &sid);
+	arg.target = NULL;
+	arg.id_out = &sid;
+	err = of_map_iommu_id(port->pcie->dev->of_node, rid, &arg);
 	if (err)
 		return err;
 
diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index b661f9c1f4fe..179f6f43a57b 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -319,9 +319,12 @@ static int xen_dt_grant_init_backend_domid(struct device *dev,
 
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
+		struct of_map_id_arg arg = {};
 		u32 rid = PCI_DEVID(pdev->bus->number, pdev->devfn);
 
-		if (of_map_iommu_id(np, rid, &iommu_spec.np, iommu_spec.args)) {
+		arg.target = &iommu_spec.np;
+		arg.id_out = iommu_spec.args;
+		if (of_map_iommu_id(np, rid, &arg)) {
 			dev_dbg(dev, "Cannot translate ID\n");
 			return -ESRCH;
 		}
diff --git a/include/linux/of.h b/include/linux/of.h
index 8cd486d89da2..21bdce2b37ca 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -25,6 +25,12 @@
 typedef u32 phandle;
 typedef u32 ihandle;
 
+struct device_node;
+struct of_map_id_arg {
+	struct device_node **target;
+	u32 *id_out;
+};
+
 struct property {
 	char	*name;
 	int	length;
@@ -458,7 +464,7 @@ bool of_console_check(const struct device_node *dn, char *name, int index);
 
 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_map_id_arg *arg);
 
 phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
 
@@ -907,7 +913,7 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
 
 static inline 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_map_id_arg *arg)
 {
 	return -EINVAL;
 }
@@ -1436,17 +1442,20 @@ static inline int of_property_read_s32(const struct device_node *np,
 }
 
 static inline int of_map_iommu_id(const struct device_node *np, u32 id,
-				  struct device_node **target, u32 *id_out)
+				  struct of_map_id_arg *arg)
 {
-	return of_map_id(np, id, "iommu-map", "iommu-map-mask",
-			 target, id_out);
+	return of_map_id(np, id, "iommu-map", "iommu-map-mask", arg);
 }
 
 static inline int of_map_msi_id(const struct device_node *np, u32 id,
 				struct device_node **target, u32 *id_out)
 {
-	return of_map_id(np, id, "msi-map", "msi-map-mask",
-			 target, id_out);
+	struct of_map_id_arg arg = {
+		.target = target,
+		.id_out = id_out,
+	};
+
+	return of_map_id(np, id, "msi-map", "msi-map-mask", &arg);
 }
 
 #define of_for_each_phandle(it, err, np, ln, cn, cc)			\
-- 
2.34.1

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

[Dmitry Baryshkov]

On 04/12/2025 11:55, Vijayanand Jitta wrote:
> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> 
> Introduce a new struct type where the optional arguments passed to
> of_map_id() are in struct. Subsequent patches add additional arguments
> to the struct that the caller expects to be filled of_map_id().

Why do we need a separate struct? Can't we use of_phandle_args instead?

> 
> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> ---
>   drivers/iommu/of_iommu.c              |  6 +++++-
>   drivers/of/base.c                     | 31 ++++++++++++++-------------
>   drivers/pci/controller/dwc/pci-imx6.c |  6 +++++-
>   drivers/pci/controller/pcie-apple.c   |  5 ++++-
>   drivers/xen/grant-dma-ops.c           |  5 ++++-
>   include/linux/of.h                    | 23 ++++++++++++++------
>   6 files changed, 50 insertions(+), 26 deletions(-)
> 
-- 
With best wishes
Dmitry

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

[Vijayanand Jitta]

On 12/5/2025 10:17 PM, Dmitry Baryshkov wrote:
> On 04/12/2025 11:55, Vijayanand Jitta wrote:
>> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>>
>> Introduce a new struct type where the optional arguments passed to
>> of_map_id() are in struct. Subsequent patches add additional arguments
>> to the struct that the caller expects to be filled of_map_id().
> 
> Why do we need a separate struct? Can't we use of_phandle_args instead?
> 

The idea is to use it for multi-map support mentioned in the cover letter,
where we need to pass additional context (callback, dev etc..). Having a
seperate structure here makes of_map_id() interface easier to extend.

>>
>> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
>> ---
>>   drivers/iommu/of_iommu.c              |  6 +++++-
>>   drivers/of/base.c                     | 31 ++++++++++++++-------------
>>   drivers/pci/controller/dwc/pci-imx6.c |  6 +++++-
>>   drivers/pci/controller/pcie-apple.c   |  5 ++++-
>>   drivers/xen/grant-dma-ops.c           |  5 ++++-
>>   include/linux/of.h                    | 23 ++++++++++++++------
>>   6 files changed, 50 insertions(+), 26 deletions(-)
>>

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

[Rob Herring]

On Mon, Dec 08, 2025 at 07:10:32PM +0530, Vijayanand Jitta wrote:
> 
> 
> On 12/5/2025 10:17 PM, Dmitry Baryshkov wrote:
> > On 04/12/2025 11:55, Vijayanand Jitta wrote:
> >> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> >>
> >> Introduce a new struct type where the optional arguments passed to
> >> of_map_id() are in struct. Subsequent patches add additional arguments
> >> to the struct that the caller expects to be filled of_map_id().
> > 
> > Why do we need a separate struct? Can't we use of_phandle_args instead?
> > 
> 
> The idea is to use it for multi-map support mentioned in the cover letter,
> where we need to pass additional context (callback, dev etc..). Having a
> seperate structure here makes of_map_id() interface easier to extend.

Then perhaps?:

struct of_map_id_args {
	struct of_phandle_args args;
        ...
}

> 
> >>
> >> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> >> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> >> ---
> >>   drivers/iommu/of_iommu.c              |  6 +++++-
> >>   drivers/of/base.c                     | 31 ++++++++++++++-------------
> >>   drivers/pci/controller/dwc/pci-imx6.c |  6 +++++-
> >>   drivers/pci/controller/pcie-apple.c   |  5 ++++-
> >>   drivers/xen/grant-dma-ops.c           |  5 ++++-
> >>   include/linux/of.h                    | 23 ++++++++++++++------
> >>   6 files changed, 50 insertions(+), 26 deletions(-)
> >>
> 

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

[Vijayanand Jitta]

On 12/10/2025 1:32 AM, Rob Herring wrote:
> On Mon, Dec 08, 2025 at 07:10:32PM +0530, Vijayanand Jitta wrote:
>>
>>
>> On 12/5/2025 10:17 PM, Dmitry Baryshkov wrote:
>>> On 04/12/2025 11:55, Vijayanand Jitta wrote:
>>>> From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>>>>
>>>> Introduce a new struct type where the optional arguments passed to
>>>> of_map_id() are in struct. Subsequent patches add additional arguments
>>>> to the struct that the caller expects to be filled of_map_id().
>>>
>>> Why do we need a separate struct? Can't we use of_phandle_args instead?
>>>
>>
>> The idea is to use it for multi-map support mentioned in the cover letter,
>> where we need to pass additional context (callback, dev etc..). Having a
>> seperate structure here makes of_map_id() interface easier to extend.
> 
> Then perhaps?:
> 
> struct of_map_id_args {
> 	struct of_phandle_args args;
>         ...
> }
> 

Thanks, this will work. Will update it in next revision.
>>
>>>>
>>>> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>>>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
>>>> ---
>>>>   drivers/iommu/of_iommu.c              |  6 +++++-
>>>>   drivers/of/base.c                     | 31 ++++++++++++++-------------
>>>>   drivers/pci/controller/dwc/pci-imx6.c |  6 +++++-
>>>>   drivers/pci/controller/pcie-apple.c   |  5 ++++-
>>>>   drivers/xen/grant-dma-ops.c           |  5 ++++-
>>>>   include/linux/of.h                    | 23 ++++++++++++++------
>>>>   6 files changed, 50 insertions(+), 26 deletions(-)
>>>>
>>

[PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[Vijayanand Jitta]

From: Robin Murphy <robin.murphy@arm.com>

So far our parsing of {iommu,msi}-map properites has always blindly
asusmed that the output specifiers will always have exactly 1 cell.
This typically does happen to be the case, but is not actually enforced
(and the PCI msi-map binding even explicitly states support for 0 or 1
cells) - as a result we've now ended up with dodgy DTs out in the field
which depend on this behaviour to map a 1-cell specifier for a 2-cell
provider, despite that being bogus per the bindings themselves.

Since there is some potential use in being able to map at least single
input IDs to multi-cell output specifiers (and properly support 0-cell
outputs as well), add support for properly parsing and using the target
nodes' #cells values, albeit with the unfortunate complication of still
having to work around expectations of the old behaviour too.

Since there are multi-cell output specifiers, the callers of of_map_id()
may need to get the exact cell output value for further processing.
Added support for that part --charan

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
 drivers/iommu/of_iommu.c |   3 +-
 drivers/of/base.c        | 107 ++++++++++++++++++++++++++++++---------
 include/linux/of.h       |  17 ++++---
 3 files changed, 94 insertions(+), 33 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index eac62bc441c5..48759cf1d900 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -45,10 +45,11 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
 				     struct device *dev,
 				     const u32 *id)
 {
-	struct of_phandle_args iommu_spec = { .args_count = 1 };
+	struct of_phandle_args iommu_spec = {};
 	struct of_map_id_arg arg = {
 		.target = &iommu_spec.np,
 		.id_out = iommu_spec.args,
+		.map_cells = &iommu_spec.args_count,
 	};
 	int err;
 
diff --git a/drivers/of/base.c b/drivers/of/base.c
index b8f78a9e6a09..68a7d6ddba66 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2045,11 +2045,30 @@ int of_find_last_cache_level(unsigned int cpu)
 	return cache_level;
 }
 
+/*
+ * Some DTs have an iommu-map targeting a 2-cell IOMMU node while
+ * specifying only 1 cell. Fortunately they all consist of length == 1
+ * entries with the same target, so check for that pattern.
+ */
+static bool of_check_bad_map(const __be32 *map, int len)
+{
+	__be32 phandle = map[1];
+
+	if (len % 4)
+		return false;
+	for (int i = 0; i < len; i += 4) {
+		if (map[i + 1] != phandle || map[i + 3] != cpu_to_be32(1))
+			return false;
+	}
+	return true;
+}
+
 /**
  * of_map_id - Translate an ID through a downstream mapping.
  * @np: root complex device node.
  * @id: device ID to map.
  * @map_name: property name of the map to use.
+ * @cells_name: property name of target specifier cells.
  * @map_mask_name: optional property name of the mask to use.
  * @arg: contains the optional params, @target which is a pointer
  *	to the target device node and id_out which is a pointer
@@ -2065,18 +2084,19 @@ int of_find_last_cache_level(unsigned int cpu)
  *
  * 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 of_map_id_arg *arg)
+int of_map_id(const struct device_node *np, u32 id, const char *map_name,
+	      const char *cells_name, const char *map_mask_name,
+	      struct of_map_id_arg *arg)
 {
 	u32 map_mask, masked_id;
-	int map_len;
+	int map_bytes, map_len, offset = 0;
+	bool bad_map = false;
 	const __be32 *map = NULL;
 
 	if (!np || !map_name || !arg || (!arg->target && !arg->id_out))
 		return -EINVAL;
 
-	map = of_get_property(np, map_name, &map_len);
+	map = of_get_property(np, map_name, &map_bytes);
 	if (!map) {
 		if (arg->target)
 			return -ENODEV;
@@ -2085,11 +2105,9 @@ int of_map_id(const struct device_node *np, u32 id,
 		return 0;
 	}
 
-	if (!map_len || map_len % (4 * sizeof(*map))) {
-		pr_err("%pOF: Error: Bad %s length: %d\n", np,
-			map_name, map_len);
-		return -EINVAL;
-	}
+	if (map_bytes % sizeof(*map))
+		goto err_map_len;
+	map_len = map_bytes / sizeof(*map);
 
 	/* The default is to select all bits. */
 	map_mask = 0xffffffff;
@@ -2102,27 +2120,62 @@ int of_map_id(const struct device_node *np, u32 id,
 		of_property_read_u32(np, map_mask_name, &map_mask);
 
 	masked_id = map_mask & id;
-	for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
+	while (offset < map_len) {
 		struct device_node *phandle_node;
-		u32 id_base = be32_to_cpup(map + 0);
-		u32 phandle = be32_to_cpup(map + 1);
-		u32 out_base = be32_to_cpup(map + 2);
-		u32 id_len = be32_to_cpup(map + 3);
+		u32 id_base, phandle, cells, id_len, id_off;
+		const __be32 *out_base;
+
+		if (map_len - offset < 2)
+			goto err_map_len;
 
+		id_base = be32_to_cpup(map + offset);
 		if (id_base & ~map_mask) {
-			pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
-				np, map_name, map_name,
+			pr_err("%pOF: Invalid %s translation - %s (0x%x) ignores id-base (0x%x)\n",
+				np, map_name, map_mask_name,
 				map_mask, id_base);
 			return -EFAULT;
 		}
 
-		if (masked_id < id_base || masked_id >= id_base + id_len)
-			continue;
-
+		phandle = be32_to_cpup(map + offset + 1);
 		phandle_node = of_find_node_by_phandle(phandle);
 		if (!phandle_node)
 			return -ENODEV;
 
+		if (!bad_map && of_property_read_u32(phandle_node, cells_name, &cells)) {
+			pr_err("%pOF: missing %s property\n", phandle_node, cells_name);
+			return -EINVAL;
+		}
+
+		if (map_len - offset < 3 + cells)
+			goto err_map_len;
+
+		if (offset == 0 && cells == 2) {
+			bad_map = of_check_bad_map(map, map_len);
+			if (bad_map) {
+				pr_warn_once("%pOF: %s mismatches target %s, assuming extra cell of 0\n",
+				np, map_name, cells_name);
+				cells = 1;
+			}
+		}
+
+		out_base = map + offset + 2;
+		offset += 3 + cells;
+
+		id_len = be32_to_cpup(map + offset - 1);
+		if (id_len > 1 && cells > 1) {
+			/*
+			 * With 1 output cell we reasonably assume its value
+			 * has a linear relationship to the input; with more,
+			 * we'd need help from the provider to know what to do.
+			 */
+			pr_err("%pOF: Unsupported %s - cannot handle %d-ID range with %d-cell output specifier\n",
+				np, map_name, id_len, cells);
+			return -EINVAL;
+		}
+		id_off = masked_id - id_base;
+		if (masked_id < id_base || id_off >= id_len)
+			continue;
+
 		if (arg->target) {
 			if (*arg->target)
 				of_node_put(phandle_node);
@@ -2133,12 +2186,14 @@ int of_map_id(const struct device_node *np, u32 id,
 				continue;
 		}
 
-		if (arg->id_out)
-			*arg->id_out = masked_id - id_base + out_base;
+		for (int i = 0; arg->id_out && i < cells; i++)
+			arg->id_out[i] = id_off + be32_to_cpu(out_base[i]);
 
+		if (arg->map_cells)
+			*arg->map_cells = 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, out_base,
-			id_len, id, masked_id - id_base + out_base);
+			np, map_name, map_mask, id_base, be32_to_cpup(out_base),
+			id_len, id, id_off + be32_to_cpup(out_base));
 		return 0;
 	}
 
@@ -2149,5 +2204,9 @@ int of_map_id(const struct device_node *np, u32 id,
 	if (arg->id_out)
 		*arg->id_out = id;
 	return 0;
+
+err_map_len:
+	pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(of_map_id);
diff --git a/include/linux/of.h b/include/linux/of.h
index 21bdce2b37ca..1981509c7918 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -29,6 +29,7 @@ struct device_node;
 struct of_map_id_arg {
 	struct device_node **target;
 	u32 *id_out;
+	u32 *map_cells;
 };
 
 struct property {
@@ -462,9 +463,9 @@ const char *of_prop_next_string(const struct property *prop, const char *cur);
 
 bool of_console_check(const struct device_node *dn, char *name, int index);
 
-int of_map_id(const struct device_node *np, u32 id,
-	       const char *map_name, const char *map_mask_name,
-	       struct of_map_id_arg *arg);
+int of_map_id(const struct device_node *np, u32 id, const char *map_name,
+	      const char *cells_name, const char *map_mask_name,
+	      struct of_map_id_arg *arg);
 
 phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
 
@@ -911,9 +912,9 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
 {
 }
 
-static inline int of_map_id(const struct device_node *np, u32 id,
-			     const char *map_name, const char *map_mask_name,
-			     struct of_map_id_arg *arg)
+static inline int of_map_id(const struct device_node *np, u32 id, const char *map_name,
+			    const char *cells_name, const char *map_mask_name,
+			    struct of_map_id_arg *arg);
 {
 	return -EINVAL;
 }
@@ -1444,7 +1445,7 @@ static inline int of_property_read_s32(const struct device_node *np,
 static inline int of_map_iommu_id(const struct device_node *np, u32 id,
 				  struct of_map_id_arg *arg)
 {
-	return of_map_id(np, id, "iommu-map", "iommu-map-mask", arg);
+	return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask", arg);
 }
 
 static inline int of_map_msi_id(const struct device_node *np, u32 id,
@@ -1455,7 +1456,7 @@ static inline int of_map_msi_id(const struct device_node *np, u32 id,
 		.id_out = id_out,
 	};
 
-	return of_map_id(np, id, "msi-map", "msi-map-mask", &arg);
+	return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask", &arg);
 }
 
 #define of_for_each_phandle(it, err, np, ln, cn, cc)			\
-- 
2.34.1

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[kernel test robot]

Hi Vijayanand,

kernel test robot noticed the following build warnings:

[auto build test WARNING on xen-tip/linux-next]
[also build test WARNING on v6.18]
[cannot apply to robh/for-next pci/next pci/for-linus linus/master next-20251204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijayanand-Jitta/of-Add-convenience-wrappers-for-of_map_id/20251204-180719
base:   https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link:    https://lore.kernel.org/r/20251204095530.8627-4-vijayanand.jitta%40oss.qualcomm.com
patch subject: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20251205/202512051152.PJ6mf512-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d8b43edf552840e59a22a7f3cc332697bd434782)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251205/202512051152.PJ6mf512-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512051152.PJ6mf512-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/of/base.c:2144:7: warning: variable 'cells' is used uninitialized whenever '&&' condition is false [-Wsometimes-uninitialized]
    2144 |                 if (!bad_map && of_property_read_u32(phandle_node, cells_name, &cells)) {
         |                     ^~~~~~~~
   drivers/of/base.c:2149:30: note: uninitialized use occurs here
    2149 |                 if (map_len - offset < 3 + cells)
         |                                            ^~~~~
   drivers/of/base.c:2144:7: note: remove the '&&' if its condition is always true
    2144 |                 if (!bad_map && of_property_read_u32(phandle_node, cells_name, &cells)) {
         |                     ^~~~~~~~~~~
   drivers/of/base.c:2125:30: note: initialize the variable 'cells' to silence this warning
    2125 |                 u32 id_base, phandle, cells, id_len, id_off;
         |                                            ^
         |                                             = 0
   1 warning generated.


vim +2144 drivers/of/base.c

  2065	
  2066	/**
  2067	 * of_map_id - Translate an ID through a downstream mapping.
  2068	 * @np: root complex device node.
  2069	 * @id: device ID to map.
  2070	 * @map_name: property name of the map to use.
  2071	 * @cells_name: property name of target specifier cells.
  2072	 * @map_mask_name: optional property name of the mask to use.
  2073	 * @arg: contains the optional params, @target which is a pointer
  2074	 *	to the target device node and id_out which is a pointer
  2075	 *	to receive the translated ID.
  2076	 *
  2077	 * Given a device ID, look up the appropriate implementation-defined
  2078	 * platform ID and/or the target device which receives transactions on that
  2079	 * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
  2080	 * @id_out may be NULL if only the other is required. If @target points to
  2081	 * a non-NULL device node pointer, only entries targeting that node will be
  2082	 * matched; if it points to a NULL value, it will receive the device node of
  2083	 * the first matching target phandle, with a reference held.
  2084	 *
  2085	 * Return: 0 on success or a standard error code on failure.
  2086	 */
  2087	int of_map_id(const struct device_node *np, u32 id, const char *map_name,
  2088		      const char *cells_name, const char *map_mask_name,
  2089		      struct of_map_id_arg *arg)
  2090	{
  2091		u32 map_mask, masked_id;
  2092		int map_bytes, map_len, offset = 0;
  2093		bool bad_map = false;
  2094		const __be32 *map = NULL;
  2095	
  2096		if (!np || !map_name || !arg || (!arg->target && !arg->id_out))
  2097			return -EINVAL;
  2098	
  2099		map = of_get_property(np, map_name, &map_bytes);
  2100		if (!map) {
  2101			if (arg->target)
  2102				return -ENODEV;
  2103			/* Otherwise, no map implies no translation */
  2104			*arg->id_out = id;
  2105			return 0;
  2106		}
  2107	
  2108		if (map_bytes % sizeof(*map))
  2109			goto err_map_len;
  2110		map_len = map_bytes / sizeof(*map);
  2111	
  2112		/* The default is to select all bits. */
  2113		map_mask = 0xffffffff;
  2114	
  2115		/*
  2116		 * Can be overridden by "{iommu,msi}-map-mask" property.
  2117		 * If of_property_read_u32() fails, the default is used.
  2118		 */
  2119		if (map_mask_name)
  2120			of_property_read_u32(np, map_mask_name, &map_mask);
  2121	
  2122		masked_id = map_mask & id;
  2123		while (offset < map_len) {
  2124			struct device_node *phandle_node;
  2125			u32 id_base, phandle, cells, id_len, id_off;
  2126			const __be32 *out_base;
  2127	
  2128			if (map_len - offset < 2)
  2129				goto err_map_len;
  2130	
  2131			id_base = be32_to_cpup(map + offset);
  2132			if (id_base & ~map_mask) {
  2133				pr_err("%pOF: Invalid %s translation - %s (0x%x) ignores id-base (0x%x)\n",
  2134					np, map_name, map_mask_name,
  2135					map_mask, id_base);
  2136				return -EFAULT;
  2137			}
  2138	
  2139			phandle = be32_to_cpup(map + offset + 1);
  2140			phandle_node = of_find_node_by_phandle(phandle);
  2141			if (!phandle_node)
  2142				return -ENODEV;
  2143	
> 2144			if (!bad_map && of_property_read_u32(phandle_node, cells_name, &cells)) {

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[kernel test robot]

Hi Vijayanand,

kernel test robot noticed the following build errors:

[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on v6.18]
[cannot apply to robh/for-next pci/next pci/for-linus linus/master next-20251204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijayanand-Jitta/of-Add-convenience-wrappers-for-of_map_id/20251204-180719
base:   https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link:    https://lore.kernel.org/r/20251204095530.8627-4-vijayanand.jitta%40oss.qualcomm.com
patch subject: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20251205/202512051121.ivp77JHU-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251205/202512051121.ivp77JHU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512051121.ivp77JHU-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/cpufreq.h:17,
                    from kernel/sched/sched.h:31,
                    from kernel/sched/rq-offsets.c:5:
>> include/linux/of.h:918:1: error: expected identifier or '(' before '{' token
     918 | {
         | ^
>> include/linux/of.h:915:19: warning: 'of_map_id' used but never defined
     915 | static inline int of_map_id(const struct device_node *np, u32 id, const char *map_name,
         |                   ^~~~~~~~~
   make[3]: *** [scripts/Makefile.build:182: kernel/sched/rq-offsets.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1280: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +918 include/linux/of.h

0384e8c6c6fa49d Pantelis Antoniou 2015-01-21  914  
c56888c32d643b3 Robin Murphy      2025-12-04 @915  static inline int of_map_id(const struct device_node *np, u32 id, const char *map_name,
c56888c32d643b3 Robin Murphy      2025-12-04  916  			    const char *cells_name, const char *map_mask_name,
c56888c32d643b3 Robin Murphy      2025-12-04  917  			    struct of_map_id_arg *arg);
2a6db719c92dbfe Nipun Gupta       2018-09-10 @918  {
2a6db719c92dbfe Nipun Gupta       2018-09-10  919  	return -EINVAL;
2a6db719c92dbfe Nipun Gupta       2018-09-10  920  }
2a6db719c92dbfe Nipun Gupta       2018-09-10  921  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[kernel test robot]

Hi Vijayanand,

kernel test robot noticed the following build errors:

[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on v6.18]
[cannot apply to robh/for-next pci/next pci/for-linus linus/master next-20251204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vijayanand-Jitta/of-Add-convenience-wrappers-for-of_map_id/20251204-180719
base:   https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link:    https://lore.kernel.org/r/20251204095530.8627-4-vijayanand.jitta%40oss.qualcomm.com
patch subject: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps
config: i386-buildonly-randconfig-001-20251205 (https://download.01.org/0day-ci/archive/20251205/202512051124.ot03fX1c-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251205/202512051124.ot03fX1c-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512051124.ot03fX1c-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/sched/rq-offsets.c:5:
   In file included from kernel/sched/sched.h:31:
   In file included from include/linux/cpufreq.h:17:
>> include/linux/of.h:918:1: error: expected identifier or '('
     918 | {
         | ^
   1 error generated.
   make[3]: *** [scripts/Makefile.build:182: kernel/sched/rq-offsets.s] Error 1 shuffle=4056561518
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1280: prepare0] Error 2 shuffle=4056561518
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=4056561518
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2 shuffle=4056561518
   make: Target 'prepare' not remade because of errors.


vim +918 include/linux/of.h

0384e8c6c6fa49 Pantelis Antoniou 2015-01-21  914  
c56888c32d643b Robin Murphy      2025-12-04  915  static inline int of_map_id(const struct device_node *np, u32 id, const char *map_name,
c56888c32d643b Robin Murphy      2025-12-04  916  			    const char *cells_name, const char *map_mask_name,
c56888c32d643b Robin Murphy      2025-12-04  917  			    struct of_map_id_arg *arg);
2a6db719c92dbf Nipun Gupta       2018-09-10 @918  {
2a6db719c92dbf Nipun Gupta       2018-09-10  919  	return -EINVAL;
2a6db719c92dbf Nipun Gupta       2018-09-10  920  }
2a6db719c92dbf Nipun Gupta       2018-09-10  921  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[Rob Herring]

On Thu, Dec 04, 2025 at 03:25:30PM +0530, Vijayanand Jitta wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> 
> So far our parsing of {iommu,msi}-map properites has always blindly
> asusmed that the output specifiers will always have exactly 1 cell.
> This typically does happen to be the case, but is not actually enforced
> (and the PCI msi-map binding even explicitly states support for 0 or 1
> cells) - as a result we've now ended up with dodgy DTs out in the field
> which depend on this behaviour to map a 1-cell specifier for a 2-cell
> provider, despite that being bogus per the bindings themselves.
> 
> Since there is some potential use in being able to map at least single
> input IDs to multi-cell output specifiers (and properly support 0-cell
> outputs as well), add support for properly parsing and using the target
> nodes' #cells values, albeit with the unfortunate complication of still
> having to work around expectations of the old behaviour too.
> 
> Since there are multi-cell output specifiers, the callers of of_map_id()
> may need to get the exact cell output value for further processing.
> Added support for that part --charan
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> ---
>  drivers/iommu/of_iommu.c |   3 +-
>  drivers/of/base.c        | 107 ++++++++++++++++++++++++++++++---------
>  include/linux/of.h       |  17 ++++---
>  3 files changed, 94 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index eac62bc441c5..48759cf1d900 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -45,10 +45,11 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
>  				     struct device *dev,
>  				     const u32 *id)
>  {
> -	struct of_phandle_args iommu_spec = { .args_count = 1 };
> +	struct of_phandle_args iommu_spec = {};
>  	struct of_map_id_arg arg = {
>  		.target = &iommu_spec.np,
>  		.id_out = iommu_spec.args,
> +		.map_cells = &iommu_spec.args_count,
>  	};
>  	int err;
>  
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index b8f78a9e6a09..68a7d6ddba66 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2045,11 +2045,30 @@ int of_find_last_cache_level(unsigned int cpu)
>  	return cache_level;
>  }
>  
> +/*
> + * Some DTs have an iommu-map targeting a 2-cell IOMMU node while
> + * specifying only 1 cell. Fortunately they all consist of length == 1
> + * entries with the same target, so check for that pattern.

Can you show what a bad entry looks like here.

> + */
> +static bool of_check_bad_map(const __be32 *map, int len)
> +{
> +	__be32 phandle = map[1];
> +
> +	if (len % 4)
> +		return false;
> +	for (int i = 0; i < len; i += 4) {
> +		if (map[i + 1] != phandle || map[i + 3] != cpu_to_be32(1))

Why does the IOMMU arg cell have to be 1? The description said 'same 
target', but it is just all have an IOMMU cell value of 1?

> +			return false;
> +	}
> +	return true;
> +}
> +
>  /**
>   * of_map_id - Translate an ID through a downstream mapping.
>   * @np: root complex device node.
>   * @id: device ID to map.
>   * @map_name: property name of the map to use.
> + * @cells_name: property name of target specifier cells.
>   * @map_mask_name: optional property name of the mask to use.
>   * @arg: contains the optional params, @target which is a pointer
>   *	to the target device node and id_out which is a pointer
> @@ -2065,18 +2084,19 @@ int of_find_last_cache_level(unsigned int cpu)
>   *
>   * 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 of_map_id_arg *arg)
> +int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> +	      const char *cells_name, const char *map_mask_name,
> +	      struct of_map_id_arg *arg)
>  {
>  	u32 map_mask, masked_id;
> -	int map_len;
> +	int map_bytes, map_len, offset = 0;
> +	bool bad_map = false;
>  	const __be32 *map = NULL;
>  
>  	if (!np || !map_name || !arg || (!arg->target && !arg->id_out))
>  		return -EINVAL;
>  
> -	map = of_get_property(np, map_name, &map_len);
> +	map = of_get_property(np, map_name, &map_bytes);
>  	if (!map) {
>  		if (arg->target)
>  			return -ENODEV;
> @@ -2085,11 +2105,9 @@ int of_map_id(const struct device_node *np, u32 id,
>  		return 0;
>  	}
>  
> -	if (!map_len || map_len % (4 * sizeof(*map))) {
> -		pr_err("%pOF: Error: Bad %s length: %d\n", np,
> -			map_name, map_len);
> -		return -EINVAL;
> -	}
> +	if (map_bytes % sizeof(*map))
> +		goto err_map_len;
> +	map_len = map_bytes / sizeof(*map);
>  
>  	/* The default is to select all bits. */
>  	map_mask = 0xffffffff;
> @@ -2102,27 +2120,62 @@ int of_map_id(const struct device_node *np, u32 id,
>  		of_property_read_u32(np, map_mask_name, &map_mask);
>  
>  	masked_id = map_mask & id;
> -	for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> +	while (offset < map_len) {
>  		struct device_node *phandle_node;
> -		u32 id_base = be32_to_cpup(map + 0);
> -		u32 phandle = be32_to_cpup(map + 1);
> -		u32 out_base = be32_to_cpup(map + 2);
> -		u32 id_len = be32_to_cpup(map + 3);
> +		u32 id_base, phandle, cells, id_len, id_off;
> +		const __be32 *out_base;
> +
> +		if (map_len - offset < 2)
> +			goto err_map_len;
>  
> +		id_base = be32_to_cpup(map + offset);
>  		if (id_base & ~map_mask) {
> -			pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
> -				np, map_name, map_name,
> +			pr_err("%pOF: Invalid %s translation - %s (0x%x) ignores id-base (0x%x)\n",
> +				np, map_name, map_mask_name,
>  				map_mask, id_base);
>  			return -EFAULT;
>  		}
>  
> -		if (masked_id < id_base || masked_id >= id_base + id_len)
> -			continue;
> -
> +		phandle = be32_to_cpup(map + offset + 1);
>  		phandle_node = of_find_node_by_phandle(phandle);
>  		if (!phandle_node)
>  			return -ENODEV;
>  
> +		if (!bad_map && of_property_read_u32(phandle_node, cells_name, &cells)) {
> +			pr_err("%pOF: missing %s property\n", phandle_node, cells_name);
> +			return -EINVAL;
> +		}
> +
> +		if (map_len - offset < 3 + cells)
> +			goto err_map_len;
> +
> +		if (offset == 0 && cells == 2) {
> +			bad_map = of_check_bad_map(map, map_len);
> +			if (bad_map) {
> +				pr_warn_once("%pOF: %s mismatches target %s, assuming extra cell of 0\n",
> +				np, map_name, cells_name);
> +				cells = 1;
> +			}
> +		}
> +
> +		out_base = map + offset + 2;
> +		offset += 3 + cells;
> +
> +		id_len = be32_to_cpup(map + offset - 1);
> +		if (id_len > 1 && cells > 1) {
> +			/*
> +			 * With 1 output cell we reasonably assume its value
> +			 * has a linear relationship to the input; with more,
> +			 * we'd need help from the provider to know what to do.
> +			 */
> +			pr_err("%pOF: Unsupported %s - cannot handle %d-ID range with %d-cell output specifier\n",
> +				np, map_name, id_len, cells);
> +			return -EINVAL;
> +		}
> +		id_off = masked_id - id_base;
> +		if (masked_id < id_base || id_off >= id_len)
> +			continue;
> +
>  		if (arg->target) {
>  			if (*arg->target)
>  				of_node_put(phandle_node);
> @@ -2133,12 +2186,14 @@ int of_map_id(const struct device_node *np, u32 id,
>  				continue;
>  		}
>  
> -		if (arg->id_out)
> -			*arg->id_out = masked_id - id_base + out_base;
> +		for (int i = 0; arg->id_out && i < cells; i++)
> +			arg->id_out[i] = id_off + be32_to_cpu(out_base[i]);
>  
> +		if (arg->map_cells)
> +			*arg->map_cells = 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, out_base,
> -			id_len, id, masked_id - id_base + out_base);
> +			np, map_name, map_mask, id_base, be32_to_cpup(out_base),
> +			id_len, id, id_off + be32_to_cpup(out_base));
>  		return 0;
>  	}
>  
> @@ -2149,5 +2204,9 @@ int of_map_id(const struct device_node *np, u32 id,
>  	if (arg->id_out)
>  		*arg->id_out = id;
>  	return 0;
> +
> +err_map_len:
> +	pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
> +	return -EINVAL;
>  }
>  EXPORT_SYMBOL_GPL(of_map_id);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 21bdce2b37ca..1981509c7918 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -29,6 +29,7 @@ struct device_node;
>  struct of_map_id_arg {
>  	struct device_node **target;
>  	u32 *id_out;
> +	u32 *map_cells;
>  };
>  
>  struct property {
> @@ -462,9 +463,9 @@ const char *of_prop_next_string(const struct property *prop, const char *cur);
>  
>  bool of_console_check(const struct device_node *dn, char *name, int index);
>  
> -int of_map_id(const struct device_node *np, u32 id,
> -	       const char *map_name, const char *map_mask_name,
> -	       struct of_map_id_arg *arg);
> +int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> +	      const char *cells_name, const char *map_mask_name,
> +	      struct of_map_id_arg *arg);
>  
>  phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
>  
> @@ -911,9 +912,9 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
>  {
>  }
>  
> -static inline int of_map_id(const struct device_node *np, u32 id,
> -			     const char *map_name, const char *map_mask_name,
> -			     struct of_map_id_arg *arg)
> +static inline int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> +			    const char *cells_name, const char *map_mask_name,
> +			    struct of_map_id_arg *arg);
>  {
>  	return -EINVAL;
>  }
> @@ -1444,7 +1445,7 @@ static inline int of_property_read_s32(const struct device_node *np,
>  static inline int of_map_iommu_id(const struct device_node *np, u32 id,
>  				  struct of_map_id_arg *arg)
>  {
> -	return of_map_id(np, id, "iommu-map", "iommu-map-mask", arg);
> +	return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask", arg);
>  }
>  
>  static inline int of_map_msi_id(const struct device_node *np, u32 id,
> @@ -1455,7 +1456,7 @@ static inline int of_map_msi_id(const struct device_node *np, u32 id,
>  		.id_out = id_out,
>  	};
>  
> -	return of_map_id(np, id, "msi-map", "msi-map-mask", &arg);
> +	return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask", &arg);

There are cases of no #msi-cells and we default to 0 cells in that case. 
Do you maintain that?

Rob

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[Vijayanand Jitta]

On 12/10/2025 1:47 AM, Rob Herring wrote:
> On Thu, Dec 04, 2025 at 03:25:30PM +0530, Vijayanand Jitta wrote:
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> So far our parsing of {iommu,msi}-map properites has always blindly
>> asusmed that the output specifiers will always have exactly 1 cell.
>> This typically does happen to be the case, but is not actually enforced
>> (and the PCI msi-map binding even explicitly states support for 0 or 1
>> cells) - as a result we've now ended up with dodgy DTs out in the field
>> which depend on this behaviour to map a 1-cell specifier for a 2-cell
>> provider, despite that being bogus per the bindings themselves.
>>
>> Since there is some potential use in being able to map at least single
>> input IDs to multi-cell output specifiers (and properly support 0-cell
>> outputs as well), add support for properly parsing and using the target
>> nodes' #cells values, albeit with the unfortunate complication of still
>> having to work around expectations of the old behaviour too.
>>
>> Since there are multi-cell output specifiers, the callers of of_map_id()
>> may need to get the exact cell output value for further processing.
>> Added support for that part --charan
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
>> ---
>>  drivers/iommu/of_iommu.c |   3 +-
>>  drivers/of/base.c        | 107 ++++++++++++++++++++++++++++++---------
>>  include/linux/of.h       |  17 ++++---
>>  3 files changed, 94 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index eac62bc441c5..48759cf1d900 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -45,10 +45,11 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
>>  				     struct device *dev,
>>  				     const u32 *id)
>>  {
>> -	struct of_phandle_args iommu_spec = { .args_count = 1 };
>> +	struct of_phandle_args iommu_spec = {};
>>  	struct of_map_id_arg arg = {
>>  		.target = &iommu_spec.np,
>>  		.id_out = iommu_spec.args,
>> +		.map_cells = &iommu_spec.args_count,
>>  	};
>>  	int err;
>>  
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index b8f78a9e6a09..68a7d6ddba66 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2045,11 +2045,30 @@ int of_find_last_cache_level(unsigned int cpu)
>>  	return cache_level;
>>  }
>>  
>> +/*
>> + * Some DTs have an iommu-map targeting a 2-cell IOMMU node while
>> + * specifying only 1 cell. Fortunately they all consist of length == 1
>> + * entries with the same target, so check for that pattern.
> 
> Can you show what a bad entry looks like here.
> 

Sure, will add an example in the comments. Basically it would look like below.

for iommu with iommu-cells  = <2>;

Device having below iommu-map property.

iommu-map = <0x0000  &smmu  0x0000  0x1>,
            <0x0100  &smmu  0x0100  0x1>;

>> + */
>> +static bool of_check_bad_map(const __be32 *map, int len)
>> +{
>> +	__be32 phandle = map[1];
>> +
>> +	if (len % 4)
>> +		return false;
>> +	for (int i = 0; i < len; i += 4) {
>> +		if (map[i + 1] != phandle || map[i + 3] != cpu_to_be32(1))
> 
> Why does the IOMMU arg cell have to be 1? The description said 'same 
> target', but it is just all have an IOMMU cell value of 1?
> 

Here, the check is for length argument to be 1. This is to maintain backward
compatibility as mentioned above, as all such bad entries right now have
length as 1.

>> +			return false;
>> +	}
>> +	return true;
>> +}
>> +
>>  /**
>>   * of_map_id - Translate an ID through a downstream mapping.
>>   * @np: root complex device node.
>>   * @id: device ID to map.
>>   * @map_name: property name of the map to use.
>> + * @cells_name: property name of target specifier cells.
>>   * @map_mask_name: optional property name of the mask to use.
>>   * @arg: contains the optional params, @target which is a pointer
>>   *	to the target device node and id_out which is a pointer
>> @@ -2065,18 +2084,19 @@ int of_find_last_cache_level(unsigned int cpu)
>>   *
>>   * 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 of_map_id_arg *arg)
>> +int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> +	      const char *cells_name, const char *map_mask_name,
>> +	      struct of_map_id_arg *arg)
>>  {
>>  	u32 map_mask, masked_id;
>> -	int map_len;
>> +	int map_bytes, map_len, offset = 0;
>> +	bool bad_map = false;
>>  	const __be32 *map = NULL;
>>  
>>  	if (!np || !map_name || !arg || (!arg->target && !arg->id_out))
>>  		return -EINVAL;
>>  
>> -	map = of_get_property(np, map_name, &map_len);
>> +	map = of_get_property(np, map_name, &map_bytes);
>>  	if (!map) {
>>  		if (arg->target)
>>  			return -ENODEV;
>> @@ -2085,11 +2105,9 @@ int of_map_id(const struct device_node *np, u32 id,
>>  		return 0;
>>  	}
>>  
>> -	if (!map_len || map_len % (4 * sizeof(*map))) {
>> -		pr_err("%pOF: Error: Bad %s length: %d\n", np,
>> -			map_name, map_len);
>> -		return -EINVAL;
>> -	}
>> +	if (map_bytes % sizeof(*map))
>> +		goto err_map_len;
>> +	map_len = map_bytes / sizeof(*map);
>>  
>>  	/* The default is to select all bits. */
>>  	map_mask = 0xffffffff;
>> @@ -2102,27 +2120,62 @@ int of_map_id(const struct device_node *np, u32 id,
>>  		of_property_read_u32(np, map_mask_name, &map_mask);
>>  
>>  	masked_id = map_mask & id;
>> -	for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
>> +	while (offset < map_len) {
>>  		struct device_node *phandle_node;
>> -		u32 id_base = be32_to_cpup(map + 0);
>> -		u32 phandle = be32_to_cpup(map + 1);
>> -		u32 out_base = be32_to_cpup(map + 2);
>> -		u32 id_len = be32_to_cpup(map + 3);
>> +		u32 id_base, phandle, cells, id_len, id_off;
>> +		const __be32 *out_base;
>> +
>> +		if (map_len - offset < 2)
>> +			goto err_map_len;
>>  
>> +		id_base = be32_to_cpup(map + offset);
>>  		if (id_base & ~map_mask) {
>> -			pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
>> -				np, map_name, map_name,
>> +			pr_err("%pOF: Invalid %s translation - %s (0x%x) ignores id-base (0x%x)\n",
>> +				np, map_name, map_mask_name,
>>  				map_mask, id_base);
>>  			return -EFAULT;
>>  		}
>>  
>> -		if (masked_id < id_base || masked_id >= id_base + id_len)
>> -			continue;
>> -
>> +		phandle = be32_to_cpup(map + offset + 1);
>>  		phandle_node = of_find_node_by_phandle(phandle);
>>  		if (!phandle_node)
>>  			return -ENODEV;
>>  
>> +		if (!bad_map && of_property_read_u32(phandle_node, cells_name, &cells)) {
>> +			pr_err("%pOF: missing %s property\n", phandle_node, cells_name);
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (map_len - offset < 3 + cells)
>> +			goto err_map_len;
>> +
>> +		if (offset == 0 && cells == 2) {
>> +			bad_map = of_check_bad_map(map, map_len);
>> +			if (bad_map) {
>> +				pr_warn_once("%pOF: %s mismatches target %s, assuming extra cell of 0\n",
>> +				np, map_name, cells_name);
>> +				cells = 1;
>> +			}
>> +		}
>> +
>> +		out_base = map + offset + 2;
>> +		offset += 3 + cells;
>> +
>> +		id_len = be32_to_cpup(map + offset - 1);
>> +		if (id_len > 1 && cells > 1) {
>> +			/*
>> +			 * With 1 output cell we reasonably assume its value
>> +			 * has a linear relationship to the input; with more,
>> +			 * we'd need help from the provider to know what to do.
>> +			 */
>> +			pr_err("%pOF: Unsupported %s - cannot handle %d-ID range with %d-cell output specifier\n",
>> +				np, map_name, id_len, cells);
>> +			return -EINVAL;
>> +		}
>> +		id_off = masked_id - id_base;
>> +		if (masked_id < id_base || id_off >= id_len)
>> +			continue;
>> +
>>  		if (arg->target) {
>>  			if (*arg->target)
>>  				of_node_put(phandle_node);
>> @@ -2133,12 +2186,14 @@ int of_map_id(const struct device_node *np, u32 id,
>>  				continue;
>>  		}
>>  
>> -		if (arg->id_out)
>> -			*arg->id_out = masked_id - id_base + out_base;
>> +		for (int i = 0; arg->id_out && i < cells; i++)
>> +			arg->id_out[i] = id_off + be32_to_cpu(out_base[i]);
>>  
>> +		if (arg->map_cells)
>> +			*arg->map_cells = 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, out_base,
>> -			id_len, id, masked_id - id_base + out_base);
>> +			np, map_name, map_mask, id_base, be32_to_cpup(out_base),
>> +			id_len, id, id_off + be32_to_cpup(out_base));
>>  		return 0;
>>  	}
>>  
>> @@ -2149,5 +2204,9 @@ int of_map_id(const struct device_node *np, u32 id,
>>  	if (arg->id_out)
>>  		*arg->id_out = id;
>>  	return 0;
>> +
>> +err_map_len:
>> +	pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
>> +	return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(of_map_id);
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 21bdce2b37ca..1981509c7918 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -29,6 +29,7 @@ struct device_node;
>>  struct of_map_id_arg {
>>  	struct device_node **target;
>>  	u32 *id_out;
>> +	u32 *map_cells;
>>  };
>>  
>>  struct property {
>> @@ -462,9 +463,9 @@ const char *of_prop_next_string(const struct property *prop, const char *cur);
>>  
>>  bool of_console_check(const struct device_node *dn, char *name, int index);
>>  
>> -int of_map_id(const struct device_node *np, u32 id,
>> -	       const char *map_name, const char *map_mask_name,
>> -	       struct of_map_id_arg *arg);
>> +int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> +	      const char *cells_name, const char *map_mask_name,
>> +	      struct of_map_id_arg *arg);
>>  
>>  phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
>>  
>> @@ -911,9 +912,9 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
>>  {
>>  }
>>  
>> -static inline int of_map_id(const struct device_node *np, u32 id,
>> -			     const char *map_name, const char *map_mask_name,
>> -			     struct of_map_id_arg *arg)
>> +static inline int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> +			    const char *cells_name, const char *map_mask_name,
>> +			    struct of_map_id_arg *arg);
>>  {
>>  	return -EINVAL;
>>  }
>> @@ -1444,7 +1445,7 @@ static inline int of_property_read_s32(const struct device_node *np,
>>  static inline int of_map_iommu_id(const struct device_node *np, u32 id,
>>  				  struct of_map_id_arg *arg)
>>  {
>> -	return of_map_id(np, id, "iommu-map", "iommu-map-mask", arg);
>> +	return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask", arg);
>>  }
>>  
>>  static inline int of_map_msi_id(const struct device_node *np, u32 id,
>> @@ -1455,7 +1456,7 @@ static inline int of_map_msi_id(const struct device_node *np, u32 id,
>>  		.id_out = id_out,
>>  	};
>>  
>> -	return of_map_id(np, id, "msi-map", "msi-map-mask", &arg);
>> +	return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask", &arg);
> 
> There are cases of no #msi-cells and we default to 0 cells in that case. 
> Do you maintain that?
> 
> Rob

Thanks for pointing this, I see this case of no #msi-cells is not covered. Will
add it in next revision.  Also, IIUC shouldn't we set default cells to '1' to
maintain backward compatibility of of_map_id in this case ? No ?

Thanks,
Vijay

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[Rob Herring]

On Wed, Dec 10, 2025 at 03:57:56PM +0530, Vijayanand Jitta wrote:
> On 12/10/2025 1:47 AM, Rob Herring wrote:
> > On Thu, Dec 04, 2025 at 03:25:30PM +0530, Vijayanand Jitta wrote:
> >> From: Robin Murphy <robin.murphy@arm.com>
> >>
> >> So far our parsing of {iommu,msi}-map properites has always blindly
> >> asusmed that the output specifiers will always have exactly 1 cell.
> >> This typically does happen to be the case, but is not actually enforced
> >> (and the PCI msi-map binding even explicitly states support for 0 or 1
> >> cells) - as a result we've now ended up with dodgy DTs out in the field
> >> which depend on this behaviour to map a 1-cell specifier for a 2-cell
> >> provider, despite that being bogus per the bindings themselves.
> >>
> >> Since there is some potential use in being able to map at least single
> >> input IDs to multi-cell output specifiers (and properly support 0-cell
> >> outputs as well), add support for properly parsing and using the target
> >> nodes' #cells values, albeit with the unfortunate complication of still
> >> having to work around expectations of the old behaviour too.
> >>
> >> Since there are multi-cell output specifiers, the callers of of_map_id()
> >> may need to get the exact cell output value for further processing.
> >> Added support for that part --charan
> >>
> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> >> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> >> ---
> >>  drivers/iommu/of_iommu.c |   3 +-
> >>  drivers/of/base.c        | 107 ++++++++++++++++++++++++++++++---------
> >>  include/linux/of.h       |  17 ++++---
> >>  3 files changed, 94 insertions(+), 33 deletions(-)
> >>
> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >> index eac62bc441c5..48759cf1d900 100644
> >> --- a/drivers/iommu/of_iommu.c
> >> +++ b/drivers/iommu/of_iommu.c
> >> @@ -45,10 +45,11 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
> >>  				     struct device *dev,
> >>  				     const u32 *id)
> >>  {
> >> -	struct of_phandle_args iommu_spec = { .args_count = 1 };
> >> +	struct of_phandle_args iommu_spec = {};
> >>  	struct of_map_id_arg arg = {
> >>  		.target = &iommu_spec.np,
> >>  		.id_out = iommu_spec.args,
> >> +		.map_cells = &iommu_spec.args_count,
> >>  	};
> >>  	int err;
> >>  
> >> diff --git a/drivers/of/base.c b/drivers/of/base.c
> >> index b8f78a9e6a09..68a7d6ddba66 100644
> >> --- a/drivers/of/base.c
> >> +++ b/drivers/of/base.c
> >> @@ -2045,11 +2045,30 @@ int of_find_last_cache_level(unsigned int cpu)
> >>  	return cache_level;
> >>  }
> >>  
> >> +/*
> >> + * Some DTs have an iommu-map targeting a 2-cell IOMMU node while
> >> + * specifying only 1 cell. Fortunately they all consist of length == 1
> >> + * entries with the same target, so check for that pattern.
> > 
> > Can you show what a bad entry looks like here.
> > 
> 
> Sure, will add an example in the comments. Basically it would look like below.
> 
> for iommu with iommu-cells  = <2>;
> 
> Device having below iommu-map property.
> 
> iommu-map = <0x0000  &smmu  0x0000  0x1>,
>             <0x0100  &smmu  0x0100  0x1>;
> 
> >> + */
> >> +static bool of_check_bad_map(const __be32 *map, int len)
> >> +{
> >> +	__be32 phandle = map[1];
> >> +
> >> +	if (len % 4)
> >> +		return false;
> >> +	for (int i = 0; i < len; i += 4) {
> >> +		if (map[i + 1] != phandle || map[i + 3] != cpu_to_be32(1))
> > 
> > Why does the IOMMU arg cell have to be 1? The description said 'same 
> > target', but it is just all have an IOMMU cell value of 1?
> > 
> 
> Here, the check is for length argument to be 1. This is to maintain backward
> compatibility as mentioned above, as all such bad entries right now have
> length as 1.

You say length and I think arg/cell length, not that the cell value 
contains a length. That's because generally cell args are provider 
defined and specific. So just say the 2nd cell has a value of 1 and 
leave out that's a length.

[...]

> >> @@ -1455,7 +1456,7 @@ static inline int of_map_msi_id(const struct device_node *np, u32 id,
> >>  		.id_out = id_out,
> >>  	};
> >>  
> >> -	return of_map_id(np, id, "msi-map", "msi-map-mask", &arg);
> >> +	return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask", &arg);
> > 
> > There are cases of no #msi-cells and we default to 0 cells in that case. 
> > Do you maintain that?
> > 
> > Rob
> 
> Thanks for pointing this, I see this case of no #msi-cells is not covered. Will
> add it in next revision.  Also, IIUC shouldn't we set default cells to '1' to
> maintain backward compatibility of of_map_id in this case ? No ?

The only default is 0. Perhaps msi-map is never used if there are 0 
cells? IDK, you tell me.

Rob

Re: [PATCH v2 3/3] of: Respect #{iommu,msi}-cells in maps

[Vijayanand Jitta]

On 12/11/2025 2:34 AM, Rob Herring wrote:
> On Wed, Dec 10, 2025 at 03:57:56PM +0530, Vijayanand Jitta wrote:
>> On 12/10/2025 1:47 AM, Rob Herring wrote:
>>> On Thu, Dec 04, 2025 at 03:25:30PM +0530, Vijayanand Jitta wrote:
>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>
>>>> So far our parsing of {iommu,msi}-map properites has always blindly
>>>> asusmed that the output specifiers will always have exactly 1 cell.
>>>> This typically does happen to be the case, but is not actually enforced
>>>> (and the PCI msi-map binding even explicitly states support for 0 or 1
>>>> cells) - as a result we've now ended up with dodgy DTs out in the field
>>>> which depend on this behaviour to map a 1-cell specifier for a 2-cell
>>>> provider, despite that being bogus per the bindings themselves.
>>>>
>>>> Since there is some potential use in being able to map at least single
>>>> input IDs to multi-cell output specifiers (and properly support 0-cell
>>>> outputs as well), add support for properly parsing and using the target
>>>> nodes' #cells values, albeit with the unfortunate complication of still
>>>> having to work around expectations of the old behaviour too.
>>>>
>>>> Since there are multi-cell output specifiers, the callers of of_map_id()
>>>> may need to get the exact cell output value for further processing.
>>>> Added support for that part --charan
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
>>>> ---
>>>>  drivers/iommu/of_iommu.c |   3 +-
>>>>  drivers/of/base.c        | 107 ++++++++++++++++++++++++++++++---------
>>>>  include/linux/of.h       |  17 ++++---
>>>>  3 files changed, 94 insertions(+), 33 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>>> index eac62bc441c5..48759cf1d900 100644
>>>> --- a/drivers/iommu/of_iommu.c
>>>> +++ b/drivers/iommu/of_iommu.c
>>>> @@ -45,10 +45,11 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
>>>>  				     struct device *dev,
>>>>  				     const u32 *id)
>>>>  {
>>>> -	struct of_phandle_args iommu_spec = { .args_count = 1 };
>>>> +	struct of_phandle_args iommu_spec = {};
>>>>  	struct of_map_id_arg arg = {
>>>>  		.target = &iommu_spec.np,
>>>>  		.id_out = iommu_spec.args,
>>>> +		.map_cells = &iommu_spec.args_count,
>>>>  	};
>>>>  	int err;
>>>>  
>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>> index b8f78a9e6a09..68a7d6ddba66 100644
>>>> --- a/drivers/of/base.c
>>>> +++ b/drivers/of/base.c
>>>> @@ -2045,11 +2045,30 @@ int of_find_last_cache_level(unsigned int cpu)
>>>>  	return cache_level;
>>>>  }
>>>>  
>>>> +/*
>>>> + * Some DTs have an iommu-map targeting a 2-cell IOMMU node while
>>>> + * specifying only 1 cell. Fortunately they all consist of length == 1
>>>> + * entries with the same target, so check for that pattern.
>>>
>>> Can you show what a bad entry looks like here.
>>>
>>
>> Sure, will add an example in the comments. Basically it would look like below.
>>
>> for iommu with iommu-cells  = <2>;
>>
>> Device having below iommu-map property.
>>
>> iommu-map = <0x0000  &smmu  0x0000  0x1>,
>>             <0x0100  &smmu  0x0100  0x1>;
>>
>>>> + */
>>>> +static bool of_check_bad_map(const __be32 *map, int len)
>>>> +{
>>>> +	__be32 phandle = map[1];
>>>> +
>>>> +	if (len % 4)
>>>> +		return false;
>>>> +	for (int i = 0; i < len; i += 4) {
>>>> +		if (map[i + 1] != phandle || map[i + 3] != cpu_to_be32(1))
>>>
>>> Why does the IOMMU arg cell have to be 1? The description said 'same 
>>> target', but it is just all have an IOMMU cell value of 1?
>>>
>>
>> Here, the check is for length argument to be 1. This is to maintain backward
>> compatibility as mentioned above, as all such bad entries right now have
>> length as 1.
> 
> You say length and I think arg/cell length, not that the cell value 
> contains a length. That's because generally cell args are provider 
> defined and specific. So just say the 2nd cell has a value of 1 and 
> leave out that's a length.
> 
> [...]
> 

Understood, will update the comment as suggested.

>>>> @@ -1455,7 +1456,7 @@ static inline int of_map_msi_id(const struct device_node *np, u32 id,
>>>>  		.id_out = id_out,
>>>>  	};
>>>>  
>>>> -	return of_map_id(np, id, "msi-map", "msi-map-mask", &arg);
>>>> +	return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask", &arg);
>>>
>>> There are cases of no #msi-cells and we default to 0 cells in that case. 
>>> Do you maintain that?
>>>
>>> Rob
>>
>> Thanks for pointing this, I see this case of no #msi-cells is not covered. Will
>> add it in next revision.  Also, IIUC shouldn't we set default cells to '1' to
>> maintain backward compatibility of of_map_id in this case ? No ?
> 
> The only default is 0. Perhaps msi-map is never used if there are 0 
> cells? IDK, you tell me.
> 
> Rob

You are right, I’ve grepped through the upstream DTs that use msi-map
(Rockchip, TI K3, HiSilicon, QCom, NXP/Freescale, etc.), and I don’t
see any cases where provider uses msi-map while having #msi-cells = <0>
or omitting #msi-cells entirely.

All of the common MSI providers referenced by msi-map (GIC ITS, v2m,
etc.) define #msi-cells to 1.

So, I think the current patch should be fine as‑is, as there are no
upstream users of msi-map relying on a 0‑cell or missing #msi-cells
provider.

Thanks,
Vijay