Hi Geert-san,

Sent: Monday, August 24, 2015 9:13 PM

Hi Shimoda-san,

On Thu, Aug 20, 2015 at 8:28 AM, Yoshihiro Shimoda
[off-list ref] wrote:

quoted

This patch adds support for R-Car generation 3 USB2 PHY driver.
This SoC has 3 EHCI/OHCI channels, and the channel 0 is shared
with the HSUSB (USB2.0 peripheral) device.

So, the purpose of this driver is:
 1) initializes some registers of SoC specific to use the
    {ehci,ohci}-platform driver.

 2) detects id pin to select host or peripheral on the channel 0.

For now, this driver only supports 1) above.

Thanks for your patch!

Thank you very much for your review!

quoted

--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt

@@ -0,0 +1,37 @@
+* Renesas R-Car generation 3 USB 2.0 PHY
+
+This file provides information on what the device node for the R-Car generation
+3 USB 2.0 PHY contains.
+
+Required properties:
+- compatible: "renesas,usb2-phy-r8a7795" if the device is a part of R8A7795 SoC.

"an R8A7795 SoC".

I will fix this in v2.
(I will submit v2 patch late today.)

quoted

+- reg: offset and length of the USB2.0 host register block.
+- reg-names: must be "usb2".
+- clocks: clock phandle and specifier pair.

"pair(s)."

I will fix this.

quoted

+- clock-names: string, clock input name, must be "usb2", and optional "hsusb".

names

Thank you for the point.
As you mentioned about runtime PM below, I will remove this property.

quoted

+- #phy-cells: see phy-bindings.txt in the same directory, must be <0>.
+
+Optional proparies:

properties

Oops, I will fix it.
I should have done aspell command before I submitted.
If I did that command, I found other typo in this patch :)

quoted

+To use a USB channel which EHCI/OHCI and HSUSB are combined, the device tree

s/which/where/

quoted

+node should set HSUSB proparies to reg and reg-names proparies:

properties (x2)

I will fix it.

quoted

+- reg: offset and length of the HSUSB register block.
+- reg-names: must be "hsusb".
+
+Example (R-Car H3):
+
+       usb-phy@ee080200 {
+               compatible = "renesas,usb2-phy-r8a7795";
+               reg = <0 0xee080200 0 0x6ff>, <0 0xe6590100 0 0x100>;
+               reg-names = "usb2", "hsusb";
+               clocks = <&mstp7_clks R8A7795_CLK_EHCI0>,
+                        <&mstp7_clks R8A7795_CLK_HSUSB>;

If you ever want to enable Runtime PM, do you want the first MSTP clock to be
managed automatically?
What about the second optional clock?

Thank you very much for the point.
I didn't take care about Runtime PM. However, I want to enable Runtime PM
because usb host and function drivers support it.

quoted

+               clock-names = "usb2", "hsusb";
+       };
+
+       usb-phy@ee0a0200 {
+               compatible = "renesas,usb2-phy-r8a7795";
+               reg = <0 0xee0a0200 0 0x6ff>;
+               reg-names = "usb2";
+               clocks = <&mstp7_clks R8A7795_CLK_EHCI0>;
+               clock-names = "usb2";
+       };

quoted

--- /dev/null
+++ b/drivers/phy/phy-rcar-gen3-usb2.c

quoted

+static int rcar_gen3_phy_usb2_init(struct phy *p)
+{
+       struct rcar_gen3_phy_usb2_channel *channel = phy_get_drvdata(p);
+       unsigned long flags;
+       void __iomem *usb2_base = channel->usb2.base;
+       void __iomem *hsusb_base = channel->hsusb.base;
+       u32 tmp;
+
+       /* Since ops->init() is called once, this driver enables both clocks */
+       clk_prepare_enable(channel->usb2.clk);
+       clk_prepare_enable(channel->hsusb.clk);

So this driver manages its clock(s) itself, and doesn't support Runtime PM.

So, I will use remove clk APIs in this driver.

Best regards,
Yoshihiro Shimoda

quoted

+static int rcar_gen3_phy_usb2_exit(struct phy *p)
+{
+       struct rcar_gen3_phy_usb2_channel *channel = phy_get_drvdata(p);
+
+       writel(0, channel->usb2.base + USB2_INT_ENABLE);
+
+       clk_disable_unprepare(channel->hsusb.clk);
+       clk_disable_unprepare(channel->usb2.clk);
+
+       return 0;
+}

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

N�����r��y���b�X��ǧv�^�)޺{.n�+���z��z��z)���w*jg���
�����ݢj.�۰\��M��gj��a����' ��ޢ�
���j:+v���w�j�m��������zZ+�����ݢj"��!�i