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]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux