[PATCH v3 00/13] Support ACPI and SETAASA device discovery

[PATCH v3 00/13] Support ACPI and SETAASA device discovery

[Akhil R]

This patch series adds SETAASA device discovery to the I3C subsystem,
enabling support for SPD5118 temperature sensors found on DDR5 memory
modules. The changes also add ACPI support for all existing DAA
methods like SETDASA, SETNEWDA as well as I2C devices on I3C bus.

SPD5118 and similar devices on DDR5 memory modules differ from typical
I3C devices in their initialization. They use SETAASA broadcast CCC
instead of ENTDAA for address assignment, and per JEDEC specification,
are not required to have a Provisioned ID or implement standard device
information CCC commands (GETPID, GETDCR, GETBCR).

The series enables to describe all I3C and I2C devices on both Device
Tree and the ACPI table, using unified device property APIs throughout
the I3C core and the Synopsys DesignWare I3C master driver.

Please note that the series modifies drivers across multiple subsystems,
like Device Tree bindings, ACPI, I3C and HWMON.

v2->v3:
  * Fix maximum value and indent bit list for mipi-i3c-static-method.
  * Move I3C_ADDR_METHOD_* macros to dt-bindings header.
  * Drop ACPICA commit IDs, keep only the Link: tags.
  * Revert the change which proceeds to register other devices if SETAASA
    is not supported so that it aligns with the rest of the driver and to
    avoid the issues pointed by Sashiko.
  * Rework multiple commit messages.

v1->v2:
  * Added patch to remove 16-bit addressing support for SPD5118
  * Guard ACPI calls with #ifdef CONFIG_ACPI
  * Remove CONFIG_OF guard for of_alias_get_highest_id()
  * Mask mipi-i3c-static-method in the driver to select only valid values.
  * Proceed to register other devices if SETAASA is not supported.
  * Update commit message and links in the description of multiple commits.


Akhil R (13):
  dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
  ACPICA: Read LVR from the I2C resource descriptor
  i3c: master: Use unified device property interface
  i3c: master: Support ACPI enumeration of child devices
  i3c: master: Add support for devices using SETAASA
  i3c: master: Add support for devices without PID
  i3c: master: match I3C device through DT and ACPI
  i3c: dw-i3c-master: Add SETAASA as supported CCC
  i3c: dw-i3c-master: Add a quirk to skip clock and reset
  i3c: dw-i3c-master: Add ACPI ID for Tegra410
  hwmon: spd5118: Remove 16-bit addressing
  hwmon: spd5118: Add I3C support
  arm64: defconfig: Enable I3C and SPD5118 hwmon

 .../devicetree/bindings/i3c/i3c.yaml          |  31 +-
 arch/arm64/configs/defconfig                  |   3 +
 drivers/acpi/acpica/rsserial.c                |   6 +-
 drivers/hwmon/Kconfig                         |   9 +-
 drivers/hwmon/spd5118.c                       | 119 +++---
 drivers/i3c/master.c                          | 354 +++++++++++++++---
 drivers/i3c/master/dw-i3c-master.c            |  66 ++--
 include/acpi/acrestyp.h                       |   1 +
 include/dt-bindings/i3c/i3c.h                 |   3 +
 include/linux/i3c/ccc.h                       |   1 +
 include/linux/i3c/master.h                    |  20 +-
 11 files changed, 463 insertions(+), 150 deletions(-)

-- 
2.50.1

[PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Akhil R]

Add the 'mipi-i3c-static-method' property mentioned in the MIPI I3C
Discovery and Configuration Specification [1] to specify which discovery
method an I3C device supports during bus initialization. The property is
a bitmap, where a bit value of 1 indicates support for that method, and 0
indicates lack of support.

Bit 0: SETDASA CCC (Direct)
Bit 1: SETAASA CCC (Broadcast)
Bit 2: Other CCC (vendor / standards extension)
All other bits are reserved.

It is specifically needed when an I3C device requires SETAASA for the
address assignment. SETDASA will be supported by default if this property
is absent, which means for now the property just serves as a flag to
enable SETAASA, but keep the property as a bitmap to align with the
specifications.

[1] https://www.mipi.org/mipi-disco-for-i3c-download

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 .../devicetree/bindings/i3c/i3c.yaml          | 31 ++++++++++++++++---
 include/dt-bindings/i3c/i3c.h                 |  3 ++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
index e25fa72fd785..225844abdd1f 100644
--- a/Documentation/devicetree/bindings/i3c/i3c.yaml
+++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
@@ -31,10 +31,12 @@ properties:
       described in the device tree, which in turn means we have to describe
       I3C devices.
 
