Re: [PATCH v3 4/8] misc: amd-sbi: Consolidate Common SBTSI Probe Path for I2C and I3C

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The auxiliary device ID (`adev->id`) is set directly to the bus device address, causing sysfs naming collisions on multi-bus systems.
--

commit 30f30dcfe1c0110443187a8e9e5a7a6452445301
Author: Prathima <Prathima.Lk@amd.com>

misc: amd-sbi: Consolidate Common SBTSI Probe Path for I2C and I3C

This commit refactors the shared probe procedures into a new common
function, sbtsi_probe_common(). This consolidation ensures that I2C
and I3C probes can share the bus-specific allocation and device
configuration logic.

> diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
> index dfdd730b906af..a4c7e1be5624d 100644
> --- a/drivers/misc/amd-sbi/tsi.c
> +++ b/drivers/misc/amd-sbi/tsi.c
> @@ -84,25 +84,35 @@ static int sbtsi_create_hwmon_adev(struct device *dev, u8 dev_addr)

[ ... ]

> +static int sbtsi_probe_common(struct device *dev, struct sbtsi_data *data)
> +{
> +	u8 val;
> +	int err;
> +
> +	err = sbtsi_xfer(data, SBTSI_REG_CONFIG, &val, true);
> +	if (err)
> +		return err;
> +
> +	data->ext_range_mode = FIELD_GET(BIT(SBTSI_CONFIG_EXT_RANGE_SHIFT), val);
> +	data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), val);
> +
> +	dev_set_drvdata(dev, data);
> +	return sbtsi_create_hwmon_adev(dev, data->dev_addr);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to use
data->dev_addr directly as the auxiliary device ID here?

Looking at sbtsi_create_hwmon_adev(), it assigns adev->id = dev_addr. This
causes sysfs naming collisions on multi-bus systems. While the existing code
comment assumes unique static addresses per socket, this breaks down on
topologies where identical sensors reside on independent I2C controllers.

Furthermore, if an upcoming patch introduces I3C support and sets this
address from the I3C dynamic address, collisions are virtually guaranteed
on multi-bus I3C systems. When a collision occurs, device_add() will fail
with a duplicate sysfs kobject name error, causing the probe to
deterministically fail.

Should this be updated to use the ida subsystem (e.g., ida_alloc()) to
assign guaranteed unique auxiliary device IDs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622135821.2190260-1-Akshay.Gupta@amd.com?part=4