[PATCH 1/5] ARM: davinci: defined missing CFGCHIP2_REFFREQ_* macros for MUSB PHY

[PATCH 1/5] ARM: davinci: defined missing CFGCHIP2_REFFREQ_* macros for MUSB PHY

[David Lechner]

From: Petr Kulhavy <petr@barix.com>

Only few MUSB PHY reference clock  frequencies were defined.
This patch defines macros for the missing frequencies:
19.2MHz, 38.4MHz, 13MHz, 26MHz, 20MHz, 40MHz

Signed-off-by: Petr Kulhavy <petr@barix.com>.
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Bin Liu <b-liu@ti.com>
---
 include/linux/platform_data/usb-davinci.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h
index e0bc4ab..961d3dc 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -33,6 +33,12 @@
 #define CFGCHIP2_REFFREQ_12MHZ	(1 << 0)
 #define CFGCHIP2_REFFREQ_24MHZ	(2 << 0)
 #define CFGCHIP2_REFFREQ_48MHZ	(3 << 0)
+#define CFGCHIP2_REFFREQ_19_2MHZ	(4 << 0)
+#define CFGCHIP2_REFFREQ_38_4MHZ	(5 << 0)
+#define CFGCHIP2_REFFREQ_13MHZ	(6 << 0)
+#define CFGCHIP2_REFFREQ_26MHZ	(7 << 0)
+#define CFGCHIP2_REFFREQ_20MHZ	(8 << 0)
+#define CFGCHIP2_REFFREQ_40MHZ	(9 << 0)
 
 struct	da8xx_ohci_root_hub;
 
-- 
1.9.1

[PATCH 2/5] ARM: davinci: da8xx: add usb phy clocks

[David Lechner]

Up to this point, the USB phy clock configuration was handled manually in
the board files and in the usb drivers. This adds proper clocks so that
the usb drivers can use clk_get and clk_enable and not have to worry about
the details. Also, the related code is removed from the board files.

Signed-off-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/board-da830-evm.c     |  12 ---
 arch/arm/mach-davinci/board-omapl138-hawk.c |   7 --
 arch/arm/mach-davinci/da830.c               | 128 ++++++++++++++++++++++++++--
 arch/arm/mach-davinci/da850.c               | 113 ++++++++++++++++++++++++
 4 files changed, 233 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 3d8cf8c..f3a8cc9 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -115,18 +115,6 @@ static __init void da830_evm_usb_init(void)
 	 */
 	cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
 