-      Another use case for describing an I3C device in the device tree is when
-      this I3C device has a static I2C address and we want to assign it a
-      specific I3C dynamic address before the DAA takes place (so that other
-      devices on the bus can't take this dynamic address).
+      Other use-cases for describing an I3C device in the device tree are:
+      - When the I3C device has a static I2C address and we want to assign
+        it a specific I3C dynamic address before the DAA takes place (so
+        that other devices on the bus can't take this dynamic address).
+      - When the I3C device requires SETAASA for its discovery and uses a
+        pre-defined static address.
 
   "#size-cells":
     const: 0
@@ -147,6 +149,27 @@ patternProperties:
           through SETDASA. If static address is not present, this address is assigned
           through SETNEWDA after assigning a temporary address via ENTDAA.
 
+      mipi-i3c-static-method:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0x1
+        maximum: 0x7
+        default: 1
+        description: |
+          Bitmap describing which methods of Dynamic Address Assignment from a
+          static address are supported by this I3C Target. A bit value of 1
+          indicates support for that method, and 0 indicates lack of support.
+
+            Bit 0: SETDASA CCC (Direct)
+            Bit 1: SETAASA CCC (Broadcast)
+            Bit 2: Other CCC (vendor / standards extension)
+            All other bits are reserved.
+
+          This property follows the MIPI I3C specification. The primary use
+          of this property is to indicate support for SETAASA, i.e Bit 1, but
+          will allow other values mentioned in the specification so that it
+          mirrors the specification. SETDASA will remain as the default method
+          even if this property is not present.
+
     required:
       - reg
 
diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
index 373439218bba..b4ceb9827ec0 100644
--- a/include/dt-bindings/i3c/i3c.h
+++ b/include/dt-bindings/i3c/i3c.h
@@ -13,4 +13,7 @@
 #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
 #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
 
+#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
+#define I3C_ADDR_METHOD_SETAASA     (1 << 1)
+
 #endif
-- 
2.50.1

[PATCH v3 02/13] ACPICA: Read LVR from the I2C resource descriptor

[Akhil R]

ACPI 6.3 specifies byte 8 of I2C Serial Bus Connection descriptor to be
used for Legacy Virtual Register (LVR) data as specified in the MIPI
I3C Specification for an I2C device connected to an I3C Host Controller.
LVR will be read by I3C host controller drivers and it provides details
about the specific speed and 50ns spike filter capabilities of I2C
devices.

Update the rsconvert_info to include this field. For I2C devices on an
I2C bus, this field is Reserved and unused.

Link: https://github.com/acpica/acpica/commit/70082dc8
Link: https://github.com/acpica/acpica/commit/b3c38dc9
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> # ACPI
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/acpi/acpica/rsserial.c | 6 +++++-
 include/acpi/acrestyp.h        | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/rsserial.c b/drivers/acpi/acpica/rsserial.c
index 279bfa27da94..c06e918ab889 100644
--- a/drivers/acpi/acpica/rsserial.c
+++ b/drivers/acpi/acpica/rsserial.c
@@ -315,7 +315,7 @@ struct acpi_rsconvert_info acpi_rs_convert_csi2_serial_bus[14] = {
  *
  ******************************************************************************/
 
-struct acpi_rsconvert_info acpi_rs_convert_i2c_serial_bus[17] = {
+struct acpi_rsconvert_info acpi_rs_convert_i2c_serial_bus[18] = {
 	{ACPI_RSC_INITGET, ACPI_RESOURCE_TYPE_SERIAL_BUS,
 	 ACPI_RS_SIZE(struct acpi_resource_i2c_serialbus),
 	 ACPI_RSC_TABLE_SIZE(acpi_rs_convert_i2c_serial_bus)},
@@ -391,6 +391,10 @@ struct acpi_rsconvert_info acpi_rs_convert_i2c_serial_bus[17] = {
 	 AML_OFFSET(i2c_serial_bus.type_specific_flags),
 	 0},
 
+	{ACPI_RSC_MOVE8, ACPI_RS_OFFSET(data.i2c_serial_bus.lvr),
+	 AML_OFFSET(i2c_serial_bus.type_specific_flags) + 1,
+	 1},
+
 	{ACPI_RSC_MOVE32, ACPI_RS_OFFSET(data.i2c_serial_bus.connection_speed),
 	 AML_OFFSET(i2c_serial_bus.connection_speed),
 	 1},
diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h
index 842f932e2c2b..38a19b1d19ac 100644
--- a/include/acpi/acrestyp.h
+++ b/include/acpi/acrestyp.h
@@ -423,6 +423,7 @@ struct acpi_resource_i2c_serialbus {
 	ACPI_RESOURCE_SERIAL_COMMON u8 access_mode;
 	u16 slave_address;
 	u32 connection_speed;
+	u8 lvr;
 };
 
 /* Values for access_mode field above */
-- 
2.50.1

[PATCH v3 03/13] i3c: master: Use unified device property interface

[Akhil R]

Replace all OF-specific functions with unified device property functions
as a prerequisite to support both ACPI and device tree.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c       | 77 +++++++++++++++++++++-----------------
 include/linux/i3c/master.h |  5 ++-
 2 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5cd4e5da2233..d0677061faab 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -11,10 +11,12 @@
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/of.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -497,7 +499,7 @@ static void i3c_bus_cleanup(struct i3c_bus *i3cbus)
 	mutex_unlock(&i3c_core_lock);
 }
 
-static int i3c_bus_init(struct i3c_bus *i3cbus, struct device_node *np)
+static int i3c_bus_init(struct i3c_bus *i3cbus, struct fwnode_handle *fwnode)
 {
 	int ret, start, end, id = -1;
 
@@ -507,8 +509,8 @@ static int i3c_bus_init(struct i3c_bus *i3cbus, struct device_node *np)
 	i3c_bus_init_addrslots(i3cbus);
 	i3cbus->mode = I3C_BUS_MODE_PURE;
 
-	if (np)
-		id = of_alias_get_id(np, "i3c");
+	if (fwnode && is_of_node(fwnode))
+		id = of_alias_get_id(to_of_node(fwnode), "i3c");
 
 	mutex_lock(&i3c_core_lock);
 	if (id >= 0) {
@@ -811,7 +813,7 @@ static void i3c_masterdev_release(struct device *dev)
 	WARN_ON(!list_empty(&bus->devs.i2c) || !list_empty(&bus->devs.i3c));
 	i3c_bus_cleanup(bus);
 
-	of_node_put(dev->of_node);
+	fwnode_handle_put(dev->fwnode);
 }
 
 static const struct device_type i3c_masterdev_type = {
@@ -993,7 +995,7 @@ static void i3c_device_release(struct device *dev)
 
 	WARN_ON(i3cdev->desc);
 
-	of_node_put(i3cdev->dev.of_node);
+	fwnode_handle_put(dev->fwnode);
 	kfree(i3cdev);
 }
 
@@ -1789,7 +1791,7 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 			     desc->info.pid);
 
 		if (desc->boardinfo)
-			desc->dev->dev.of_node = desc->boardinfo->of_node;
+			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
 
 		ret = device_register(&desc->dev->dev);
 		if (ret) {
@@ -2408,8 +2410,8 @@ EXPORT_SYMBOL_GPL(i3c_master_add_i3c_dev_locked);
 #define OF_I3C_REG1_IS_I2C_DEV			BIT(31)
 
 static int
-of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
-				struct device_node *node, u32 *reg)
+i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
+			     struct fwnode_handle *fwnode, u32 *reg)
 {
 	struct i2c_dev_boardinfo *boardinfo;
 	struct device *dev = &master->dev;
@@ -2419,9 +2421,13 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 	if (!boardinfo)
 		return -ENOMEM;
 
-	ret = of_i2c_get_board_info(dev, node, &boardinfo->base);
-	if (ret)
-		return ret;
+	if (is_of_node(fwnode)) {
+		ret = of_i2c_get_board_info(dev, to_of_node(fwnode), &boardinfo->base);
+		if (ret)
+			return ret;
+	} else {
+		return -EINVAL;
+	}
 
 	/*
 	 * The I3C Specification does not clearly say I2C devices with 10-bit
@@ -2437,14 +2443,14 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 	boardinfo->lvr = reg[2];
 
 	list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
-	of_node_get(node);
+	fwnode_handle_get(fwnode);
 
 	return 0;
 }
 
 static int
-of_i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
-				struct device_node *node, u32 *reg)
+i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
+			     struct fwnode_handle *fwnode, u32 *reg)
 {
 	struct i3c_dev_boardinfo *boardinfo;
 	struct device *dev = &master->dev;
@@ -2467,7 +2473,7 @@ of_i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 
 	boardinfo->static_addr = reg[0];
 
-	if (!of_property_read_u32(node, "assigned-address", &init_dyn_addr)) {
+	if (!fwnode_property_read_u32(fwnode, "assigned-address", &init_dyn_addr)) {
 		if (init_dyn_addr > I3C_MAX_ADDR)
 			return -EINVAL;
 
@@ -2484,14 +2490,14 @@ of_i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 		return -EINVAL;
 
 	boardinfo->init_dyn_addr = init_dyn_addr;
-	boardinfo->of_node = of_node_get(node);
+	boardinfo->fwnode = fwnode_handle_get(fwnode);
 	list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
 
 	return 0;
 }
 
-static int of_i3c_master_add_dev(struct i3c_master_controller *master,
-				 struct device_node *node)
+static int i3c_master_add_dev(struct i3c_master_controller *master,
+			      struct fwnode_handle *fwnode)
 {
 	u32 reg[3];
 	int ret;
@@ -2499,7 +2505,7 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
 	if (!master)
 		return -EINVAL;
 
-	ret = of_property_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg));
+	ret = fwnode_property_read_u32_array(fwnode, "reg", reg, ARRAY_SIZE(reg));
 	if (ret)
 		return ret;
 
@@ -2508,25 +2514,25 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
 	 * dealing with an I2C device.
 	 */
 	if (!reg[1])
-		ret = of_i3c_master_add_i2c_boardinfo(master, node, reg);
+		ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
 	else
-		ret = of_i3c_master_add_i3c_boardinfo(master, node, reg);
+		ret = i3c_master_add_i3c_boardinfo(master, fwnode, reg);
 
 	return ret;
 }
 
-static int of_populate_i3c_bus(struct i3c_master_controller *master)
+static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
 {
 	struct device *dev = &master->dev;
-	struct device_node *i3cbus_np = dev->of_node;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
 	int ret;
 	u32 val;
 
-	if (!i3cbus_np)
+	if (!fwnode)
 		return 0;
 
-	for_each_available_child_of_node_scoped(i3cbus_np, node) {
-		ret = of_i3c_master_add_dev(master, node);
+	fwnode_for_each_available_child_node_scoped(fwnode, child) {
+		ret = i3c_master_add_dev(master, child);
 		if (ret)
 			return ret;
 	}
@@ -2536,10 +2542,10 @@ static int of_populate_i3c_bus(struct i3c_master_controller *master)
 	 * on the bus are not supporting typical rates, or if the bus topology
 	 * prevents it from using max possible rate.
 	 */
-	if (!of_property_read_u32(i3cbus_np, "i2c-scl-hz", &val))
+	if (!device_property_read_u32(dev, "i2c-scl-hz", &val))
 		master->bus.scl_rate.i2c = val;
 
-	if (!of_property_read_u32(i3cbus_np, "i3c-scl-hz", &val))
+	if (!device_property_read_u32(dev, "i3c-scl-hz", &val))
 		master->bus.scl_rate.i3c = val;
 
 	return 0;
@@ -2594,7 +2600,7 @@ static u8 i3c_master_i2c_get_lvr(struct i2c_client *client)
 	u8 lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
 	u32 reg[3];
 
-	if (!of_property_read_u32_array(client->dev.of_node, "reg", reg, ARRAY_SIZE(reg)))
+	if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg", reg, ARRAY_SIZE(reg)))
 		lvr = reg[2];
 
 	return lvr;
@@ -2713,7 +2719,8 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
 	struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master);
 	struct i2c_dev_desc *i2cdev;
 	struct i2c_dev_boardinfo *i2cboardinfo;
-	int ret, id;
+	struct fwnode_handle *fwnode = dev_fwnode(&master->dev);
+	int ret, id = -1;
 
 	adap->dev.parent = master->dev.parent;
 	adap->owner = master->dev.parent->driver->owner;
@@ -2722,7 +2729,9 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
 	adap->timeout = HZ;
 	adap->retries = 3;
 
-	id = of_alias_get_id(master->dev.of_node, "i2c");
+	if (fwnode && is_of_node(fwnode))
+		id = of_alias_get_id(to_of_node(fwnode), "i2c");
+
 	if (id >= 0) {
 		adap->nr = id;
 		ret = i2c_add_numbered_adapter(adap);
@@ -3023,7 +3032,7 @@ int i3c_master_register(struct i3c_master_controller *master,
 		return ret;
 
 	master->dev.parent = parent;
-	master->dev.of_node = of_node_get(parent->of_node);
+	device_set_node(&master->dev, fwnode_handle_get(dev_fwnode(parent)));
 	master->dev.bus = &i3c_bus_type;
 	master->dev.type = &i3c_masterdev_type;
 	master->dev.release = i3c_masterdev_release;
@@ -3042,13 +3051,13 @@ int i3c_master_register(struct i3c_master_controller *master,
 	master->dev.coherent_dma_mask = parent->coherent_dma_mask;
 	master->dev.dma_parms = parent->dma_parms;
 
-	ret = i3c_bus_init(i3cbus, master->dev.of_node);
+	ret = i3c_bus_init(i3cbus, dev_fwnode(&master->dev));
 	if (ret)
 		goto err_put_dev;
 
 	dev_set_name(&master->dev, "i3c-%d", i3cbus->id);
 
-	ret = of_populate_i3c_bus(master);
+	ret = fwnode_populate_i3c_bus(master);
 	if (ret)
 		goto err_put_dev;
 
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 592b646f6134..6b03a3ce574c 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -177,7 +177,8 @@ struct i3c_device_ibi_info {
  * @pid: I3C Provisioned ID exposed by the device. This is a unique identifier
  *	 that may be used to attach boardinfo to i3c_dev_desc when the device
  *	 does not have a static address
- * @of_node: optional DT node in case the device has been described in the DT
+ * @fwnode: Firmware node (DT or ACPI) in case the device has been
+ *	    described in firmware
  *
  * This structure is used to attach board-level information to an I3C device.
  * Not all I3C devices connected on the bus will have a boardinfo. It's only
@@ -189,7 +190,7 @@ struct i3c_dev_boardinfo {
 	u8 init_dyn_addr;
 	u8 static_addr;
 	u64 pid;
-	struct device_node *of_node;
+	struct fwnode_handle *fwnode;
 };
 
 /**
-- 
2.50.1

[PATCH v3 04/13] i3c: master: Support ACPI enumeration of child devices

[Akhil R]

Although the existing subsystem allows host controllers to register
through the ACPI table, it was not possible to describe I3C or I2C devices
when using ACPI. This is because the driver relied on the reg property to
retrieve the PID, static address, etc., whereas ACPI uses _ADR or serial
resources to describe such devices.

Read _ADR and LVR from the ACPI resources and extract the data as per the
ACPI specification for an I3C bus. Also read mipi-i3c-static-address as
per the MIPI DISCO specifications [1] to get the static address to be
used. Hence enable describing the I3C or I2C devices in the ACPI table,
which is required if the device is using a static address or if it needs
some specific properties to be attached to it.

[1] https://www.mipi.org/mipi-disco-for-i3c-download

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c | 140 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 132 insertions(+), 8 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index d0677061faab..ffec97157d48 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -5,6 +5,7 @@
  * Author: Boris Brezillon <boris.brezillon@bootlin.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/atomic.h>
 #include <linux/bug.h>
 #include <linux/device.h>
@@ -2409,6 +2410,53 @@ EXPORT_SYMBOL_GPL(i3c_master_add_i3c_dev_locked);
 
 #define OF_I3C_REG1_IS_I2C_DEV			BIT(31)
 
+#ifdef CONFIG_ACPI
+static int i3c_acpi_get_i2c_resource(struct acpi_resource *ares, void *data)
+{
+	struct i2c_dev_boardinfo *boardinfo = data;
+	struct acpi_resource_i2c_serialbus *sb;
+
+	if (!i2c_acpi_get_i2c_resource(ares, &sb))
+		return 1;
+
+	boardinfo->base.addr = sb->slave_address;
+	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
+		boardinfo->base.flags |= I2C_CLIENT_TEN;
+
+	boardinfo->lvr = sb->lvr;
+
+	return 0;
+}
+
+static int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
+				      struct fwnode_handle *fwnode)
+{
+	struct acpi_device *adev = to_acpi_device_node(fwnode);
+	LIST_HEAD(resources);
+	int ret;
+
+	boardinfo->base.fwnode = acpi_fwnode_handle(adev);
+
+	ret = acpi_dev_get_resources(adev, &resources,
+				     i3c_acpi_get_i2c_resource, boardinfo);
+	if (ret < 0)
+		return ret;
+
+	acpi_dev_free_resource_list(&resources);
+
+	if (!boardinfo->base.addr)
+		return -ENODEV;
+
+	return 0;
+}
+#else
+static inline int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
+					     struct fwnode_handle *fwnode)
+{
+	return -ENODEV;
+}
+#endif
+
 static int
 i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 			     struct fwnode_handle *fwnode, u32 *reg)
@@ -2425,6 +2473,13 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 		ret = of_i2c_get_board_info(dev, to_of_node(fwnode), &boardinfo->base);
 		if (ret)
 			return ret;
+
+		/* LVR is encoded in reg[2] for Device Tree. */
+		boardinfo->lvr = reg[2];
+	} else if (is_acpi_device_node(fwnode)) {
+		ret = i3c_acpi_add_i2c_boardinfo(boardinfo, fwnode);
+		if (ret)
+			return ret;
 	} else {
 		return -EINVAL;
 	}
@@ -2439,9 +2494,6 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 		return -EOPNOTSUPP;
 	}
 
-	/* LVR is encoded in reg[2]. */
-	boardinfo->lvr = reg[2];
-
 	list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
 	fwnode_handle_get(fwnode);
 
@@ -2496,8 +2548,8 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 	return 0;
 }
 
-static int i3c_master_add_dev(struct i3c_master_controller *master,
-			      struct fwnode_handle *fwnode)
+static int i3c_master_add_of_dev(struct i3c_master_controller *master,
+				 struct fwnode_handle *fwnode)
 {
 	u32 reg[3];
 	int ret;
@@ -2521,6 +2573,67 @@ static int i3c_master_add_dev(struct i3c_master_controller *master,
 	return ret;
 }
 
+#ifdef CONFIG_ACPI
+static int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
+				   struct fwnode_handle *fwnode)
+{
+	struct acpi_device *adev = to_acpi_device_node(fwnode);
+	acpi_bus_address adr;
+	u32 reg[3] = { 0 };
+
+	/*
+	 * If the ACPI table entry does not have _ADR method, it's an I2C device
+	 * If the ACPI table entry has _ADR method, it's an I3C device
+	 */
+	if (!acpi_has_method(adev->handle, "_ADR"))
+		return i3c_master_add_i2c_boardinfo(master, fwnode, reg);
+
+	adr = acpi_device_adr(adev);
+
+	/* For I3C devices, _ADR will have the 48 bit PID of the device  */
+	reg[1] = upper_32_bits(adr);
+	reg[2] = lower_32_bits(adr);
+
+	fwnode_property_read_u32(fwnode, "mipi-i3c-static-address", &reg[0]);
+
+	return i3c_master_add_i3c_boardinfo(master, fwnode, reg);
+}
+
+static u8 i3c_acpi_i2c_get_lvr(struct i2c_client *client)
+{
+	struct acpi_device *adev = to_acpi_device_node(client->dev.fwnode);
+	struct i2c_dev_boardinfo boardinfo = {};
+	LIST_HEAD(resources);
+	int ret;
+	u8 lvr;
+
+	lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
+
+	ret = acpi_dev_get_resources(adev, &resources,
+				     i3c_acpi_get_i2c_resource, &boardinfo);
+	if (ret < 0)
+		return lvr;
+
+	if (boardinfo.base.addr)
+		lvr = boardinfo.lvr;
+
+	acpi_dev_free_resource_list(&resources);
+
+	return lvr;
+}
+#else
+static inline int i3c_master_add_acpi_dev(struct i3c_master_controller *master,
+					  struct fwnode_handle *fwnode)
+{
+	return -ENODEV;
+}
+
+static inline u8 i3c_acpi_i2c_get_lvr(struct i2c_client *client)
+{
+	return I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
+}
+#endif
+
 static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
 {
 	struct device *dev = &master->dev;
@@ -2532,7 +2645,13 @@ static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
 		return 0;
 
 	fwnode_for_each_available_child_node_scoped(fwnode, child) {
-		ret = i3c_master_add_dev(master, child);
+		if (is_of_node(child))
+			ret = i3c_master_add_of_dev(master, child);
+		else if (is_acpi_device_node(child))
+			ret = i3c_master_add_acpi_dev(master, child);
+		else
+			continue;
+
 		if (ret)
 			return ret;
 	}
@@ -2600,8 +2719,13 @@ static u8 i3c_master_i2c_get_lvr(struct i2c_client *client)
 	u8 lvr = I3C_LVR_I2C_INDEX(2) | I3C_LVR_I2C_FM_MODE;
 	u32 reg[3];
 
-	if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg", reg, ARRAY_SIZE(reg)))
-		lvr = reg[2];
+	if (is_of_node(client->dev.fwnode)) {
+		if (!fwnode_property_read_u32_array(client->dev.fwnode, "reg",
+						    reg, ARRAY_SIZE(reg)))
+			lvr = reg[2];
+	} else if (is_acpi_device_node(client->dev.fwnode)) {
+		lvr = i3c_acpi_i2c_get_lvr(client);
+	}
 
 	return lvr;
 }
-- 
2.50.1

[PATCH v3 05/13] i3c: master: Add support for devices using SETAASA

[Akhil R]

Add support for devices using SETAASA, such as SPD5118 and SPD5108
attached to DDR5 memory modules that do not support ENTDAA. Follow the
guidelines proposed by the MIPI Discovery and Configuration
Specification [1] for discovering such devices.

SETAASA (Set All Addresses to Static Address) differs from standard I3C
address assignment that uses ENTDAA or SETDASA to assign dynamic
addresses. Devices using SETAASA assign their pre-defined static addresses
as their dynamic addresses during DAA, and it is not mandatory for these
devices to implement standard CCC commands like GETPID, GETDCR, or GETBCR.
For such devices, it is generally recommended to issue SETHID (specified
by JEDEC JESD300) as a prerequisite for SETAASA to stop HID bit flipping.

[1] https://www.mipi.org/mipi-disco-for-i3c-download

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c       | 70 +++++++++++++++++++++++++++++++++++++-
 include/linux/i3c/ccc.h    |  1 +
 include/linux/i3c/master.h | 15 ++++++++
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index ffec97157d48..de72317c9a5c 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -5,6 +5,7 @@
  * Author: Boris Brezillon <boris.brezillon@bootlin.com>
  */
 
+#include <dt-bindings/i3c/i3c.h>
 #include <linux/acpi.h>
 #include <linux/atomic.h>
 #include <linux/bug.h>
@@ -1051,6 +1052,47 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
 	return ret;
 }
 
+/**
+ * i3c_master_setaasa_locked() - start a SETAASA procedure (Set All Addresses to Static Address)
+ * @master: I3C master object
+ *
+ * Send a SETAASA CCC command to set all attached I3C devices' dynamic addresses to
+ * their static address.
+ *
+ * This function must be called with the bus lock held in write mode.
+ *
+ * First, the SETHID CCC command is sent, followed by the SETAASA CCC.
+ *
+ * Return: 0 in case of success, a positive I3C error code if the error is
+ * one of the official Mx error codes, and a negative error code otherwise.
+ */
+static int i3c_master_setaasa_locked(struct i3c_master_controller *master)
+{
+	struct i3c_ccc_cmd_dest dest;
+	struct i3c_ccc_cmd cmd;
+	int ret;
+
+	/*
+	 * Send SETHID CCC command. Though it is a standard CCC command specified
+	 * in JESD300-5, we are not defining a separate macro to be explicit that
+	 * the value falls under the vendor specific range.
+	 */
+	i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
+	i3c_ccc_cmd_init(&cmd, false, I3C_CCC_VENDOR(0, true), &dest, 1);
+	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
+	i3c_ccc_cmd_dest_cleanup(&dest);
+	if (ret)
+		return ret;
+
+	/* Send SETAASA CCC command */
+	i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
+	i3c_ccc_cmd_init(&cmd, false, I3C_CCC_SETAASA, &dest, 1);
+	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
+	i3c_ccc_cmd_dest_cleanup(&dest);
+
+	return ret;
+}
+
 /**
  * i3c_master_entdaa_locked() - start a DAA (Dynamic Address Assignment)
  *				procedure
@@ -1739,6 +1781,18 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
 	if (ret)
 		goto err_free_dev;
 
+	/*
+	 * For devices using SETAASA instead of ENTDAA, the address is statically
+	 * assigned. Update the dynamic address to the provided static address.
+	 * Reattaching the I3C device is not useful. It is also not mandatory
+	 * for such devices to implement CCC commands like GETPID, GETDCR etc.
+	 * Hence, we can return here.
+	 */
+	if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+		i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
+		return 0;
+	}
+
 	ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
 					i3cdev->boardinfo->init_dyn_addr);
 	if (ret)
@@ -2145,6 +2199,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
 	if (ret)
 		goto err_bus_cleanup;
 
+	if (master->addr_method & I3C_ADDR_METHOD_SETAASA) {
+		ret = i3c_master_setaasa_locked(master);
+		if (ret)
+			goto err_bus_cleanup;
+	}
+
 	/*
 	 * Reserve init_dyn_addr first, and then try to pre-assign dynamic
 	 * address and retrieve device information if needed.
@@ -2507,7 +2567,7 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 	struct i3c_dev_boardinfo *boardinfo;
 	struct device *dev = &master->dev;
 	enum i3c_addr_slot_status addrstatus;
-	u32 init_dyn_addr = 0;
+	u32 init_dyn_addr = 0, static_addr_method = 0;
 
 	boardinfo = devm_kzalloc(dev, sizeof(*boardinfo), GFP_KERNEL);
 	if (!boardinfo)
@@ -2535,6 +2595,13 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 			return -EINVAL;
 	}
 
+	if (!fwnode_property_read_u32(fwnode, "mipi-i3c-static-method", &static_addr_method))
+		boardinfo->static_addr_method = static_addr_method &
+					(I3C_ADDR_METHOD_SETDASA | I3C_ADDR_METHOD_SETAASA);
+
+	/* Update the address methods required for device discovery */
+	master->addr_method |= boardinfo->static_addr_method;
+
 	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
 
 	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
@@ -3162,6 +3229,7 @@ int i3c_master_register(struct i3c_master_controller *master,
 	master->dev.release = i3c_masterdev_release;
 	master->ops = ops;
 	master->secondary = secondary;
+	master->addr_method = I3C_ADDR_METHOD_SETDASA;
 	INIT_LIST_HEAD(&master->boardinfo.i2c);
 	INIT_LIST_HEAD(&master->boardinfo.i3c);
 
diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
index ad59a4ae60d1..a145d766ab6f 100644
--- a/include/linux/i3c/ccc.h
+++ b/include/linux/i3c/ccc.h
@@ -32,6 +32,7 @@
 #define I3C_CCC_DEFSLVS			I3C_CCC_ID(0x8, true)
 #define I3C_CCC_ENTTM			I3C_CCC_ID(0xb, true)
 #define I3C_CCC_ENTHDR(x)		I3C_CCC_ID(0x20 + (x), true)
+#define I3C_CCC_SETAASA			I3C_CCC_ID(0x29, true)
 
 /* Unicast-only commands */
 #define I3C_CCC_SETDASA			I3C_CCC_ID(0x7, false)
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 6b03a3ce574c..5a6d217fa5eb 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -174,6 +174,14 @@ struct i3c_device_ibi_info {
  *		 assigned a dynamic address by the master. Will be used during
  *		 bus initialization to assign it a specific dynamic address
  *		 before starting DAA (Dynamic Address Assignment)
+ * @static_addr_method: Bitmap describing which methods of Dynamic Address
+ *		 Assignment from a Static Address are supported by this I3C Target.
+ *		 A value of 1 in a bit position indicates that the Bus Controller
+ *		 supports that method, and a value of 0 indicates that the Bus
+ *		 Controller does not support that method.
+ *		 Bit 0: SETDASA
+ *		 Bit 1: SETAASA
+ *		 All other bits are reserved.
  * @pid: I3C Provisioned ID exposed by the device. This is a unique identifier
  *	 that may be used to attach boardinfo to i3c_dev_desc when the device
  *	 does not have a static address
@@ -189,6 +197,7 @@ struct i3c_dev_boardinfo {
 	struct list_head node;
 	u8 init_dyn_addr;
 	u8 static_addr;
+	u8 static_addr_method;
 	u64 pid;
 	struct fwnode_handle *fwnode;
 };
@@ -516,6 +525,11 @@ struct i3c_master_controller_ops {
  * @boardinfo.i2c: list of I2C boardinfo objects
  * @boardinfo: board-level information attached to devices connected on the bus
  * @bus: I3C bus exposed by this master
+ * @addr_method: Bitmap describing which methods of Address Assignment required
+ *		 to be run for discovering all the devices on the bus.
+ *		 Bit 0: SETDASA
+ *		 Bit 1: SETAASA
+ *		 All other bits are reserved.
  * @wq: workqueue which can be used by master
  *	drivers if they need to postpone operations that need to take place
  *	in a thread context. Typical examples are Hot Join processing which
@@ -543,6 +557,7 @@ struct i3c_master_controller {
 		struct list_head i2c;
 	} boardinfo;
 	struct i3c_bus bus;
+	u8 addr_method;
 	struct workqueue_struct *wq;
 	unsigned int dev_nack_retry_count;
 };
-- 
2.50.1

[PATCH v3 06/13] i3c: master: Add support for devices without PID

[Akhil R]

Devices using SETAASA for address assignment are not required to have
a 48-bit PID according to the I3C specification. Allow such devices to
register and use the static address where PID was required.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c | 51 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index de72317c9a5c..fbe8710c4c56 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1842,8 +1842,17 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		desc->dev->dev.type = &i3c_device_type;
 		desc->dev->dev.bus = &i3c_bus_type;
 		desc->dev->dev.release = i3c_device_release;
-		dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
-			     desc->info.pid);
+
+		/*
+		 * For devices without PID (e.g., SETAASA devices), use
+		 * static address for naming instead.
+		 */
+		if (desc->info.pid)
+			dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
+				     desc->info.pid);
+		else
+			dev_set_name(&desc->dev->dev, "%d-%02x", master->bus.id,
+				     desc->info.static_addr);
 
 		if (desc->boardinfo)
 			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
@@ -2288,8 +2297,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
 	struct i3c_dev_boardinfo *i3cboardinfo;
 
 	list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
-		if (i3cdev->info.pid != i3cboardinfo->pid)
-			continue;
+		/*
+		 * For devices without PID (e.g., SETAASA devices), match by
+		 * static address. For devices with PID, match by PID.
+		 */
+		if (i3cboardinfo->pid) {
+			if (i3cdev->info.pid != i3cboardinfo->pid)
+				continue;
+		} else {
+			if (!i3cboardinfo->static_addr ||
+			    i3cdev->info.static_addr != i3cboardinfo->static_addr)
+				continue;
+		}
 
 		i3cdev->boardinfo = i3cboardinfo;
 		i3cdev->info.static_addr = i3cboardinfo->static_addr;
@@ -2303,8 +2322,12 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
 	struct i3c_master_controller *master = i3c_dev_get_master(refdev);
 	struct i3c_dev_desc *i3cdev;
 
+	if (!refdev->info.pid)
+		return NULL;
+
 	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
-		if (i3cdev != refdev && i3cdev->info.pid == refdev->info.pid)
+		if (i3cdev != refdev && i3cdev->info.pid &&
+		    i3cdev->info.pid == refdev->info.pid)
 			return i3cdev;
 	}
 
@@ -2604,9 +2627,15 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 
 	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
 
-	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
-	    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
-		return -EINVAL;
+	/* For SETAASA devices, validate the static address instead of PID */
+	if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+		if (!boardinfo->static_addr)
+			return -EINVAL;
+	} else {
+		if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
+		    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
+			return -EINVAL;
+	}
 
 	boardinfo->init_dyn_addr = init_dyn_addr;
 	boardinfo->fwnode = fwnode_handle_get(fwnode);
