Re: [PATCH v3 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 2 potential issue(s) to consider:
- [High] Auxiliary device ID collision prevents probing multiple sensors with identical addresses on different buses.
- [Low] SENSORS_SBTSI selects AMD_SBTSI but fails to inherit its dependencies, causing Kconfig unmet dependency warnings and potential build failures.
--

commit 66b0cf6c2e144993bdd6fedccce15184c3257aaf
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 subsystem
and into drivers/misc/amd-sbi so it can be reused by non-hwmon consumers.
It registers an auxiliary device that the hwmon driver can bind to.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index e4c4f2b09732d..8f204cf49b6e0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1963,7 +1963,7 @@ config SENSORS_SL28CPLD
>  
>  config SENSORS_SBTSI
>  	tristate "Emulated SB-TSI temperature sensor"
> -	depends on I2C
> +	select AMD_SBTSI

[Severity: Low]
Will this cause a Kconfig unmet dependency warning?

Because the target config depends on I2C and ARM || ARM64 || COMPILE_TEST,
selecting it here without those same dependencies means SENSORS_SBTSI can
be enabled on unsupported architectures or when I2C is disabled.

This forces the target to be built despite its unsatisfied dependencies,
which could lead to build or link errors.

>  	help
>  	  If you say yes here you get support for emulated temperature
>  	  sensors on AMD SoCs with SB-TSI interface connected to a BMC device.

[ ... ]

> diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
> new file mode 100644
> index 0000000000000..dfdd730b906af
> --- /dev/null
> +++ b/drivers/misc/amd-sbi/tsi.c
> @@ -0,0 +1,134 @@

[ ... ]

> +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;
> +	/*
> +	 * In a multi-socket system, otherwise identical devices do not
> +	 * share the same static address; each instance has its own address,
> +	 * which must be supplied via the device tree (DTS).
> +	 */
> +	adev->id = dev_addr;

[Severity: High]
Does this risk a sysfs name collision on systems with multiple I2C buses?

If multiple independent I2C buses each have a sensor at the same address,
the assigned id will be identical for both.

Because the auxiliary device name is constructed as
amd-sbtsi.temp-sensor.<id>, the second device to probe might fail the
device addition with a sysfs duplicate name warning, leaving the sensor
unusable.

Could a globally unique ID be generated here instead, perhaps using the
IDA framework?

> +	adev->dev.parent = dev;
> +	adev->dev.release = sbtsi_adev_release;

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