[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]

Hi Geert-san,

> From: Geert Uytterhoeven
> Sent: Wednesday, July 5, 2017 7:09 PM
> 
> 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!

Thank you very much for the comments!

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

Yes.

> > +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?

Oops! I should call clk_disable_unprepare(priv->ehci_clk) here.
I will fix it.

> > +       }
> > +
> > +       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.

Same here. So, I will use "goto" for it.

> > +       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);

Thanks. I will fix it.

> > +/* 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().

Best regards,
Yoshihiro Shimoda

> 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

[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-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
>  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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Oops, I will fix it.

> With that, for the binding:
> 
> 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
> 
> 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.

I got 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?

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 = {
> > +	.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]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2804 bytes --]

Hi Geert-san,

> From: Geert Uytterhoeven
> Sent: Wednesday, July 5, 2017 9:22 PM
> 
> 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.

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?

Best regards,
Yoshihiro Shimoda

> 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*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

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]

Hi Geert-san,

> -----Original Message-----
> From: Geert Uytterhoeven
> Sent: Monday, July 24, 2017 9:59 PM
> 
> 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?

Oh, this is my fault.
I had assumed a clock driver should use subsys_initcall()...
Like clk-fixed-rate.c, we can use builtin_platform_driver in general.

> What happens if the rcar-usb2-clock-sel uses builtin_platform_driver()?

I tried it today, and then the rcar-usb2-clock-sel works :)
So, I will submit v3 patch.

Best regards,
Yoshihiro Shimoda

> 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