Re: [PATCHv3] dt-bindings: mfd: omap-usb-tll: convert to YAML

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx/
> 
> 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@xxxxxxxxx>
> ---

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@4a062000 {
> +      compatible = "ti,usbhs-tll";
> +      reg = <0x4a062000 0x1000>;
> +      interrupts = <78>;

Nothing improved here. See my last comment.

Best regards,
Krzysztof





[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux