[PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

R-Car USB 2.0 controller can change the clock source from an oscillator
to an external clock via a register. So, this patch adds support
the clock source selector as a clock driver.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 This patch is based on the renesas-drivers.git /
renesas-drivers-2017-06-27-v4.12-rc7 tag.

 Changes from v1:
  - Change this driver as a clock driver from a generic phy driver.
    https://patchwork.kernel.org/patch/9788697/
  - Remove "RFC" tag.

 .../bindings/clock/renesas,rcar-usb2-clock-sel.txt |  54 ++++++
 drivers/clk/renesas/Kconfig                        |   5 +
 drivers/clk/renesas/Makefile                       |   1 +
 drivers/clk/renesas/rcar-usb2-clock-sel.c          | 205 +++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
 create mode 100644 drivers/clk/renesas/rcar-usb2-clock-sel.c

diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
new file mode 100644
index 0000000..75ccafc
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
@@ -0,0 +1,54 @@
+* Renesas R-Car USB 2.0 clock selector
+
+This file provides information on what the device node for the R-Car USB 2.0
+clock selector.
+
+If you connect an external clock to the USB_EXTAL pin only, you should set
+the clock rate to "usb_extal" node only.
+If you connect an oscillator to both the USB_XTAL and USB_EXTAL, this module
+is not needed because this is default setting. (Of course, you can set the
+clock rates to both "usb_extal" and "usb_xtal" nodes.
+
+Case 1: An external clock connects to R-Car SoC
+        +----------+   +--- R-Car ---------------------+
+        |External  |---|USB_EXTAL ---> all usb channels|
+        |clock     |   |USB_XTAL                       |
+        +----------+   +-------------------------------+
+In this case, we need this driver with "usb_extal" clock.
+
+Case 2: An oscillator connects to R-Car SoC
+        +----------+   +--- R-Car ---------------------+
+        |Oscillator|---|USB_EXTAL -+-> all usb channels|
+        |          |---|USB_XTAL --+                   |
+        +----------+   +-------------------------------+
+In this case, we don't need this selector.
+
+Required properties:
+- compatible: "renesas,r8a7795-rcar-usb2-clock-sel" if the device is a part of
+	      an R8A7795 SoC.
+              "renesas,r8a7796-rcar-usb2-clock-sel" if the device if a part of
+	      an R8A7796 SoC.
+              "renesas,rcar-gen3-usb2-clock-sel" 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 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 USB_EXTAL clock pin must be "usb_extal"
+ - The USB_XTAL clock pin must be "usb_xtal"
+- #clock-cells: Must be 0
+
+Exxample (R-Car H3):
+
+        usb2_clksel: clock-controller@e6590630 {
+                compatible = "renesas,r8a77950-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";
+        };
diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 78d1df9..094df5c 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -114,6 +114,11 @@ config CLK_RCAR_GEN3_CPG
 	bool
 	select CLK_RENESAS_CPG_MSSR
 
+config CLK_RCAR_USB2_CLOCK_SEL
+	bool "Renesas R-Car USB2 clock selector support"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	help
+	  This is a driver for R-Car USB2 clock selector
 
 # Generic
 config CLK_RENESAS_CPG_MSSR
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index 02d0412..4eec85b 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_CLK_SH73A0)		+= clk-sh73a0.o
 obj-$(CONFIG_CLK_RCAR_GEN2)		+= clk-rcar-gen2.o
 obj-$(CONFIG_CLK_RCAR_GEN2_CPG)		+= rcar-gen2-cpg.o
 obj-$(CONFIG_CLK_RCAR_GEN3_CPG)		+= rcar-gen3-cpg.o
+obj-$(CONFIG_CLK_RCAR_USB2_CLOCK_SEL)	+= rcar-usb2-clock-sel.o
 
 # Generic
 obj-$(CONFIG_CLK_RENESAS_CPG_MSSR)	+= renesas-cpg-mssr.o
diff --git a/drivers/clk/renesas/rcar-usb2-clock-sel.c b/drivers/clk/renesas/rcar-usb2-clock-sel.c
new file mode 100644
index 0000000..b48b2fc
--- /dev/null
+++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
@@ -0,0 +1,205 @@
+/*
+ * Renesas R-Car USB2.0 clock selector
+ *
+ * Copyright (C) 2017 Renesas Electronics Corp.
+ *
+ * Based on renesas-cpg-mssr.c
+ *
+ * Copyright (C) 2015 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/slab.h>
+
+#define USB20_CLKSET0		0x00
+#define CLKSET0_INTCLK_EN	BIT(11)
+#define CLKSET0_PRIVATE		BIT(0)
+#define CLKSET0_EXTAL_ONLY	(CLKSET0_INTCLK_EN | CLKSET0_PRIVATE)
+
+struct usb2_clock_sel_priv {
+	void __iomem *base;
+	struct clk_hw hw;
+	struct clk *ehci_clk;
+	bool extal;
+	bool xtal;
+};
+#define to_priv(_hw)	container_of(_hw, struct usb2_clock_sel_priv, hw)
+
+static void usb2_clock_sel_enable_extal_only(struct usb2_clock_sel_priv *priv)
+{
+	u16 val = readw(priv->base + USB20_CLKSET0);
+
+	pr_debug("%s: enter %d %d %x\n", __func__,
+		 priv->extal, priv->xtal, val);
+
+	if (priv->extal && !priv->xtal && val != CLKSET0_EXTAL_ONLY)
+		writew(CLKSET0_EXTAL_ONLY, priv->base + USB20_CLKSET0);
+}
+
+static void usb2_clock_sel_disable_extal_only(struct usb2_clock_sel_priv *priv)
+{
+	if (priv->extal && !priv->xtal)
+		writew(CLKSET0_PRIVATE, priv->base + USB20_CLKSET0);
+}
+
+static int usb2_clock_sel_enable(struct clk_hw *hw)
+{
+	usb2_clock_sel_enable_extal_only(to_priv(hw));
+
+	return 0;
+}
+
+static void usb2_clock_sel_disable(struct clk_hw *hw)
+{
+	usb2_clock_sel_disable_extal_only(to_priv(hw));
+}
+
+/*
+ * This module seems a mux, but this driver assumes a gate because
+ * ehci/ohci platform drivers don't support clk_set_parent() for now.
+ * If this driver acts as a gate, ehci/ohci-platform drivers don't need
+ * any modification.
+ */
+static const struct clk_ops usb2_clock_sel_clock_ops = {
+	.enable = usb2_clock_sel_enable,
+	.disable = usb2_clock_sel_disable,
+};
+
+static const struct of_device_id rcar_usb2_clock_sel_match[] = {
+	{ .compatible = "renesas,rcar-gen3-usb2-clock-sel" },
+	{ }
+};
+
+static int rcar_usb2_clock_sel_suspend(struct device *dev)
+{
+	struct usb2_clock_sel_priv *priv = dev_get_drvdata(dev);
+
+	usb2_clock_sel_disable_extal_only(priv);
+	clk_disable_unprepare(priv->ehci_clk);
+
+	return 0;
+}
+
+static int rcar_usb2_clock_sel_resume(struct device *dev)
+{
+	struct usb2_clock_sel_priv *priv = dev_get_drvdata(dev);
+	int error;
+
+	error = clk_prepare_enable(priv->ehci_clk);
+	if (error)
+		return error;
+	usb2_clock_sel_enable_extal_only(priv);
+
+	return 0;
+}
+
+static int rcar_usb2_clock_sel_remove(struct platform_device *pdev)
+{
+	struct usb2_clock_sel_priv *priv = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(priv->ehci_clk);
+	of_clk_del_provider(pdev->dev.of_node);
+	clk_hw_unregister(&priv->hw);
+
+	return 0;
+}
+
+static int __init rcar_usb2_clock_sel_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct usb2_clock_sel_priv *priv;
+	struct resource *res;
+	struct clk *clk;
+	struct clk_init_data init;
+	int error;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->ehci_clk = devm_clk_get(dev, "ehci_ohci");
+	if (!IS_ERR(priv->ehci_clk)) {
+		error = clk_prepare_enable(priv->ehci_clk);
+		if (error)
+			return error;
+	}
+
+	clk = devm_clk_get(dev, "usb_extal");
+	if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
+		priv->extal = !!clk_get_rate(clk);
+		clk_disable_unprepare(clk);
+	}
+	clk = devm_clk_get(dev, "usb_xtal");
+	if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
+		priv->xtal = !!clk_get_rate(clk);
+		clk_disable_unprepare(clk);
+	}
+
+	if (!priv->extal && !priv->extal) {
+		dev_err(dev, "This driver needs usb_extal or usb_xtal\n");
+		return -ENOENT;
+	}
+
+	platform_set_drvdata(pdev, priv);
+	dev_set_drvdata(dev, priv);
+
+	init.name = "rcar_usb2_clock_sel";
+	init.ops = &usb2_clock_sel_clock_ops;
+	init.flags = CLK_IS_BASIC;
+	init.parent_names = NULL;
+	init.num_parents = 0;
+	priv->hw.init = &init;
+
+	clk = clk_register(NULL, &priv->hw);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	error = of_clk_add_provider(np, of_clk_src_simple_get, clk);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static const struct dev_pm_ops rcar_usb2_clock_sel_pm_ops = {
+	.suspend	= rcar_usb2_clock_sel_suspend,
+	.resume		= rcar_usb2_clock_sel_resume,
+};
+
+static struct platform_driver rcar_usb2_clock_sel_driver = {
+	.driver		= {
+		.name	= "rcar-usb2-clock-sel",
+		.pm	= &rcar_usb2_clock_sel_pm_ops,
+		.of_match_table = rcar_usb2_clock_sel_match,
+	},
+	.remove		= rcar_usb2_clock_sel_remove,
+};
+
+static int __init rcar_usb2_clock_sel_init(void)
+{
+	return platform_driver_probe(&rcar_usb2_clock_sel_driver,
+				     rcar_usb2_clock_sel_probe);
+}
+
+/* Since this driver needs other ccf drivers, this uses subsys_initcall_sync */
+subsys_initcall_sync(rcar_usb2_clock_sel_init);
+
+MODULE_DESCRIPTION("Renesas R-Car USB2 clock selector Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Geert Uytterhoeven]

Hi Simoda-san,

On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> R-Car USB 2.0 controller can change the clock source from an oscillator
> to an external clock via a register. So, this patch adds support
> the clock source selector as a clock driver.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c

> +static void usb2_clock_sel_enable_extal_only(struct usb2_clock_sel_priv *priv)
> +{
> +       u16 val = readw(priv->base + USB20_CLKSET0);
> +
> +       pr_debug("%s: enter %d %d %x\n", __func__,
> +                priv->extal, priv->xtal, val);
> +
> +       if (priv->extal && !priv->xtal && val != CLKSET0_EXTAL_ONLY)
> +               writew(CLKSET0_EXTAL_ONLY, priv->base + USB20_CLKSET0);
> +}
> +
> +static void usb2_clock_sel_disable_extal_only(struct usb2_clock_sel_priv *priv)
> +{
> +       if (priv->extal && !priv->xtal)
> +               writew(CLKSET0_PRIVATE, priv->base + USB20_CLKSET0);
> +}
> +
> +static int usb2_clock_sel_enable(struct clk_hw *hw)
> +{
> +       usb2_clock_sel_enable_extal_only(to_priv(hw));
> +
> +       return 0;
> +}
> +
> +static void usb2_clock_sel_disable(struct clk_hw *hw)
> +{
> +       usb2_clock_sel_disable_extal_only(to_priv(hw));
> +}
> +
> +/*
> + * This module seems a mux, but this driver assumes a gate because
> + * ehci/ohci platform drivers don't support clk_set_parent() for now.
> + * If this driver acts as a gate, ehci/ohci-platform drivers don't need
> + * any modification.
> + */
> +static const struct clk_ops usb2_clock_sel_clock_ops = {
> +       .enable = usb2_clock_sel_enable,
> +       .disable = usb2_clock_sel_disable,
> +};

So you do "smart muxing" in the enable routine above?

> +static int __init rcar_usb2_clock_sel_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       struct usb2_clock_sel_priv *priv;
> +       struct resource *res;
> +       struct clk *clk;
> +       struct clk_init_data init;
> +       int error;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       priv->base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(priv->base))
> +               return PTR_ERR(priv->base);
> +
> +       priv->ehci_clk = devm_clk_get(dev, "ehci_ohci");
> +       if (!IS_ERR(priv->ehci_clk)) {
> +               error = clk_prepare_enable(priv->ehci_clk);
> +               if (error)
> +                       return error;
> +       }
> +
> +       clk = devm_clk_get(dev, "usb_extal");
> +       if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
> +               priv->extal = !!clk_get_rate(clk);
> +               clk_disable_unprepare(clk);
> +       }
> +       clk = devm_clk_get(dev, "usb_xtal");
> +       if (!IS_ERR(clk) && !clk_prepare_enable(clk)) {
> +               priv->xtal = !!clk_get_rate(clk);
> +               clk_disable_unprepare(clk);
> +       }
> +
> +       if (!priv->extal && !priv->extal) {
> +               dev_err(dev, "This driver needs usb_extal or usb_xtal\n");
> +               return -ENOENT;

Perhaps enabled clocks should be disabled on error?

> +       }
> +
> +       platform_set_drvdata(pdev, priv);
> +       dev_set_drvdata(dev, priv);
> +
> +       init.name = "rcar_usb2_clock_sel";
> +       init.ops = &usb2_clock_sel_clock_ops;
> +       init.flags = CLK_IS_BASIC;
> +       init.parent_names = NULL;
> +       init.num_parents = 0;
> +       priv->hw.init = &init;
> +
> +       clk = clk_register(NULL, &priv->hw);
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);

