linux-kernel - Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

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: <20221121124518.00000da4@Huawei.com>
Date:   Mon, 21 Nov 2022 12:45:18 +0000
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Edmund Berenson <edmund.berenson@...ix.com>
CC:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Lukasz Zemla <Lukasz.Zemla@...dward.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        "Michael Hennerich" <Michael.Hennerich@...log.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] dt-bindings: iio: adc: ad7923: adjust documentation

On Mon, 21 Nov 2022 11:45:32 +0100
Edmund Berenson <edmund.berenson@...ix.com> wrote:

> On Mon, Nov 21, 2022 at 11:31:33AM +0100, Krzysztof Kozlowski wrote:
> > On 21/11/2022 11:26, Edmund Berenson wrote:  
> > > On Mon, Nov 21, 2022 at 10:13:57AM +0100, Krzysztof Kozlowski wrote:  
> > >> On 20/11/2022 18:06, Edmund Berenson wrote:  
> > >>> - Add the ad7927 compatibility string, with fallback compatibility
> > >>> to ad7928.
> > >>> - ad7923 and ad7924 are treated the same in the driver, show
> > >>> the relationship in the documentation.
> > >>>
> > >>> Suggested-by: Lukasz Zemla <Lukasz.Zemla@...dward.com>
> > >>> Signed-off-by: Edmund Berenson <edmund.berenson@...ix.com>
> > >>> ---
> > >>>  .../bindings/iio/adc/adi,ad7923.yaml          | 26 ++++++++++++-------  
> > >>
> > >> Do not respond with new patch to some old thread. Each patchset starts a
> > >> new thread.
> > >>  
> > > Sorry I didn't know this is the preferred way. I will send new patch
> > > version as new thread in the future.  
> > >>>  1 file changed, 17 insertions(+), 9 deletions(-)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > >>> index 07f9d1c09c7d..e553853e25d5 100644
> > >>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > >>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml
> > >>> @@ -11,7 +11,7 @@ maintainers:
> > >>>  
> > >>>  description: |
> > >>>    Analog Devices AD7904, AD7914, AD7923, AD7924 4 Channel ADCs, and AD7908,
> > >>> -   AD7918, AD7928 8 Channels ADCs.
> > >>> +   AD7918, AD7927, AD7928 8 Channels ADCs.
> > >>>  
> > >>>    Specifications about the part can be found at:
> > >>>      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7923.pdf
> > >>> @@ -20,14 +20,22 @@ description: |
> > >>>  
> > >>>  properties:
> > >>>    compatible:
> > >>> -    enum:
> > >>> -      - adi,ad7904
> > >>> -      - adi,ad7914
> > >>> -      - adi,ad7923
> > >>> -      - adi,ad7924
> > >>> -      - adi,ad7908
> > >>> -      - adi,ad7918
> > >>> -      - adi,ad7928
> > >>> +    oneOf:
> > >>> +      - enum:
> > >>> +          - adi,ad7904
> > >>> +          - adi,ad7914
> > >>> +          - adi,ad7908  
> > >>
> > >> You already started shuffling the entries, so make them ordered. What's
> > >> the point of changing the order from one non-sorted to another non-sorted?
> > >>  
> > >>> +          - adi,ad7918
> > >>> +          - adi,ad7923
> > >>> +          - adi,ad7924  
> > >>
> > >> Then deprecate this as alone compatible.
> > >>  
> > >>> +          - adi,ad7927> +          - adi,ad7928  
> > >>
> > >> Ditto
> > >>  
> > >>> +      - items:
> > >>> +          - const: adi,ad7923
> > >>> +          - const: adi,ad7924  
> > >>
> > >> I would expect lower number as fallback.  
> > > If I remove alone compatibility of 7924 and 7927 in the documentation,  
> > 
> > I don't understand. 7924 and 7927 are not compatible with each other -
> > neither in old code nor in new - so what do you want to remove?
> >   
> > > I will have to remove explicit compatibility match on the driver side,
> > > correct?
> > > Just want to make sure I don't misunderstand you.  
> > 
> > My comment to which you responded was about order of items. Usually
> > lower number means older device and usually older device is the fallback.  

Oldest in which sense?  I think it should be oldest in order of having
a binding defined, not in order of part releases (and ADI seem to scramble
part numbers fairly randomly so definitely not generally the case that
ordering of numbers has anything much to do with age of part).




> My response was meant to respond to both your comment to "deprecate
> alone compatibility" and "lower number should be fallback"
> Which I understood in the following way: because 7923, 7924 for one and
> 7927, 7928 are compatible with each other I will remove
> 7924 compatible string from driver and not add 7927 to the driver and
> only add it to the documentation.
> > Best regards,
> > Krzysztof
> >   
> 
> Thank you and best regards,
> Edmund
>