-	/* USB2.0 PHY reference clock is 24 MHz */
-	cfgchip2 &= ~CFGCHIP2_REFFREQ;
-	cfgchip2 |=  CFGCHIP2_REFFREQ_24MHZ;
-
-	/*
-	 * Select internal reference clock for USB 2.0 PHY
-	 * and use it as a clock source for USB 1.1 PHY
-	 * (this is the default setting anyway).
-	 */
-	cfgchip2 &= ~CFGCHIP2_USB1PHYCLKMUX;
-	cfgchip2 |=  CFGCHIP2_USB2PHYCLKMUX;
-
 	/*
 	 * We have to override VBUS/ID signals when MUSB is configured into the
 	 * host-only mode -- ID pin will float if no cable is connected, so the
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index ee62486..d27e753 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -251,13 +251,6 @@ static __init void omapl138_hawk_usb_init(void)
 		return;
 	}
 
-	/* Setup the Ref. clock frequency for the HAWK at 24 MHz. */
-
-	cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
-	cfgchip2 &= ~CFGCHIP2_REFFREQ;
-	cfgchip2 |=  CFGCHIP2_REFFREQ_24MHZ;
-	__raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
-
 	ret = gpio_request_one(DA850_USB1_VBUS_PIN,
 			GPIOF_DIR_OUT, "USB1 VBUS");
 	if (ret < 0) {
diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 7187e7f..213fb17e 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -12,6 +12,7 @@
 #include <linux/init.h>
 #include <linux/clk.h>
 #include <linux/platform_data/gpio-davinci.h>
+#include <linux/platform_data/usb-davinci.h>
 
 #include <asm/mach/map.h>
 
@@ -297,13 +298,6 @@ static struct clk mcasp2_clk = {
 	.gpsc		= 1,
 };
 
-static struct clk usb20_clk = {
-	.name		= "usb20",
-	.parent		= &pll0_sysclk2,
-	.lpsc		= DA8XX_LPSC1_USB20,
-	.gpsc		= 1,
-};
-
 static struct clk aemif_clk = {
 	.name		= "aemif",
 	.parent		= &pll0_sysclk3,
@@ -346,6 +340,12 @@ static struct clk i2c1_clk = {
 	.gpsc		= 1,
 };
 
+static struct clk usb_ref_clk = {
+	.name		= "usb_ref_clk",
+	.rate		= 48000000,
+	.set_rate	= davinci_simple_set_rate,
+};
+
 static struct clk usb11_clk = {
 	.name		= "usb11",
 	.parent		= &pll0_sysclk4,
@@ -353,6 +353,115 @@ static struct clk usb11_clk = {
 	.gpsc		= 1,
 };
 
+static struct clk usb20_clk = {
+	.name		= "usb20",
+	.parent		= &pll0_sysclk2,
+	.lpsc		= DA8XX_LPSC1_USB20,
+	.gpsc		= 1,
+};
+
+static void usb20_phy_clk_enable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	/*
+	 * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
+	 * host may use the PLL clock without USB 2.0 OTG being used.
+	 */
+		val &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
+		val |= CFGCHIP2_PHY_PLLON;
+
+	/* Set the mux depending on the parent clock. */
+	if (clk->parent == &pll0_aux_clk)
+		val |= CFGCHIP2_USB2PHYCLKMUX;
+	else if (clk->parent == &usb_ref_clk)
+		val &= ~CFGCHIP2_USB2PHYCLKMUX;
+	else
+		pr_err("Bad parent on USB 2.0 PHY clock.\n");
+
+	/* reference frequency also comes from parent clock */
+	val &= ~CFGCHIP2_REFFREQ;
+	switch (clk_get_rate(clk->parent)) {
+	case 12000000:
+		val |= CFGCHIP2_REFFREQ_12MHZ;
+		break;
+	case 13000000:
+		val |= CFGCHIP2_REFFREQ_13MHZ;
+		break;
+	case 19200000:
+		val |= CFGCHIP2_REFFREQ_19_2MHZ;
+		break;
+	case 20000000:
+		val |= CFGCHIP2_REFFREQ_20MHZ;
+		break;
+	case 24000000:
+		val |= CFGCHIP2_REFFREQ_24MHZ;
+		break;
+	case 26000000:
+		val |= CFGCHIP2_REFFREQ_26MHZ;
+		break;
+	case 38400000:
+		val |= CFGCHIP2_REFFREQ_38_4MHZ;
+		break;
+	case 40000000:
+		val |= CFGCHIP2_REFFREQ_40MHZ;
+		break;
+	case 48000000:
+		val |= CFGCHIP2_REFFREQ_48MHZ;
+		break;
+	default:
+		pr_err("Bad parent clock rate on USB 2.0 PHY clock.\n");
+		break;
+	}
+
+		writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	pr_info("Waiting for USB 2.0 PHY clock good...\n");
+		while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
+							 & CFGCHIP2_PHYCLKGD))
+			cpu_relax();
+	}
+
+static void usb20_phy_clk_disable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+	val |= CFGCHIP2_PHYPWRDN;
+	__raw_writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+}
+
+static struct clk usb20_phy_clk = {
+	.name		= "usb20_phy",
+	.parent		= &pll0_aux_clk, /* or &usb_ref_clk */
+	.clk_enable	= usb20_phy_clk_enable,
+	.clk_disable	= usb20_phy_clk_disable,
+};
+
+static void usb11_phy_clk_enable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	/* Set the USB 1.1 PHY clock mux based on the parent clock. */
+	if (clk->parent == &usb20_phy_clk)
+		val &= ~CFGCHIP2_USB1PHYCLKMUX;
+	else if (clk->parent == &usb_ref_clk)
+		val &= ~CFGCHIP2_USB1PHYCLKMUX;
+	else
+		pr_err("Bad parent on USB 1.1 PHY clock.\n");
+
+	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+}
+
+static struct clk usb11_phy_clk = {
+	.name		= "usb11_phy",
+	.parent		= &usb20_phy_clk, /* or &usb_ref_clk */
+	.clk_enable	= usb11_phy_clk_enable,
+};
 static struct clk emif3_clk = {
 	.name		= "emif3",
 	.parent		= &pll0_sysclk5,
@@ -412,7 +521,6 @@ static struct clk_lookup da830_clks[] = {
 	CLK("davinci-mcasp.0",	NULL,		&mcasp0_clk),
 	CLK("davinci-mcasp.1",	NULL,		&mcasp1_clk),
 	CLK("davinci-mcasp.2",	NULL,		&mcasp2_clk),
-	CLK(NULL,		"usb20",	&usb20_clk),
 	CLK(NULL,		"aemif",	&aemif_clk),
 	CLK(NULL,		"aintc",	&aintc_clk),
 	CLK(NULL,		"secu_mgr",	&secu_mgr_clk),
@@ -420,7 +528,11 @@ static struct clk_lookup da830_clks[] = {
 	CLK("davinci_mdio.0",   "fck",          &emac_clk),
 	CLK(NULL,		"gpio",		&gpio_clk),
 	CLK("i2c_davinci.2",	NULL,		&i2c1_clk),
+	CLK(NULL,		"usb_ref_clk",	&usb_ref_clk),
 	CLK(NULL,		"usb11",	&usb11_clk),
+	CLK(NULL,		"usb20",	&usb20_clk),
+	CLK(NULL,		"usb20_phy",	&usb20_phy_clk),
+	CLK(NULL,		"usb11_phy",	&usb11_phy_clk),
 	CLK(NULL,		"emif3",	&emif3_clk),
 	CLK(NULL,		"arm",		&arm_clk),
 	CLK(NULL,		"rmii",		&rmii_clk),
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 97d8779..649d3fa 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -19,6 +19,7 @@
 #include <linux/cpufreq.h>
 #include <linux/regulator/consumer.h>
 #include <linux/platform_data/gpio-davinci.h>
+#include <linux/platform_data/usb-davinci.h>
 
 #include <asm/mach/map.h>
 
@@ -333,6 +334,12 @@ static struct clk aemif_clk = {
 	.flags		= ALWAYS_ENABLED,
 };
 
+static struct clk usb_ref_clk = {
+	.name		= "usb_ref_clk",
+	.rate		= 48000000,
+	.set_rate	= davinci_simple_set_rate,
+};
+
 static struct clk usb11_clk = {
 	.name		= "usb11",
 	.parent		= &pll0_sysclk4,
@@ -347,6 +354,109 @@ static struct clk usb20_clk = {
 	.gpsc		= 1,
 };
 
+static void usb20_phy_clk_enable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	/*
+	 * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
+	 * host may use the PLL clock without USB 2.0 OTG being used.
+	 */
+	val &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
+	val |= CFGCHIP2_PHY_PLLON;
+
+	/* Set the mux depending on the parent clock. */
+	if (clk->parent == &pll0_aux_clk)
+		val |= CFGCHIP2_USB2PHYCLKMUX;
+	else if (clk->parent == &usb_ref_clk)
+		val &= ~CFGCHIP2_USB2PHYCLKMUX;
+	else
+		pr_err("Bad parent on USB 2.0 PHY clock.\n");
+
+	/* reference frequency also comes from parent clock */
+	val &= ~CFGCHIP2_REFFREQ;
+	switch (clk_get_rate(clk->parent)) {
+	case 12000000:
+		val |= CFGCHIP2_REFFREQ_12MHZ;
+		break;
+	case 13000000:
+		val |= CFGCHIP2_REFFREQ_13MHZ;
+		break;
+	case 19200000:
+		val |= CFGCHIP2_REFFREQ_19_2MHZ;
+		break;
+	case 20000000:
+		val |= CFGCHIP2_REFFREQ_20MHZ;
+		break;
+	case 24000000:
+		val |= CFGCHIP2_REFFREQ_24MHZ;
+		break;
+	case 26000000:
+		val |= CFGCHIP2_REFFREQ_26MHZ;
+		break;
+	case 38400000:
+		val |= CFGCHIP2_REFFREQ_38_4MHZ;
+		break;
+	case 40000000:
+		val |= CFGCHIP2_REFFREQ_40MHZ;
+		break;
+	case 48000000:
+		val |= CFGCHIP2_REFFREQ_48MHZ;
+		break;
+	default:
+		pr_err("Bad parent clock rate on USB 2.0 PHY clock.\n");
+		break;
+	}
+
+	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	pr_info("Waiting for USB 2.0 PHY clock good...\n");
+	while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
+						& CFGCHIP2_PHYCLKGD))
+		cpu_relax();
+}
+
+static void usb20_phy_clk_disable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+	val |= CFGCHIP2_PHYPWRDN;
+	__raw_writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+}
+
+static struct clk usb20_phy_clk = {
+	.name		= "usb20_phy",
+	.parent		= &pll0_aux_clk, /* or &usb_ref_clk */
+	.clk_enable	= usb20_phy_clk_enable,
+	.clk_disable	= usb20_phy_clk_disable,
+};
+
+static void usb11_phy_clk_enable(struct clk *clk)
+{
+	u32 val;
+
+	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+
+	/* Set the USB 1.1 PHY clock mux based on the parent clock. */
+	if (clk->parent == &usb20_phy_clk)
+		val &= ~CFGCHIP2_USB1PHYCLKMUX;
+	else if (clk->parent == &usb_ref_clk)
+		val &= ~CFGCHIP2_USB1PHYCLKMUX;
+	else
+		pr_err("Bad parent on USB 1.1 PHY clock.\n");
+
+	writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
+}
+
+static struct clk usb11_phy_clk = {
+	.name		= "usb11_phy",
+	.parent		= &usb20_phy_clk, /* or &usb_ref_clk */
+	.clk_enable	= usb11_phy_clk_enable,
+};
+
 static struct clk spi0_clk = {
 	.name		= "spi0",
 	.parent		= &pll0_sysclk2,
@@ -468,8 +578,11 @@ static struct clk_lookup da850_clks[] = {
 	CLK("da830-mmc.0",	NULL,		&mmcsd0_clk),
 	CLK("da830-mmc.1",	NULL,		&mmcsd1_clk),
 	CLK(NULL,		"aemif",	&aemif_clk),
+	CLK(NULL,		"usb_ref_clk",	&usb_ref_clk),
 	CLK(NULL,		"usb11",	&usb11_clk),
 	CLK(NULL,		"usb20",	&usb20_clk),
+	CLK(NULL,		"usb20_phy",	&usb20_phy_clk),
+	CLK(NULL,		"usb11_phy",	&usb11_phy_clk),
 	CLK("spi_davinci.0",	NULL,		&spi0_clk),
 	CLK("spi_davinci.1",	NULL,		&spi1_clk),
 	CLK("vpif",		NULL,		&vpif_clk),
-- 
1.9.1

[PATCH 2/5] ARM: davinci: da8xx: add usb phy clocks

[Sergei Shtylyov]

Hello.

On 3/16/2016 1:37 AM, David Lechner wrote:

> Up to this point, the USB phy clock configuration was handled manually in
> the board files and in the usb drivers. This adds proper clocks so that
> the usb drivers can use clk_get and clk_enable and not have to worry about
> the details. Also, the related code is removed from the board files.
>
> Signed-off-by: David Lechner <david@lechnology.com>
[...]
> diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
> index 7187e7f..213fb17e 100644
> --- a/arch/arm/mach-davinci/da830.c
> +++ b/arch/arm/mach-davinci/da830.c
[...]
> @@ -346,6 +340,12 @@ static struct clk i2c1_clk = {
>   	.gpsc		= 1,
>   };
>
> +static struct clk usb_ref_clk = {
> +	.name		= "usb_ref_clk",
> +	.rate		= 48000000,
> +	.set_rate	= davinci_simple_set_rate,
> +};
> +
>   static struct clk usb11_clk = {
>   	.name		= "usb11",
>   	.parent		= &pll0_sysclk4,
> @@ -353,6 +353,115 @@ static struct clk usb11_clk = {
>   	.gpsc		= 1,
>   };
>
> +static struct clk usb20_clk = {
> +	.name		= "usb20",
> +	.parent		= &pll0_sysclk2,
> +	.lpsc		= DA8XX_LPSC1_USB20,
> +	.gpsc		= 1,
> +};

    Why move it?

> +
> +static void usb20_phy_clk_enable(struct clk *clk)
> +{
> +	u32 val;
> +
> +	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> +	/*
> +	 * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
> +	 * host may use the PLL clock without USB 2.0 OTG being used.
> +	 */
> +		val &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
> +		val |= CFGCHIP2_PHY_PLLON;

    Wrong indentation.

> +
> +	/* Set the mux depending on the parent clock. */
> +	if (clk->parent == &pll0_aux_clk)
> +		val |= CFGCHIP2_USB2PHYCLKMUX;
> +	else if (clk->parent == &usb_ref_clk)
> +		val &= ~CFGCHIP2_USB2PHYCLKMUX;

    Don't we have clk_set_parent()for that?

> +	else
> +		pr_err("Bad parent on USB 2.0 PHY clock.\n");
> +
[...]
> +		writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));

    Wrong indentation again.

> +
> +	pr_info("Waiting for USB 2.0 PHY clock good...\n");
> +		while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
> +							 & CFGCHIP2_PHYCLKGD))
> +			cpu_relax();

    And again.

> +	}
> +
> +static void usb20_phy_clk_disable(struct clk *clk)
> +{
> +	u32 val;
> +
> +	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +	val |= CFGCHIP2_PHYPWRDN;

    I'm not sure that powering down the PHY can be regarded as disabling the 
clock...

> +	__raw_writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));

    Please don't mix readl() and __raw_writel().

[...]
> +static void usb11_phy_clk_enable(struct clk *clk)
> +{
> +	u32 val;
> +
> +	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> +	/* Set the USB 1.1 PHY clock mux based on the parent clock. */
> +	if (clk->parent == &usb20_phy_clk)
> +		val &= ~CFGCHIP2_USB1PHYCLKMUX;
> +	else if (clk->parent == &usb_ref_clk)
> +		val &= ~CFGCHIP2_USB1PHYCLKMUX;

     Huh? When do you set this bit?

[...]
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 97d8779..649d3fa 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -19,6 +19,7 @@
>   #include <linux/cpufreq.h>
>   #include <linux/regulator/consumer.h>
>   #include <linux/platform_data/gpio-davinci.h>
> +#include <linux/platform_data/usb-davinci.h>
>
>   #include <asm/mach/map.h>
>
> @@ -333,6 +334,12 @@ static struct clk aemif_clk = {
>   	.flags		= ALWAYS_ENABLED,
>   };
>
> +static struct clk usb_ref_clk = {
> +	.name		= "usb_ref_clk",
> +	.rate		= 48000000,
> +	.set_rate	= davinci_simple_set_rate,
> +};
> +
>   static struct clk usb11_clk = {
>   	.name		= "usb11",
>   	.parent		= &pll0_sysclk4,
> @@ -347,6 +354,109 @@ static struct clk usb20_clk = {
[...]
> +static void usb11_phy_clk_enable(struct clk *clk)
> +{
> +	u32 val;
> +
> +	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> +	/* Set the USB 1.1 PHY clock mux based on the parent clock. */
> +	if (clk->parent == &usb20_phy_clk)
> +		val &= ~CFGCHIP2_USB1PHYCLKMUX;
> +	else if (clk->parent == &usb_ref_clk)
> +		val &= ~CFGCHIP2_USB1PHYCLKMUX;

    When do you set this bit?

[...]

MBR, Sergei

[PATCH 2/5] ARM: davinci: da8xx: add usb phy clocks

[David Lechner]

On 03/16/2016 07:27 AM, Sergei Shtylyov wrote:
>>
>> +static struct clk usb20_clk = {
>> +    .name        = "usb20",
>> +    .parent        = &pll0_sysclk2,
>> +    .lpsc        = DA8XX_LPSC1_USB20,
>> +    .gpsc        = 1,
>> +};
>
>     Why move it?

For organization, to keep all of the USB clocks together. I can leave it 
alone if that is preferred.

>> +
>> +    /* Set the mux depending on the parent clock. */
>> +    if (clk->parent == &pll0_aux_clk)
>> +        val |= CFGCHIP2_USB2PHYCLKMUX;
>> +    else if (clk->parent == &usb_ref_clk)
>> +        val &= ~CFGCHIP2_USB2PHYCLKMUX;
>
>     Don't we have clk_set_parent()for that?

Yes. clk_set_parent() is already called in a loop for all clocks 
elsewhere, so not needed here.

---

Thank you for the careful review. I will address the other problems you 
pointed out.

[PATCH 2/5] ARM: davinci: da8xx: add usb phy clocks

[Sergei Shtylyov]

On 03/16/2016 08:58 PM, David Lechner wrote:

>>> +static struct clk usb20_clk = {
>>> +    .name        = "usb20",
>>> +    .parent        = &pll0_sysclk2,
>>> +    .lpsc        = DA8XX_LPSC1_USB20,
>>> +    .gpsc        = 1,
>>> +};
>>
>>     Why move it?
>
> For organization, to keep all of the USB clocks together. I can leave it alone
> if that is preferred.

    I'd prefer to minimize the noise...

>>> +
>>> +    /* Set the mux depending on the parent clock. */
>>> +    if (clk->parent == &pll0_aux_clk)
>>> +        val |= CFGCHIP2_USB2PHYCLKMUX;
>>> +    else if (clk->parent == &usb_ref_clk)
>>> +        val &= ~CFGCHIP2_USB2PHYCLKMUX;
>>
>>     Don't we have clk_set_parent()for that?
>
> Yes. clk_set_parent() is already called in a loop for all clocks elsewhere, so
> not needed here.

    No, I mean why is not this implemented as a part of clk_set_parent()?

MBR, Sergei

[PATCH 2/5] ARM: davinci: da8xx: add usb phy clocks

[David Lechner]

On 03/16/2016 01:04 PM, Sergei Shtylyov wrote:
>
>     No, I mean why is not this implemented as a part of clk_set_parent()?
>

There is not currently any framework for mux clocks in the davinci 
clocks. I am hoping to eventually get the davinci clocks moved to the 
common clock framework, so this was just a hack to get things working 
with what was already existing. I did not want to spend the time fixing 
it twice.

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[David Lechner]

The usb ohci driver has been change to not include mach/*, so we need
to pass the cfgchip2 address to the driver so that it can turn the usb
phy on and off.

Signed-off-by: David Lechner <david@lechnology.com>
---
 arch/arm/mach-davinci/usb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
index b0a6b52..9607b0c 100644
--- a/arch/arm/mach-davinci/usb.c
+++ b/arch/arm/mach-davinci/usb.c
@@ -148,6 +148,11 @@ static struct resource da8xx_usb11_resources[] = {
 		.flags	= IORESOURCE_MEM,
 	},
 	[1] = {
+		.start	= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG,
+		.end	= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG + 4 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	[2] = {
 		.start	= IRQ_DA8XX_IRQN,
 		.end	= IRQ_DA8XX_IRQN,
 		.flags	= IORESOURCE_IRQ,
-- 
1.9.1

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[Sergei Shtylyov]

Hello.

On 03/16/2016 01:37 AM, David Lechner wrote:

> The usb ohci driver has been change to not include mach/*, so we need
> to pass the cfgchip2 address to the driver so that it can turn the usb
> phy on and off.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>   arch/arm/mach-davinci/usb.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
> index b0a6b52..9607b0c 100644
> --- a/arch/arm/mach-davinci/usb.c
> +++ b/arch/arm/mach-davinci/usb.c
> @@ -148,6 +148,11 @@ static struct resource da8xx_usb11_resources[] = {
>   		.flags	= IORESOURCE_MEM,
>   	},
>   	[1] = {
> +		.start	= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG,
> +		.end	= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG + 4 - 1,
> +		.flags	= IORESOURCE_MEM,
> +	},

    No, this register is shared b/w MUSB and OHCI. The proper thing to do is 
to write the PHY driver and let it control this shared register.

[...]

MBR, Sergei

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[David Lechner]

On 03/15/2016 05:45 PM, Sergei Shtylyov wrote:
>
>     No, this register is shared b/w MUSB and OHCI. The proper thing to
> do is to write the PHY driver and let it control this shared register.
>

OK. I've started working on this. I am looking at using struct usb_phy, 
however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED, 
USB_PHY_TYPE_USB2, and USB_PHY_TYPE_USB3. Would it be acceptable to use 
USB_PHY_TYPE_UNDEFINED for the ohci since it is USB 1.1? Or perhaps I 
should use the more generic struct phy for that one?

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[David Lechner]

On 03/15/2016 10:46 PM, David Lechner wrote:
> On 03/15/2016 05:45 PM, Sergei Shtylyov wrote:
>>
>>     No, this register is shared b/w MUSB and OHCI. The proper thing to
>> do is to write the PHY driver and let it control this shared register.
>>
>
> OK. I've started working on this. I am looking at using struct usb_phy,
> however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED,
> USB_PHY_TYPE_USB2, and USB_PHY_TYPE_USB3. Would it be acceptable to use
> USB_PHY_TYPE_UNDEFINED for the ohci since it is USB 1.1? Or perhaps I
> should use the more generic struct phy for that one?
>

Also, I am not finding any existing data structure to pass the musb 
set_mode function to the phy in either usb_phy or usb_otg. Setting the 
mode (host/peripheral/otg) is done in the same PHY register, so it seems 
like it should be implemented in the new phy driver as well.

I guess I could use a generic phy instead and use phy_set_drvdata() to 
share data between the phy driver and the musb driver. Does this sound 
like a reasonable thing to do?

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[Sergei Shtylyov]

On 03/16/2016 07:57 AM, David Lechner wrote:

>>>     No, this register is shared b/w MUSB and OHCI. The proper thing to
>>> do is to write the PHY driver and let it control this shared register.
>>>
>> OK. I've started working on this. I am looking at using struct usb_phy,
>> however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED,
>> USB_PHY_TYPE_USB2, and USB_PHY_TYPE_USB3. Would it be acceptable to use
>> USB_PHY_TYPE_UNDEFINED for the ohci since it is USB 1.1? Or perhaps I
>> should use the more generic struct phy for that one?
>>
> Also, I am not finding any existing data structure to pass the musb set_mode
> function to the phy in either usb_phy or usb_otg. Setting the mode
> (host/peripheral/otg) is done in the same PHY register, so it seems like it
> should be implemented in the new phy driver as well.

    Perhaps we'd have to sacrifice that functionality...

> I guess I could use a generic phy instead and use phy_set_drvdata() to share
> data between the phy driver and the musb driver. Does this sound like a
> reasonable thing to do?

    Not sure what you mean, could you elaborate?

MBR, Sergei

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[David Lechner]

On 03/16/2016 12:38 PM, Sergei Shtylyov wrote:
> On 03/16/2016 07:57 AM, David Lechner wrote:
>
>> Also, I am not finding any existing data structure to pass the musb
>> set_mode
>> function to the phy in either usb_phy or usb_otg. Setting the mode
>> (host/peripheral/otg) is done in the same PHY register, so it seems
>> like it
>> should be implemented in the new phy driver as well.
>
>     Perhaps we'd have to sacrifice that functionality...

The device I am working on (LEGO MINDSTORMS EV3) has the port wired as 
peripheral only, so I don't think leaving this out is an option. Leaving 
it in OTG mode doesn't work because the required electrical connections 
are just not there.

>> I guess I could use a generic phy instead and use phy_set_drvdata() to
>> share
>> data between the phy driver and the musb driver. Does this sound like a
>> reasonable thing to do?
>
>     Not sure what you mean, could you elaborate?

I found another driver that essentially does what I was trying to 
explain here. See the sun4i_usb_phy_set_squelch_detect function in 
drivers/phy/phy-sun4i-usb.c:394[1] as an example. It is called at 
drivers/usb/musb/sunxi.c:160[2] and :167.

I would move the da8xx_musb_set_mode function from 
drivers/usb/musb/da8xx.c to the new drivers/phy/phy-da8xx-usb.c and call 
it in a similar manner to the sunix example I gave.


---

[1]: drivers/phy/phy-sun4i-usb.c

void sun4i_usb_phy_set_squelch_detect(struct phy *_phy, bool enabled)
{
struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);

sun4i_usb_phy_write(phy, PHY_SQUELCH_DETECT, enabled ? 0 : 2, 2);
}
EXPORT_SYMBOL_GPL(sun4i_usb_phy_set_squelch_detect);



[2]: drivers/usb/musb/sunxi.c

static void sunxi_musb_pre_root_reset_end(struct musb *musb)
{
	struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);

	sun4i_usb_phy_set_squelch_detect(glue->phy, false);
}

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[Sergei Shtylyov]

On 03/16/2016 09:14 PM, David Lechner wrote:

>>> Also, I am not finding any existing data structure to pass the musb
>>> set_mode
>>> function to the phy in either usb_phy or usb_otg. Setting the mode
>>> (host/peripheral/otg) is done in the same PHY register, so it seems
>>> like it
>>> should be implemented in the new phy driver as well.
>>
>>     Perhaps we'd have to sacrifice that functionality...
>
> The device I am working on (LEGO MINDSTORMS EV3) has the port wired as
> peripheral only, so I don't think leaving this out is an option. Leaving it in
> OTG mode doesn't work because the required electrical connections are just not
> there.

    The set_mode() method doesn't have anything to do with the predefined 
roles. What CFGCHIP2 setting do is to override the ID input (and also the VBUS 
level comparator). This is not required for the normal functioning of either 
host or peripheral AFAIR.

[...]

MBR, Sergei

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[Sergei Shtylyov]

On 03/16/2016 09:22 PM, Sergei Shtylyov wrote:

>>>> Also, I am not finding any existing data structure to pass the musb
>>>> set_mode
>>>> function to the phy in either usb_phy or usb_otg. Setting the mode
>>>> (host/peripheral/otg) is done in the same PHY register, so it seems
>>>> like it
>>>> should be implemented in the new phy driver as well.
>>>
>>>     Perhaps we'd have to sacrifice that functionality...
>>
>> The device I am working on (LEGO MINDSTORMS EV3) has the port wired as
>> peripheral only, so I don't think leaving this out is an option. Leaving it in
>> OTG mode doesn't work because the required electrical connections are just not
>> there.
>
>     The set_mode() method doesn't have anything to do with the predefined
> roles. What CFGCHIP2 setting do is to override the ID input (and also the VBUS
> level comparator). This is not required for the normal functioning of either
> host or peripheral AFAIR.

   Or at least it wasn't when I last looked. Now it does... :-/

> [...]

MBR, Sergei

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[David Lechner]

On 03/16/2016 01:22 PM, Sergei Shtylyov wrote:
>
>     The set_mode() method doesn't have anything to do with the
> predefined roles. What CFGCHIP2 setting do is to override the ID input
> (and also the VBUS level comparator). This is not required for the
> normal functioning of either host or peripheral AFAIR.
>

OK, so it sounds like I should just remove set_mode from the musb driver 
completely and make this configurable in the phy driver via platform 
data or device tree.

[PATCH 3/5] ARM: davinci: da8xx: add cfgchip2 to resources

[Sergei Shtylyov]

Hello.

On 3/16/2016 6:46 AM, David Lechner wrote:

>>     No, this register is shared b/w MUSB and OHCI. The proper thing to
>> do is to write the PHY driver and let it control this shared register.

> OK. I've started working on this. I am looking at using struct usb_phy,
> however, enum usb_phy_type only has USB_PHY_TYPE_UNDEFINED, USB_PHY_TYPE_USB2,
> and USB_PHY_TYPE_USB3. Would it be acceptable to use USB_PHY_TYPE_UNDEFINED
> for the ohci since it is USB 1.1? Or perhaps I should use the more generic
> struct phy for that one?

    No new USB PHY drivers accepted, look at drivers/phy/ instead please.

MBR, Sergei

[PATCH 4/5] usb: ohci-da8xx: Remove clock code that references mach

[David Lechner]

Including mach/* is frowned upon in device drivers, so get rid of it.

This replaces usb20_clk with usb11_phy_clk that represents the 48MHz usb
phy clock. The interaction with the usb20 (musb) subsystem does no belong
here and has been implemented in da830.c/da850.c with the rest of the
da8xx clock code.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/usb/host/ohci-da8xx.c | 81 ++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index e5c33bc..37fa3fb 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -16,57 +16,43 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 
-#include <mach/da8xx.h>
 #include <linux/platform_data/usb-davinci.h>
 
 #ifndef CONFIG_ARCH_DAVINCI_DA8XX
 #error "This file is DA8xx bus glue.  Define CONFIG_ARCH_DAVINCI_DA8XX."
 #endif
 
-#define CFGCHIP2	DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG)
+#define USB1SUSPENDM	(1 << 7)
 
 static struct clk *usb11_clk;
-static struct clk *usb20_clk;
+static struct clk *usb11_phy_clk;
+static __iomem u32 *cfgchip2;
 
 /* Over-current indicator change bitmask */
 static volatile u16 ocic_mask;
 
-static void ohci_da8xx_clock(int on)
+static void ohci_da8xx_enable(void)
 {
-	u32 cfgchip2;
-
-	cfgchip2 = __raw_readl(CFGCHIP2);
-	if (on) {
-		clk_enable(usb11_clk);
-
-		/*
-		 * If USB 1.1 reference clock is sourced from USB 2.0 PHY, we
-		 * need to enable the USB 2.0 module clocking, start its PHY,
-		 * and not allow it to stop the clock during USB 2.0 suspend.
-		 */
-		if (!(cfgchip2 & CFGCHIP2_USB1PHYCLKMUX)) {
-			clk_enable(usb20_clk);
-
-			cfgchip2 &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
-			cfgchip2 |= CFGCHIP2_PHY_PLLON;
-			__raw_writel(cfgchip2, CFGCHIP2);
-
-			pr_info("Waiting for USB PHY clock good...\n");
-			while (!(__raw_readl(CFGCHIP2) & CFGCHIP2_PHYCLKGD))
-				cpu_relax();
-		}
+	u32 val;
 
-		/* Enable USB 1.1 PHY */
-		cfgchip2 |= CFGCHIP2_USB1SUSPENDM;
-	} else {
-		clk_disable(usb11_clk);
-		if (!(cfgchip2 & CFGCHIP2_USB1PHYCLKMUX))
-			clk_disable(usb20_clk);
+	clk_prepare_enable(usb11_clk);
+	clk_prepare_enable(usb11_phy_clk);
 
-		/* Disable USB 1.1 PHY */
-		cfgchip2 &= ~CFGCHIP2_USB1SUSPENDM;
-	}
-	__raw_writel(cfgchip2, CFGCHIP2);
+	val = __raw_readl(cfgchip2);
+	val |= USB1SUSPENDM;
+	__raw_writel(val, cfgchip2);
+}
+
+static void ohci_da8xx_disable(void)
+{
+	u32 val;
+
+	val = __raw_readl(cfgchip2);
+	val &= ~USB1SUSPENDM;
+	__raw_writel(val, cfgchip2);
+
+	clk_disable_unprepare(usb11_phy_clk);
+	clk_disable_unprepare(usb11_clk);
 }
 
 /*
@@ -92,7 +78,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
 
 	dev_dbg(dev, "starting USB controller\n");
 
-	ohci_da8xx_clock(1);
+	ohci_da8xx_enable();
 
 	/*
 	 * DA8xx only have 1 port connected to the pins but the HC root hub
@@ -129,7 +115,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
 static void ohci_da8xx_stop(struct usb_hcd *hcd)
 {
 	ohci_stop(hcd);
-	ohci_da8xx_clock(0);
+	ohci_da8xx_disable();
 }
 
 static int ohci_da8xx_start(struct usb_hcd *hcd)
@@ -304,9 +290,9 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
 	if (IS_ERR(usb11_clk))
 		return PTR_ERR(usb11_clk);
 
-	usb20_clk = devm_clk_get(&pdev->dev, "usb20");
-	if (IS_ERR(usb20_clk))
-		return PTR_ERR(usb20_clk);
+	usb11_phy_clk = devm_clk_get(&pdev->dev, "usb11_phy");
+	if (IS_ERR(usb11_phy_clk))
+		return PTR_ERR(usb11_phy_clk);
 
 	hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
 	if (!hcd)
@@ -316,11 +302,20 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
 	hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(hcd->regs)) {
 		error = PTR_ERR(hcd->regs);
+		dev_err(&pdev->dev, "failed to map ohci.\n");
 		goto err;
 	}
 	hcd->rsrc_start = mem->start;
 	hcd->rsrc_len = resource_size(mem);
 
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	cfgchip2 = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(cfgchip2)) {
+		error = PTR_ERR(cfgchip2);
+		dev_err(&pdev->dev, "failed to map cfgchip2.\n");
+		goto err;
+	}
+
 	ohci_hcd_init(hcd_to_ohci(hcd));
 
 	irq = platform_get_irq(pdev, 0);
@@ -397,7 +392,7 @@ static int ohci_da8xx_suspend(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	ohci_da8xx_clock(0);
+	ohci_da8xx_disable();
 	hcd->state = HC_STATE_SUSPENDED;
 
 	return ret;
@@ -412,7 +407,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
 		msleep(5);
 	ohci->next_statechange = jiffies;
 
-	ohci_da8xx_clock(1);
+	ohci_da8xx_enable();
 	dev->dev.power.power_state = PMSG_ON;
 	usb_hcd_resume_root_hub(hcd);
 	return 0;
-- 
1.9.1

[PATCH 5/5] usb: musb-da8xx: remove board-specific clock handling

[David Lechner]

This driver should not have to worry about how the clocks are configured
on a system. Added a usb20_phy clock to handle the USB 2.0 PLL clock
externally.

Also changed to using devm_ to simplify code a bit.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/usb/musb/da8xx.c | 93 +++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 57 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b03d3b8..4e4c872 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -87,50 +87,29 @@ struct da8xx_glue {
 	struct platform_device	*musb;
 	struct platform_device	*phy;
 	struct clk		*clk;
+	struct clk		*phy_clk;
 };
 
-/*
- * REVISIT (PM): we should be able to keep the PHY in low power mode most
- * of the time (24 MHz oscillator and PLL off, etc.) by setting POWER.D0
- * and, when in host mode, autosuspending idle root ports... PHY_PLLON
- * (overriding SUSPENDM?) then likely needs to stay off.
- */
-
 static inline void phy_on(void)
 {
 	u32 cfgchip2 = __raw_readl(CFGCHIP2);
 
 	/*
-	 * Start the on-chip PHY and its PLL.
+	 * Most of the configuration is already done by the usb20_phy clock.
+	 * We just need to enable the OTG PHY here.
 	 */
-	cfgchip2 &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN);
-	cfgchip2 |= CFGCHIP2_PHY_PLLON;
+	cfgchip2 &= ~CFGCHIP2_OTGPWRDN;
 	__raw_writel(cfgchip2, CFGCHIP2);
-
-	pr_info("Waiting for USB PHY clock good...\n");
-	while (!(__raw_readl(CFGCHIP2) & CFGCHIP2_PHYCLKGD))
-		cpu_relax();
 }
 
 static inline void phy_off(void)
 {
 	u32 cfgchip2 = __raw_readl(CFGCHIP2);
-
-	/*
-	 * Ensure that USB 1.1 reference clock is not being sourced from
-	 * USB 2.0 PHY.  Otherwise do not power down the PHY.
-	 */
-	if (!(cfgchip2 & CFGCHIP2_USB1PHYCLKMUX) &&
-	     (cfgchip2 & CFGCHIP2_USB1SUSPENDM)) {
-		pr_warning("USB 1.1 clocked from USB 2.0 PHY -- "
-			   "can't power it down\n");
-		return;
-	}
-
 	/*
-	 * Power down the on-chip PHY.
+	 * Just turn off the OTG PHY. The usb20_phy clock takes care of
+	 * everything else.
 	 */
-	cfgchip2 |= CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN;
+	cfgchip2 |= CFGCHIP2_OTGPWRDN;
 	__raw_writel(cfgchip2, CFGCHIP2);
 }
 
@@ -489,38 +468,45 @@ static int da8xx_probe(struct platform_device *pdev)
 	struct platform_device		*musb;
 	struct da8xx_glue		*glue;
 	struct platform_device_info	pinfo;
-	struct clk			*clk;
-
-	int				ret = -ENOMEM;
+	int				ret;
 
-	glue = kzalloc(sizeof(*glue), GFP_KERNEL);
+	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
 	if (!glue) {
 		dev_err(&pdev->dev, "failed to allocate glue context\n");
-		goto err0;
+		return -ENOMEM;
 	}
 
-	clk = clk_get(&pdev->dev, "usb20");
-	if (IS_ERR(clk)) {
+	glue->clk = devm_clk_get(&pdev->dev, "usb20");
+	if (IS_ERR(glue->clk)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
-		ret = PTR_ERR(clk);
-		goto err3;
+		return PTR_ERR(glue->clk);
 	}
 
-	ret = clk_enable(clk);
+	ret = clk_prepare_enable(glue->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to enable clock\n");
-		goto err4;
+		return ret;
 	}
 
-	glue->dev			= &pdev->dev;
-	glue->clk			= clk;
+	glue->phy_clk = devm_clk_get(&pdev->dev, "usb20_phy");
+	if (IS_ERR(glue->phy_clk)) {
+		dev_err(&pdev->dev, "failed to get phy clock\n");
+		return PTR_ERR(glue->phy_clk);
+	}
+
+	ret = clk_prepare_enable(glue->phy_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable phy clock\n");
+		goto err0;
+	}
 
+	glue->dev			= &pdev->dev;
 	pdata->platform_ops		= &da8xx_ops;
 
 	glue->phy = usb_phy_generic_register();
 	if (IS_ERR(glue->phy)) {
 		ret = PTR_ERR(glue->phy);
-		goto err5;
+		goto err1;
 	}
 	platform_set_drvdata(pdev, glue);
 
@@ -548,24 +534,18 @@ static int da8xx_probe(struct platform_device *pdev)
 	if (IS_ERR(musb)) {
 		ret = PTR_ERR(musb);
 		dev_err(&pdev->dev, "failed to register musb device: %d\n", ret);
-		goto err6;
+		goto err2;
 	}
 
 	return 0;
 
-err6:
+err2:
 	usb_phy_generic_unregister(glue->phy);
-
-err5:
-	clk_disable(clk);
-
-err4:
-	clk_put(clk);
-
-err3:
-	kfree(glue);
-
+err1:
+	clk_disable_unprepare(glue->phy_clk);
 err0:
+	clk_disable_unprepare(glue->clk);
+
 	return ret;
 }
 
@@ -575,9 +555,8 @@ static int da8xx_remove(struct platform_device *pdev)
 
 	platform_device_unregister(glue->musb);
 	usb_phy_generic_unregister(glue->phy);
-	clk_disable(glue->clk);
-	clk_put(glue->clk);
-	kfree(glue);
+	clk_disable_unprepare(glue->phy_clk);
+	clk_disable_unprepare(glue->clk);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 5/5] usb: musb-da8xx: remove board-specific clock handling

[Sergei Shtylyov]

Hello.

On 3/16/2016 1:37 AM, David Lechner wrote:

> This driver should not have to worry about how the clocks are configured
> on a system. Added a usb20_phy clock to handle the USB 2.0 PLL clock
> externally.
>
> Also changed to using devm_ to simplify code a bit.

    This should preferably be split to another patch. Doing one thing per 
patch is a rule of thumb.

> Signed-off-by: David Lechner <david@lechnology.com>
[...]

MBR, Sergei

[PATCH 4/5] usb: ohci-da8xx: Remove clock code that references mach

[David Lechner]

On 03/16/2016 09:50 AM, Alan Stern wrote:

> Is it theoretically impossible for a da8xx platform to have more than
> one OHCI controller?
>
> If it isn't, you better not use static variables to hold per-device
> data.

Yes, it is theoretically impossible, but probably better to not use 
static variables anyway.