linux-kernel - RE: [PATCH v2] phy: rcar-gen3-usb3: add support for R-Car Gen3 USB 3.0 PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <TY1PR06MB09923C2EEECD6C86EB53A994D8FF0@TY1PR06MB0992.apcprd06.prod.outlook.com>
Date:   Thu, 25 May 2017 07:52:11 +0000
From:   Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
CC:     Kishon Vijay Abraham I <kishon@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH v2] phy: rcar-gen3-usb3: add support for R-Car Gen3 USB
 3.0 PHY

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@...esas.com> 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@...esas.com>
> > ---
<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@...ux-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