Re: [PATCH v2 3/3] iio: flow: add Sensirion SLF3S liquid flow sensor driver

[Wadim Mueller]

On Thu, 28 May 2026 12:22:44 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

Thanks for the review (and to Rodrigo for jumping in). Inline
replies, will address all of it in v3.

> Check these.  I'd expect dev_printk.h for instance.
> Follow IWYU (approximately) for includes.

Will go through the includes and add the missing ones for what is
used directly.

> I don't think this [SLF3S_MEAS_LEN] constant being a define is
> that useful. [...] use ARRAY_SIZE() for the loop

Will move the literal to the buf declaration and use
ARRAY_SIZE(buf).

> I'd put this value [SLF3S_TEMP_SCALE_MILLIC] inline with the
> comment.

Will inline at the call site and drop the define.

> Probably more readable with 600 * MICRO / 30 * MICRO / 1920 * MILLI

Ack, will do.

> I'd be consistent and have const u8 block[at_least 3]
> [...] Use at_least, not static for kernel code

Will switch both helpers to [at_least N].

> wrap at 80 chars

Will reflow.

> If this happens I'd go with the detection over the dt provided.
> And dev_info for the mismatch [...]

Will invert: detection wins, DT is fallback for unknown sub-type,
mismatch goes to dev_info.

> More than likely we need some level of sleep here for the device to
> wake up. Is there anything in the datasheet?

Yes, tPU = 25 ms max (time to sensor ready). Will add
fsleep(25000) after the regulator enable. Also tw = 60 ms typ
(warm-up until output within spec), so will bump the existing
SLF3S_MEAS_START_DELAY_US from 12 ms to 60 ms.

> How useful is the generic compatible?

Will be dropped in v3 (agreed with Krzysztof in the binding
thread). Also moving sensirion,medium out of DT into a sysfs
attribute, since it's runtime config.

> Sashiko (probably correctly) identifies that the formatting that the
> IIO core does for an IIO_VAL_FRACTIONAL only goes to 9 decimal places.
> [...] maybe switch to using IIO_VAL_DECIMAL64_PICO
> +CC Rodrigo

Confirmed, the SLF3S-0600F scale (~1.67e-9 l/s/LSB) gets
truncated to 1 digit, the 1300F loses some precision too.

I would prefer waiting for Rodrigo's IIO_VAL_DECIMAL64_PICO over
bumping the FRACTIONAL formatter, since pico covers all variants
and avoids changing core behaviour for everyone. Plan would be
to send v3 with all other points addressed and respin to v4 once
PICO is in mainline. Ok with you, or would you rather see the
FRACTIONAL bump now?

Thanks again,
Wadim