Re: [RFC PATCH v1 3/4] iio: flow: add Sensirion SLF3x liquid flow sensor driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The fractional scale for the SLF3S-4000B variant evaluates to exactly 0.0 in sysfs due to IIO formatting logic, permanently zeroing out flow readings for userspace.
- [Medium] Concurrent initialization of the shared global `slf3x_crc_table` in `probe()` causes a data race.
- [Medium] Continuous measurement mode is not restarted after a system suspend/resume cycle.
--

commit 2a3dbf69e1c64c32d26f6f97455487233c63c7c0
Author: Wadim Mueller <wadim.mueller@cmblu.de>

iio: flow: add Sensirion SLF3x liquid flow sensor driver

Add an IIO driver for the Sensirion SLF3S family of digital
liquid-flow sensors. The supported sub-types (SLF3S-0600F,
SLF3S-4000B) share the same register map and command set and are
distinguished only by the flow scale.

> diff --git a/drivers/iio/flow/slf3x.c b/drivers/iio/flow/slf3x.c
> new file mode 100644
> index 0000000000000..e4ee1a04a46d7
> --- /dev/null
> +++ b/drivers/iio/flow/slf3x.c

[ ... ]

> +static const struct slf3x_variant slf3x_variants[] = {
> +	{ .sub_type = 0x03, .name = "slf3s-0600f",
> +	  .scale_num = 1, .scale_den = 6000000 },
> +	{ .sub_type = 0x05, .name = "slf3s-4000b",
> +	  .scale_num = 1, .scale_den = 1666680000 },

[Severity: High]
Will the fractional scale for the SLF3S-4000B variant permanently zero out
flow readings for userspace?

The driver returns IIO_VAL_FRACTIONAL with val = 1 and val2 = 1666680000.
The IIO core formats fractional values for sysfs up to nanounit precision
(9 decimal places) using the calculation (val * 1000000000LL) / val2.

Because the denominator (1,666,680,000) is strictly larger than the 10^9
multiplier, the integer division computes to exactly 0. As a result, the
string emitted to sysfs will be 0.000000000.

Userspace applications rely on this sysfs attribute to convert raw readings
into physical units, meaning any flow rate reading will be multiplied by 0.0,
rendering the sensor's flow channel completely non-functional.

> +};

[ ... ]

> +static int slf3x_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	struct slf3x_data *sf;
> +	int ret;
> +
> +	ret = devm_regulator_get_enable_optional(dev, "vdd");
> +	if (ret < 0 && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "failed to enable vdd\n");
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*sf));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	sf = iio_priv(indio_dev);
> +	sf->client = client;
> +	crc8_populate_msb(slf3x_crc_table, SLF3X_CRC8_POLY);

[Severity: Medium]
Can this concurrent initialization of the shared global slf3x_crc_table cause
a data race?

The driver defines a global static array slf3x_crc_table and dynamically
populates it by calling crc8_populate_msb() unconditionally during
slf3x_probe().

If multiple SLF3x sensors are present and probed concurrently on different I2C
buses, or if a newly plugged sensor is probed while an already-registered
sensor is handling an active read, multiple threads will concurrently modify
or read the global table without synchronization.

This could result in transient -EIO errors or Kernel Concurrency Sanitizer
warnings.

> +
> +	ret = slf3x_read_product_info(sf);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "product info read failed\n");

[ ... ]

> +static struct i2c_driver slf3x_driver = {
> +	.driver = {
> +		.name = "slf3x",
> +		.of_match_table = slf3x_of_match,
> +	},
> +	.probe = slf3x_probe,
> +	.id_table = slf3x_id,
> +};
> +module_i2c_driver(slf3x_driver);

[Severity: Medium]
Will continuous measurement mode fail to restart after a system suspend/resume
cycle?

The driver initiates the sensor's continuous measurement mode (0x36 0x08)
exclusively in probe() and assumes the device stays in this mode indefinitely
to fetch samples on demand in read_raw().

However, the driver does not implement power management suspend and resume
callbacks. If the system suspends and the sensor loses power (e.g., via
regulator cutoff), it will wake up in its default idle state.

Without a resume callback to re-issue the continuous measurement command,
subsequent read operations in read_raw() will return invalid data, silently
breaking the driver after a suspend/resume cycle.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524205112.26638-1-wafgo01@gmail.com?part=3