Likewise.

> +       error = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +       if (error)
> +               return error;
> +
> +       return 0;

return of_clk_add_provider(np, of_clk_src_simple_get, clk);

> +/* Since this driver needs other ccf drivers, this uses subsys_initcall_sync */
> +subsys_initcall_sync(rcar_usb2_clock_sel_init);

I suppose this is a workaround for the lack of probe deferral support in the
USB subsystem?

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

RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

SGkgR2VlcnQtc2FuLA0KDQo+IEZyb206IEdlZXJ0IFV5dHRlcmhvZXZlbg0KPiBTZW50OiBXZWRu
ZXNkYXksIEp1bHkgNSwgMjAxNyA3OjA5IFBNDQo+IA0KPiBIaSBTaW1vZGEtc2FuLA0KPiANCj4g
T24gV2VkLCBKdW4gMjgsIDIwMTcgYXQgODoyOCBBTSwgWW9zaGloaXJvIFNoaW1vZGENCj4gPHlv
c2hpaGlyby5zaGltb2RhLnVoQHJlbmVzYXMuY29tPiB3cm90ZToNCj4gPiBSLUNhciBVU0IgMi4w
IGNvbnRyb2xsZXIgY2FuIGNoYW5nZSB0aGUgY2xvY2sgc291cmNlIGZyb20gYW4gb3NjaWxsYXRv
cg0KPiA+IHRvIGFuIGV4dGVybmFsIGNsb2NrIHZpYSBhIHJlZ2lzdGVyLiBTbywgdGhpcyBwYXRj
aCBhZGRzIHN1cHBvcnQNCj4gPiB0aGUgY2xvY2sgc291cmNlIHNlbGVjdG9yIGFzIGEgY2xvY2sg
ZHJpdmVyLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogWW9zaGloaXJvIFNoaW1vZGEgPHlvc2hp
aGlyby5zaGltb2RhLnVoQHJlbmVzYXMuY29tPg0KPiANCj4gVGhhbmtzIGZvciB5b3VyIHBhdGNo
IQ0KDQpUaGFuayB5b3UgdmVyeSBtdWNoIGZvciB0aGUgY29tbWVudHMhDQoNCj4gPiAtLS0gL2Rl
di9udWxsDQo+ID4gKysrIGIvZHJpdmVycy9jbGsvcmVuZXNhcy9yY2FyLXVzYjItY2xvY2stc2Vs
LmMNCj4gDQo+ID4gK3N0YXRpYyB2b2lkIHVzYjJfY2xvY2tfc2VsX2VuYWJsZV9leHRhbF9vbmx5
KHN0cnVjdCB1c2IyX2Nsb2NrX3NlbF9wcml2ICpwcml2KQ0KPiA+ICt7DQo+ID4gKyAgICAgICB1
MTYgdmFsID0gcmVhZHcocHJpdi0+YmFzZSArIFVTQjIwX0NMS1NFVDApOw0KPiA+ICsNCj4gPiAr
ICAgICAgIHByX2RlYnVnKCIlczogZW50ZXIgJWQgJWQgJXhcbiIsIF9fZnVuY19fLA0KPiA+ICsg
ICAgICAgICAgICAgICAgcHJpdi0+ZXh0YWwsIHByaXYtPnh0YWwsIHZhbCk7DQo+ID4gKw0KPiA+
ICsgICAgICAgaWYgKHByaXYtPmV4dGFsICYmICFwcml2LT54dGFsICYmIHZhbCAhPSBDTEtTRVQw
X0VYVEFMX09OTFkpDQo+ID4gKyAgICAgICAgICAgICAgIHdyaXRldyhDTEtTRVQwX0VYVEFMX09O
TFksIHByaXYtPmJhc2UgKyBVU0IyMF9DTEtTRVQwKTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3Rh
dGljIHZvaWQgdXNiMl9jbG9ja19zZWxfZGlzYWJsZV9leHRhbF9vbmx5KHN0cnVjdCB1c2IyX2Ns
b2NrX3NlbF9wcml2ICpwcml2KQ0KPiA+ICt7DQo+ID4gKyAgICAgICBpZiAocHJpdi0+ZXh0YWwg
JiYgIXByaXYtPnh0YWwpDQo+ID4gKyAgICAgICAgICAgICAgIHdyaXRldyhDTEtTRVQwX1BSSVZB
VEUsIHByaXYtPmJhc2UgKyBVU0IyMF9DTEtTRVQwKTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3Rh
dGljIGludCB1c2IyX2Nsb2NrX3NlbF9lbmFibGUoc3RydWN0IGNsa19odyAqaHcpDQo+ID4gK3sN
Cj4gPiArICAgICAgIHVzYjJfY2xvY2tfc2VsX2VuYWJsZV9leHRhbF9vbmx5KHRvX3ByaXYoaHcp
KTsNCj4gPiArDQo+ID4gKyAgICAgICByZXR1cm4gMDsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3Rh
dGljIHZvaWQgdXNiMl9jbG9ja19zZWxfZGlzYWJsZShzdHJ1Y3QgY2xrX2h3ICpodykNCj4gPiAr
ew0KPiA+ICsgICAgICAgdXNiMl9jbG9ja19zZWxfZGlzYWJsZV9leHRhbF9vbmx5KHRvX3ByaXYo
aHcpKTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArLyoNCj4gPiArICogVGhpcyBtb2R1bGUgc2VlbXMg
YSBtdXgsIGJ1dCB0aGlzIGRyaXZlciBhc3N1bWVzIGEgZ2F0ZSBiZWNhdXNlDQo+ID4gKyAqIGVo
Y2kvb2hjaSBwbGF0Zm9ybSBkcml2ZXJzIGRvbid0IHN1cHBvcnQgY2xrX3NldF9wYXJlbnQoKSBm
b3Igbm93Lg0KPiA+ICsgKiBJZiB0aGlzIGRyaXZlciBhY3RzIGFzIGEgZ2F0ZSwgZWhjaS9vaGNp
LXBsYXRmb3JtIGRyaXZlcnMgZG9uJ3QgbmVlZA0KPiA+ICsgKiBhbnkgbW9kaWZpY2F0aW9uLg0K
PiA+ICsgKi8NCj4gPiArc3RhdGljIGNvbnN0IHN0cnVjdCBjbGtfb3BzIHVzYjJfY2xvY2tfc2Vs
X2Nsb2NrX29wcyA9IHsNCj4gPiArICAgICAgIC5lbmFibGUgPSB1c2IyX2Nsb2NrX3NlbF9lbmFi
bGUsDQo+ID4gKyAgICAgICAuZGlzYWJsZSA9IHVzYjJfY2xvY2tfc2VsX2Rpc2FibGUsDQo+ID4g
K307DQo+IA0KPiBTbyB5b3UgZG8gInNtYXJ0IG11eGluZyIgaW4gdGhlIGVuYWJsZSByb3V0aW5l
IGFib3ZlPw0KDQpZZXMuDQoNCj4gPiArc3RhdGljIGludCBfX2luaXQgcmNhcl91c2IyX2Nsb2Nr
X3NlbF9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KPiA+ICt7DQo+ID4gKyAg
ICAgICBzdHJ1Y3QgZGV2aWNlICpkZXYgPSAmcGRldi0+ZGV2Ow0KPiA+ICsgICAgICAgc3RydWN0
IGRldmljZV9ub2RlICpucCA9IGRldi0+b2Zfbm9kZTsNCj4gPiArICAgICAgIHN0cnVjdCB1c2Iy
X2Nsb2NrX3NlbF9wcml2ICpwcml2Ow0KPiA+ICsgICAgICAgc3RydWN0IHJlc291cmNlICpyZXM7
DQo+ID4gKyAgICAgICBzdHJ1Y3QgY2xrICpjbGs7DQo+ID4gKyAgICAgICBzdHJ1Y3QgY2xrX2lu
aXRfZGF0YSBpbml0Ow0KPiA+ICsgICAgICAgaW50IGVycm9yOw0KPiA+ICsNCj4gPiArICAgICAg
IHByaXYgPSBkZXZtX2t6YWxsb2MoZGV2LCBzaXplb2YoKnByaXYpLCBHRlBfS0VSTkVMKTsNCj4g
PiArICAgICAgIGlmICghcHJpdikNCj4gPiArICAgICAgICAgICAgICAgcmV0dXJuIC1FTk9NRU07
DQo+ID4gKw0KPiA+ICsgICAgICAgcmVzID0gcGxhdGZvcm1fZ2V0X3Jlc291cmNlKHBkZXYsIElP
UkVTT1VSQ0VfTUVNLCAwKTsNCj4gPiArICAgICAgIHByaXYtPmJhc2UgPSBkZXZtX2lvcmVtYXBf
cmVzb3VyY2UoZGV2LCByZXMpOw0KPiA+ICsgICAgICAgaWYgKElTX0VSUihwcml2LT5iYXNlKSkN
Cj4gPiArICAgICAgICAgICAgICAgcmV0dXJuIFBUUl9FUlIocHJpdi0+YmFzZSk7DQo+ID4gKw0K
PiA+ICsgICAgICAgcHJpdi0+ZWhjaV9jbGsgPSBkZXZtX2Nsa19nZXQoZGV2LCAiZWhjaV9vaGNp
Iik7DQo+ID4gKyAgICAgICBpZiAoIUlTX0VSUihwcml2LT5laGNpX2NsaykpIHsNCj4gPiArICAg
ICAgICAgICAgICAgZXJyb3IgPSBjbGtfcHJlcGFyZV9lbmFibGUocHJpdi0+ZWhjaV9jbGspOw0K
PiA+ICsgICAgICAgICAgICAgICBpZiAoZXJyb3IpDQo+ID4gKyAgICAgICAgICAgICAgICAgICAg
ICAgcmV0dXJuIGVycm9yOw0KPiA+ICsgICAgICAgfQ0KPiA+ICsNCj4gPiArICAgICAgIGNsayA9
IGRldm1fY2xrX2dldChkZXYsICJ1c2JfZXh0YWwiKTsNCj4gPiArICAgICAgIGlmICghSVNfRVJS
KGNsaykgJiYgIWNsa19wcmVwYXJlX2VuYWJsZShjbGspKSB7DQo+ID4gKyAgICAgICAgICAgICAg
IHByaXYtPmV4dGFsID0gISFjbGtfZ2V0X3JhdGUoY2xrKTsNCj4gPiArICAgICAgICAgICAgICAg
Y2xrX2Rpc2FibGVfdW5wcmVwYXJlKGNsayk7DQo+ID4gKyAgICAgICB9DQo+ID4gKyAgICAgICBj
bGsgPSBkZXZtX2Nsa19nZXQoZGV2LCAidXNiX3h0YWwiKTsNCj4gPiArICAgICAgIGlmICghSVNf
RVJSKGNsaykgJiYgIWNsa19wcmVwYXJlX2VuYWJsZShjbGspKSB7DQo+ID4gKyAgICAgICAgICAg
ICAgIHByaXYtPnh0YWwgPSAhIWNsa19nZXRfcmF0ZShjbGspOw0KPiA+ICsgICAgICAgICAgICAg
ICBjbGtfZGlzYWJsZV91bnByZXBhcmUoY2xrKTsNCj4gPiArICAgICAgIH0NCj4gPiArDQo+ID4g
KyAgICAgICBpZiAoIXByaXYtPmV4dGFsICYmICFwcml2LT5leHRhbCkgew0KPiA+ICsgICAgICAg
ICAgICAgICBkZXZfZXJyKGRldiwgIlRoaXMgZHJpdmVyIG5lZWRzIHVzYl9leHRhbCBvciB1c2Jf
eHRhbFxuIik7DQo+ID4gKyAgICAgICAgICAgICAgIHJldHVybiAtRU5PRU5UOw0KPiANCj4gUGVy
aGFwcyBlbmFibGVkIGNsb2NrcyBzaG91bGQgYmUgZGlzYWJsZWQgb24gZXJyb3I/DQoNCk9vcHMh
IEkgc2hvdWxkIGNhbGwgY2xrX2Rpc2FibGVfdW5wcmVwYXJlKHByaXYtPmVoY2lfY2xrKSBoZXJl
Lg0KSSB3aWxsIGZpeCBpdC4NCg0KPiA+ICsgICAgICAgfQ0KPiA+ICsNCj4gPiArICAgICAgIHBs
YXRmb3JtX3NldF9kcnZkYXRhKHBkZXYsIHByaXYpOw0KPiA+ICsgICAgICAgZGV2X3NldF9kcnZk
YXRhKGRldiwgcHJpdik7DQo+ID4gKw0KPiA+ICsgICAgICAgaW5pdC5uYW1lID0gInJjYXJfdXNi
Ml9jbG9ja19zZWwiOw0KPiA+ICsgICAgICAgaW5pdC5vcHMgPSAmdXNiMl9jbG9ja19zZWxfY2xv
Y2tfb3BzOw0KPiA+ICsgICAgICAgaW5pdC5mbGFncyA9IENMS19JU19CQVNJQzsNCj4gPiArICAg
ICAgIGluaXQucGFyZW50X25hbWVzID0gTlVMTDsNCj4gPiArICAgICAgIGluaXQubnVtX3BhcmVu
dHMgPSAwOw0KPiA+ICsgICAgICAgcHJpdi0+aHcuaW5pdCA9ICZpbml0Ow0KPiA+ICsNCj4gPiAr
ICAgICAgIGNsayA9IGNsa19yZWdpc3RlcihOVUxMLCAmcHJpdi0+aHcpOw0KPiA+ICsgICAgICAg
aWYgKElTX0VSUihjbGspKQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gUFRSX0VSUihjbGsp
Ow0KPiANCj4gTGlrZXdpc2UuDQoNClNhbWUgaGVyZS4gU28sIEkgd2lsbCB1c2UgImdvdG8iIGZv
ciBpdC4NCg0KPiA+ICsgICAgICAgZXJyb3IgPSBvZl9jbGtfYWRkX3Byb3ZpZGVyKG5wLCBvZl9j
bGtfc3JjX3NpbXBsZV9nZXQsIGNsayk7DQo+ID4gKyAgICAgICBpZiAoZXJyb3IpDQo+ID4gKyAg
ICAgICAgICAgICAgIHJldHVybiBlcnJvcjsNCj4gPiArDQo+ID4gKyAgICAgICByZXR1cm4gMDsN
Cj4gDQo+IHJldHVybiBvZl9jbGtfYWRkX3Byb3ZpZGVyKG5wLCBvZl9jbGtfc3JjX3NpbXBsZV9n
ZXQsIGNsayk7DQoNClRoYW5rcy4gSSB3aWxsIGZpeCBpdC4NCg0KPiA+ICsvKiBTaW5jZSB0aGlz
IGRyaXZlciBuZWVkcyBvdGhlciBjY2YgZHJpdmVycywgdGhpcyB1c2VzIHN1YnN5c19pbml0Y2Fs
bF9zeW5jICovDQo+ID4gK3N1YnN5c19pbml0Y2FsbF9zeW5jKHJjYXJfdXNiMl9jbG9ja19zZWxf
aW5pdCk7DQo+IA0KPiBJIHN1cHBvc2UgdGhpcyBpcyBhIHdvcmthcm91bmQgZm9yIHRoZSBsYWNr
IG9mIHByb2JlIGRlZmVycmFsIHN1cHBvcnQgaW4gdGhlDQo+IFVTQiBzdWJzeXN0ZW0/DQoNClRo
aXMgaXMgbXkgZmF1bHQuIEkgYWRkZWQgInBvd2VyLWRvbWFpbnMiIHByb3BlcnR5IGludG8gdGhp
cyBkZXZpY2UncyBub2RlLg0KQWZ0ZXIgSSByZW1vdmUgdGhlIHBvd2VyLWRvbWFpbnMgbGlrZSB0
aGUgY3BnIG5vZGUsIHRoaXMgZHJpdmVyIGNhbiB1c2Ugc3Vic3lzX2luaXRjYWxsKCkNCmluc3Rl
YWQgb2Ygc3Vic3lzX2luaXRjYWxsX3N5bmMoKS4NCg0KQmVzdCByZWdhcmRzLA0KWW9zaGloaXJv
IFNoaW1vZGENCg0KPiBHcntvZXRqZSxlZXRpbmd9cywNCj4gDQo+ICAgICAgICAgICAgICAgICAg
ICAgICAgIEdlZXJ0DQo+IA0KPiAtLQ0KPiBHZWVydCBVeXR0ZXJob2V2ZW4gLS0gVGhlcmUncyBs
b3RzIG9mIExpbnV4IGJleW9uZCBpYTMyIC0tIGdlZXJ0QGxpbnV4LW02OGsub3JnDQo+IA0KPiBJ
biBwZXJzb25hbCBjb252ZXJzYXRpb25zIHdpdGggdGVjaG5pY2FsIHBlb3BsZSwgSSBjYWxsIG15
c2VsZiBhIGhhY2tlci4gQnV0DQo+IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlzdHMgSSBq
dXN0IHNheSAicHJvZ3JhbW1lciIgb3Igc29tZXRoaW5nIGxpa2UgdGhhdC4NCj4gICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0K

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Jul 5, 2017 at 1:57 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>> From: Geert Uytterhoeven
>> Sent: Wednesday, July 5, 2017 7:09 PM
>> On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh@renesas.com> wrote:
>> > R-Car USB 2.0 controller can change the clock source from an oscillator
>> > to an external clock via a register. So, this patch adds support
>> > the clock source selector as a clock driver.

