linux-kernel - Re: [PATCH v3 01/10] dt-bindings: iio: adc: Add AD4170

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250521-functional-knowing-hornet-b33a1f@kuoka>
Date: Wed, 21 May 2025 10:33:43 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: David Lechner <dlechner@...libre.com>, 
	Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, jic23@...nel.org, lars@...afoo.de, 
	Michael.Hennerich@...log.com, nuno.sa@...log.com, andy@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, linus.walleij@...aro.org, brgl@...ev.pl
Subject: Re: [PATCH v3 01/10] dt-bindings: iio: adc: Add AD4170

On Fri, May 16, 2025 at 12:45:05PM GMT, Marcelo Schmitt wrote:
> ...
> > > +
> > > +    properties:
> > > +      adi,reference-select:
> > > +        description: |
> > > +          Selects the reference source to use when converting on the specific
> > > +          channel. Valid values are:
> > > +          0: Differential reference voltage REFIN+ - REFIN−.
> > > +          1: Differential reference voltage REFIN2+ - REFIN2−.
> > > +          2: Internal 2.5V referece (REFOUT) relative to AVSS.
> > > +          3: Analog supply voltage (AVDD) relative AVSS.
> > > +        $ref: /schemas/types.yaml#/definitions/uint8
> > > +        enum: [0, 1, 2, 3]
> > Using strings instead of int for this and most of the other custom enums here
> > would make them self-documenting and easier to use.
> 
> The numbers match the values that are documented in the datasheet for each
> option of voltage reference available to use with a channel. So we would be
> using numbers mostly to define values of some unit and pin numbers (e.g. 100 for
> the microamp property)? Not really excited about doing this change because I
> think it will make the dtb a bit larger and the driver code a bit more lengthy,
> but can do that for v4.

You must use what is already there, the same property, the same type.
And since existing property was integer, I am unhappy that Dumitru
Ceclan decided to go with a string in commit 3d50d03f2194 ("dt-bindings:
adc: add AD7173"), although that is probably on us.

So now there is a mess and we are going to have either exception or
warnings for these devices (always), because schema expects exactly one
type.

So instead of coming or proposing the third type, just use what was
already in the bindings.

Same for every other property here, really, don't come with your own
custom stuff.

> 
> ...
> > > +      adi,sensor-type:
> > > +        description: |
> > > +          Type of sensor connected to the device. Depending on the sensor type
> > > +          (weigh scale, RTD, or thermocouple) the values of sensor-node
> > > +          properties have slightly different constraints. This property
> > > +          specifies which particular external sensor is connected to the ADC so
> > > +          the sensor-node properties can be properly parsed and verified. The
> > > +          possible sensor types are:
> > > +          0: weigh scale;
> > > +          1: RTD;
> > > +          2: thermocouple.
> > > +        $ref: /schemas/types.yaml#/definitions/uint8
> > This property seems reduandant since it has to match the node name.
> > 
> > i.e. weighscale@... is is always adi,sensor-type = <0>; and so on.
> 
> Yes, can we rely on node names I'll do that for v4.

Again, don't come with own new way of doing things. What node name?
Isn't this always "channel"?

Best regards,
Krzysztof