@@ -2629,10 +2658,10 @@ static int i3c_master_add_of_dev(struct i3c_master_controller *master,
 		return ret;
 
 	/*
-	 * The manufacturer ID can't be 0. If reg[1] == 0 that means we're
-	 * dealing with an I2C device.
+	 * I3C device should have either the manufacturer ID specified or the
+	 * address discovery method specified. Else treat it as an I2C device.
 	 */
-	if (!reg[1])
+	if (!reg[1] && !fwnode_property_present(fwnode, "mipi-i3c-static-method"))
 		ret = i3c_master_add_i2c_boardinfo(master, fwnode, reg);
 	else
 		ret = i3c_master_add_i3c_boardinfo(master, fwnode, reg);
-- 
2.50.1

[PATCH v3 07/13] i3c: master: match I3C device through DT and ACPI

[Akhil R]

SETAASA-based devices cannot always be identified by PID or DCR; the
standard I3C id_table matching may not be applicable. Allow such devices
to be matched through Device Tree or ACPI.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index fbe8710c4c56..0be83bfdb563 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/slab.h>
@@ -343,15 +344,32 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
 {
 	struct i3c_device *i3cdev;
 	const struct i3c_driver *i3cdrv;
+	u8 static_addr_method = 0;
 
 	if (dev->type != &i3c_device_type)
 		return 0;
 
 	i3cdev = dev_to_i3cdev(dev);
 	i3cdrv = drv_to_i3cdrv(drv);
-	if (i3c_device_match_id(i3cdev, i3cdrv->id_table))
+
+	if (i3cdev->desc && i3cdev->desc->boardinfo)
+		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
+
+	/*
+	 * SETAASA based device need not always have a matching ID since
+	 * it is not mandatory for such devices to implement deviceinfo
+	 * CCC commands. Allow them to register through DT or ACPI.
+	 */
+	if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))
 		return 1;
 
+	if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {
+		if (of_driver_match_device(dev, drv))
+			return 1;
+		if (acpi_driver_match_device(dev, drv))
+			return 1;
+	}
+
 	return 0;
 }
 
-- 
2.50.1

[PATCH v3 08/13] i3c: dw-i3c-master: Add SETAASA as supported CCC

[Akhil R]

Add SETAASA and SETHID to the supported list of CCC commands for
DesignWare I3C host controller.

SETAASA is a broadcast command that assigns predefined static addresses
to all I3C devices on the bus.

SETHID is to stop HID bit flipping by the SPD Hub to which the SPD devices
are connected. It is a prerequisite command to be sent before SETAASA as
recommended by JESD300-5 and JESD403 sideband bus specifications.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master/dw-i3c-master.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 655693a2187e..edd42daf7553 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -309,6 +309,8 @@ static bool dw_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
 	case I3C_CCC_GETSTATUS:
 	case I3C_CCC_GETMXDS:
 	case I3C_CCC_GETHDRCAP:
