Re: [PATCH v2 3/5] dt-bindings: iio: temperature: ltc2983: refine

[Jonathan Cameron]

On Sun, 23 Oct 2022 15:46:13 +0200
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:

> On Sun, 2022-10-23 at 13:51 +0100, Jonathan Cameron wrote:
> > On Thu, 20 Oct 2022 12:02:55 +0300
> > Cosmin Tanislav <demonsingur@xxxxxxxxx> wrote:
> >   
> > > From: Cosmin Tanislav <cosmin.tanislav@xxxxxxxxxx>
> > > 
> > >  * make sure addresses are represented as hex
> > >  * add note about wrong unit value for adi,mux-delay-config-us
> > >  * simplify descriptions
> > >  * add descriptions for the items of custom sensor tables
> > >  * add default property values where applicable
> > >  * use conditionals to extend minimum reg value
> > >    for single ended sensors
> > >  * remove " around phandle schema $ref
> > >  * remove label from example and use generic temperature
> > >    sensor name
> > > 
> > > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@xxxxxxxxxx>  
> > 
> > Hi Cosmin,
> > 
> > Just one question inline from me (other than the build bot report
> > that I'll
> > assume you'll fix for v3).
> > 
> > Otherwise looks like a nice cleanup to me.
> > 
> > I wonder a bit on whether it is worth splitting up, but that would be
> > rather messy to actually do so will leave that to the dt experts to
> > comment
> > on.
> > 
> > Jonathan
> > 
> >   
> > > ---
> > >  .../bindings/iio/temperature/adi,ltc2983.yaml | 309 +++++++++++---
> > > ----
> > >  1 file changed, 182 insertions(+), 127 deletions(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > > l
> > > b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > > l
> > > index 722781aa4697..3e97ec841fd6 100644
> > > ---
> > > a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > > l
> > > +++
> > > b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yam
> > > l
> > > @@ -26,25 +26,25 @@ properties:
> > >  
> > >    adi,mux-delay-config-us:
> > >      description:
> > > -      The LTC2983 performs 2 or 3 internal conversion cycles per
> > > temperature
> > > -      result. Each conversion cycle is performed with different
> > > excitation and
> > > -      input multiplexer configurations. Prior to each conversion,
> > > these
> > > -      excitation circuits and input switch configurations are
> > > changed and an
> > > -      internal 1ms delay ensures settling prior to the conversion
> > > cycle in most
> > > -      cases. An extra delay can be configured using this property.
> > > The value is
> > > -      rounded to nearest 100us.
> > > +      Extra delay prior to each conversion, in addition to the
> > > internal 1ms
> > > +      delay, for the multiplexer to switch input configurations
> > > and
> > > +      excitation values.
> > > +
> > > +      This property is supposed to be in microseconds, but to
> > > maintain
> > > +      compatibility, this value will be multiplied by 100 before
> > > usage.  
> > 
> > This new text has me a little confused.  Previously we talked
> > rounding, now it
> > is saying the value is multiplied (which would make it definitely not
> > in micro
> > secs!)..  So are we papering over a driver bug here?
> > 
> >   
> 
> Hi Jonathan,
> 
> Let me try to make this one clear as it was my mess...
> 
> The multiplication is done internally by the device. I messed up in
> this one as this value is clearly not in us but it is the raw value.
> So, tecnically, there's nothing wrong in the driver as it just reads
> this property and directly writes it. But of course this is misleading
> and wrong from the bindings point of view.
> 
> That said, me and Cosmin did spoke about just having this property
> 'deprecated' and add a new one (the driver would need to be changed
> accordingly) - no idea also about a new name for it :)
> 
> But for this round, Cosmin decided to have this stated on the
> description and see what you and dt maintainers had to say about it and
> if making it 'deprecated' is the way to go (or something else).

Deprecating and replacing with something right definitely seems like
correct option to me.  Naming always fun though when we already used the
most obvious name ;)

Thanks for the explanation.

Jonathan

> 
> - Nuno Sá
>