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

[Frank Li]

On Mon, Jul 27, 2026 at 08:26:52AM +0000, Akhil R wrote:
> Hi Adrian,
>
> On Wed, 22 Jul 2026 13:38:43 +0300 Adrian Hunter wrote:
> > On 21/07/2026 07:07, Akhil R wrote:
> >> 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@xxxxxxx>
> >> Signed-off-by: Akhil R <akhilrajeev@xxxxxxxxxx>
> >> ---
> >>  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 f1be38a640ca..4b3d9628bc39 100644
> >> --- a/drivers/i3c/master.c
> >> +++ b/drivers/i3c/master.c
> >> @@ -13,10 +13,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>
> >> @@ -491,7 +493,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;
> >>
> >> @@ -501,8 +503,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) {
> >> @@ -837,7 +839,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 = {
> >> @@ -1044,7 +1046,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);
> >>  }
> >>
> >> @@ -1928,7 +1930,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);
> >
> > i3c_device_release() "puts" the node so there should be a "get" here.
> > Should probably fix that before switching to device properties.
> >
> > Also seems like the of_node_get() for boardinfo for both i3c and i2c
> > has no balancing put.  Again, it would be good to get that sorted out
> > before switching to device properties.
>
> Thanks for the review.
>
> I understand the issue of unbalanced get/put, but given the size and
> spread of this series, mixing it up with fixes may make it difficult
> to review and may dilute its intent. This patch is a 1:1 of_node ->
> fwnode conversion and keeps the exact same get/put balance, so it
> should not regress anything.
>
> If you agree, the fixes and pre-existing issues can be taken up
> separately.

You can omit all pre-existing issue for this patch set. Encourage fix
pre-existing issues by new patch.

Frank

>
> Best Regards,
> Akhil