+	case I3C_CCC_SETAASA:
+	case I3C_CCC_VENDOR(0, true): /* SETHID */
 		return true;
 	default:
 		return false;
-- 
2.50.1

[PATCH v3 09/13] i3c: dw-i3c-master: Add a quirk to skip clock and reset

[Akhil R]

Some ACPI-enumerated devices like Tegra410 do not have clock and reset
resources exposed via the clk/reset frameworks. Unlike device tree, ACPI
on Arm does not model such provider functions. The hardware is expected
to be brought out of reset and have its clocks enabled by the firmware
before the OS takes over. Any data to be shared with the OS is passed
using the _DSD property.

Add a match data for such devices to skip acquiring clock and reset
controls during probe and read the clock rate from the "clock-frequency"
_DSD property. Note that the "clock-frequency" here is the controller's
core clock and not the bus speed. I3C specifies the bus speed separately
using "i3c-scl-hz" and "i2c-scl-hz" and hence this should not cause any
conflict.

Also, move match data parsing before clock/reset acquisition so the quirk
is available early enough.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master/dw-i3c-master.c | 57 ++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index edd42daf7553..b90756ade2db 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -242,6 +242,7 @@
 /* List of quirks */
 #define AMD_I3C_OD_PP_TIMING		BIT(1)
 #define DW_I3C_DISABLE_RUNTIME_PM_QUIRK	BIT(2)
+#define DW_I3C_ACPI_SKIP_CLK_RST		BIT(3)
 
 struct dw_i3c_cmd {
 	u32 cmd_lo;
@@ -556,13 +557,26 @@ static void dw_i3c_master_set_intr_regs(struct dw_i3c_master *master)
 	writel(IBI_REQ_REJECT_ALL, master->regs + IBI_MR_REQ_REJECT);
 }
 
+static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
+{
+	unsigned int core_rate_prop;
+
+	if (!(master->quirks & DW_I3C_ACPI_SKIP_CLK_RST))
+		return clk_get_rate(master->core_clk);
+
+	if (device_property_read_u32(master->dev, "clock-frequency", &core_rate_prop))
+		return 0;
+
+	return core_rate_prop;
+}
+
 static int dw_i3c_clk_cfg(struct dw_i3c_master *master)
 {
 	unsigned long core_rate, core_period;
 	u32 scl_timing;
 	u8 hcnt, lcnt;
 
-	core_rate = clk_get_rate(master->core_clk);
+	core_rate = dw_i3c_master_get_core_rate(master);
 	if (!core_rate)
 		return -EINVAL;
 
@@ -615,7 +629,7 @@ static int dw_i2c_clk_cfg(struct dw_i3c_master *master)
 	u16 hcnt, lcnt;
 	u32 scl_timing;
 
-	core_rate = clk_get_rate(master->core_clk);
+	core_rate = dw_i3c_master_get_core_rate(master);
 	if (!core_rate)
 		return -EINVAL;
 
@@ -1580,19 +1594,33 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	if (IS_ERR(master->regs))
 		return PTR_ERR(master->regs);
 
-	master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
-	if (IS_ERR(master->core_clk))
-		return PTR_ERR(master->core_clk);
+	if (has_acpi_companion(&pdev->dev)) {
+		quirks = (unsigned long)device_get_match_data(&pdev->dev);
+	} else if (pdev->dev.of_node) {
+		drvdata = device_get_match_data(&pdev->dev);
+		if (drvdata)
+			quirks = drvdata->flags;
+	}
+	master->quirks = quirks;
+
+	if (master->quirks & DW_I3C_ACPI_SKIP_CLK_RST) {
+		master->core_clk = NULL;
+		master->core_rst = NULL;
+	} else {
+		master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
+		if (IS_ERR(master->core_clk))
+			return PTR_ERR(master->core_clk);
+
+		master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
+											"core_rst");
+		if (IS_ERR(master->core_rst))
+			return PTR_ERR(master->core_rst);
+	}
 
 	master->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk");
 	if (IS_ERR(master->pclk))
 		return PTR_ERR(master->pclk);
 
-	master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
-										"core_rst");
-	if (IS_ERR(master->core_rst))
-		return PTR_ERR(master->core_rst);
-
 	spin_lock_init(&master->xferqueue.lock);
 	INIT_LIST_HEAD(&master->xferqueue.list);
 
@@ -1625,15 +1653,6 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	master->maxdevs = ret >> 16;
 	master->free_pos = GENMASK(master->maxdevs - 1, 0);
 
-	if (has_acpi_companion(&pdev->dev)) {
-		quirks = (unsigned long)device_get_match_data(&pdev->dev);
-	} else if (pdev->dev.of_node) {
-		drvdata = device_get_match_data(&pdev->dev);
-		if (drvdata)
-			quirks = drvdata->flags;
-	}
-	master->quirks = quirks;
-
 	/* Keep controller enabled by preventing runtime suspend */
 	if (master->quirks & DW_I3C_DISABLE_RUNTIME_PM_QUIRK)
 		pm_runtime_get_noresume(&pdev->dev);
-- 
2.50.1

[PATCH v3 10/13] i3c: dw-i3c-master: Add ACPI ID for Tegra410

[Akhil R]

Update variable names to generic names and add Tegra410 ACPI ID to
support the I3C controller in Tegra410, which is a DesignWare I3C host
controller.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master/dw-i3c-master.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index b90756ade2db..e63822df2ae4 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1847,11 +1847,12 @@ static const struct of_device_id dw_i3c_master_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, dw_i3c_master_of_match);
 
