linux-kernel - Re: [PATCH] dt-bindings: max310x: convert to YAML

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] [day] [month] [year] [list]

Message-Id: <20231012101402.6cbea154169f64c6b0922ed7@hugovil.com>
Date:   Thu, 12 Oct 2023 10:14:02 -0400
From:   Hugo Villeneuve <hugo@...ovil.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Hugo Villeneuve <hvilleneuve@...onoff.com>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
        devicetree@...r.kernel.org, andriy.shevchenko@...ux.intel.com
Subject: Re: [PATCH] dt-bindings: max310x: convert to YAML

On Thu, 12 Oct 2023 16:01:54 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:

> On 12/10/2023 15:59, Hugo Villeneuve wrote:
> >>> +  clock-frequency:
> >>> +    description:
> >>> +      When there is no clock provider visible to the platform, this
> >>> +      is the source crystal frequency for the IC in Hz.
> >>> +    minimum: 1000000
> >>> +    maximum: 4000000
> >>
> >> This wasn't in original binding. Explain this in the commit msg.
> > 
> > I will add the corresponding explanation in V2. 
> > 
> > The 'clock-frequency' property is already supported
> > by the driver but was not documented in the original txt binding.
> > 
> > This is related to the commit d4d6f03c4fb3: serial: max310x: Try to
> > get crystal clock rate from property (Author: Andy Shevchenko):
> >     
> >     In some configurations, mainly ACPI-based, the clock frequency of
> >     the device is supplied by very well established 'clock-frequency'
> >     property. Hence, try to get it from the property at last if no other
> >     providers are available.
> 
> This does not justify adding it to bindings. ACPI stuff stays with ACPI.
> Drop it.

Hi,
ok, no problem. I will drop it. I will also drop a similar property
from nxp,sc16is7xx.yaml which was added by me a few weeks ago (in a
separate patch, of course).


> > 
> > 
> >>
> >>> +
> >>> +  clock-names:
> >>> +    enum:
> >>> +      - xtal # External crystal
> >>> +      - osc  # External clock source
> >>
> >> clock-names follow immediately clocks.
> > 
> > Will fix for V2.
> > 
> > 
> >>
> >>> +
> >>> +  gpio-controller: true
> >>> +
> >>> +  "#gpio-cells":
> >>> +    const: 2
> >>> +
> >>> +  gpio-line-names:
> >>> +    minItems: 1
> >>> +    maxItems: 16
> >>> +
> >>> +allOf:
> >>
> >> allOf: block goes after required: block.
> > 
> > Will fix for V2.
> > 
> > 
> >>
> >>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >>> +  - $ref: /schemas/serial/serial.yaml#
> >>> +  - $ref: /schemas/serial/rs485.yaml#
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - interrupts
> >>> +
> >>> +oneOf:
> >>> +  - required:
> >>> +      - clocks
> >>> +      - clock-names
> >>> +  - required:
> >>> +      - clock-frequency
> >>
> >> That's also something new as well. The original binding required clocks.
> >> Why are you changing this?
> > 
> > See explanation above about clock-frequency.
> > 
> > If clocks is not provided, than at least 'clock-frequency' must be
> > provided.
> > 
> > 
> >>> +
> >>> +unevaluatedProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    #include <dt-bindings/interrupt-controller/irq.h>
> >>> +    i2c {
> >>> +        #address-cells = <1>;
> >>> +        #size-cells = <0>;
> >>> +
> >>> +        serial@2c {
> >>> +            compatible = "maxim,max3107";
> >>> +            reg = <0x2c>;
> >>> +            clocks = <&xtal4m>;
> >>> +            clock-names = "xtal";
> >>> +            interrupt-parent = <&gpio3>;
> >>> +            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> >>> +            gpio-controller;
> >>> +            #gpio-cells = <2>;
> >>> +        };
> >>> +
> >>
> >> One example is enuogh. All other are the same.
> > 
> > Not really, clock-names is different for example 2 (osc), and example 3
> > shows that 'clock-frequency' can be used if no clock is provided?
> 
> Difference by one property does not really justify new example. The
> third case - clock frequency - is okay, but the property is going away.

Ok, I will drop examples 2 and 3.

Hugo.