[PATCH v2 1/4] ARM i.MX6: use reserved bit for mxs phy clock gate

[PATCH v2 1/4] ARM i.MX6: use reserved bit for mxs phy clock gate

[Peter Chen]

For mxs-phy user i.mx6q, the PHY's clock is controlled by
hardware automatically, the software only needs to enable it
at probe, disable it at remove. During the runtime,
we don't need to control it. So for the usbphy clk policy:

- Keep refcount for usbphy as clk framework needs to know if
it is off or on.
- Use reserved bit, in that case, clk_enable/disable will
only update refcount, but without any hardware effects.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v2:
- Use reserved bit for usb phy clk control

 arch/arm/mach-imx/clk-imx6q.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 7f2c10c..85dcc89 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -208,8 +208,14 @@ int __init mx6q_clocks_init(void)
 	clk[pll7_usb_host] = imx_clk_pllv3(IMX_PLLV3_USB,	"pll7_usb_host","osc", base + 0x20, 0x3);
 	clk[pll8_mlb]      = imx_clk_pllv3(IMX_PLLV3_MLB,	"pll8_mlb",	"osc", base + 0xd0, 0x0);
 
-	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 6);
-	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 6);
+	/*
+	 * Bit 20 is the reserved and read-only bit, we do this only for:
+	 * - Do nothing for usbphy clk_enable/disable
+	 * - Keep refcount when do usbphy clk_enable/disable, in that case,
+	 * the clk framework can know the USB phy clk is on or off
+	 */
+	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 20);
+	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 20);
 
 	clk[sata_ref] = imx_clk_fixed_factor("sata_ref", "pll6_enet", 1, 5);
 	clk[pcie_ref] = imx_clk_fixed_factor("pcie_ref", "pll6_enet", 1, 4);
-- 
1.7.0.4

[PATCH v2 2/4] usb: mxs-phy: change clock usage for i.mx6q

[Peter Chen]

For mxs-phy user i.mx6q, the PHY's clock is controlled by
hardware automatically, the software only needs to enable it
at probe, disable it at remove. But other mxs-phy users need
to control that clock runtime, so we hardcode clk on/off,
and give a reserved bit for clk on/off at clk code for i.mx6q.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v2:
- Only control gate bit for phy clk control
- Only open the gate at probe, and close the gate at remove

 Documentation/devicetree/bindings/usb/mxs-phy.txt |    2 +
 arch/arm/boot/dts/imx6q.dtsi                      |    2 +
 drivers/usb/otg/mxs-phy.c                         |   61 +++++++++++++++++++++
 3 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
index 5835b27..384e700 100644
--- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
+++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
@@ -4,10 +4,12 @@ Required properties:
 - compatible: Should be "fsl,imx23-usbphy"
 - reg: Should contain registers location and length
 - interrupts: Should contain phy interrupt
+- The reg offset for PHY clock at anatop
 
 Example:
 usbphy1: usbphy at 020c9000 {
 	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
 	reg = <0x020c9000 0x1000>;
 	interrupts = <0 44 0x04>;
+	anatop-phy-reg-offset = <0x10>;
 };
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index d6265ca..1517e93 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -519,6 +519,7 @@
 				reg = <0x020c9000 0x1000>;
 				interrupts = <0 44 0x04>;
 				clocks = <&clks 182>;
+				anatop-phy-reg-offset = <0x10>;
 			};
 
 			usbphy2: usbphy at 020ca000 {
@@ -526,6 +527,7 @@
 				reg = <0x020ca000 0x1000>;
 				interrupts = <0 45 0x04>;
 				clocks = <&clks 183>;
+				anatop-phy-reg-offset = <0x20>;
 			};
 
 			snvs at 020cc000 {
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index 7630272..49727dd 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -20,6 +20,9 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #define DRIVER_NAME "mxs_phy"
 
@@ -34,6 +37,11 @@
 #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
 #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
 
+#define CTRL_SET				0x4
+#define CTRL_CLR				0x8
+
+#define BM_ANADIG_USB_PLL_480_CTRL_EN_USB_CLKS		(1 << 6)
+
 struct mxs_phy {
 	struct usb_phy phy;
 	struct clk *clk;
@@ -108,6 +116,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct regmap *anatop;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -146,11 +155,63 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
+	/*
+	 * At mx6x, USB PHY PLL and its output gate is controlled by hardware.
+	 * It just needs to open the gate at init, if the usb device is
+	 * in suspend, it will close related PLL automatically without
+	 * the gate is on or off.
+	 */
+
+	anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
+
+	if (!IS_ERR(anatop)) {
+		struct device *dev = &pdev->dev;
+		struct device_node *np = dev->of_node;
+		u32 phy_reg_offset;
+		int ret;
+
+		ret = of_property_read_u32(np, "anatop-phy-reg-offset",
+					   &phy_reg_offset);
+		if (ret) {
+			dev_err(dev, "no anatop-phy-reg-offset property set\n");
+			return -EINVAL;
+		}
+
+		regmap_write(anatop, phy_reg_offset + CTRL_SET,
+				BM_ANADIG_USB_PLL_480_CTRL_EN_USB_CLKS);
+	} else {
+		pr_warn("failed to find fsl,imx6q-anatop regmap\n");
+	}
+
 	return 0;
 }
 
 static int mxs_phy_remove(struct platform_device *pdev)
 {
+	struct regmap *anatop;
+
+	/* close the clock gate for USB PHY */
+	anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
+
+	if (!IS_ERR(anatop)) {
+		struct device *dev = &pdev->dev;
+		struct device_node *np = dev->of_node;
+		u32 phy_reg_offset;
+		int ret;
+
+		ret = of_property_read_u32(np, "anatop-phy-reg-offset",
+					   &phy_reg_offset);
+		if (ret) {
+			dev_err(dev, "no anatop-phy-reg-offset property set\n");
+			return -EINVAL;
+		}
+
+		regmap_write(anatop, phy_reg_offset + CTRL_CLR,
+				BM_ANADIG_USB_PLL_480_CTRL_EN_USB_CLKS);
+	} else {
+		pr_warn("failed to find fsl,imx6q-anatop regmap\n");
+	}
+
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 
1.7.0.4

[PATCH v2 3/4] usb: mxs-phy: add set_suspend API

[Peter Chen]

It needs to call set_suspend during USB suspend/resume

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/otg/mxs-phy.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index 49727dd..cad16e5 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -84,6 +84,25 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
 	clk_disable_unprepare(mxs_phy->clk);
 }
 
+static int mxs_phy_suspend(struct usb_phy *x, int suspend)
+{
+	struct mxs_phy *mxs_phy = to_mxs_phy(x);
+
+	if (suspend) {
+		writel_relaxed(0xffffffff, x->io_priv + HW_USBPHY_PWD);
+		writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
+			x->io_priv + HW_USBPHY_CTRL_SET);
+		clk_disable_unprepare(mxs_phy->clk);
+	} else {
+		clk_prepare_enable(mxs_phy->clk);
+		writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
+			x->io_priv + HW_USBPHY_CTRL_CLR);
+		writel_relaxed(0, x->io_priv + HW_USBPHY_PWD);
+	}
+
+	return 0;
+}
+
 static int mxs_phy_on_connect(struct usb_phy *phy,
 		enum usb_device_speed speed)
 {
@@ -146,6 +165,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	mxs_phy->phy.label		= DRIVER_NAME;
 	mxs_phy->phy.init		= mxs_phy_init;
 	mxs_phy->phy.shutdown		= mxs_phy_shutdown;
+	mxs_phy->phy.set_suspend	= mxs_phy_suspend;
 	mxs_phy->phy.notify_connect	= mxs_phy_on_connect;
 	mxs_phy->phy.notify_disconnect	= mxs_phy_on_disconnect;
 
-- 
1.7.0.4

[PATCH v2 4/4] usb: chipidea: imx: Add system suspend/resume API

[Peter Chen]

During the system suspend/resume procedure, the USB also
needs to go suspend/resume procedure, this patch adds
related APIs. It is tested at i.mx6q sabrelite. Meanwhile,
it fixes the bug that the USB will out of work after
system suspend/resume.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/bits.h        |    1 +
 drivers/usb/chipidea/ci13xxx_imx.c |   61 ++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index ba9c6ef..d1467bb 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -47,6 +47,7 @@
 #define PORTSC_FPR            BIT(6)
 #define PORTSC_SUSP           BIT(7)
 #define PORTSC_HSP            BIT(9)
+#define PORTSC_PHCD           BIT(23) /* phy suspend mode */
 #define PORTSC_PTC            (0x0FUL << 16)
 
 /* DEVLC */
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index 342eab0..dd257b1 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -25,6 +25,7 @@
 #include <linux/mfd/syscon.h>
 
 #include "ci.h"
+#include "bits.h"
 #include "ci13xxx_imx.h"
 
 #define pdev_to_phy(pdev) \
@@ -321,6 +322,63 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ci13xxx_imx_suspend(struct device *dev)
+{
+	struct ci13xxx_imx_data *data =
+		platform_get_drvdata(to_platform_device(dev));
+	struct platform_device *plat_ci;
+	struct ci13xxx	*ci;
+
+	plat_ci = data->ci_pdev;
+	ci = platform_get_drvdata(plat_ci);
+
+	hw_write(ci, OP_PORTSC, PORTSC_PHCD, 1);
+
+	if (data->phy)
+		usb_phy_set_suspend(data->phy, 1);
+
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int ci13xxx_imx_resume(struct device *dev)
+{
+	int ret;
+	struct ci13xxx_imx_data *data =
+		platform_get_drvdata(to_platform_device(dev));
+	struct platform_device *plat_ci;
+	struct ci13xxx	*ci;
+
+	plat_ci = data->ci_pdev;
+	ci = platform_get_drvdata(plat_ci);
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret) {
+		dev_err(dev,
+			"Failed to prepare or enable clock, err=%d\n", ret);
+		return ret;
+	}
+
+	if (hw_read(ci, OP_PORTSC, PORTSC_PHCD)) {
+		hw_write(ci, OP_PORTSC, PORTSC_PHCD, 0);
+		/* Some clks sync between Controller and USB PHY */
+		mdelay(1);
+	}
+
+	if (data->phy)
+		usb_phy_set_suspend(data->phy, 0);
+
+	return ret;
+}
+
+static const struct dev_pm_ops ci13xxx_imx_pm_ops = {
+	.suspend	= ci13xxx_imx_suspend,
+	.resume		= ci13xxx_imx_resume,
+};
+#endif
+
 static const struct of_device_id ci13xxx_imx_dt_ids[] = {
 	{ .compatible = "fsl,imx27-usb", },
 	{ /* sentinel */ }
@@ -334,6 +392,9 @@ static struct platform_driver ci13xxx_imx_driver = {
 		.name = "imx_usb",
 		.owner = THIS_MODULE,
 		.of_match_table = ci13xxx_imx_dt_ids,
+#ifdef CONFIG_PM
+		.pm	= &ci13xxx_imx_pm_ops,
+#endif
 	 },
 };
 
-- 
1.7.0.4

[PATCH v2 1/4] ARM i.MX6: use reserved bit for mxs phy clock gate

[Shawn Guo]

On Tue, Jan 15, 2013 at 02:08:34PM +0800, Peter Chen wrote:
> For mxs-phy user i.mx6q, the PHY's clock is controlled by
> hardware automatically, the software only needs to enable it
> at probe, disable it at remove. During the runtime,
> we don't need to control it. So for the usbphy clk policy:
> 
> - Keep refcount for usbphy as clk framework needs to know if
> it is off or on.

Just checked the reference manual, it seems that not only bit 6
(EN_USB_CLKS) but also bit 12 (POWER) are controlled by USB hardware.
If this is true, I think that PLL3 and PLL7 are really designed for
USB PHY use only.  Do you actually see other real users of these two
PLLs on imx6q besides USB PHY?  If not, we can just provide a dummy
clock to mxs-phy driver on imx6q, and keep real clocks usbphy1 and
usbphy2 there for platform init function to enable them if USB support
is built in.  Then, we can save all these dirty hacks.  Thoughts?

Shawn

> - Use reserved bit, in that case, clk_enable/disable will
> only update refcount, but without any hardware effects.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
> Changes for v2:
> - Use reserved bit for usb phy clk control
> 
>  arch/arm/mach-imx/clk-imx6q.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index 7f2c10c..85dcc89 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -208,8 +208,14 @@ int __init mx6q_clocks_init(void)
>  	clk[pll7_usb_host] = imx_clk_pllv3(IMX_PLLV3_USB,	"pll7_usb_host","osc", base + 0x20, 0x3);
>  	clk[pll8_mlb]      = imx_clk_pllv3(IMX_PLLV3_MLB,	"pll8_mlb",	"osc", base + 0xd0, 0x0);
>  
> -	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 6);
> -	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 6);
> +	/*
> +	 * Bit 20 is the reserved and read-only bit, we do this only for:
> +	 * - Do nothing for usbphy clk_enable/disable
> +	 * - Keep refcount when do usbphy clk_enable/disable, in that case,
> +	 * the clk framework can know the USB phy clk is on or off
> +	 */
> +	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 20);
> +	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 20);
>  
>  	clk[sata_ref] = imx_clk_fixed_factor("sata_ref", "pll6_enet", 1, 5);
>  	clk[pcie_ref] = imx_clk_fixed_factor("pcie_ref", "pll6_enet", 1, 4);
> -- 
> 1.7.0.4
> 
> 

[PATCH v2 1/4] ARM i.MX6: use reserved bit for mxs phy clock gate

[Peter Chen]

On Tue, Jan 15, 2013 at 07:33:16PM +0800, Shawn Guo wrote:
> On Tue, Jan 15, 2013 at 02:08:34PM +0800, Peter Chen wrote:
> > For mxs-phy user i.mx6q, the PHY's clock is controlled by
> > hardware automatically, the software only needs to enable it
> > at probe, disable it at remove. During the runtime,
> > we don't need to control it. So for the usbphy clk policy:
> > 
> > - Keep refcount for usbphy as clk framework needs to know if
> > it is off or on.
> 
> Just checked the reference manual, it seems that not only bit 6
> (EN_USB_CLKS) but also bit 12 (POWER) are controlled by USB hardware.
> If this is true, I think that PLL3 and PLL7 are really designed for
> USB PHY use only.  Do you actually see other real users of these two
> PLLs on imx6q besides USB PHY?  If not, we can just provide a dummy
> clock to mxs-phy driver on imx6q, and keep real clocks usbphy1 and
> usbphy2 there for platform init function to enable them if USB support
> is built in.  Then, we can save all these dirty hacks.  Thoughts?
It was my v1 patch did.

I send the v2 patch due to below reasons,
There are other PLL3 users, it is better keep refcount
to all its children, or the user will be confused when the
PLL3 goes to disable, but the usb otg part is still used

For PLL7, we can do it, and let the PLL7 be out of clock framework
management.

But I still think keep usbphy as PLL child is better solution,
in that case, the clock maintainer can know the real clock usage.

> 
> Shawn
> 
> > - Use reserved bit, in that case, clk_enable/disable will
> > only update refcount, but without any hardware effects.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> > Changes for v2:
> > - Use reserved bit for usb phy clk control
> > 
> >  arch/arm/mach-imx/clk-imx6q.c |   10 ++++++++--
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > index 7f2c10c..85dcc89 100644
> > --- a/arch/arm/mach-imx/clk-imx6q.c
> > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > @@ -208,8 +208,14 @@ int __init mx6q_clocks_init(void)
> >  	clk[pll7_usb_host] = imx_clk_pllv3(IMX_PLLV3_USB,	"pll7_usb_host","osc", base + 0x20, 0x3);
> >  	clk[pll8_mlb]      = imx_clk_pllv3(IMX_PLLV3_MLB,	"pll8_mlb",	"osc", base + 0xd0, 0x0);
> >  
> > -	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 6);
> > -	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 6);
> > +	/*
> > +	 * Bit 20 is the reserved and read-only bit, we do this only for:
> > +	 * - Do nothing for usbphy clk_enable/disable
> > +	 * - Keep refcount when do usbphy clk_enable/disable, in that case,
> > +	 * the clk framework can know the USB phy clk is on or off
> > +	 */
> > +	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 20);
> > +	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 20);
> >  
> >  	clk[sata_ref] = imx_clk_fixed_factor("sata_ref", "pll6_enet", 1, 5);
> >  	clk[pcie_ref] = imx_clk_fixed_factor("pcie_ref", "pll6_enet", 1, 4);
> > -- 
> > 1.7.0.4
> > 
> > 

-- 

Best Regards,
Peter Chen

[PATCH v2 1/4] ARM i.MX6: use reserved bit for mxs phy clock gate

[Shawn Guo]