-static const struct acpi_device_id amd_i3c_device_match[] = {
+static const struct acpi_device_id dw_i3c_master_acpi_match[] = {
 	{ "AMDI0015", AMD_I3C_OD_PP_TIMING },
+	{ "NVDA2018", DW_I3C_ACPI_SKIP_CLK_RST },
 	{ }
 };
-MODULE_DEVICE_TABLE(acpi, amd_i3c_device_match);
+MODULE_DEVICE_TABLE(acpi, dw_i3c_master_acpi_match);
 
 static struct platform_driver dw_i3c_driver = {
 	.probe = dw_i3c_probe,
@@ -1860,7 +1861,7 @@ static struct platform_driver dw_i3c_driver = {
 	.driver = {
 		.name = "dw-i3c-master",
 		.of_match_table = dw_i3c_master_of_match,
-		.acpi_match_table = amd_i3c_device_match,
+		.acpi_match_table = dw_i3c_master_acpi_match,
 		.pm = &dw_i3c_pm_ops,
 	},
 };
-- 
2.50.1

[PATCH v3 11/13] hwmon: spd5118: Remove 16-bit addressing

[Akhil R]

The intent of introducing 16-bit addressing was to support I3C, but it
turns out that I3C does not require reading the Legacy Mode register,
nor any specific encoding for page translation. The testing of 16-bit
code was limited and there are no known users for this feature. Remove
the sections that support 16-bit addressing and prepare the driver to
support I3C appropriately.

Suggested-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/hwmon/spd5118.c | 79 +++--------------------------------------
 1 file changed, 5 insertions(+), 74 deletions(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..8fdde35e68cd 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -66,9 +66,6 @@ static const unsigned short normal_i2c[] = {
 #define SPD5118_EEPROM_BASE		0x80
 #define SPD5118_EEPROM_SIZE		(SPD5118_PAGE_SIZE * SPD5118_NUM_PAGES)
 
-#define PAGE_ADDR0(page)		(((page) & BIT(0)) << 6)
-#define PAGE_ADDR1_4(page)		(((page) & GENMASK(4, 1)) >> 1)
-
 /* Temperature unit in millicelsius */
 #define SPD5118_TEMP_UNIT		(MILLIDEGREE_PER_DEGREE / 4)
 /* Representable temperature range in millicelsius */
@@ -78,7 +75,6 @@ static const unsigned short normal_i2c[] = {
 struct spd5118_data {
 	struct regmap *regmap;
 	struct mutex nvmem_lock;
-	bool is_16bit;
 };
 
 /* hwmon */
@@ -348,12 +344,7 @@ static ssize_t spd5118_nvmem_read_page(struct spd5118_data *data, char *buf,
 	if (offset + count > SPD5118_PAGE_SIZE)
 		count = SPD5118_PAGE_SIZE - offset;
 
-	if (data->is_16bit) {
-		addr = SPD5118_EEPROM_BASE | PAGE_ADDR0(page) |
-		  (PAGE_ADDR1_4(page) << 8);
-	} else {
-		addr = page * 0x100 + SPD5118_EEPROM_BASE;
-	}
+	addr = page * 0x100 + SPD5118_EEPROM_BASE;
 	err = regmap_bulk_read(regmap, addr + offset, buf, count);
 	if (err)
 		return err;
@@ -473,15 +464,6 @@ static const struct regmap_config spd5118_regmap8_config = {
 	.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
 };
 
-static const struct regmap_config spd5118_regmap16_config = {
-	.reg_bits = 16,
-	.val_bits = 8,
-	.max_register = 0x7ff,
-	.writeable_reg = spd5118_writeable_reg,
-	.volatile_reg = spd5118_volatile_reg,
-	.cache_type = REGCACHE_MAPLE,
-};
-
 static int spd5118_suspend(struct device *dev)
 {
 	struct spd5118_data *data = dev_get_drvdata(dev);
@@ -519,8 +501,7 @@ static int spd5118_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
 
-static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
-				bool is_16bit)
+static int spd5118_common_probe(struct device *dev, struct regmap *regmap)
 {
 	unsigned int capability, revision, vendor, bank;
 	struct spd5118_data *data;
@@ -537,8 +518,6 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
 	if (!(capability & SPD5118_CAP_TS_SUPPORT))
 		return -ENODEV;
 
-	data->is_16bit = is_16bit;
-
 	err = regmap_read(regmap, SPD5118_REG_REVISION, &revision);
 	if (err)
 		return err;
@@ -680,69 +659,21 @@ static int spd5118_i2c_init(struct i2c_client *client)
 	return 0;
 }
 
-/*
- * 16-bit addressing note:
- *
- * If I2C_FUNC_I2C is not supported by an I2C adapter driver, regmap uses
- * SMBus operations as alternative. To simulate a read operation with a 16-bit
- * address, it writes the address using i2c_smbus_write_byte_data(), followed
- * by one or more calls to i2c_smbus_read_byte() to read the data.
- * Per spd5118 standard, a read operation after writing the address must start
- * with <Sr> (Repeat Start). However, a SMBus read byte operation starts with
- * <S> (Start). This resets the register address in the spd5118 chip. As result,
- * i2c_smbus_read_byte() always returns data from register address 0x00.
- *
- * A working alternative to access chips with 16-bit register addresses in the
- * absence of I2C_FUNC_I2C support is not known.
- *
- * For this reason, 16-bit addressing can only be supported with I2C if the
- * adapter supports I2C_FUNC_I2C.
- *
- * For I2C, the addressing mode selected by the BIOS must not be changed.
- * Experiments show that at least some PC BIOS versions will not change the
- * addressing mode on a soft reboot and end up in setup, claiming that some
- * configuration change happened. This will happen again after a power cycle,
- * which does reset the addressing mode. To prevent this from happening,
- * detect if 16-bit addressing is enabled and always use the currently
- * configured addressing mode.
- */
-
 static int spd5118_i2c_probe(struct i2c_client *client)
 {
-	const struct regmap_config *config;
 	struct device *dev = &client->dev;
 	struct regmap *regmap;
-	int err, mode;
-	bool is_16bit;
+	int err;
 
 	err = spd5118_i2c_init(client);
 	if (err)
 		return err;
 
-	mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
-	if (mode < 0)
-		return mode;
-
-	is_16bit = mode & SPD5118_LEGACY_MODE_ADDR;
-	if (is_16bit) {
-		/*
-		 * See 16-bit addressing note above explaining why it is
-		 * necessary to check for I2C_FUNC_I2C support here.
-		 */
-		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
-			dev_err(dev, "Adapter does not support 16-bit register addresses\n");
-			return -ENODEV;
-		}
-		config = &spd5118_regmap16_config;
-	} else {
-		config = &spd5118_regmap8_config;
-	}
-
-	regmap = devm_regmap_init_i2c(client, config);
+	regmap = devm_regmap_init_i2c(client, &spd5118_regmap8_config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
 
-	return spd5118_common_probe(dev, regmap, is_16bit);
+	return spd5118_common_probe(dev, regmap);
 }
 
 static const struct i2c_device_id spd5118_i2c_id[] = {
-- 
2.50.1

[PATCH v3 12/13] hwmon: spd5118: Add I3C support

[Akhil R]

Add a regmap config and a probe function to support I3C-based
communication with SPD5118 devices.

On an I3C bus, SPD5118 devices are enumerated via SETAASA and always
require an ACPI or device tree entry. The device matching is hence
through the OF match tables only and do not need an I3C class match
table. The device identity is verified in the type registers before
proceeding to the common probe function.

Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/hwmon/Kconfig   |  9 ++++---
 drivers/hwmon/spd5118.c | 56 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 14e4cea48acc..a7f60f8868eb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2309,12 +2309,15 @@ config SENSORS_INA3221
 
 config SENSORS_SPD5118
 	tristate "SPD5118 Compliant Temperature Sensors"
-	depends on I2C
+	depends on I3C_OR_I2C
 	select REGMAP_I2C
+	select REGMAP_I3C if I3C
 	help
 	  If you say yes here you get support for SPD5118 (JEDEC JESD300)
-	  compliant temperature sensors. Such sensors are found on DDR5 memory
-	  modules.
+	  compliant temperature sensors using I2C or I3C bus interface.
+	  Such sensors are found on DDR5 memory modules.
+
+	  This driver supports both I2C and I3C interfaces.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called spd5118.
diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 8fdde35e68cd..db66028d2f59 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -18,6 +18,7 @@
 #include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/i3c/device.h>
 #include <linux/hwmon.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -464,6 +465,27 @@ static const struct regmap_config spd5118_regmap8_config = {
 	.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
 };
 
+/*
+ * SPD5118 2-byte register address format (JESD300-5, Tables 7 & 20):
+ *   Byte 1 (on wire first):  MemReg | BlkAddr[0] | Address[5:0]
+ *   Byte 2 (on wire second): 0000   | BlkAddr[4:1]
+ *
+ * The address byte (with MemReg and lower address bits) must be sent first,
+ * followed by the upper block address byte. With regmap 16-bit register
+ * format, this maps to little-endian: the low byte of the 16-bit value is
+ * transmitted first. No range config is needed since I3C does not use MR11
+ * page switching.
+ */
+static const struct regmap_config spd5118_regmap_i3c_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = 0x7ff,
+	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	.writeable_reg = spd5118_writeable_reg,
+	.volatile_reg = spd5118_volatile_reg,
+	.cache_type = REGCACHE_MAPLE,
+};
+
 static int spd5118_suspend(struct device *dev)
 {
 	struct spd5118_data *data = dev_get_drvdata(dev);
@@ -701,7 +723,39 @@ static struct i2c_driver spd5118_i2c_driver = {
 	.address_list	= IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
 };
 
-module_i2c_driver(spd5118_i2c_driver);
+/* I3C */
+
+static int spd5118_i3c_probe(struct i3c_device *i3cdev)
+{
+	struct device *dev = i3cdev_to_dev(i3cdev);
+	struct regmap *regmap;
+	u8 regval[2];
+	int err;
+
+	regmap = devm_regmap_init_i3c(i3cdev, &spd5118_regmap_i3c_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
+
+	err = regmap_bulk_read(regmap, SPD5118_REG_TYPE, regval, 2);
+	if (err)
+		return err;
+
+	if (regval[0] != 0x51 || regval[1] != 0x18)
+		return -ENODEV;
+
+	return spd5118_common_probe(dev, regmap);
+}
+
+static struct i3c_driver spd5118_i3c_driver = {
+	.driver = {
+		.name	= "spd5118_i3c",
+		.of_match_table = spd5118_of_ids,
+		.pm = pm_sleep_ptr(&spd5118_pm_ops),
+	},
+	.probe		= spd5118_i3c_probe,
+};
+
+module_i3c_i2c_driver(spd5118_i3c_driver, &spd5118_i2c_driver);
 
 MODULE_AUTHOR("René Rebe <rene@exactcode.de>");
 MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
-- 
2.50.1

[PATCH v3 13/13] arm64: defconfig: Enable I3C and SPD5118 hwmon

[Akhil R]

Enable I3C subsystem (I3C), Synopsys DesignWare I3C master controller
(DW_I3C_MASTER), and SPD5118 hwmon temperature sensor (SENSORS_SPD5118)
as modules.

The NVIDIA Vera CPU uses SOCAMM LPDDR5X memory module, which contain
SPD5118 (JEDEC JESD300) compliant temperature sensor. This sensor is
accessible over the I3C bus through the DesignWare I3C controller present
on the SoC. Enabling these configs allows monitoring memory module
temperatures on platforms such as Vera Rubin. Vera is an ACPI-based
platform and does not use device tree.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 arch/arm64/configs/defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index dd1ac01ee29b..cc573c481c65 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -588,6 +588,8 @@ CONFIG_I2C_UNIPHIER_F=y
 CONFIG_I2C_XILINX=m
 CONFIG_I2C_RCAR=y
 CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_I3C=m
+CONFIG_DW_I3C_MASTER=m
 CONFIG_SPI=y
 CONFIG_SPI_APPLE=m
 CONFIG_SPI_ARMADA_3700=y
@@ -772,6 +774,7 @@ CONFIG_SENSORS_SL28CPLD=m
 CONFIG_SENSORS_AMC6821=m
 CONFIG_SENSORS_INA2XX=m
 CONFIG_SENSORS_INA3221=m
+CONFIG_SENSORS_SPD5118=m
 CONFIG_SENSORS_TMP102=m
 CONFIG_MISC_RP1=m
 CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
-- 
2.50.1

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[sashiko-bot]

Sashiko AI review found 3 potential issue(s):
- [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
- [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
- [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
--

commit dbfc3abafb12042fa0c42239f7ff49c55d30eb3d
Author: Akhil R <akhilrajeev@nvidia.com>

dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

This commit adds the 'mipi-i3c-static-method' property to specify which
discovery method an I3C device supports during bus initialization. The
property serves as a bitmap where each bit indicates support for methods
like SETDASA or SETAASA.

[1] https://www.mipi.org/mipi-disco-for-i3c-download

> diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
> index e25fa72fd7857..225844abdd1fd 100644
> --- a/Documentation/devicetree/bindings/i3c/i3c.yaml
> +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
[ ... ]
> @@ -147,6 +149,27 @@ patternProperties:
>            through SETDASA. If static address is not present, this address is assigned
>            through SETNEWDA after assigning a temporary address via ENTDAA.

Does the description for assigned-address need an update here? It currently
states that if a static address is present, the address is assigned through
SETDASA. This seems to contradict the new capabilities allowing SETAASA.

>  
> +      mipi-i3c-static-method:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0x1

The description below states that "0 indicates lack of support", but this
minimum validation prevents setting the bitmap to 0. If an I3C target has a
static address but relies purely on ENTDAA, how can a developer override the
default value of 1 to specify that no static methods are supported?

> +        maximum: 0x7
> +        default: 1
> +        description: |
> +          Bitmap describing which methods of Dynamic Address Assignment from a
> +          static address are supported by this I3C Target. A bit value of 1
> +          indicates support for that method, and 0 indicates lack of support.
> +
> +            Bit 0: SETDASA CCC (Direct)
> +            Bit 1: SETAASA CCC (Broadcast)
> +            Bit 2: Other CCC (vendor / standards extension)
> +            All other bits are reserved.
[ ... ]
> diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
> index 373439218bba6..8409addff0115 100644
> --- a/include/dt-bindings/i3c/i3c.h
> +++ b/include/dt-bindings/i3c/i3c.h
> @@ -13,4 +13,7 @@
>  #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
>  #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
>  
> +#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
> +#define I3C_ADDR_METHOD_SETAASA     (1 << 1)

Since Bit 2 is documented as "Other CCC" in the binding and the maximum value
allows combinations up to 0x7, should there be a macro defined for Bit 2?
Adding one could prevent device tree authors from needing to use magic
numbers to represent this capability.

> +
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=1

Re: [PATCH v3 02/13] ACPICA: Read LVR from the I2C resource descriptor

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Medium] The newly added `lvr` field in `struct acpi_resource_i2c_serialbus` is omitted from the resource dumping mechanism in `rsdumpinfo.c`.
--

commit 06f9727bd6ba574a252f3ac9267b0193a2f50dd1
Author: Akhil R <akhilrajeev@nvidia.com>

ACPICA: Read LVR from the I2C resource descriptor

This patch updates the ACPICA I2C serial bus descriptor handling to read
the legacy virtual register (lvr) data. It adds the lvr field to struct
acpi_resource_i2c_serialbus and updates the conversion info to map it.

Link: https://github.com/acpica/acpica/commit/70082dc8
Link: https://github.com/acpica/acpica/commit/b3c38dc9

> diff --git a/drivers/acpi/acpica/rsserial.c b/drivers/acpi/acpica/rsserial.c
> index 279bfa27da94d..c06e918ab8895 100644
> --- a/drivers/acpi/acpica/rsserial.c
> +++ b/drivers/acpi/acpica/rsserial.c

[ ... ]

> diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h
> index 842f932e2c2bc..38a19b1d19ace 100644
> --- a/include/acpi/acrestyp.h
> +++ b/include/acpi/acrestyp.h
> @@ -423,6 +423,7 @@ struct acpi_resource_i2c_serialbus {
>  	ACPI_RESOURCE_SERIAL_COMMON u8 access_mode;
>  	u16 slave_address;
>  	u32 connection_speed;
> +	u8 lvr;
>  };

Does this newly added lvr field also need to be included in the resource
dumping mechanism?

Looking at drivers/acpi/acpica/rsdumpinfo.c, acpi_rs_dump_i2c_serial_bus
lacks an entry for this new field:

struct acpi_rsdump_info acpi_rs_dump_i2c_serial_bus[14] = {
	{ACPI_RSD_TITLE, ACPI_RSD_TABLE_SIZE(acpi_rs_dump_i2c_serial_bus),
	 "I2C Serial Bus", NULL},
	ACPI_RS_DUMP_COMMON_SERIAL_BUS {ACPI_RSD_1BITFLAG,
					ACPI_RSD_OFFSET(i2c_serial_bus.
							access_mode),
					"AccessMode", acpi_gbl_am_decode},
	{ACPI_RSD_UINT32, ACPI_RSD_OFFSET(i2c_serial_bus.connection_speed),
	 "ConnectionSpeed", NULL},
	{ACPI_RSD_UINT16, ACPI_RSD_OFFSET(i2c_serial_bus.slave_address),
	 "SlaveAddress", NULL},
};

Without a corresponding ACPI_RSD_UINT8 entry here, will utilities like
acpidump or acpiexec ignore the lvr field when disassembling and printing
I2C Serial Bus Resource descriptors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=2

Re: [PATCH v3 03/13] i3c: master: Use unified device property interface

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [Critical] Dangling fwnode pointer in boardinfo leading to Use-After-Free on device hot-plug.
- [High] Master registration fails on ACPI systems due to mandatory OF-specific property checks.
--

commit bc7c2a6c59d2e2a9a674fb4e8a851cbc8852b1e2
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: Use unified device property interface

This commit replaces OF-specific functions with unified device property
functions to serve as a prerequisite for supporting both ACPI and device
tree.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5cd4e5da22333..d0677061faab4 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -993,7 +995,7 @@ static void i3c_device_release(struct device *dev)
>  
>  	WARN_ON(i3cdev->desc);
>  
> -	of_node_put(i3cdev->dev.of_node);
> +	fwnode_handle_put(dev->fwnode);
>  	kfree(i3cdev);
>  }
>  
[ ... ]
> @@ -1789,7 +1791,7 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  			     desc->info.pid);
>  
>  		if (desc->boardinfo)
> -			desc->dev->dev.of_node = desc->boardinfo->of_node;
> +			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);

Can this result in a use-after-free if a device is hot-unplugged and later
re-registered?

Since device_set_node() does not increment the fwnode reference count, the
device shares the reference owned by boardinfo.

During device unregistration, i3c_device_release() unconditionally calls
fwnode_handle_put(dev->fwnode), dropping that single reference.

If the device is subsequently hot-plugged, will the cached boardinfo reuse
the now-freed fwnode pointer? Would it be safer to acquire a new reference
when setting the device node, like fwnode_handle_get(desc->boardinfo->fwnode)?

[ ... ]
> @@ -2499,7 +2505,7 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
>  	if (!master)
>  		return -EINVAL;
>  
> -	ret = of_property_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg));
> +	ret = fwnode_property_read_u32_array(fwnode, "reg", reg, ARRAY_SIZE(reg));
>  	if (ret)
>  		return ret;

Does this cause master registration to fail on ACPI systems?

Previously, of_populate_i3c_bus() safely ignored ACPI systems because
dev->of_node was NULL. Now, fwnode_populate_i3c_bus() retrieves the ACPI
fwnode and iterates through its child devices.

For each child, this unconditionally attempts to read the OF-specific reg
property. Since ACPI defines resources via _CRS and _ADR rather than a
reg array, this read will fail and return an error.

This error then propagates up and aborts the entire i3c_master_register()
process. I noticed this regression is fixed later in the patch series by
commit e979566482d52 (i3c: master: Support ACPI enumeration of child
devices), but I am pointing it out here as it temporarily breaks
bisectability for ACPI platforms.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=3

Re: [PATCH v3 04/13] i3c: master: Support ACPI enumeration of child devices

[sashiko-bot]

Sashiko AI review found 3 potential issue(s):
- [High] `i3c_acpi_get_i2c_resource()` incorrectly overwrites device configuration and accumulates flags when multiple `I2cSerialBus` resources are present.
- [High] Auxiliary non-I2C/I3C ACPI child nodes cause the entire I3C master bus registration to fail.
- [High] The ACPI I2C enumeration path leaves `boardinfo->base.type` uninitialized, resulting in empty device names.
--

commit 32dd6acd0427ec4e092205396a3b4f2782c52903
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: Support ACPI enumeration of child devices

This commit adds support for describing I3C and I2C devices using ACPI.
It extracts device configuration such as the static address and LVR
from ACPI resources to enable enumeration on the I3C bus.

Link: https://www.mipi.org/mipi-disco-for-i3c-download

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2409,6 +2410,53 @@ EXPORT_SYMBOL_GPL(i3c_master_add_i3c_dev_locked);
>  
>  #define OF_I3C_REG1_IS_I2C_DEV			BIT(31)
>  
> +#ifdef CONFIG_ACPI
> +static int i3c_acpi_get_i2c_resource(struct acpi_resource *ares, void *data)
> +{
> +	struct i2c_dev_boardinfo *boardinfo = data;
> +	struct acpi_resource_i2c_serialbus *sb;
> +
> +	if (!i2c_acpi_get_i2c_resource(ares, &sb))
> +		return 1;
> +
> +	boardinfo->base.addr = sb->slave_address;
> +	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
> +		boardinfo->base.flags |= I2C_CLIENT_TEN;
> +
> +	boardinfo->lvr = sb->lvr;
> +
> +	return 0;
> +}

Does this code overwrite the device configuration if multiple I2cSerialBus
resources are present? By returning 0, it tells the ACPI core to continue
walking resources. The primary address seems to be overwritten by the last
resource, and flags like I2C_CLIENT_TEN from earlier resources will
accumulate into the final configuration.

[ ... ]

> +static int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo,
> +				      struct fwnode_handle *fwnode)
> +{
> +	struct acpi_device *adev = to_acpi_device_node(fwnode);
> +	LIST_HEAD(resources);
> +	int ret;
> +
> +	boardinfo->base.fwnode = acpi_fwnode_handle(adev);
> +
> +	ret = acpi_dev_get_resources(adev, &resources,
> +				     i3c_acpi_get_i2c_resource, boardinfo);

Is boardinfo->base.type left uninitialized here? Because the structure is
allocated with devm_kzalloc() earlier, the type field remains an empty
string.

Standard I2C APIs rely on this field to populate the client name, which might
break sysfs node creation and legacy string-based driver matching.

[ ... ]

>  static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
>  {
>  	struct device *dev = &master->dev;
> @@ -2532,7 +2645,13 @@ static int fwnode_populate_i3c_bus(struct i3c_master_controller *master)
>  		return 0;
>  
>  	fwnode_for_each_available_child_node_scoped(fwnode, child) {
> -		ret = i3c_master_add_dev(master, child);
> +		if (is_of_node(child))
> +			ret = i3c_master_add_of_dev(master, child);
> +		else if (is_acpi_device_node(child))
> +			ret = i3c_master_add_acpi_dev(master, child);
> +		else
> +			continue;
> +
>  		if (ret)
>  			return ret;
>  	}

If an auxiliary ACPI child node lacks an _ADR method and lacks I2cSerialBus
resources, i3c_master_add_acpi_dev() calls i3c_master_add_i2c_boardinfo(),
which will eventually return -ENODEV.

Will this cause the entire I3C master bus registration to fail because the
loop aborts on the first error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=4

Re: [PATCH v3 05/13] i3c: master: Add support for devices using SETAASA

[sashiko-bot]

Sashiko AI review found 7 potential issue(s):
- [High] Missing `i3c_master_reattach_i3c_dev` call for SETAASA devices causes hardware state desynchronization and address collisions.
- [High] Unhandled `I3C_ERROR_M2` in `i3c_master_setaasa_locked` aborts entire I3C bus initialization if no devices acknowledge the broadcast.
- [High] SETAASA devices without an explicit `assigned-address` property are silently ignored and never initialized.
- [Medium] Broadcasting SETAASA before issuing targeted SETDASA commands breaks devices intended for SETDASA.
- [Medium] Bypassing `i3c_master_retrieve_dev_info` for SETAASA devices leaves critical device capabilities zeroed.
- [Medium] Broadcasting Vendor Extension CCC (SETHID) without a defining byte violates the MIPI I3C specification.
- [Medium] Mismatch between `assigned-address` and `static_addr` for SETAASA devices permanently leaks address slots.
--

commit 198585b433bb26dea4a874b95e4dc205539e927a
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: Add support for devices using SETAASA

This commit adds support for devices using the SETAASA (Set All Addresses to
Static Address) method. It enables the discovery and dynamic address
assignment of these devices without relying on ENTDAA or SETDASA.

Link: https://www.mipi.org/mipi-disco-for-i3c-download

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ffec97157d4883..de72317c9a5c73 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> @@ -1051,6 +1052,47 @@ static int i3c_master_rstdaa_locked(struct i3c_master_controller *master,
>  	return ret;
>  }
>  
> +/**
> + * i3c_master_setaasa_locked() - start a SETAASA procedure (Set All Addresses to Static Address)
> + * @master: I3C master object
> + *
> + * Send a SETAASA CCC command to set all attached I3C devices' dynamic addresses to
> + * their static address.
> + *
> + * This function must be called with the bus lock held in write mode.
> + *
> + * First, the SETHID CCC command is sent, followed by the SETAASA CCC.
> + *
> + * Return: 0 in case of success, a positive I3C error code if the error is
> + * one of the official Mx error codes, and a negative error code otherwise.
> + */
> +static int i3c_master_setaasa_locked(struct i3c_master_controller *master)
> +{
> +	struct i3c_ccc_cmd_dest dest;
> +	struct i3c_ccc_cmd cmd;
> +	int ret;
> +
> +	/*
> +	 * Send SETHID CCC command. Though it is a standard CCC command specified
> +	 * in JESD300-5, we are not defining a separate macro to be explicit that
> +	 * the value falls under the vendor specific range.
> +	 */
> +	i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
> +	i3c_ccc_cmd_init(&cmd, false, I3C_CCC_VENDOR(0, true), &dest, 1);

Does this violate the MIPI I3C specification? The command is initialized
with a 0-length payload. The specification mandates that Vendor Extension
CCCs must include a defining byte (Vendor ID) in their payload when
broadcast.

Could broadcasting 0x61 without an identifier cause non-JEDEC devices to
misinterpret the command as their own vendor-specific extension?

> +	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> +	i3c_ccc_cmd_dest_cleanup(&dest);
> +	if (ret)
> +		return ret;

Should this code handle the I3C_ERROR_M2 case? If no active devices
acknowledge the frame, the hardware controller returns an I3C_ERROR_M2
error.

Unlike RSTDAA and ENTDAA which explicitly suppress this error, propagating
it back to i3c_master_bus_init() causes the initialization to abort and
tears down all I2C and I3C devices on the bus.

> +
> +	/* Send SETAASA CCC command */
> +	i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
> +	i3c_ccc_cmd_init(&cmd, false, I3C_CCC_SETAASA, &dest, 1);
> +	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> +	i3c_ccc_cmd_dest_cleanup(&dest);
> +
> +	return ret;
> +}

[ ... ]

> @@ -1739,6 +1781,18 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
>  	if (ret)
>  		goto err_free_dev;
>  
> +	/*
> +	 * For devices using SETAASA instead of ENTDAA, the address is statically
> +	 * assigned. Update the dynamic address to the provided static address.
> +	 * Reattaching the I3C device is not useful. It is also not mandatory
> +	 * for such devices to implement CCC commands like GETPID, GETDCR etc.
> +	 * Hence, we can return here.
> +	 */
> +	if (i3cdev->boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		i3cdev->info.dyn_addr = i3cdev->boardinfo->static_addr;
> +		return 0;
> +	}

By returning early, this bypasses the i3c_master_reattach_i3c_dev() call.
Does this leave the hardware state desynchronized? The dynamic address slot
is never marked as I3C_ADDR_SLOT_I3C_DEV in master->bus.addrslots, which
could lead to bus collisions during ENTDAA or Hot-Join.

Furthermore, the hardware master driver's .reattach_i3c_dev() callback is
skipped, meaning the physical controller's Device Address Table is never
updated and subsequent private transfers might fail.

Does skipping i3c_master_retrieve_dev_info() leave structural bounds like
max_read_len and max_write_len initialized to zero? Even if standard CCCs
are optional for these devices, downstream drivers might expect these fields
to be populated or have safe default values.

Also, if a user provides an assigned-address (init_dyn_addr) that differs
from the static_addr, this early return seems to leak the reserved address
slot. The init_dyn_addr slot is reserved earlier in i3c_master_bus_init(),
but this path never clears or updates it.

> +
>  	ret = i3c_master_setdasa_locked(master, i3cdev->info.static_addr,
>  					i3cdev->boardinfo->init_dyn_addr);
>  	if (ret)

[ ... ]

> @@ -2145,6 +2199,12 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>  	if (ret)
>  		goto err_bus_cleanup;
>  
> +	if (master->addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		ret = i3c_master_setaasa_locked(master);
> +		if (ret)
> +			goto err_bus_cleanup;
> +	}

Could broadcasting SETAASA before issuing targeted SETDASA commands break
devices intended for SETDASA?

If a device supports both methods, the initial SETAASA broadcast will cause
it to adopt its static address immediately. The subsequent SETDASA command
meant to assign a specific software-configured dynamic address would then
fail or be ignored.

> +
>  	/*
>  	 * Reserve init_dyn_addr first, and then try to pre-assign dynamic
>  	 * address and retrieve device information if needed.

Since SETAASA forces the dynamic address to equal the static address,
devices using this method might omit the optional assigned-address property.

If assigned-address is omitted, init_dyn_addr is 0. Looking at the loop
immediately following this code:

	list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
		if (!i3cboardinfo->init_dyn_addr)
			continue;

Does this cause SETAASA devices without an assigned-address to be completely
skipped and never initialized during the static device setup?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=5

Re: [PATCH v3 06/13] i3c: master: Add support for devices without PID

[sashiko-bot]

Sashiko AI review found 4 potential issue(s):
- [Critical] Pre-existing use-after-free and double-free on device registration failure.
- [High] Validation bypass allows SETDASA devices with a missing or zero PID to be incorrectly registered.
- [Medium] PID validation is completely bypassed for SETAASA devices, allowing malformed PIDs to be accepted and exposed.
- [Medium] Dead code and logic flaws in device matching and duplicate detection for dynamically discovered devices.
--

commit dbd7e3bde8caae211d2b1844852f3a001da04d22
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: Add support for devices without PID

This commit allows devices without a 48-bit PID to register using their
static address. It primarily supports SETAASA address assignment as per the
I3C specification where a PID is not strictly required.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index de72317c9a5c73..fbe8710c4c56f8 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1842,8 +1842,17 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  		desc->dev->dev.type = &i3c_device_type;
>  		desc->dev->dev.bus = &i3c_bus_type;
>  		desc->dev->dev.release = i3c_device_release;
> -		dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
> -			     desc->info.pid);
> +
> +		/*
> +		 * For devices without PID (e.g., SETAASA devices), use
> +		 * static address for naming instead.
> +		 */
> +		if (desc->info.pid)
> +			dev_set_name(&desc->dev->dev, "%d-%llx", master->bus.id,
> +				     desc->info.pid);
> +		else
> +			dev_set_name(&desc->dev->dev, "%d-%02x", master->bus.id,
> +				     desc->info.static_addr);
>  
>  		if (desc->boardinfo)
>  			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);

This problem wasn't introduced by this patch, but does the error path further
down in this function leave a dangling pointer?

When device_register() fails, put_device() is called, which triggers
i3c_device_release() and frees desc->dev. However, desc->dev is not set to
NULL.

Later, during master teardown or driver unbind,
i3c_master_unregister_i3c_devs() iterates over the bus devices and checks
if (!i3cdev->dev) which will incorrectly pass. Could this lead to a
use-after-free and a subsequent double-free?

[ ... ]

> @@ -2288,8 +2297,18 @@ static void i3c_master_attach_boardinfo(struct i3c_dev_desc *i3cdev)
>  	struct i3c_dev_boardinfo *i3cboardinfo;
>  
>  	list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> -		if (i3cdev->info.pid != i3cboardinfo->pid)
> -			continue;
> +		/*
> +		 * For devices without PID (e.g., SETAASA devices), match by
> +		 * static address. For devices with PID, match by PID.
> +		 */
> +		if (i3cboardinfo->pid) {
> +			if (i3cdev->info.pid != i3cboardinfo->pid)
> +				continue;
> +		} else {
> +			if (!i3cboardinfo->static_addr ||
> +			    i3cdev->info.static_addr != i3cboardinfo->static_addr)
> +				continue;
> +		}

Is it possible for the static address match to succeed here?

This function appears to be called by i3c_master_add_i3c_dev_locked() for
devices dynamically discovered via the ENTDAA procedure. During ENTDAA, the
static address is not retrieved, meaning i3cdev->info.static_addr would be
zero.

Could this cause the match against non-zero SETAASA static addresses
to always fail?

[ ... ]

> @@ -2303,8 +2322,12 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
>  	struct i3c_master_controller *master = i3c_dev_get_master(refdev);
>  	struct i3c_dev_desc *i3cdev;
>  
> +	if (!refdev->info.pid)
> +		return NULL;
> +

Can refdev->info.pid ever be zero here?

Similar to attach_boardinfo, this duplicate search seems to be used during
ENTDAA discovery. During ENTDAA, the hardware PID is always retrieved, so
refdev->info.pid is never zero. Devices statically configured with SETAASA
bypass these functions entirely.

[ ... ]

> @@ -2604,9 +2627,15 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
>  
>  	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
>  
> -	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> -	    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> -		return -EINVAL;
> +	/* For SETAASA devices, validate the static address instead of PID */
> +	if (boardinfo->static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		if (!boardinfo->static_addr)
> +			return -EINVAL;
> +	} else {
> +		if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> +		    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> +			return -EINVAL;
> +	}

Does this bypass PID validation entirely for SETAASA devices, even if a
PID is provided?

If a device tree node explicitly provides a PID in its reg property for a
SETAASA device, it skips the reserved upper bits check. The unvalidated PID
is then stored in boardinfo->pid, and later used to name the device via
dev_set_name(). Could this expose malformed data to userspace in sysfs?

Also, if the mipi-i3c-static-method is SETDASA (or any method other than
SETAASA) and the device has a missing or zero PID, the execution reaches
this else block. Because boardinfo->pid is zero, the bitwise validation
mathematically evaluates to zero and passes.

Does this allow SETDASA devices to bypass validation and be registered
without a valid PID, violating the I3C specification?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=6

Re: [PATCH v3 07/13] i3c: master: match I3C device through DT and ACPI

[sashiko-bot]

Sashiko AI review found 4 potential issue(s):
- [High] I3C devices matched via DT or ACPI will not automatically load their corresponding kernel modules because the I3C subsystem's `uevent` handler fails to generate OF or ACPI modaliases.
- [High] Lockless access to `i3cdev->desc` in `i3c_device_match()` introduces a TOCTOU race and a Use-After-Free / NULL Pointer Dereference.
- [High] Uninitialized memory read in `i3c_device_match_id()` when `i3cdev->desc` is NULL.
- [Medium] Architectural restriction of firmware matching to SETAASA devices.
--

commit b086c2a6b40084b4476548d93f6440348d3a2ee5
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: match I3C device through DT and ACPI

This patch allows SETAASA-based I3C devices to be matched through Device
Tree or ACPI, as they might not implement deviceinfo CCC commands and lack
standard ID matching.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index fbe8710c4c56f8..0be83bfdb563f5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -343,15 +344,32 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
>  {
>  	struct i3c_device *i3cdev;
>  	const struct i3c_driver *i3cdrv;
> +	u8 static_addr_method = 0;
>  
>  	if (dev->type != &i3c_device_type)
>  		return 0;
>  
>  	i3cdev = dev_to_i3cdev(dev);
>  	i3cdrv = drv_to_i3cdrv(drv);
> -	if (i3c_device_match_id(i3cdev, i3cdrv->id_table))
> +
> +	if (i3cdev->desc && i3cdev->desc->boardinfo)
> +		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;

Is it safe to access i3cdev->desc and boardinfo here without holding the I3C
bus lock?

During a Dynamic Address Assignment or Hot-Join event,
i3c_master_add_i3c_dev_locked() can replace the descriptor and free the old
one. Because this match function runs without locks, it might see a non-NULL
descriptor, get preempted while the descriptor is freed, and then dereference
freed memory.

Additionally, the compiler might fetch i3cdev->desc twice here, which could
result in a NULL pointer dereference if it gets set to NULL concurrently
during device unregistration.

> +
> +	/*
> +	 * SETAASA based device need not always have a matching ID since
> +	 * it is not mandatory for such devices to implement deviceinfo
> +	 * CCC commands. Allow them to register through DT or ACPI.
> +	 */
> +	if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))

If i3cdev->desc is NULL, does i3c_device_match_id() operate on uninitialized
memory?

The check above indicates i3cdev->desc can be NULL, but i3c_device_get_info()
will return early without initializing the devinfo structure when the
descriptor is missing, leaving i3c_device_match_id() to extract PID
components from uninitialized stack memory.

>  		return 1;
>  
> +	if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> +		if (of_driver_match_device(dev, drv))
> +			return 1;
> +		if (acpi_driver_match_device(dev, drv))
> +			return 1;
> +	}

Is there a reason to restrict DT and ACPI matching to only SETAASA devices?

Generic firmware matching is generally available to any device with a valid
firmware node. Since of_driver_match_device() and acpi_driver_match_device()
safely return 0 if no firmware match exists, gating this behind a specific
address assignment type might unnecessarily restrict firmware-based matching
for standard I3C devices.

Also, does i3c_device_uevent() need an update to generate OF and ACPI
modaliases?

If drivers omit an id_table and rely solely on DT or ACPI matching, they
might not be automatically loaded by userspace during device discovery, since
the uevent handler only emits standard I3C modaliases.

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=7

Re: [PATCH v3 09/13] i3c: dw-i3c-master: Add a quirk to skip clock and reset

[sashiko-bot]

Sashiko AI review found 5 potential issue(s):
- [Critical] The driver fails to disable hardware interrupts before destroying the workqueue during removal, leading to a potential Use-After-Free.
- [High] The patch alters the initialization order of `pclk` and `core_rst`, causing the core reset to be deasserted before the peripheral clock is enabled.
- [High] Casting the return value of `device_get_match_data()` to `unsigned long` causes pointer addresses to be interpreted as quirk flags on ACPI PRP0001 systems, which can lead to random hardware initialization failures.
- [High] A crafted `clock-frequency` device property can cause an integer overflow leading to a divide-by-zero kernel panic on 32-bit systems.
- [High] Combining `devm_clk_get_enabled()` with Runtime PM clock management causes a clock refcount underflow if the device is suspended during driver removal.
--

commit 62e84914f9fc0efd829919bb31559f49b5cb8b00
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: dw-i3c-master: Add a quirk to skip clock and reset

Some ACPI-enumerated devices like Tegra410 do not have clock and reset
resources exposed via the clk/reset frameworks. This patch adds match data
to skip acquiring these controls during probe and reads the core clock rate
from a device property instead.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index edd42daf75539b..b90756ade2db6d 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
[ ... ]
> @@ -556,13 +557,26 @@ static void dw_i3c_master_set_intr_regs(struct dw_i3c_master *master)
>  	writel(IBI_REQ_REJECT_ALL, master->regs + IBI_MR_REQ_REJECT);
>  }
>  
> +static unsigned long dw_i3c_master_get_core_rate(struct dw_i3c_master *master)
> +{
> +	unsigned int core_rate_prop;
> +
> +	if (!(master->quirks & DW_I3C_ACPI_SKIP_CLK_RST))
> +		return clk_get_rate(master->core_clk);
> +
> +	if (device_property_read_u32(master->dev, "clock-frequency", &core_rate_prop))
> +		return 0;
> +
> +	return core_rate_prop;
> +}

Could reading a large clock-frequency property cause an integer overflow?

If core_rate is read as a very large value from the property (e.g., 
3400000000), it seems core_period in dw_i3c_clk_cfg() could evaluate to 0 
due to the addition in the DIV_ROUND_UP() macro wrapping around on 32-bit 
systems. 

Would the subsequent DIV_ROUND_UP(I3C_BUS_THIGH_MAX_NS, core_period) then 
trigger a divide-by-zero panic?

[ ... ]
> @@ -1580,19 +1594,33 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>  	if (IS_ERR(master->regs))
>  		return PTR_ERR(master->regs);
>  
> -	master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);
> -	if (IS_ERR(master->core_clk))
> -		return PTR_ERR(master->core_clk);
> +	if (has_acpi_companion(&pdev->dev)) {
> +		quirks = (unsigned long)device_get_match_data(&pdev->dev);

Can this pointer cast lead to random hardware initialization failures?

If an ACPI device uses the PRP0001 mechanism, device_get_match_data()
might return a pointer to struct dw_i3c_drvdata instead of an integer.

If the pointer address happens to have BIT(3) set, could the driver
inadvertently skip acquiring core_clk and core_rst, leading to an external
abort when reading hardware registers on an unclocked device?

> +	} else if (pdev->dev.of_node) {
> +		drvdata = device_get_match_data(&pdev->dev);
> +		if (drvdata)
> +			quirks = drvdata->flags;
> +	}
> +	master->quirks = quirks;
> +
> +	if (master->quirks & DW_I3C_ACPI_SKIP_CLK_RST) {
> +		master->core_clk = NULL;
> +		master->core_rst = NULL;
> +	} else {
> +		master->core_clk = devm_clk_get_enabled(&pdev->dev, NULL);

Is there a risk of a clock refcount underflow here during device removal?

The clocks are dynamically disabled in dw_i3c_master_runtime_suspend().
If the device is already suspended when dw_i3c_common_remove() calls
pm_runtime_disable() without forcefully resuming first, would the
devres cleanup unconditionally call clk_disable_unprepare() again?

> +		if (IS_ERR(master->core_clk))
> +			return PTR_ERR(master->core_clk);
> +
> +		master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
> +											"core_rst");
> +		if (IS_ERR(master->core_rst))
> +			return PTR_ERR(master->core_rst);
> +	}
>  
>  	master->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk");

