diff mbox series

[1/3] dt-bindings: clock: renesas: rcar-usb2-clock-sel: Fix clock[-name]s properties

Message ID 1571915821-1620-2-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series clk: renesas: rcar-usb2-clock-sel: Fix clks/resets handling | expand

Commit Message

Yoshihiro Shimoda Oct. 24, 2019, 11:16 a.m. UTC
Since the hardware requires to enable both USB 2.0 host and peripheral
functional clock, this patch fixes the documentation.
Fortunately, no one has this device node for now, so that we don't
need to think of backward compatibility.

Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 .../devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt     | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Geert Uytterhoeven Oct. 24, 2019, 11:45 a.m. UTC | #1
Hi Shimoda-san,

On Thu, Oct 24, 2019 at 1:17 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since the hardware requires to enable both USB 2.0 host and peripheral
> functional clock, this patch fixes the documentation.
> Fortunately, no one has this device node for now, so that we don't
> need to think of backward compatibility.
>
> Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks four your patch!

Looks good to me, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

I'm just wondering about the inconsistent use of "_" and "-" in clock
names, but I don't have a better suggestion ("hs-usb-if", "usb_extal",
and "usb_xtal" do match the datasheet), so let's ignore my OCD ;-)

> --- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> @@ -38,7 +38,8 @@ Required properties:
>  - reg: offset and length of the USB 2.0 clock selector register block.
>  - clocks: A list of phandles and specifier pairs.
>  - clock-names: Name of the clocks.
> - - The functional clock must be "ehci_ohci"
> + - The functional clock of USB 2.0 host side must be "ehci_ohci"

"_" means "and".

> + - The functional clock of HS-USB side must be "hs-usb-if"

"-" means concatenation of terms.

>   - The USB_EXTAL clock pin must be "usb_extal"
>   - The USB_XTAL clock pin must be "usb_xtal"

"_" means concatenation of terms.

Gr{oetje,eeting}s,

                        Geert
Yoshihiro Shimoda Oct. 25, 2019, 1:29 a.m. UTC | #2
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, October 24, 2019 8:46 PM
> 
> Hi Shimoda-san,
> 
> On Thu, Oct 24, 2019 at 1:17 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Since the hardware requires to enable both USB 2.0 host and peripheral
> > functional clock, this patch fixes the documentation.
> > Fortunately, no one has this device node for now, so that we don't
> > need to think of backward compatibility.
> >
> > Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Thanks four your patch!
> 
> Looks good to me, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thank you for your review!

> I'm just wondering about the inconsistent use of "_" and "-" in clock
> names, but I don't have a better suggestion ("hs-usb-if", "usb_extal",
> and "usb_xtal" do match the datasheet), so let's ignore my OCD ;-)

I intended to match the names with the datasheet, but "ehci_ohci" doesn't match though...
So, should I change the "ehci_ohci" to "ehci/ohci"?

Best regards,
Yoshihiro Shimoda

> > --- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > @@ -38,7 +38,8 @@ Required properties:
> >  - reg: offset and length of the USB 2.0 clock selector register block.
> >  - clocks: A list of phandles and specifier pairs.
> >  - clock-names: Name of the clocks.
> > - - The functional clock must be "ehci_ohci"
> > + - The functional clock of USB 2.0 host side must be "ehci_ohci"
> 
> "_" means "and".
> 
> > + - The functional clock of HS-USB side must be "hs-usb-if"
> 
> "-" means concatenation of terms.
> 
> >   - The USB_EXTAL clock pin must be "usb_extal"
> >   - The USB_XTAL clock pin must be "usb_xtal"
> 
> "_" means concatenation of terms.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Geert Uytterhoeven Oct. 25, 2019, 7:45 a.m. UTC | #3
Hi Shimoda-san,

On Fri, Oct 25, 2019 at 3:29 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Thursday, October 24, 2019 8:46 PM
> > On Thu, Oct 24, 2019 at 1:17 PM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > Since the hardware requires to enable both USB 2.0 host and peripheral
> > > functional clock, this patch fixes the documentation.
> > > Fortunately, no one has this device node for now, so that we don't
> > > need to think of backward compatibility.
> > >
> > > Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> > Thanks four your patch!
> >
> > Looks good to me, so
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Thank you for your review!
>
> > I'm just wondering about the inconsistent use of "_" and "-" in clock
> > names, but I don't have a better suggestion ("hs-usb-if", "usb_extal",
> > and "usb_xtal" do match the datasheet), so let's ignore my OCD ;-)
>
> I intended to match the names with the datasheet, but "ehci_ohci" doesn't match though...
> So, should I change the "ehci_ohci" to "ehci/ohci"?

I think that's up to you.  Both are fine to me.
Given this is USB2, using "ehci" only may be fine, too.

Note that
  - the only other clock with a slash in its name in the datasheet is
    "ths/thc", which we call "thermal" in the clock drivers,
  - the "ehci/ohci" clocks are called "ehciN" or "usb-ehci" in the clock
    drivers.
Nothing relies on those names, though, and they're not part of any ABI,
unlike the ehci/ohci clock input in this binding.

> > > --- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > > +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > > @@ -38,7 +38,8 @@ Required properties:
> > >  - reg: offset and length of the USB 2.0 clock selector register block.
> > >  - clocks: A list of phandles and specifier pairs.
> > >  - clock-names: Name of the clocks.
> > > - - The functional clock must be "ehci_ohci"
> > > + - The functional clock of USB 2.0 host side must be "ehci_ohci"
> >
> > "_" means "and".
> >
> > > + - The functional clock of HS-USB side must be "hs-usb-if"
> >
> > "-" means concatenation of terms.
> >
> > >   - The USB_EXTAL clock pin must be "usb_extal"
> > >   - The USB_XTAL clock pin must be "usb_xtal"
> >
> > "_" means concatenation of terms.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
index 83f6c6a..5c1903f 100644
--- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
@@ -38,7 +38,8 @@  Required properties:
 - reg: offset and length of the USB 2.0 clock selector register block.
 - clocks: A list of phandles and specifier pairs.
 - clock-names: Name of the clocks.
- - The functional clock must be "ehci_ohci"
+ - The functional clock of USB 2.0 host side must be "ehci_ohci"
+ - The functional clock of HS-USB side must be "hs-usb-if"
  - The USB_EXTAL clock pin must be "usb_extal"
  - The USB_XTAL clock pin must be "usb_xtal"
 - #clock-cells: Must be 0
@@ -49,7 +50,8 @@  Example (R-Car H3):
 		compatible = "renesas,r8a7795-rcar-usb2-clock-sel",
 			     "renesas,rcar-gen3-usb2-clock-sel";
 		reg = <0 0xe6590630 0 0x02>;
-		clocks = <&cpg CPG_MOD 703>, <&usb_extal>, <&usb_xtal>;
-		clock-names = "ehci_ohci", "usb_extal", "usb_xtal";
+		clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>,
+			 <&usb_extal>, <&usb_xtal>;
+		clock-names = "ehci_ohci", "hs-usb-if", "usb_extal", "usb_xtal";
 		#clock-cells = <0>;
 	};