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

[Frank Li]

On Wed, Mar 18, 2026 at 10:57:16PM +0530, Akhil R wrote:
> Replace all OF-specific functions with unified device property functions
> as a prerequisite to support both ACPI and device tree.
>
> Signed-off-by: Akhil R <akhilrajeev@xxxxxxxxxx>

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> ---
>  drivers/i3c/master.c       | 91 ++++++++++++++++++++++----------------
>  include/linux/i3c/master.h |  5 ++-
>  2 files changed, 55 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index c32847bc4d0d..2c479fecbfdf 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -5,16 +5,19 @@
>   * Author: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/atomic.h>
>  #include <linux/bug.h>
>  #include <linux/device.h>
>  #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 +500,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 +510,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 +814,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 = {
> @@ -995,7 +998,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);
>  }
>
> @@ -1783,7 +1786,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) {
> @@ -2402,8 +2405,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;
> @@ -2413,9 +2416,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
> @@ -2431,14 +2438,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;
> @@ -2461,7 +2468,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;
>
> @@ -2478,14 +2485,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;
> @@ -2493,7 +2500,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;
>
> @@ -2502,25 +2509,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;
>  	}
> @@ -2530,10 +2537,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;
> @@ -2588,7 +2595,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;
> @@ -2707,7 +2714,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;
> @@ -2716,7 +2724,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);
> @@ -3017,7 +3027,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;
> @@ -3036,13 +3046,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;
>
> @@ -3300,11 +3310,14 @@ static int __init i3c_init(void)
>  {
>  	int res;
>
> -	res = of_alias_get_highest_id("i3c");
> -	if (res >= 0) {
> -		mutex_lock(&i3c_core_lock);
> -		__i3c_first_dynamic_bus_num = res + 1;
> -		mutex_unlock(&i3c_core_lock);
> +	/* of_alias_get_highest_id is DT-specific, only call for DT systems */
> +	if (IS_ENABLED(CONFIG_OF)) {
> +		res = of_alias_get_highest_id("i3c");
> +		if (res >= 0) {
> +			mutex_lock(&i3c_core_lock);
> +			__i3c_first_dynamic_bus_num = res + 1;
> +			mutex_unlock(&i3c_core_lock);
> +		}
>  	}
>
>  	res = bus_register_notifier(&i2c_bus_type, &i2cdev_notifier);
> 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
>