Does this change alter the initialization order between the peripheral clock
and the core reset?

In the original code, pclk was enabled before core_rst was acquired and
deasserted. By moving core_rst into the else block above pclk, the reset
appears to be deasserted while pclk is still disabled.

Since hardware IPs typically require the peripheral bus clock to be running
for reset signals to properly synchronize and clear APB registers, could
this inverted sequence cause bus hangs or initialization failures?

[ ... ]

Additionally, is there a potential use-after-free regarding hj_work during
driver removal?

In dw_i3c_common_remove(), cancel_work_sync(&master->hj_work) is called
followed by i3c_master_unregister(), which destroys master->base.wq.

Since the IRQ was requested with devm_request_irq(), it remains active
until devres runs after remove() returns. If a Hot-Join interrupt fires
after the workqueue is destroyed, would the handler execute and call
queue_work(), dereferencing the freed pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=9

Re: [PATCH v3 10/13] i3c: dw-i3c-master: Add ACPI ID for Tegra410

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [High] Type mismatch when retrieving match data for ACPI devices instantiated via PRP0001 (OF compatible strings).
--

commit 333ca78106137e8c6bbf34e1b45a4801fdfbbebe
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: dw-i3c-master: Add ACPI ID for Tegra410

Update variable names to generic names and add Tegra410 ACPI ID to
support the I3C controller in Tegra410, which is a DesignWare I3C host
controller.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index b90756ade2db6d..e63822df2ae48e 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1847,11 +1847,12 @@ static const struct of_device_id dw_i3c_master_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, dw_i3c_master_of_match);
>  
> -static const struct acpi_device_id amd_i3c_device_match[] = {
> +static const struct acpi_device_id dw_i3c_master_acpi_match[] = {
>  	{ "AMDI0015", AMD_I3C_OD_PP_TIMING },
> +	{ "NVDA2018", DW_I3C_ACPI_SKIP_CLK_RST },
>  	{ }
>  };

This isn't a regression from this commit, but I noticed a potential issue
with how ACPI match data is extracted in dw_i3c_common_probe():

drivers/i3c/master/dw-i3c-master.c:dw_i3c_common_probe() {
    ...
	if (has_acpi_companion(&pdev->dev)) {
		quirks = (unsigned long)device_get_match_data(&pdev->dev);
	} else if (pdev->dev.of_node) {
		drvdata = device_get_match_data(&pdev->dev);
		if (drvdata)
			quirks = drvdata->flags;
	}
    ...
}

If an ACPI device is instantiated using the standard PRP0001 mechanism, it
maps to OF compatible strings like "altr,agilex5-dw-i3c-master". In this
scenario, has_acpi_companion() evaluates to true, but device_get_match_data()
will return the pointer to altr_agilex5_drvdata from the OF match table.

Could this direct cast of the pointer to an unsigned long bitmask cause
unintended quirk flags to be interpreted as true depending on memory layout?

Would it be safer to unify the match data format to always use struct
pointers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=10

Re: [PATCH v3 11/13] hwmon: spd5118: Remove 16-bit addressing

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Medium] The driver violates hwmon subsystem locking rules by using a custom `nvmem_lock`, and correcting this exposes an initialization order race condition where the NVMEM interface is exposed before the hwmon device is registered.
--

