Re: [PATCH v4 1/8] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using the I2C client address directly as the auxiliary device instance ID causes sysfs name collisions and probe failures if identical devices reside at the same address on different I2C buses.
--

--- Patch [1]: [PATCH v4 1/8] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc ---
commit 00ce49f593f00850072fde683389c8cb1954ee34
Author: Prathima <Prathima.Lk@amd.com>

hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc

This commit moves the SBTSI core functionality out of the hwmon-only path
and into drivers/misc/amd-sbi so it can be reused. It creates an auxiliary
device for the hwmon driver to bind to and adds the appropriate build
dependencies.

> diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
> new file mode 100644
> index 0000000000000..67d08df28429d
> --- /dev/null
> +++ b/drivers/misc/amd-sbi/tsi.c

[ ... ]

> +static int sbtsi_create_hwmon_adev(struct device *dev, u8 dev_addr)
> +{
> +	struct auxiliary_device *adev;
> +	int ret;
> +
> +	adev = kzalloc_obj(*adev);
> +	if (!adev)
> +		return -ENOMEM;
> +
> +	adev->name = AMD_SBTSI_AUX_HWMON;
> +	adev->id = dev_addr;

[Severity: Medium]
Could using the I2C client address directly as the auxiliary device instance
ID cause sysfs name collisions and probe failures if identical devices reside
at the same address on different I2C buses?

If multiple sensors are present on different I2C buses but share the same I2C
address, the driver assigns them identical auxiliary device IDs. The auxiliary
bus subsystem aggregates all devices into a single virtual namespace
(/sys/bus/auxiliary/devices/amd-sbtsi.temp-sensor.<id>), so device_add()
could return -EEXIST and cause subsequent probes to fail.

While the comments below mention that multi-socket systems provide unique I2C
addresses even across different buses, relying on platform-specific hardware
strapping assumptions for globally unique software identifiers can be fragile.

Would it make sense to use the IDA framework here to dynamically allocate
globally unique instance IDs?

> +	adev->dev.parent = dev;
> +	adev->dev.release = sbtsi_adev_release;
> +
> +	ret = auxiliary_device_init(adev);

[ ... ]

> +static int sbtsi_i2c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct sbtsi_data *data;
> +	int err;

[ ... ]

> +	dev_set_drvdata(dev, data);
> +	/* In a multi-socket system, devices that are otherwise identical do not
> +	 * share the same static address; each instance resides at a unique I2C
> +	 * client address on the same or different bus. Use the I2C client
> +	 * address as the auxiliary device instance ID to ensure each socket
> +	 * receives a distinct auxiliary device name.
> +	 */
> +	return sbtsi_create_hwmon_adev(dev, client->addr);
> +}

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