Re: [PATCH v6 3/4] hwmon: (sht3x) Add devicetree support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using enum value 0 as OF match data combined with missing I2C ID table entries causes incorrect device identification and brittle fallback logic.
--

commit e029bdd9863615efb54ed7864138224170daf0d1
Author: Zaixiang Xu <zaixiang.xu.dev@gmail.com>

hwmon: (sht3x) Add devicetree support

Add of_match_table to support devicetree based instantiation.
Specific match data is used to distinguish between temperature/humidity
(SHT) and temperature-only (STS) sensors.

> diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
> index 08306ccb6d0b..6437568aaf73 100644
> --- a/drivers/hwmon/sht3x.c
> +++ b/drivers/hwmon/sht3x.c
> @@ -939,8 +939,25 @@ static const struct i2c_device_id sht3x_ids[] = {
>  
>  MODULE_DEVICE_TABLE(i2c, sht3x_ids);
>  
> +static const struct of_device_id sht3x_of_match[] = {
> +	{ .compatible = "sensirion,sht30", .data = (void *)(uintptr_t)sht3x },
> +	{ .compatible = "sensirion,sht31", .data = (void *)(uintptr_t)sht3x },
> +	{ .compatible = "sensirion,sht35", .data = (void *)(uintptr_t)sht3x },
> +	{ .compatible = "sensirion,sht85", .data = (void *)(uintptr_t)sht3x },
> +	{ .compatible = "sensirion,sts30", .data = (void *)(uintptr_t)sts3x },
> +	{ .compatible = "sensirion,sts31", .data = (void *)(uintptr_t)sts3x },
> +	{ .compatible = "sensirion,sts32", .data = (void *)(uintptr_t)sts3x },
> +	{ .compatible = "sensirion,sts35", .data = (void *)(uintptr_t)sts3x },
> +	{ }
> +};

Since sht3x is the first element of enum sht3x_chips and evaluates to 0, does
setting .data to (void *)(uintptr_t)sht3x essentially set it to NULL?

When instantiated via device tree, does device_get_match_data() return NULL
for these entries, causing i2c_get_match_data() to incorrectly fall back to
i2c_match_id()?

If instantiated manually via sysfs without a device tree node,
device_get_match_data() also returns NULL. If i2c_get_match_data() then falls
back to i2c_match_id(), does it fail to match the STS devices because they
are missing from sht3x_ids, causing it to return NULL?

When sht3x_probe() casts this NULL back to 0, does it incorrectly identify an
STS sensor as an SHT sensor, exposing non-functional humidity attributes?

Would it help to offset the chip enum values so they avoid 0, and to
explicitly add the newly added string variants to the I2C id_table?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1777374569-59233-1-git-send-email-zaixiang.xu.dev@gmail.com?part=3