>> > --- /dev/null
>> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c

>> > +/* Since this driver needs other ccf drivers, this uses subsys_initcall_sync */
>> > +subsys_initcall_sync(rcar_usb2_clock_sel_init);
>>
>> I suppose this is a workaround for the lack of probe deferral support in the
>> USB subsystem?
>
> This is my fault. I added "power-domains" property into this device's node.
> After I remove the power-domains like the cpg node, this driver can use subsys_initcall()
> instead of subsys_initcall_sync().

Does the clock sel module requires the functional clock "ehci_ohci" to be
running before you can access its registers?
If yes, I think there should be a "power-domains" property.

Then, you can simplify the code by calling

        pm_runtime_enable(dev);
        pm_runtime_get_sync(dev);

and remove the explicit handling of the functional clock.

That does not solve probe deferral handling in the USB subsystem, though.

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

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Rob Herring]

On Wed, Jun 28, 2017 at 03:28:35PM +0900, Yoshihiro Shimoda wrote:
> R-Car USB 2.0 controller can change the clock source from an oscillator
> to an external clock via a register. So, this patch adds support
> the clock source selector as a clock driver.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  This patch is based on the renesas-drivers.git /
> renesas-drivers-2017-06-27-v4.12-rc7 tag.
> 
>  Changes from v1:
>   - Change this driver as a clock driver from a generic phy driver.
>     https://patchwork.kernel.org/patch/9788697/
>   - Remove "RFC" tag.
> 
>  .../bindings/clock/renesas,rcar-usb2-clock-sel.txt |  54 ++++++
>  drivers/clk/renesas/Kconfig                        |   5 +
>  drivers/clk/renesas/Makefile                       |   1 +
>  drivers/clk/renesas/rcar-usb2-clock-sel.c          | 205 +++++++++++++++++++++
>  4 files changed, 265 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
>  create mode 100644 drivers/clk/renesas/rcar-usb2-clock-sel.c
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> new file mode 100644
> index 0000000..75ccafc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> @@ -0,0 +1,54 @@
> +* Renesas R-Car USB 2.0 clock selector
> +
> +This file provides information on what the device node for the R-Car USB 2.0
> +clock selector.
> +
> +If you connect an external clock to the USB_EXTAL pin only, you should set
> +the clock rate to "usb_extal" node only.
> +If you connect an oscillator to both the USB_XTAL and USB_EXTAL, this module
> +is not needed because this is default setting. (Of course, you can set the
> +clock rates to both "usb_extal" and "usb_xtal" nodes.
> +
> +Case 1: An external clock connects to R-Car SoC
> +        +----------+   +--- R-Car ---------------------+
> +        |External  |---|USB_EXTAL ---> all usb channels|
> +        |clock     |   |USB_XTAL                       |
> +        +----------+   +-------------------------------+
> +In this case, we need this driver with "usb_extal" clock.
> +
> +Case 2: An oscillator connects to R-Car SoC
> +        +----------+   +--- R-Car ---------------------+
> +        |Oscillator|---|USB_EXTAL -+-> all usb channels|
> +        |          |---|USB_XTAL --+                   |
> +        +----------+   +-------------------------------+
> +In this case, we don't need this selector.
> +
> +Required properties:
> +- compatible: "renesas,r8a7795-rcar-usb2-clock-sel" if the device is a part of
> +	      an R8A7795 SoC.
> +              "renesas,r8a7796-rcar-usb2-clock-sel" if the device if a part of
> +	      an R8A7796 SoC.
> +              "renesas,rcar-gen3-usb2-clock-sel" 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 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 USB_EXTAL clock pin must be "usb_extal"
> + - The USB_XTAL clock pin must be "usb_xtal"
> +- #clock-cells: Must be 0
> +
> +Exxample (R-Car H3):
> +
> +        usb2_clksel: clock-controller@e6590630 {
> +                compatible = "renesas,r8a77950-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";

Missing #clock-cells

With that, for the binding:

Acked-by: Rob Herring <robh@kernel.org>

Rob

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Stephen Boyd]

On 06/28, Yoshihiro Shimoda wrote:
> +
> +
> +	platform_set_drvdata(pdev, priv);
> +	dev_set_drvdata(dev, priv);
> +
> +	init.name = "rcar_usb2_clock_sel";
> +	init.ops = &usb2_clock_sel_clock_ops;
> +	init.flags = CLK_IS_BASIC;

Please drop CLK_IS_BASIC unless you need it.

> +	init.parent_names = NULL;
> +	init.num_parents = 0;
> +	priv->hw.init = &init;
> +
> +	clk = clk_register(NULL, &priv->hw);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	error = of_clk_add_provider(np, of_clk_src_simple_get, clk);

Can you use clk_hw_register() and of_clk_add_hw_provider()
please?

> +	if (error)
> +		return error;
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops rcar_usb2_clock_sel_pm_ops = {
> +	.suspend	= rcar_usb2_clock_sel_suspend,
> +	.resume		= rcar_usb2_clock_sel_resume,
> +};
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

Hi Rob,

> -----Original Message-----
> From: Rob Herring
> Sent: Wednesday, July 5, 2017 11:16 PM
>=20
> On Wed, Jun 28, 2017 at 03:28:35PM +0900, Yoshihiro Shimoda wrote:
> > R-Car USB 2.0 controller can change the clock source from an oscillator
> > to an external clock via a register. So, this patch adds support
> > the clock source selector as a clock driver.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  This patch is based on the renesas-drivers.git /
> > renesas-drivers-2017-06-27-v4.12-rc7 tag.
> >
> >  Changes from v1:
> >   - Change this driver as a clock driver from a generic phy driver.
> >     https://patchwork.kernel.org/patch/9788697/
> >   - Remove "RFC" tag.
> >
> >  .../bindings/clock/renesas,rcar-usb2-clock-sel.txt |  54 ++++++
> >  drivers/clk/renesas/Kconfig                        |   5 +
> >  drivers/clk/renesas/Makefile                       |   1 +
> >  drivers/clk/renesas/rcar-usb2-clock-sel.c          | 205 +++++++++++++=
++++++++
> >  4 files changed, 265 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/renesas,rca=
r-usb2-clock-sel.txt
> >  create mode 100644 drivers/clk/renesas/rcar-usb2-clock-sel.c
> >
> > diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-=
clock-sel.txt
> b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > new file mode 100644
> > index 0000000..75ccafc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-s=
el.txt
> > @@ -0,0 +1,54 @@
> > +* Renesas R-Car USB 2.0 clock selector
> > +
> > +This file provides information on what the device node for the R-Car U=
SB 2.0
> > +clock selector.
> > +
> > +If you connect an external clock to the USB_EXTAL pin only, you should=
 set
> > +the clock rate to "usb_extal" node only.
> > +If you connect an oscillator to both the USB_XTAL and USB_EXTAL, this =
module
> > +is not needed because this is default setting. (Of course, you can set=
 the
> > +clock rates to both "usb_extal" and "usb_xtal" nodes.
> > +
> > +Case 1: An external clock connects to R-Car SoC
> > +        +----------+   +--- R-Car ---------------------+
> > +        |External  |---|USB_EXTAL ---> all usb channels|
> > +        |clock     |   |USB_XTAL                       |
> > +        +----------+   +-------------------------------+
> > +In this case, we need this driver with "usb_extal" clock.
> > +
> > +Case 2: An oscillator connects to R-Car SoC
> > +        +----------+   +--- R-Car ---------------------+
> > +        |Oscillator|---|USB_EXTAL -+-> all usb channels|
> > +        |          |---|USB_XTAL --+                   |
> > +        +----------+   +-------------------------------+
> > +In this case, we don't need this selector.
> > +
> > +Required properties:
> > +- compatible: "renesas,r8a7795-rcar-usb2-clock-sel" if the device is a=
 part of
> > +	      an R8A7795 SoC.
> > +              "renesas,r8a7796-rcar-usb2-clock-sel" if the device if a=
 part of
> > +	      an R8A7796 SoC.
> > +              "renesas,rcar-gen3-usb2-clock-sel" for a generic R-Car G=
en3
> > +              compatible device.
> > +
> > +              When compatible with the generic version, nodes must lis=
t the
> > +              SoC-specific version corresponding to the platform first
> > +              followed by the generic version.
> > +
> > +- 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 USB_EXTAL clock pin must be "usb_extal"
> > + - The USB_XTAL clock pin must be "usb_xtal"
> > +- #clock-cells: Must be 0
> > +
> > +Exxample (R-Car H3):
> > +
> > +        usb2_clksel: clock-controller@e6590630 {
> > +                compatible =3D "renesas,r8a77950-rcar-usb2-clock-sel",
> > +                             "renesas,rcar-gen3-usb2-clock-sel";
> > +                reg =3D <0 0xe6590630 0 0x02>;
> > +                clocks =3D <&cpg CPG_MOD 703>, <&usb_extal>, <&usb_xta=
l>;
> > +                clock-names =3D "ehci_ohci", "usb_extal", "usb_xtal";
>=20
> Missing #clock-cells

Oops, I will fix it.

> With that, for the binding:
>=20
> Acked-by: Rob Herring <robh@kernel.org>

Thank you for your Acked-by!

Best regards,
Yoshihiro Shimoda

> Rob

RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

Hi Stephen-san,

Thank you for your comments!

> From: Stephen Boyd
> Sent: Thursday, July 6, 2017 6:18 AM
>=20
> On 06/28, Yoshihiro Shimoda wrote:
> > +
> > +
> > +	platform_set_drvdata(pdev, priv);
> > +	dev_set_drvdata(dev, priv);
> > +
> > +	init.name =3D "rcar_usb2_clock_sel";
> > +	init.ops =3D &usb2_clock_sel_clock_ops;
> > +	init.flags =3D CLK_IS_BASIC;
>=20
> Please drop CLK_IS_BASIC unless you need it.

I got it.

> > +	init.parent_names =3D NULL;
> > +	init.num_parents =3D 0;
> > +	priv->hw.init =3D &init;
> > +
> > +	clk =3D clk_register(NULL, &priv->hw);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	error =3D of_clk_add_provider(np, of_clk_src_simple_get, clk);
>=20
> Can you use clk_hw_register() and of_clk_add_hw_provider()
> please?

I could use of_clk_add_hw_provider().

Best regards,
Yoshihiro Shimoda

> > +	if (error)
> > +		return error;
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dev_pm_ops rcar_usb2_clock_sel_pm_ops =3D {
> > +	.suspend	=3D rcar_usb2_clock_sel_suspend,
> > +	.resume		=3D rcar_usb2_clock_sel_resume,
> > +};
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

SGkgR2VlcnQtc2FuLA0KDQo+IEZyb206IEdlZXJ0IFV5dHRlcmhvZXZlbg0KPiBTZW50OiBXZWRu
ZXNkYXksIEp1bHkgNSwgMjAxNyA5OjIyIFBNDQo+IA0KPiBIaSBTaGltb2RhLXNhbiwNCj4gDQo+
IE9uIFdlZCwgSnVsIDUsIDIwMTcgYXQgMTo1NyBQTSwgWW9zaGloaXJvIFNoaW1vZGENCj4gPHlv
c2hpaGlyby5zaGltb2RhLnVoQHJlbmVzYXMuY29tPiB3cm90ZToNCj4gPj4gRnJvbTogR2VlcnQg
VXl0dGVyaG9ldmVuDQo+ID4+IFNlbnQ6IFdlZG5lc2RheSwgSnVseSA1LCAyMDE3IDc6MDkgUE0N
Cj4gPj4gT24gV2VkLCBKdW4gMjgsIDIwMTcgYXQgODoyOCBBTSwgWW9zaGloaXJvIFNoaW1vZGEN
Cj4gPj4gPHlvc2hpaGlyby5zaGltb2RhLnVoQHJlbmVzYXMuY29tPiB3cm90ZToNCj4gPj4gPiBS
LUNhciBVU0IgMi4wIGNvbnRyb2xsZXIgY2FuIGNoYW5nZSB0aGUgY2xvY2sgc291cmNlIGZyb20g
YW4gb3NjaWxsYXRvcg0KPiA+PiA+IHRvIGFuIGV4dGVybmFsIGNsb2NrIHZpYSBhIHJlZ2lzdGVy
LiBTbywgdGhpcyBwYXRjaCBhZGRzIHN1cHBvcnQNCj4gPj4gPiB0aGUgY2xvY2sgc291cmNlIHNl
bGVjdG9yIGFzIGEgY2xvY2sgZHJpdmVyLg0KPiANCj4gPj4gPiAtLS0gL2Rldi9udWxsDQo+ID4+
ID4gKysrIGIvZHJpdmVycy9jbGsvcmVuZXNhcy9yY2FyLXVzYjItY2xvY2stc2VsLmMNCj4gDQo+
ID4+ID4gKy8qIFNpbmNlIHRoaXMgZHJpdmVyIG5lZWRzIG90aGVyIGNjZiBkcml2ZXJzLCB0aGlz
IHVzZXMgc3Vic3lzX2luaXRjYWxsX3N5bmMgKi8NCj4gPj4gPiArc3Vic3lzX2luaXRjYWxsX3N5
bmMocmNhcl91c2IyX2Nsb2NrX3NlbF9pbml0KTsNCj4gPj4NCj4gPj4gSSBzdXBwb3NlIHRoaXMg
aXMgYSB3b3JrYXJvdW5kIGZvciB0aGUgbGFjayBvZiBwcm9iZSBkZWZlcnJhbCBzdXBwb3J0IGlu
IHRoZQ0KPiA+PiBVU0Igc3Vic3lzdGVtPw0KPiA+DQo+ID4gVGhpcyBpcyBteSBmYXVsdC4gSSBh
ZGRlZCAicG93ZXItZG9tYWlucyIgcHJvcGVydHkgaW50byB0aGlzIGRldmljZSdzIG5vZGUuDQo+
ID4gQWZ0ZXIgSSByZW1vdmUgdGhlIHBvd2VyLWRvbWFpbnMgbGlrZSB0aGUgY3BnIG5vZGUsIHRo
aXMgZHJpdmVyIGNhbiB1c2Ugc3Vic3lzX2luaXRjYWxsKCkNCj4gPiBpbnN0ZWFkIG9mIHN1YnN5
c19pbml0Y2FsbF9zeW5jKCkuDQo+IA0KPiBEb2VzIHRoZSBjbG9jayBzZWwgbW9kdWxlIHJlcXVp
cmVzIHRoZSBmdW5jdGlvbmFsIGNsb2NrICJlaGNpX29oY2kiIHRvIGJlDQo+IHJ1bm5pbmcgYmVm
b3JlIHlvdSBjYW4gYWNjZXNzIGl0cyByZWdpc3RlcnM/DQo+IElmIHllcywgSSB0aGluayB0aGVy
ZSBzaG91bGQgYmUgYSAicG93ZXItZG9tYWlucyIgcHJvcGVydHkuDQoNClllcy4gQnV0Li4uDQoN
Cj4gVGhlbiwgeW91IGNhbiBzaW1wbGlmeSB0aGUgY29kZSBieSBjYWxsaW5nDQo+IA0KPiAgICAg
ICAgIHBtX3J1bnRpbWVfZW5hYmxlKGRldik7DQo+ICAgICAgICAgcG1fcnVudGltZV9nZXRfc3lu
YyhkZXYpOw0KPiANCj4gYW5kIHJlbW92ZSB0aGUgZXhwbGljaXQgaGFuZGxpbmcgb2YgdGhlIGZ1
bmN0aW9uYWwgY2xvY2suDQo+IA0KPiBUaGF0IGRvZXMgbm90IHNvbHZlIHByb2JlIGRlZmVycmFs
IGhhbmRsaW5nIGluIHRoZSBVU0Igc3Vic3lzdGVtLCB0aG91Z2guDQoNCkkgYWRkZWQgYSBkZWJ1
ZyBtZXNzYWdlIGF0IGVuZCBvZiBjcGdfbXNzcl9wcm9iZSgpLCBhbmQgaWYgSSB1c2VkIHN1YnN5
c19pbml0Y2FsbCgpIG9uDQpyY2FyLXVzYjItY2xvY2stc2VsIGRyaXZlciwga2VybmVsIGxvZyBv
dXRwdXQgYmVsb3c6DQo9PT09PT09PT09PT09PT09PT09PT09PT0NClsgICAgMC4yNzI1NDddIHJj
YXItdXNiMi1jbG9jay1zZWwgZTY1OTA2MzAuY2xvY2stY29udHJvbGxlcjogcHJvYmUgZGVmZXJy
YWwgbm90IHN1cHBvcnRlZA0KWyAgICAwLjI3Mzk0NF0gIC0tLS0tLS0tLS0tLS0tLS0tIGNwZ19t
c3NyX3Byb2JlOiBwcm9iZWQhDQo9PT09PT09PT09PT09PT09PT09PT09PT0NClNvLCBpdCBzZWVt
cyB0aGUgcmVuZXNhcy1jcGctbXNzci5jIGRvZXNuJ3Qgc29sdmUgcHJvYmUgZGVmZXJyYWwgaGFu
ZGxpbmcuDQooVGhlIGRyaXZlciByZW5lc2FzLWNwZy1tc3NyLmMgdXNlcyBwbGF0Zm9ybV9kcml2
ZXJfcHJvYmUoKSBmb3Igbm93LikNCg0KU28sIElJVUMsIHRoaXMgcmNhci11c2IyLWNsb2NrLXNl
bCBkcml2ZXIgY2Fubm90IHVzZSBzdWJzeXNfaW5pdGNhbGwoKS4NCk9yLCBzaG91bGQgd2UgbW9k
aWZ5IHRoZSByZW5lc2FzLWNwZy1tc3NyLmMgc29tZWhvdz8NCg0KQmVzdCByZWdhcmRzLA0KWW9z
aGloaXJvIFNoaW1vZGENCg0KPiBHcntvZXRqZSxlZXRpbmd9cywNCj4gDQo+ICAgICAgICAgICAg
ICAgICAgICAgICAgIEdlZXJ0DQo+IA0KPiAtLQ0KPiBHZWVydCBVeXR0ZXJob2V2ZW4gLS0gVGhl
cmUncyBsb3RzIG9mIExpbnV4IGJleW9uZCBpYTMyIC0tIGdlZXJ0QGxpbnV4LW02OGsub3JnDQo+
IA0KPiBJbiBwZXJzb25hbCBjb252ZXJzYXRpb25zIHdpdGggdGVjaG5pY2FsIHBlb3BsZSwgSSBj
YWxsIG15c2VsZiBhIGhhY2tlci4gQnV0DQo+IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlz
dHMgSSBqdXN0IHNheSAicHJvZ3JhbW1lciIgb3Igc29tZXRoaW5nIGxpa2UgdGhhdC4NCj4gICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0K

Re: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, Jul 24, 2017 at 12:33 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>> From: Geert Uytterhoeven
>> Sent: Wednesday, July 5, 2017 9:22 PM
>> On Wed, Jul 5, 2017 at 1:57 PM, Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh@renesas.com> wrote:
>> >> From: Geert Uytterhoeven
>> >> Sent: Wednesday, July 5, 2017 7:09 PM
>> >> On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
>> >> <yoshihiro.shimoda.uh@renesas.com> wrote:
>> >> > R-Car USB 2.0 controller can change the clock source from an oscillator
>> >> > to an external clock via a register. So, this patch adds support
>> >> > the clock source selector as a clock driver.
>>
>> >> > --- /dev/null
>> >> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
>>
>> >> > +/* Since this driver needs other ccf drivers, this uses subsys_initcall_sync */
>> >> > +subsys_initcall_sync(rcar_usb2_clock_sel_init);
>> >>
>> >> I suppose this is a workaround for the lack of probe deferral support in the
>> >> USB subsystem?
>> >
>> > This is my fault. I added "power-domains" property into this device's node.
>> > After I remove the power-domains like the cpg node, this driver can use subsys_initcall()
>> > instead of subsys_initcall_sync().
>>
>> Does the clock sel module requires the functional clock "ehci_ohci" to be
>> running before you can access its registers?
>> If yes, I think there should be a "power-domains" property.
>
> Yes. But...
>
>> Then, you can simplify the code by calling
>>
>>         pm_runtime_enable(dev);
>>         pm_runtime_get_sync(dev);
>>
>> and remove the explicit handling of the functional clock.
>>
>> That does not solve probe deferral handling in the USB subsystem, though.
>
> I added a debug message at end of cpg_mssr_probe(), and if I used subsys_initcall() on
> rcar-usb2-clock-sel driver, kernel log output below:
> ========================
> [    0.272547] rcar-usb2-clock-sel e6590630.clock-controller: probe deferral not supported
> [    0.273944]  ----------------- cpg_mssr_probe: probed!
> ========================
> So, it seems the renesas-cpg-mssr.c doesn't solve probe deferral handling.
> (The driver renesas-cpg-mssr.c uses platform_driver_probe() for now.)
>
> So, IIUC, this rcar-usb2-clock-sel driver cannot use subsys_initcall().
> Or, should we modify the renesas-cpg-mssr.c somehow?

Drivers should avoid using subsys_initcall().
Why do you use a subsys_initcall()? To avoid probe deferral in the USB
susbystem?
What happens if the rcar-usb2-clock-sel uses builtin_platform_driver()?

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

RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Yoshihiro Shimoda]

SGkgR2VlcnQtc2FuLA0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEdl
ZXJ0IFV5dHRlcmhvZXZlbg0KPiBTZW50OiBNb25kYXksIEp1bHkgMjQsIDIwMTcgOTo1OSBQTQ0K
PiANCj4gSGkgU2hpbW9kYS1zYW4sDQo+IA0KPiBPbiBNb24sIEp1bCAyNCwgMjAxNyBhdCAxMjoz
MyBQTSwgWW9zaGloaXJvIFNoaW1vZGENCj4gPHlvc2hpaGlyby5zaGltb2RhLnVoQHJlbmVzYXMu
Y29tPiB3cm90ZToNCj4gPj4gRnJvbTogR2VlcnQgVXl0dGVyaG9ldmVuDQo+ID4+IFNlbnQ6IFdl
ZG5lc2RheSwgSnVseSA1LCAyMDE3IDk6MjIgUE0NCj4gPj4gT24gV2VkLCBKdWwgNSwgMjAxNyBh
dCAxOjU3IFBNLCBZb3NoaWhpcm8gU2hpbW9kYQ0KPiA+PiA8eW9zaGloaXJvLnNoaW1vZGEudWhA
cmVuZXNhcy5jb20+IHdyb3RlOg0KPiA+PiA+PiBGcm9tOiBHZWVydCBVeXR0ZXJob2V2ZW4NCj4g
Pj4gPj4gU2VudDogV2VkbmVzZGF5LCBKdWx5IDUsIDIwMTcgNzowOSBQTQ0KPiA+PiA+PiBPbiBX
ZWQsIEp1biAyOCwgMjAxNyBhdCA4OjI4IEFNLCBZb3NoaWhpcm8gU2hpbW9kYQ0KPiA+PiA+PiA8
eW9zaGloaXJvLnNoaW1vZGEudWhAcmVuZXNhcy5jb20+IHdyb3RlOg0KPiA+PiA+PiA+IFItQ2Fy
IFVTQiAyLjAgY29udHJvbGxlciBjYW4gY2hhbmdlIHRoZSBjbG9jayBzb3VyY2UgZnJvbSBhbiBv
c2NpbGxhdG9yDQo+ID4+ID4+ID4gdG8gYW4gZXh0ZXJuYWwgY2xvY2sgdmlhIGEgcmVnaXN0ZXIu
IFNvLCB0aGlzIHBhdGNoIGFkZHMgc3VwcG9ydA0KPiA+PiA+PiA+IHRoZSBjbG9jayBzb3VyY2Ug
c2VsZWN0b3IgYXMgYSBjbG9jayBkcml2ZXIuDQo+ID4+DQo+ID4+ID4+ID4gLS0tIC9kZXYvbnVs
bA0KPiA+PiA+PiA+ICsrKyBiL2RyaXZlcnMvY2xrL3JlbmVzYXMvcmNhci11c2IyLWNsb2NrLXNl
bC5jDQo+ID4+DQo+ID4+ID4+ID4gKy8qIFNpbmNlIHRoaXMgZHJpdmVyIG5lZWRzIG90aGVyIGNj
ZiBkcml2ZXJzLCB0aGlzIHVzZXMgc3Vic3lzX2luaXRjYWxsX3N5bmMgKi8NCj4gPj4gPj4gPiAr
c3Vic3lzX2luaXRjYWxsX3N5bmMocmNhcl91c2IyX2Nsb2NrX3NlbF9pbml0KTsNCj4gPj4gPj4N
Cj4gPj4gPj4gSSBzdXBwb3NlIHRoaXMgaXMgYSB3b3JrYXJvdW5kIGZvciB0aGUgbGFjayBvZiBw
cm9iZSBkZWZlcnJhbCBzdXBwb3J0IGluIHRoZQ0KPiA+PiA+PiBVU0Igc3Vic3lzdGVtPw0KPiA+
PiA+DQo+ID4+ID4gVGhpcyBpcyBteSBmYXVsdC4gSSBhZGRlZCAicG93ZXItZG9tYWlucyIgcHJv
cGVydHkgaW50byB0aGlzIGRldmljZSdzIG5vZGUuDQo+ID4+ID4gQWZ0ZXIgSSByZW1vdmUgdGhl
IHBvd2VyLWRvbWFpbnMgbGlrZSB0aGUgY3BnIG5vZGUsIHRoaXMgZHJpdmVyIGNhbiB1c2Ugc3Vi
c3lzX2luaXRjYWxsKCkNCj4gPj4gPiBpbnN0ZWFkIG9mIHN1YnN5c19pbml0Y2FsbF9zeW5jKCku
DQo+ID4+DQo+ID4+IERvZXMgdGhlIGNsb2NrIHNlbCBtb2R1bGUgcmVxdWlyZXMgdGhlIGZ1bmN0
aW9uYWwgY2xvY2sgImVoY2lfb2hjaSIgdG8gYmUNCj4gPj4gcnVubmluZyBiZWZvcmUgeW91IGNh
biBhY2Nlc3MgaXRzIHJlZ2lzdGVycz8NCj4gPj4gSWYgeWVzLCBJIHRoaW5rIHRoZXJlIHNob3Vs
ZCBiZSBhICJwb3dlci1kb21haW5zIiBwcm9wZXJ0eS4NCj4gPg0KPiA+IFllcy4gQnV0Li4uDQo+
ID4NCj4gPj4gVGhlbiwgeW91IGNhbiBzaW1wbGlmeSB0aGUgY29kZSBieSBjYWxsaW5nDQo+ID4+
DQo+ID4+ICAgICAgICAgcG1fcnVudGltZV9lbmFibGUoZGV2KTsNCj4gPj4gICAgICAgICBwbV9y
dW50aW1lX2dldF9zeW5jKGRldik7DQo+ID4+DQo+ID4+IGFuZCByZW1vdmUgdGhlIGV4cGxpY2l0
IGhhbmRsaW5nIG9mIHRoZSBmdW5jdGlvbmFsIGNsb2NrLg0KPiA+Pg0KPiA+PiBUaGF0IGRvZXMg
bm90IHNvbHZlIHByb2JlIGRlZmVycmFsIGhhbmRsaW5nIGluIHRoZSBVU0Igc3Vic3lzdGVtLCB0
aG91Z2guDQo+ID4NCj4gPiBJIGFkZGVkIGEgZGVidWcgbWVzc2FnZSBhdCBlbmQgb2YgY3BnX21z
c3JfcHJvYmUoKSwgYW5kIGlmIEkgdXNlZCBzdWJzeXNfaW5pdGNhbGwoKSBvbg0KPiA+IHJjYXIt
dXNiMi1jbG9jay1zZWwgZHJpdmVyLCBrZXJuZWwgbG9nIG91dHB1dCBiZWxvdzoNCj4gPiA9PT09
PT09PT09PT09PT09PT09PT09PT0NCj4gPiBbICAgIDAuMjcyNTQ3XSByY2FyLXVzYjItY2xvY2st
c2VsIGU2NTkwNjMwLmNsb2NrLWNvbnRyb2xsZXI6IHByb2JlIGRlZmVycmFsIG5vdCBzdXBwb3J0
ZWQNCj4gPiBbICAgIDAuMjczOTQ0XSAgLS0tLS0tLS0tLS0tLS0tLS0gY3BnX21zc3JfcHJvYmU6
IHByb2JlZCENCj4gPiA9PT09PT09PT09PT09PT09PT09PT09PT0NCj4gPiBTbywgaXQgc2VlbXMg
dGhlIHJlbmVzYXMtY3BnLW1zc3IuYyBkb2Vzbid0IHNvbHZlIHByb2JlIGRlZmVycmFsIGhhbmRs
aW5nLg0KPiA+IChUaGUgZHJpdmVyIHJlbmVzYXMtY3BnLW1zc3IuYyB1c2VzIHBsYXRmb3JtX2Ry
aXZlcl9wcm9iZSgpIGZvciBub3cuKQ0KPiA+DQo+ID4gU28sIElJVUMsIHRoaXMgcmNhci11c2Iy
LWNsb2NrLXNlbCBkcml2ZXIgY2Fubm90IHVzZSBzdWJzeXNfaW5pdGNhbGwoKS4NCj4gPiBPciwg
c2hvdWxkIHdlIG1vZGlmeSB0aGUgcmVuZXNhcy1jcGctbXNzci5jIHNvbWVob3c/DQo+IA0KPiBE
cml2ZXJzIHNob3VsZCBhdm9pZCB1c2luZyBzdWJzeXNfaW5pdGNhbGwoKS4NCj4gV2h5IGRvIHlv
dSB1c2UgYSBzdWJzeXNfaW5pdGNhbGwoKT8gVG8gYXZvaWQgcHJvYmUgZGVmZXJyYWwgaW4gdGhl
IFVTQg0KPiBzdXNieXN0ZW0/DQoNCk9oLCB0aGlzIGlzIG15IGZhdWx0Lg0KSSBoYWQgYXNzdW1l
ZCBhIGNsb2NrIGRyaXZlciBzaG91bGQgdXNlIHN1YnN5c19pbml0Y2FsbCgpLi4uDQpMaWtlIGNs
ay1maXhlZC1yYXRlLmMsIHdlIGNhbiB1c2UgYnVpbHRpbl9wbGF0Zm9ybV9kcml2ZXIgaW4gZ2Vu
ZXJhbC4NCg0KPiBXaGF0IGhhcHBlbnMgaWYgdGhlIHJjYXItdXNiMi1jbG9jay1zZWwgdXNlcyBi
dWlsdGluX3BsYXRmb3JtX2RyaXZlcigpPw0KDQpJIHRyaWVkIGl0IHRvZGF5LCBhbmQgdGhlbiB0
aGUgcmNhci11c2IyLWNsb2NrLXNlbCB3b3JrcyA6KQ0KU28sIEkgd2lsbCBzdWJtaXQgdjMgcGF0
Y2guDQoNCkJlc3QgcmVnYXJkcywNCllvc2hpaGlybyBTaGltb2RhDQoNCj4gR3J7b2V0amUsZWV0
aW5nfXMsDQo+IA0KPiAgICAgICAgICAgICAgICAgICAgICAgICBHZWVydA0KPiANCj4gLS0NCj4g
R2VlcnQgVXl0dGVyaG9ldmVuIC0tIFRoZXJlJ3MgbG90cyBvZiBMaW51eCBiZXlvbmQgaWEzMiAt
LSBnZWVydEBsaW51eC1tNjhrLm9yZw0KPiANCj4gSW4gcGVyc29uYWwgY29udmVyc2F0aW9ucyB3
aXRoIHRlY2huaWNhbCBwZW9wbGUsIEkgY2FsbCBteXNlbGYgYSBoYWNrZXIuIEJ1dA0KPiB3aGVu
IEknbSB0YWxraW5nIHRvIGpvdXJuYWxpc3RzIEkganVzdCBzYXkgInByb2dyYW1tZXIiIG9yIHNv
bWV0aGluZyBsaWtlIHRoYXQuDQo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgLS0g
TGludXMgVG9ydmFsZHMNCg==