Messages in this thread Patch in this message | | | From | Yoshihiro Shimoda <> | Subject | RE: [PATCH v2 3/4] dt-bindings: phy: renesas: usb3-phy: convert bindings to json-schema | Date | Fri, 27 Mar 2020 02:39:35 +0000 |
| |
Hi Geert-san,
Thank you for your review! > From: Geert Uytterhoeven, Sent: Thursday, March 26, 2020 5:53 PM <snip> > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/renesas,usb3-phy.yaml > > @@ -0,0 +1,78 @@ > > +# SPDX-License-Identifier: GPL-2.0-only
> In think Rob would prefer > > # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
I got it.
> > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/phy/renesas,usb3-phy.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Renesas R-Car generation 3 USB 3.0 PHY > > + > > +maintainers: > > + - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - renesas,r8a774a1-usb3-phy # RZ/G2M > > + - renesas,r8a774b1-usb3-phy # RZ/G2N > > + - renesas,r8a7795-usb3-phy # R-Car H3 > > + - renesas,r8a7796-usb3-phy # R-Car M3-W > > + - renesas,r8a77965-usb3-phy # R-Car M3-N > > + - const: renesas,rcar-gen3-usb3-phy > > + > > + reg: > > + # base address and length of the registers block for the PHY. > > I think this comment can be removed.
I think so. So, I'll remove it.
> > + maxItems: 1 > > + > > + clocks: > > + # A list of phandles and clock-specifier pairs. > > Likewise.
Yes, I'll remove it.
> "minItems: 2"?
Thank you for pointed it out. Yes, this should have "minItems: 2".
> > + maxItems: 3 > > + > > + clock-names: > > + # If you want to use the ssc, the clock-frequency of usb_extal > > + # must not be 0. > > "minItems: 2"?
Yes, I'll add it.
> > + maxItems: 3 > > + items: > > + - const: usb3-if # The funcional clock > > + - const: usb3s_clk # The usb3's external clock > > + - const: usb_extal # The usb2's external clock > > + > > + '#phy-cells': > > + # see phy-bindings.txt in the same directory > > + const: 0 > > + > > + power-domains: > > + maxItems: 1 > > + > > + resets: > > + maxItems: 1 > > + > > + renesas,ssc-range: > > + description: | > > + Enable/disable spread spectrum clock (ssc). 0 or the property doesn't > > + exist means disabliing the ssc. > > disabling
Oops, I'll fix it.
> The actual value will be -<value> ppm.
Yes, that's right. So, I'll add this sentence.
> > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - enum: [ 0, 4003, 4492, 4980 ] > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - '#phy-cells' > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/r8a7795-cpg-mssr.h> > > + #include <dt-bindings/power/r8a7795-sysc.h> > > + > > + usb-phy@e65ee000 { > > + compatible = "renesas,r8a7795-usb3-phy", "renesas,rcar-gen3-usb3-phy"; > > + reg = <0 0xe65ee000 0 0x90>; > > Examples use the default #{address,size}-cells of <1>, so > > reg = <0xe65ee000 0x90>;
Oh, I didn't know. I'll fix it.
Best regards, Yoshihiro Shimoda
| |