From: Krzysztof Kozlowski <krzk@kernel.org>
To: Jonathan Cameron <jic23@kernel.org>
Cc: "Wadim Mueller" <wafgo01@gmail.com>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Maxwell Doose" <m32285159@gmail.com>,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Marcelo Schmitt" <marcelo.schmitt1@gmail.com>,
"Rodrigo Alencar" <455.rodrigo.alencar@gmail.com>
Subject: Re: [PATCH v4 4/4] iio: flow: add Sensirion SLF3S liquid flow sensor driver
Date: Sun, 19 Jul 2026 08:18:28 +0200 [thread overview]
Message-ID: <31eb950b-3848-4f0e-80ae-804d89ca4c5d@kernel.org> (raw)
In-Reply-To: <20260621145117.70b2d50e@jic23-huawei>
On 21/06/2026 15:51, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 06:27:00 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
>> On 14/06/2026 17:10, Jonathan Cameron wrote:
>>> On Sat, 13 Jun 2026 09:51:13 +0200
>>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>>> On 12/06/2026 19:47, Jonathan Cameron wrote:
>>>>> On Thu, 11 Jun 2026 16:01:12 +0200
>>>>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>
>>>>>> On 11/06/2026 15:27, Wadim Mueller wrote:
>>>>>>> +
>>>>>>> +static const struct of_device_id slf3s_of_match[] = {
>>>>>>> + { .compatible = "sensirion,slf3s-0600f", .data = &slf3s_variants[0] },
>>>>>>> + { .compatible = "sensirion,slf3s-1300f", .data = &slf3s_variants[1] },
>>>>>>> + { .compatible = "sensirion,slf3s-4000b", .data = &slf3s_variants[2] },
>>>>>>
>>>>>> You should have only 1300f here and detect the variants. That was my
>>>>>> point when I suggested to use the fallback.
>>>>>>
>>>>>
>>>>> I'm lost. How does that work? They cannot fallback to that part because
>>>>> it relies on in driver detection of the fact that they are incompatible.
>>>>>
>>>>
>>>> I am lost too. Then why were they made compatible in the binding?
>>>>
>>>> Entire discussion was that these are FULLY compatible due to variant
>>>> detection. That was the entire point of discussing more generic
>>>> fallback. Using specific fallback does not change that - it is the same
>>>> concept.
>>>>
>>>> If devices are not detectable, why were we discussing any compatibility?
>>>
>>> They are detectable, but the feature set is not, so to me there is zero valid
>>> in a generic fallback, we have to update the driver every time a new part comes
>>> along. (i.e. I agree with Conor's reply to the previous version thread).
>>> A specific fallback to a completely compatible part would be fine as there
>>> would be sufficient info to not need the ID lookup.
>>
>> This patch has specific fallback and we discuss this now.
>>
>>>
>>> The case in the binding for this version is the worst of all options because
>>> it implies it is valid to fallback to something that gives a false impression
>>> of being specific when it's relying on ID matching to say actually it's something
>>> else.
>>>
>>> So definitely not
>>> + compatible:
>>> + oneOf:
>>> + - const: sensirion,slf3s-1300f
>>> + - items:
>>> + - enum:
>>> + - sensirion,slf3s-0600f
>>> + - sensirion,slf3s-4000b
>>> + - const: sensirion,slf3s-1300f
>>> +
>>>
>>> Falling back to slf3s is better than this, but I'd rather not have a fallback
>>> at all, thus allowing correct fallback to the parts listed here in future.
>>
>> Why? These devices are fully detectable thus fully compatible.
>
> Ok. We aren't really making progress. Let me layout the three approaches.
> + advantages and disadvantages as I understand it.
>
> What we have here:
> compatible:
> oneOf:
> - const: sensirion,slf3s-1300f
> - items:
> - enum:
> - sensirion,slf3s-0600f
> - sensirion,slf3s-4000b
> - const: sensirion,slf3s-1300f
>
> Advantages: Fallback to a real part.
> Disadvantages:
> - Requires a driver change even for drop in replacements (which will have
> different WHOAMI IDs.
> - To me it implies that a driver that supports slf3s-1300f should just
> work with parts we haven't seen yet if they have that in their fallback
> lists. I appreciate you think that we shouldn't put effort into supporting
> that case, but to me it seems more or less free to support and is useful.
>
> Generic compatible for fallback.
> compatible:
> - items:
> - enum:
> - sensirion,slf3s-0600f
> - sensirion,slf3s-1300f
> - sensirion,slf3s-4000b
> - const: sensirion,slf3s
>
> Advantages: Fallback to something that doesn't imply the post fix is coming from
> DT. So kind of hints we need detection code.
> Disadvantages: requires driver changes for each fully compatible drop in part.
> Note I'm not sure what this brings us over just having one compatible... Maybe
> we'll some day get a non detectable difference where we need to use the binding
> but seems fairly unlikely (I've seen that happen once when a manufacturer failed
> to update a WHOAMI reg value for a new part).
>
> No fallbacks
> compatible:
> enum:
> - sensirion,slf3s-0600f
> - sensirion,slf3s-1300f
> - sensirion,slf3s-4000b
>
> Advantages: (Common!) a future drop in compatible part works with old kernels
> as the driver knows a match + part information to use if the ID
> match fails.
> Disadvantages: Not sure I see any... However I'm guessing I'm missing something.
The same disadvantages as in other cases:
"Requires a driver change even for drop in replacements (which will have
different WHOAMI IDs."
But I think I get your main point - since we need to compare WHOAMI
registers for fallback cases, then there is no benefit of using the
fallback in the first place, so let's go with original approach.
>
> Note for this topic I think it would be great to capture some best practice
> information somewhere so we can improve consistency. I've looked around and
> there are some bits of feedback in numerous threads but those are hard to track
> down.
>
> The Dt spec refers to these as being about programming mode + driver selection.
> I will note that we have very broad families of devices that have a somewhat
> standard register interface + do always put the WHOAMI in the same place.
> In the vendor drivers those match a single hydra of a driver; upstream we
> have multiple drivers as devices do very different things and hence in
> practice almost no code would have been shared - so I have no idea what
> we should have done for those more commplex cases. Following your logic
> (I may have it wrong!) I think we would have had a generic compatible
> for the whole family but in upstream Linux no driver would have bound
> to that. I guess there are different extremes:
>
> 1) This case - register interfaces is consistent for all registers,
> we are just matching on constant tables needed to interpret the
> data in those tables.
Interpreting data in registers is the programming interface, therefore
the programming interface of that part is different. However since there
is reliable autodetection of a model, the programming interface is
considered compatible.
> 2) Similarish. Main register interface shared, but optional additional
> parts. If one device is a strict superset, a fallback is
> appropriate (in my view) but if I follow your points here then they
> should all have a single fallback compatible.
> 3) Not particularly similar, but still detectable in a consistent
> way. To me not appropriate to have a common fallback as based on
> the dt-spec thing about matching drivers, they will typically
> match different drivers. Maybe we should have a common fallback
> but typically not bind to it?
Unused common fallback rather suggests they are not compatible. The
purpose of the fallback is for SW to use. If it is not used, let's just
skip the fallback.
> 4) Detection code becomes a mess and is different for each part
> (see the old i2c_detect stuff) So hopefully no one thinks we
> should use a fallback once we reach
> this level of complexity (unless part are genuinely compatible)
Yes.
Best regards,
Krzysztof
next prev parent reply other threads:[~2026-07-19 6:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-11 13:26 [PATCH v4 0/4] iio: flow: Sensirion SLF3S liquid flow sensor Wadim Mueller
2026-06-11 13:26 ` [PATCH v4 1/4] iio: types: add IIO_VOLUMEFLOW channel type Wadim Mueller
2026-06-11 13:26 ` [PATCH v4 2/4] dt-bindings: iio: flow: add Sensirion SLF3S liquid flow sensor Wadim Mueller
2026-06-11 14:01 ` Krzysztof Kozlowski
2026-06-11 13:26 ` [PATCH v4 3/4] iio: core: add IIO_VAL_DECIMAL64_FEMTO format type Wadim Mueller
2026-06-12 9:11 ` Rodrigo Alencar
2026-06-11 13:27 ` [PATCH v4 4/4] iio: flow: add Sensirion SLF3S liquid flow sensor driver Wadim Mueller
2026-06-11 14:01 ` Krzysztof Kozlowski
2026-06-12 17:47 ` Jonathan Cameron
2026-06-13 7:51 ` Krzysztof Kozlowski
2026-06-14 15:10 ` Jonathan Cameron
2026-06-15 4:27 ` Krzysztof Kozlowski
2026-06-21 13:51 ` Jonathan Cameron
2026-07-08 6:50 ` Wadim Mueller
2026-07-14 0:56 ` Maxwell Doose
2026-07-19 6:18 ` Krzysztof Kozlowski [this message]
2026-06-11 19:18 ` Andy Shevchenko
2026-06-14 15:19 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=31eb950b-3848-4f0e-80ae-804d89ca4c5d@kernel.org \
--to=krzk@kernel.org \
--cc=455.rodrigo.alencar@gmail.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m32285159@gmail.com \
--cc=marcelo.schmitt1@gmail.com \
--cc=nuno.sa@analog.com \
--cc=robh@kernel.org \
--cc=wafgo01@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome