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

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Multiple `I2cSerialBus` resources in an ACPI device cause resource overwrite and flag accumulation.
- [High] I3C master bus registration aborts completely if an unrelated ACPI child node is encountered.
--

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

i3c: master: Support ACPI enumeration of child devices

This commit adds support for enumerating I3C and I2C child devices of an I3C
master controller via ACPI. It extracts static addresses, LVR, and PID
information from ACPI resources instead of relying exclusively on Device Tree
properties.

[ ... ]

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5e0e926f61f3d..08d0fcabd6f10 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> @@ -2403,6 +2404,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;
> +}

If an ACPI device defines multiple I2cSerialBus resources, will this callback
overwrite boardinfo->base.addr and lvr with the values from the last parsed
resource?

Also, since boardinfo->base.flags uses a bitwise OR without clearing, if any
of the parsed resources are 10-bit but the final one is 7-bit, could the final
device be registered with a 7-bit address but with the 10-bit flag incorrectly
set?

[ ... ]

> @@ -2526,7 +2639,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 the ACPI child node happens to be a non-device companion node (like a
PowerResource or thermal zone) lacking an _ADR method and an I2cSerialBus
resource, i3c_master_add_acpi_dev() will return -ENODEV.

Will returning an error here cause the entire I3C master controller bus
registration to abort just because of an unrelated companion node in the
ACPI namespace?

[ ... ]

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