commit 8460c5a9338d8916bf61ccb3e598518750ec6cf9
Author: Akhil R <akhilrajeev@nvidia.com>

hwmon: spd5118: Remove 16-bit addressing

This commit removes 16-bit addressing support for the spd5118 driver,
as it is not required for I3C and has no known users. It updates the
regmap configuration and probe functions to only use 8-bit addressing.

> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 5da44571b6a0ce..8fdde35e68cd3f 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -78,7 +75,6 @@ static const unsigned short normal_i2c[] = {
>  struct spd5118_data {
>  	struct regmap *regmap;
>  	struct mutex nvmem_lock;
> -	bool is_16bit;
>  };

Does this custom lock violate the hwmon subsystem locking guidelines?

The guidelines state that drivers must serialize attributes registered by
other means using hwmon_lock() and hwmon_unlock().

If this code is updated to comply with the guideline by replacing
nvmem_lock with hwmon_lock(data->hwmon_dev), could it expose a race
condition during initialization?

Looking at spd5118_common_probe():

spd5118_common_probe()
    spd5118_nvmem_init(dev, data);
    ...
    hwmon_dev = devm_hwmon_device_register_with_info(dev, ...);

Since spd5118_nvmem_init() is called before the hwmon device is registered,
would this expose the nvmem sysfs file to userspace while the hwmon
locking context is still unregistered, potentially leading to a NULL
pointer dereference?

Should the hwmon device registration happen before the nvmem device
registration to prevent this sequence?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=11

Re: [PATCH v3 12/13] hwmon: spd5118: Add I3C support

[sashiko-bot]

Sashiko AI review found 4 potential issue(s):
- [High] NVMEM reads over I3C return corrupted data due to an unhandled 64-byte block rollover.
- [High] Stack-allocated buffers are passed to `regmap_bulk_read()`, which is unsafe for DMA-capable I3C controllers and will cause kernel panics or memory corruption.
- [Low] The driver can be selected when `CONFIG_I2C` is disabled, leading to a Kconfig unmet direct dependency warning and a compilation failure due to undefined I2C APIs.
- [Low] Building the driver when `CONFIG_I3C` is disabled results in a linker error for `__devm_regmap_init_i3c`.
--

commit 4b70b869fa4332fdf133591e152cc997f3522e25
Author: Akhil R <akhilrajeev@nvidia.com>

hwmon: spd5118: Add I3C support
    
Add a regmap config and a probe function to support I3C-based
communication with SPD5118 devices.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 14e4cea48acc47..a7f60f8868eb6b 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2309,12 +2309,15 @@ config SENSORS_INA3221
>  
>  config SENSORS_SPD5118
>  	tristate "SPD5118 Compliant Temperature Sensors"
> -	depends on I2C
> +	depends on I3C_OR_I2C
>  	select REGMAP_I2C

If CONFIG_I2C is disabled and CONFIG_I3C is enabled as a module, could this
cause Kconfig unmet direct dependency warnings?

Because select REGMAP_I2C strictly depends on I2C, it will result in warnings.
Compilation might also fail when module_i3c_i2c_driver() unconditionally
references i2c_add_driver(), which is undefined when CONFIG_I2C is not set.

> +	select REGMAP_I3C if I3C

[ ... ]

> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index 8fdde35e68cd3f..db66028d2f590e 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -464,6 +465,27 @@ static const struct regmap_config spd5118_regmap8_config = {
>  	.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
>  };
>  
> +/*
> + * SPD5118 2-byte register address format (JESD300-5, Tables 7 & 20):
> + *   Byte 1 (on wire first):  MemReg | BlkAddr[0] | Address[5:0]
> + *   Byte 2 (on wire second): 0000   | BlkAddr[4:1]
> + *
> + * The address byte (with MemReg and lower address bits) must be sent first,
> + * followed by the upper block address byte. With regmap 16-bit register
> + * format, this maps to little-endian: the low byte of the 16-bit value is
> + * transmitted first. No range config is needed since I3C does not use MR11
> + * page switching.
> + */
> +static const struct regmap_config spd5118_regmap_i3c_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.max_register = 0x7ff,

Since the 2-byte address format specifies only a 6-bit Address field, does the
internal address counter roll over at the 64-byte boundary during I3C mode?

If spd5118_nvmem_read_page() executes a 128-byte regmap_bulk_read(), it
appears the device will silently wrap around to the beginning of the same
64-byte block, returning repeated and incorrect data instead of advancing to
the next block.

> +	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.writeable_reg = spd5118_writeable_reg,
> +	.volatile_reg = spd5118_volatile_reg,
> +	.cache_type = REGCACHE_MAPLE,
> +};
> +

