Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1649589 > unrolled thread

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

Started byGeert Uytterhoeven <geert@linux-m68k.org>
First post2017-05-24 14:50 +0200
Last post2017-05-25 10:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2] phy: rcar-gen3-usb3: add support for R-Car Gen3 USB  3.0 PHY Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-24 14:50 +0200
    RE: [PATCH v2] phy: rcar-gen3-usb3: add support for R-Car Gen3 USB  3.0 PHY Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2017-05-25 10:00 +0200

#1649589 — Re: [PATCH v2] phy: rcar-gen3-usb3: add support for R-Car Gen3 USB 3.0 PHY

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-05-24 14:50 +0200
SubjectRe: [PATCH v2] phy: rcar-gen3-usb3: add support for R-Car Gen3 USB 3.0 PHY
Message-ID<tKDTI-2pQ-23@gated-at.bofh.it>
Hi Shimoda-san,

On Wed, May 24, 2017 at 2:17 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.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@renesas.com>
> ---
>  This patch is based on the latest linux-phy.git / next branch and
>  "[PATCH v5 3/3] phy: Group vendor specific phy drivers" patch.
>
>  I discussed with Geert-san about this driver on the ML:
>  https://patchwork.kernel.org/patch/9731759/
>
>  Changes from v1:
>  - Changes from dev_info to dev_vdbg in rcar_gen3_phy_usb3_init().
>
>  .../devicetree/bindings/phy/rcar-gen3-phy-usb3.txt |  47 +++++
>  MAINTAINERS                                        |   4 +-
>  drivers/phy/renesas/Kconfig                        |   7 +
>  drivers/phy/renesas/Makefile                       |   1 +
>  drivers/phy/renesas/phy-rcar-gen3-usb3.c           | 207 +++++++++++++++++++++
>  5 files changed, 264 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb3.txt
>  create mode 100644 drivers/phy/renesas/phy-rcar-gen3-usb3.c
>
> 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
> @@ -0,0 +1,47 @@
> +* 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.

> +  - The usb2's external clock must be "usb_extal". If you want to use the ssc,
> +    the clock-frequenvy must not be 0.

frequency

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?

> --- /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.

> +       { .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@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

[toc] | [next] | [standalone]


#1650263

FromYoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date2017-05-25 10:00 +0200
Message-ID<tKVQB-5n9-1@gated-at.bofh.it>
In reply to#1649589
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@renesas.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@renesas.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@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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web