linux-kernel - Re: [PATCHv3] dt-bindings: mfd: omap-usb-tll: 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: <r4gzj34igulq7ivb6jed5liw5ya62b7fyhp4aqpuuc6wok2bp5@wbyxkpa7q6wd>
Date: Tue, 17 Dec 2024 07:38:15 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Karol Przybylski <karprzy7@...il.com>
Cc: andreas@...nade.info, rogerq@...nel.org, lee@...nel.org, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	devicetree@...r.kernel.org, linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org, 
	skhan@...uxfoundation.org
Subject: Re: [PATCHv3] dt-bindings: mfd: omap-usb-tll: convert to YAML

On Sat, Dec 14, 2024 at 10:52:07PM +0100, Karol Przybylski wrote:
> Conversion of omap-usb-tll.txt into yaml format, inspired by discussion in
> lore.kernel.org/all/cd915c18-7230-4c38-a860-d2a777223147@...nel.org/
> 
> Changes made to the binding during conversion:
> - Added Roger Quadros as maintainer
> - Removed deprecated ti,hwmod
> - Renamed .yaml file to ti,usbhs-tll.yaml
> 
> Signed-off-by: Karol Przybylski <karprzy7@...il.com>
> ---

Changelog goes here. I asked about this last time.

Use b4 or `git format-patch -v3`, so there will be proper space before
v3 in subject prefix.

These are nits but the problem is that you never responded to my review
and did not implement it fully.

>  .../devicetree/bindings/mfd/omap-usb-tll.txt  | 27 -----------
>  .../devicetree/bindings/mfd/ti,usbhs-tll.yaml | 47 +++++++++++++++++++
>  2 files changed, 47 insertions(+), 27 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/omap-usb-tll.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,usbhs-tll.yaml

...

> +  clocks:
> +    minItems: 1
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: usb_tll_hs_usb_ch0_clk
> +      - const: usb_tll_hs_usb_ch1_clk
> +      - const: usb_tll_hs_usb_ch2_clk
> +    minItems: 1

Same question as last time - why the number of clocks is flexible?
Original binding suggested that if clocks are present, then three clocks
are required, so you are changing here something which should be
explained in commit msg.

More over, implementation suggests that you could provide clock "ch2"
without "ch0" and "ch1", which your binding does not allow. So again a
change or deviation from the current ABI.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usbhstll@...62000 {
> +      compatible = "ti,usbhs-tll";
> +      reg = <0x4a062000 0x1000>;
> +      interrupts = <78>;

Nothing improved here. See my last comment.

Best regards,
Krzysztof