[ ... ]

> @@ -701,7 +723,39 @@ static struct i2c_driver spd5118_i2c_driver = {
>  	.address_list	= IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
>  };
>  
> -module_i2c_driver(spd5118_i2c_driver);
> +/* I3C */
> +
> +static int spd5118_i3c_probe(struct i3c_device *i3cdev)
> +{
> +	struct device *dev = i3cdev_to_dev(i3cdev);
> +	struct regmap *regmap;
> +	u8 regval[2];
> +	int err;
> +
> +	regmap = devm_regmap_init_i3c(i3cdev, &spd5118_regmap_i3c_config);

If CONFIG_I3C is disabled, will this unconditionally call
devm_regmap_init_i3c() and result in a linker error?

When CONFIG_I3C is disabled, REGMAP_I3C is not selected, and
__devm_regmap_init_i3c is only exported by regmap-i3c.c when REGMAP_I3C is
enabled.

> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
> +
> +	err = regmap_bulk_read(regmap, SPD5118_REG_TYPE, regval, 2);

Is it safe to pass the stack-allocated buffer regval to regmap_bulk_read()?

Because val_bits = 8 uses regmap_parse_inplace_noop, regmap_bulk_read() avoids
bounce buffers and passes the user buffer directly down to regmap_i3c_read()
and i3c_device_do_xfers().

Passing stack memory to DMA-capable APIs can lead to memory corruption or
kernel panics.

> +	if (err)
> +		return err;
> +
> +	if (regval[0] != 0x51 || regval[1] != 0x18)
> +		return -ENODEV;
> +
> +	return spd5118_common_probe(dev, regmap);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423085718.70762-1-akhilrajeev@nvidia.com?part=12

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Rob Herring]

On Thu, Apr 23, 2026 at 11:23:11AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 3 potential issue(s):

Does anyone read replies to the bot? If so, why was everyone hwmon list 
dropped?

> - [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
> - [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
> - [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
> --

This should go below the quoted commit msg.

> 
> commit dbfc3abafb12042fa0c42239f7ff49c55d30eb3d
> Author: Akhil R <akhilrajeev@nvidia.com>
> 
> dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
> 
> This commit adds the 'mipi-i3c-static-method' property to specify which
> discovery method an I3C device supports during bus initialization. The
> property serves as a bitmap where each bit indicates support for methods
> like SETDASA or SETAASA.
> 
> [1] https://www.mipi.org/mipi-disco-for-i3c-download
> 
> > diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
> > index e25fa72fd7857..225844abdd1fd 100644
> > --- a/Documentation/devicetree/bindings/i3c/i3c.yaml
> > +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
> [ ... ]
> > @@ -147,6 +149,27 @@ patternProperties:
> >            through SETDASA. If static address is not present, this address is assigned
> >            through SETNEWDA after assigning a temporary address via ENTDAA.
> 
> Does the description for assigned-address need an update here? It currently
> states that if a static address is present, the address is assigned through
> SETDASA. This seems to contradict the new capabilities allowing SETAASA.

Yes.

> 
> >  
> > +      mipi-i3c-static-method:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        minimum: 0x1
> 
> The description below states that "0 indicates lack of support", but this
> minimum validation prevents setting the bitmap to 0. If an I3C target has a
> static address but relies purely on ENTDAA, how can a developer override the
> default value of 1 to specify that no static methods are supported?

0 is referring to the bit values, not the overall value. However, 
whether 0 should be allowed is a valid point.

> > +        maximum: 0x7
> > +        default: 1
> > +        description: |
> > +          Bitmap describing which methods of Dynamic Address Assignment from a
> > +          static address are supported by this I3C Target. A bit value of 1
> > +          indicates support for that method, and 0 indicates lack of support.
> > +
> > +            Bit 0: SETDASA CCC (Direct)
> > +            Bit 1: SETAASA CCC (Broadcast)
> > +            Bit 2: Other CCC (vendor / standards extension)
> > +            All other bits are reserved.
> [ ... ]
> > diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
> > index 373439218bba6..8409addff0115 100644
> > --- a/include/dt-bindings/i3c/i3c.h
> > +++ b/include/dt-bindings/i3c/i3c.h
> > @@ -13,4 +13,7 @@
> >  #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
> >  #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
> >  
> > +#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
> > +#define I3C_ADDR_METHOD_SETAASA     (1 << 1)
> 
> Since Bit 2 is documented as "Other CCC" in the binding and the maximum value
> allows combinations up to 0x7, should there be a macro defined for Bit 2?
> Adding one could prevent device tree authors from needing to use magic
> numbers to represent this capability.

Yes.

Rob

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Guenter Roeck]

On 5/5/26 14:32, Rob Herring wrote:
> On Thu, Apr 23, 2026 at 11:23:11AM +0000, sashiko-bot@kernel.org wrote:
>> Sashiko AI review found 3 potential issue(s):
> 
> Does anyone read replies to the bot? If so, why was everyone hwmon list
> dropped?
> 

It appears that people either complain about getting AI feedback, or
they complain that they don't. That is pretty much a lose-lose situation.
For patches sent to linux-hwmon, Sashiko is configured to respond only
to the mailing list and to the submitter in the hope to minimize the
"I do not want to see this" feedback. That matches the configuration
for several subsystems.

As far as I can see it is not currently possible to limit AI review to
patches actually associated with the given subsystem if sent to that
subsystem's mailing list. I'll discuss with Roman to see if patches
like this one can be filtered out when reviewing patches sent to
hwmon mailing list.

>> - [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
>> - [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
>> - [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
>> --
> 
> This should go below the quoted commit msg.
> 

Others (like me) want to see it at the very top to see a quick summary.
Another lose-lose situation.

Again, I'll discuss with Roman if responses can be limited to patches
actually affecting the hwmon subsystem. This should hopefully reduce
unwanted feedback.

Thanks,
Guenter

>>
>> commit dbfc3abafb12042fa0c42239f7ff49c55d30eb3d
>> Author: Akhil R <akhilrajeev@nvidia.com>
>>
>> dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
>>
>> This commit adds the 'mipi-i3c-static-method' property to specify which
>> discovery method an I3C device supports during bus initialization. The
>> property serves as a bitmap where each bit indicates support for methods
>> like SETDASA or SETAASA.
>>
>> [1] https://www.mipi.org/mipi-disco-for-i3c-download
>>
>>> diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
>>> index e25fa72fd7857..225844abdd1fd 100644
>>> --- a/Documentation/devicetree/bindings/i3c/i3c.yaml
>>> +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
>> [ ... ]
>>> @@ -147,6 +149,27 @@ patternProperties:
>>>             through SETDASA. If static address is not present, this address is assigned
>>>             through SETNEWDA after assigning a temporary address via ENTDAA.
>>
>> Does the description for assigned-address need an update here? It currently
>> states that if a static address is present, the address is assigned through
>> SETDASA. This seems to contradict the new capabilities allowing SETAASA.
> 
> Yes.
> 
>>
>>>   
>>> +      mipi-i3c-static-method:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0x1
>>
>> The description below states that "0 indicates lack of support", but this
>> minimum validation prevents setting the bitmap to 0. If an I3C target has a
>> static address but relies purely on ENTDAA, how can a developer override the
>> default value of 1 to specify that no static methods are supported?
> 
> 0 is referring to the bit values, not the overall value. However,
> whether 0 should be allowed is a valid point.
> 
>>> +        maximum: 0x7
>>> +        default: 1
>>> +        description: |
>>> +          Bitmap describing which methods of Dynamic Address Assignment from a
>>> +          static address are supported by this I3C Target. A bit value of 1
>>> +          indicates support for that method, and 0 indicates lack of support.
>>> +
>>> +            Bit 0: SETDASA CCC (Direct)
>>> +            Bit 1: SETAASA CCC (Broadcast)
>>> +            Bit 2: Other CCC (vendor / standards extension)
>>> +            All other bits are reserved.
>> [ ... ]
>>> diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
>>> index 373439218bba6..8409addff0115 100644
>>> --- a/include/dt-bindings/i3c/i3c.h
>>> +++ b/include/dt-bindings/i3c/i3c.h
>>> @@ -13,4 +13,7 @@
>>>   #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
>>>   #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
>>>   
>>> +#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
>>> +#define I3C_ADDR_METHOD_SETAASA     (1 << 1)
>>
>> Since Bit 2 is documented as "Other CCC" in the binding and the maximum value
>> allows combinations up to 0x7, should there be a macro defined for Bit 2?
>> Adding one could prevent device tree authors from needing to use magic
>> numbers to represent this capability.
> 
> Yes.
> 
> Rob
> 

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Rob Herring]

On Tue, May 5, 2026 at 5:17 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 5/5/26 14:32, Rob Herring wrote:
> > On Thu, Apr 23, 2026 at 11:23:11AM +0000, sashiko-bot@kernel.org wrote:
> >> Sashiko AI review found 3 potential issue(s):
> >
> > Does anyone read replies to the bot? If so, why was everyone hwmon list

everyone *but* the hwmon list...

> > dropped?
> >
>
> It appears that people either complain about getting AI feedback, or
> they complain that they don't. That is pretty much a lose-lose situation.
> For patches sent to linux-hwmon, Sashiko is configured to respond only
> to the mailing list and to the submitter in the hope to minimize the
> "I do not want to see this" feedback. That matches the configuration
> for several subsystems.
>
> As far as I can see it is not currently possible to limit AI review to
> patches actually associated with the given subsystem if sent to that
> subsystem's mailing list. I'll discuss with Roman to see if patches
> like this one can be filtered out when reviewing patches sent to
> hwmon mailing list.

No, I want the replies. I was just wondering why it is not reply-all
which is standard. I was also wondering why I was only sometimes
seeing the reviews. I have since figured that out and submitted a PR
to add DT. I set it to reply-all, so I guess there will be some
comment about that.

I don't think there's any solution if a patch hits 2 conflicting
configurations. Surely the folks that don't want to see it can figure
out how to delete or ignore emails... I have thousands of ignored
emails. It's not hard. :)

> >> - [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
> >> - [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
> >> - [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
> >> --
> >
> > This should go below the quoted commit msg.
> >
>
> Others (like me) want to see it at the very top to see a quick summary.
> Another lose-lose situation.

If there's anything kernel developers agree on, it is not top posting.
I don't see how sashiko is special here. Isn't below the commit
message close enough to the top? I don't mind so much about the list
of issues being at the top, but I really don't see why the commit
message is not quoted (I would guess that's just an artifact of
creating the reply out of the git tree it was applied to?). The commit
hash is also completely useless to anyone without access to the git
tree sashiko uses.

One thing I would love to see is reviews of the commit message itself.
I would love to never have to say "don't use 'This commit/patch'" or
"the commit msg should answer why, not what" again.

Rob