Devicetree
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Krzysztof Kozlowski <krzk@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, 21 Jun 2026 14:51:17 +0100	[thread overview]
Message-ID: <20260621145117.70b2d50e@jic23-huawei> (raw)
In-Reply-To: <84503093-4bff-4c93-aff8-aa07e1a6a1a1@kernel.org>

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.

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.
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?
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)

I'm a little bothered by the fuzzy boundaries between those cases
so would ideally like rules that say 1-3 need fallbacks or none do
but we live in a fuzzy world, so maybe best we can do is leave things
flexible.

Jonathan



> 
> Best regards,
> Krzysztof


  reply	other threads:[~2026-06-21 13:51 UTC|newest]

Thread overview: 15+ 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 [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=20260621145117.70b2d50e@jic23-huawei \
    --to=jic23@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=krzk+dt@kernel.org \
    --cc=krzk@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