On Wed, Jan 16, 2013 at 09:18:46AM +0800, Peter Chen wrote:
> On Tue, Jan 15, 2013 at 07:33:16PM +0800, Shawn Guo wrote:
> > On Tue, Jan 15, 2013 at 02:08:34PM +0800, Peter Chen wrote:
> > > For mxs-phy user i.mx6q, the PHY's clock is controlled by
> > > hardware automatically, the software only needs to enable it
> > > at probe, disable it at remove. During the runtime,
> > > we don't need to control it. So for the usbphy clk policy:
> > > 
> > > - Keep refcount for usbphy as clk framework needs to know if
> > > it is off or on.
> > 
> > Just checked the reference manual, it seems that not only bit 6
> > (EN_USB_CLKS) but also bit 12 (POWER) are controlled by USB hardware.
> > If this is true, I think that PLL3 and PLL7 are really designed for
> > USB PHY use only.  Do you actually see other real users of these two
> > PLLs on imx6q besides USB PHY?  If not, we can just provide a dummy
> > clock to mxs-phy driver on imx6q, and keep real clocks usbphy1 and
> > usbphy2 there for platform init function to enable them if USB support
> > is built in.  Then, we can save all these dirty hacks.  Thoughts?
> It was my v1 patch did.
> 
I thought it might be okay to access anatop for enabling the clock in
usb driver.  But I found it's just too ugly to do that when I look at
the code a little bit closer.  I prefer to keep clock driver unchanged
and just call clk_prepare_enable() at platform level to enable the
clock.  I really dislike the code accessing anatop for enabling the
clock.  That's the part different from you v1 patch.

> I send the v2 patch due to below reasons,
> There are other PLL3 users, it is better keep refcount
> to all its children, or the user will be confused when the
> PLL3 goes to disable, but the usb otg part is still used
> 
This is the part I'm not sure how it works.  You said, the PLL will be
managed by hardware during USB suspend/resume.  I assume that it means
the PLL will be powered off when USB suspends and be powered on when
USB resumes.  If that's the case, how other users of the PLL work when
PLL is powered off by USB hardware.

Shawn

> For PLL7, we can do it, and let the PLL7 be out of clock framework
> management.
> 
> But I still think keep usbphy as PLL child is better solution,
> in that case, the clock maintainer can know the real clock usage.
> 

[PATCH v2 1/4] ARM i.MX6: use reserved bit for mxs phy clock gate

[Peter Chen]

On Wed, Jan 16, 2013 at 09:48:57AM +0800, Shawn Guo wrote:
> On Wed, Jan 16, 2013 at 09:18:46AM +0800, Peter Chen wrote:
> > On Tue, Jan 15, 2013 at 07:33:16PM +0800, Shawn Guo wrote:
> > > On Tue, Jan 15, 2013 at 02:08:34PM +0800, Peter Chen wrote:
> > > > For mxs-phy user i.mx6q, the PHY's clock is controlled by
> > > > hardware automatically, the software only needs to enable it
> > > > at probe, disable it at remove. During the runtime,
> > > > we don't need to control it. So for the usbphy clk policy:
> > > > 
> > > > - Keep refcount for usbphy as clk framework needs to know if
> > > > it is off or on.
> > > 
> I thought it might be okay to access anatop for enabling the clock in
> usb driver.  But I found it's just too ugly to do that when I look at
> the code a little bit closer.  I prefer to keep clock driver unchanged
> and just call clk_prepare_enable() at platform level to enable the
> clock.  I really dislike the code accessing anatop for enabling the
> clock.  That's the part different from you v1 patch.

It is a way, but how to consolidate other mxx-phy users, like mx28.
It needs to call clk_prepare_enable runtime.
> 
> > I send the v2 patch due to below reasons,
> > There are other PLL3 users, it is better keep refcount
> > to all its children, or the user will be confused when the
> > PLL3 goes to disable, but the usb otg part is still used
> > 
> This is the part I'm not sure how it works.  You said, the PLL will be
> managed by hardware during USB suspend/resume.  I assume that it means
> the PLL will be powered off when USB suspends and be powered on when
> USB resumes.  If that's the case, how other users of the PLL work when
> PLL is powered off by USB hardware.

It is a little complicated:
You can understand both PLL7 and PLL3 like below:
PLL_POWER_BIT && usb_is_not_suspend
For PLL7: USB host 1 is the only user, so we can keep PLL7_POWER_BIT is always on
and let the usb hardware controller it automatically. Other way is let the
clock framework and usb hardware controller it together.
For PLL3, as there are other users, so software must control PLL3_POWER_BIT,
or the PLL3 will be off if the usb enters suspend.

This PLL_POWER_BIT is not related to usbphy clock gate, this power bit can be
usbphy's parent.

> 
> Shawn
> 
> > For PLL7, we can do it, and let the PLL7 be out of clock framework
> > management.
> > 
> > But I still think keep usbphy as PLL child is better solution,
> > in that case, the clock maintainer can know the real clock usage.
> > 

-- 

Best Regards,
Peter Chen