linux-kernel - Re: [PATCH 1/8] dt-bindings: iio: adc: ad7606: Make corrections on spi conditions

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20240817160551.35353140@jic23-huawei>
Date: Sat, 17 Aug 2024 16:05:51 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Conor Dooley <conor@...nel.org>
Cc: Guillaume Stols <gstols@...libre.com>, Uwe Kleine-König <ukleinek@...nel.org>, Lars-Peter Clausen
 <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Jonathan Corbet <corbet@....net>, linux-pwm@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
 aardelean@...libre.com
Subject: Re: [PATCH 1/8] dt-bindings: iio: adc: ad7606: Make corrections on
 spi conditions

On Thu, 15 Aug 2024 15:35:34 +0100
Conor Dooley <conor@...nel.org> wrote:

> On Thu, Aug 15, 2024 at 12:11:55PM +0000, Guillaume Stols wrote:
> > The SPI conditions are not always required, because there is also a
> > parallel interface. The way used to detect that the SPI interface is
> > used is to check if the reg value is between 0 and 256.
> > There is also a correction on the spi-cpha that is not required when SPI
> > interface is selected, while spi-cpol is.  
> 
> This feels like it should be two patches, with the first having a Fixes:
> tag etc, if the original binding was incorrect.
> 
> > 
> > Signed-off-by: Guillaume Stols <gstols@...libre.com>
> > ---
> >  .../devicetree/bindings/iio/adc/adi,ad7606.yaml         | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > index 69408cae3db9..c0008d36320f 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > @@ -117,15 +117,26 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > -  - spi-cpha
> >    - avcc-supply
> >    - vdrive-supply
> >    - interrupts
> >    - adi,conversion-start-gpios
> >  
> > -allOf:
> > -  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +# This checks if reg is a chipselect so the device is on an SPI
> > +# bus, the if-clause will fail if reg is a tuple such as for a
> > +# platform device.
> > +if:
> > +  properties:
> > +    reg:
> > +      minimum: 0
> > +      maximum: 256

That's not particularly nice - in theory the parallel bus memory map could
be at 0 - it's just very unlikely on a real platform.

I'd just do what we do with i2c/spi drivers and just not make it required at all.
Rely on comments to say why.

In ideal case we'd figure out from the parent node if it was an spi bus
but I have no idea how that might be enforced.

> > +then:
> > +  allOf:
> > +    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +    - required:
> > +        - spi-cpol
> >  
> > +allOf:
> >    - if:
> >        properties:
> >          compatible:
> > 
> > -- 
> > 2.34.1
> >