Hi Geert-san, > From: Geert Uytterhoeven > Sent: Wednesday, May 24, 2017 9:39 PM > > Hi Shimoda-san, > > On Wed, May 24, 2017 at 2:17 PM, Yoshihiro Shimoda > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > > The USB 3.0 PHY modules of R-Car Gen3 SoCs have: > > - Spread spectrum clock (ssc). > > - Using USB 2.0 EXTAL clock instead of USB 3.0 clock. > > - Enabling VBUS detection for usb3.0 peripheral. > > > > So, this driver supports these features. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > --- <snip> > > diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb3.txt > b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb3.txt > > new file mode 100644 > > index 0000000..e58ba6b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb3.txt > > +* Renesas R-Car generation 3 USB 3.0 PHY > > + > > +This file provides information on what the device node for the R-Car generation > > +3 USB 3.0 PHY contains. > > +If you want to enable spread spectrum clock (ssc), you should use USB_EXTAL > > +instead of USB3_CLK. However, if you don't want to these features, you don't > > +need this driver. > > + > > +Required properties: > > +- compatible: "renesas,r8a7795-usb3-phy" if the device is a part of an R8A7795 > > + SoC. > > + "renesas,r8a7796-usb3-phy" if the device is a part of an R8A7796 > > + SoC. > > + "renesas,rcar-gen3-usb3-phy" for a generic R-Car Gen3 compatible > > + device. > > + > > + When compatible with the generic version, nodes must list the > > + SoC-specific version corresponding to the platform first > > + followed by the generic version. > > + > > +- reg: offset and length of the USB 3.0 PHY register block. > > +- clocks: A list of phandles and clock-specifier pairs. > > +- clock-names: Name of the clocks. > > + - The funcional clock must be "usb3-if". > > + - The usb3's external clock must be "usb3s_clk". If you want to use the ssc, > > + the clock-frequency must be 0. > > Given you have "renesas,ssc-range" to enable ssc, I think it doesn't matter > if the clock frequency is 0 or not, so the "if you..." part can be removed. I see. If both usb3s_clk and usb_extal are not 0 and "ssc-range" is enabled, the driver should choose usb_extal and enable the ssc. So, I understand "if you.." part can be removed. Thanks! > > + - The usb2's external clock must be "usb_extal". If you want to use the ssc, > > + the clock-frequenvy must not be 0. > > frequency Oops, I will fix it. > Here it does matter, as you need this clock for ssc. > BTW, the driver can fallback to the other clock if ssc cannot be used. > > > +Optional properties: > > +- renesas,ssc-range: Enable/disable spread spectrum clock (ssc) by using > > + the following values as u32: > > + - 0 (or the property doesn't exist): disable the ssc > > + - 4980: enable the ssc as -4980 ppm > > + - 4492: enable the ssc as -4492 ppm > > + - 4003: enable the ssc as -4003 ppm > > Using ssc or not sounds like software policy, not hardware description? > Can this be decided at runtime? I'm thinking this is hardware description :) This setting needs before usb3.0 controller starts. So, this cannot be decided at runtime. I found phy/brcm-sata-phy.txt also had a similar property "brcm,enable-ssc". > > --- /dev/null > > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb3.c > > > +static const struct of_device_id rcar_gen3_phy_usb3_match_table[] = { > > + { .compatible = "renesas,r8a7795-usb3-phy" }, > > + { .compatible = "renesas,r8a7796-usb3-phy" }, > > As the driver matches against the generic compatible property, the above two > lines can be removed. I got it. I will remove it. Best regards, Yoshihiro Shimoda > > + { .compatible = "renesas,rcar-gen3-usb3-phy" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, rcar_gen3_phy_usb3_match_table); > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > 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