[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Sylvain Rochet]

This patch add suspend/resume support for Atmel EHCI, mostly
about disabling and unpreparing clocks so USB PLL is stopped
before entering sleep state.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 56a8850..790de92 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -19,6 +19,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/atmel.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
 
@@ -174,6 +175,36 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = ehci_suspend(hcd, false);
+	if (ret)
+		return ret;
+
+	if (at91_suspend_entering_slow_clock())
+		atmel_stop_clock();
+
+	return 0;
+}
+
+static int ehci_atmel_drv_resume(struct platform_device *pdev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+
+	if (!clocked)
+		atmel_start_clock();
+
+	return ehci_resume(hcd, false);
+}
+#else
+#define ehci_atmel_drv_suspend NULL
+#define ehci_atmel_drv_resume  NULL
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id atmel_ehci_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9g45-ehci" },
@@ -187,6 +218,8 @@ static struct platform_driver ehci_atmel_driver = {
 	.probe		= ehci_atmel_drv_probe,
 	.remove		= ehci_atmel_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
+	.suspend	= ehci_atmel_drv_suspend,
+	.resume		= ehci_atmel_drv_resume,
 	.driver		= {
 		.name	= "atmel-ehci",
 		.of_match_table	= of_match_ptr(atmel_ehci_dt_ids),
-- 
2.1.4

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Alexandre Belloni]

Hi,

You should probably put the susbsytem maintainers in copy too. As
reported by get_maintainer.pl:
Alan Stern <stern@rowland.harvard.edu> (maintainer:USB EHCI DRIVER)
Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:USB SUBSYSTEM)

They will be the one taking the patch.

And when dealing with PM on AT91, please copy
Wenyou Yang <wenyou.yang@atmel.com>


On 16/01/2015 at 20:41:14 +0100, Sylvain Rochet wrote :
> This patch add suspend/resume support for Atmel EHCI, mostly
> about disabling and unpreparing clocks so USB PLL is stopped
> before entering sleep state.
> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>

Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> +
> +	if (at91_suspend_entering_slow_clock())
> +		atmel_stop_clock();
> +

We should definitely find a way to get rid of
at91_suspend_entering_slow_clock() at some point in time.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Boris Brezillon]

Hi Sylvain,

On Sat, 17 Jan 2015 02:34:42 +0100
Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> Hi,
> 
> You should probably put the susbsytem maintainers in copy too. As
> reported by get_maintainer.pl:
> Alan Stern <stern@rowland.harvard.edu> (maintainer:USB EHCI DRIVER)
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:USB SUBSYSTEM)
> 
> They will be the one taking the patch.
> 
> And when dealing with PM on AT91, please copy
> Wenyou Yang <wenyou.yang@atmel.com>
> 
> 
> On 16/01/2015 at 20:41:14 +0100, Sylvain Rochet wrote :
> > This patch add suspend/resume support for Atmel EHCI, mostly
> > about disabling and unpreparing clocks so USB PLL is stopped
> > before entering sleep state.
> > 
> > Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> 
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> 
> > +
> > +	if (at91_suspend_entering_slow_clock())
> > +		atmel_stop_clock();
> > +
> 
> We should definitely find a way to get rid of
> at91_suspend_entering_slow_clock() at some point in time.
> 
> 

Can't we just disable clocks without testing for target_state ==
PM_SUSPEND_MEM (which is exactly what at91_suspend_entering_slow_clock
does [1]) when entering suspend ?
I mean, IMHO other kind of suspend should still benefit from the power
save induced by this PLL deactivation.

Is there such a big penalty when resuming the device if the PLL and
peripheral clocks are disabled ?

[1]http://lxr.free-electrons.com/source/arch/arm/mach-at91/pm.c#L116


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Sylvain Rochet]

Hi Boris,


On Sat, Jan 17, 2015 at 10:36:09AM +0100, Boris Brezillon wrote:
> On Sat, 17 Jan 2015 02:34:42 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> > 
> > We should definitely find a way to get rid of
> > at91_suspend_entering_slow_clock() at some point in time.
> 
> Can't we just disable clocks without testing for target_state ==
> PM_SUSPEND_MEM (which is exactly what at91_suspend_entering_slow_clock
> does [1]) when entering suspend ?
> I mean, IMHO other kind of suspend should still benefit from the power
> save induced by this PLL deactivation.

I agree, but it depends on what we mean with standby vs mem, there 
should be a difference between the two sleep mode.

This behavior follows what the Atmel OHCI driver is currently doing.


> Is there such a big penalty when resuming the device if the PLL and
> peripheral clocks are disabled ?

There is a penalty, starting up a PLL takes about 500 ?s, however I 
can't decide if this is a small or a big penalty.


Sylvain

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Boris Brezillon]

On Sat, 17 Jan 2015 11:43:00 +0100
Sylvain Rochet <sylvain.rochet@finsecur.com> wrote:

> Hi Boris,
> 
> 
> On Sat, Jan 17, 2015 at 10:36:09AM +0100, Boris Brezillon wrote:
> > On Sat, 17 Jan 2015 02:34:42 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> > > 
> > > We should definitely find a way to get rid of
> > > at91_suspend_entering_slow_clock() at some point in time.
> > 
> > Can't we just disable clocks without testing for target_state ==
> > PM_SUSPEND_MEM (which is exactly what at91_suspend_entering_slow_clock
> > does [1]) when entering suspend ?
> > I mean, IMHO other kind of suspend should still benefit from the power
> > save induced by this PLL deactivation.
> 
> I agree, but it depends on what we mean with standby vs mem, there 
> should be a difference between the two sleep mode.

AFAIU PM_SUSPEND_MEM mode only implies putting the SDRAM in
self-refresh mode to avoid waking the processor up for periodic SDRAM
bank refresh.
IMO this should not impact other peripherals behavior (BTW I haven't
found any USB driver testing for this suspend state before disabling
their clks).

And what if we decide to implement runtime PM for this peripheral ?
Shouldn't we disable these clks (which are one of the main source of
power consumption of this IP) ?

> 
> This behavior follows what the Atmel OHCI driver is currently doing.

Yes, but it doesn't mean we should do the same ;-), especially since
we're trying to remove this at91_suspend_entering_slow_clock function.

> 
> 
> > Is there such a big penalty when resuming the device if the PLL and
> > peripheral clocks are disabled ?
> 
> There is a penalty, starting up a PLL takes about 500 ?s, however I 
> can't decide if this is a small or a big penalty.

That's indeed not negligible, but when one enters suspend, I guess it
does expect some latency to resume from the suspended state.


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCHv2 1/2] USB: host: ehci_atmel: Add suspend/resume support

[Sylvain Rochet]

This patch add suspend/resume support for Atmel EHCI, mostly
about disabling and unpreparing clocks so USB PLL is stopped
before entering sleep state.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 56a8850..2e0043b 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -37,6 +37,8 @@ static int clocked;
 
 static void atmel_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -48,6 +50,8 @@ static void atmel_start_clock(void)
 
 static void atmel_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
@@ -174,6 +178,32 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = ehci_suspend(hcd, false);
+	if (ret)
+		return ret;
+
+	atmel_stop_clock();
+	return 0;
+}
+
+static int ehci_atmel_drv_resume(struct platform_device *pdev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+
+	atmel_start_clock();
+	return ehci_resume(hcd, false);
+}
+#else
+#define ehci_atmel_drv_suspend NULL
+#define ehci_atmel_drv_resume  NULL
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id atmel_ehci_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9g45-ehci" },
@@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
 	.probe		= ehci_atmel_drv_probe,
 	.remove		= ehci_atmel_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
+	.suspend	= ehci_atmel_drv_suspend,
+	.resume		= ehci_atmel_drv_resume,
 	.driver		= {
 		.name	= "atmel-ehci",
 		.of_match_table	= of_match_ptr(atmel_ehci_dt_ids),
-- 
2.1.4

[PATCHv2 2/2] USB: host: ohci_at91: Stop/start USB PLL for all sleep modes

[Sylvain Rochet]

Disable/unprepare clocks without testing the sleep target_state, removed
the at91_suspend_entering_slow_clock() call (which is only a
target_state == PM_SUSPEND_MEM).

Other kind of suspend now benefit from the power save induced by this
PLL deactivation. The resume penalty is about 500 us, which is not
negligible but acceptable considering the amount of power we are saving.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/usb/host/ohci-at91.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index dc9e4e6..f0da734 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -49,6 +49,8 @@ extern int usb_disabled(void);
 
 static void at91_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -61,6 +63,8 @@ static void at91_start_clock(void)
 
 static void at91_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	clk_disable_unprepare(hclk);
@@ -615,16 +619,14 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 	 *
 	 * REVISIT: some boards will be able to turn VBUS off...
 	 */
-	if (at91_suspend_entering_slow_clock()) {
-		ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
-		ohci->hc_control &= OHCI_CTRL_RWC;
-		ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
-		ohci->rh_state = OHCI_RH_HALTED;
-
-		/* flush the writes */
-		(void) ohci_readl (ohci, &ohci->regs->control);
-		at91_stop_clock();
-	}
+	ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
+	ohci->hc_control &= OHCI_CTRL_RWC;
+	ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
+	ohci->rh_state = OHCI_RH_HALTED;
+
+	/* flush the writes */
+	(void) ohci_readl (ohci, &ohci->regs->control);
+	at91_stop_clock();
 
 	return ret;
 }
@@ -636,8 +638,7 @@ static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(hcd->irq);
 
-	if (!clocked)
-		at91_start_clock();
+	at91_start_clock();
 
 	ohci_resume(hcd, false);
 	return 0;
-- 
2.1.4

[PATCHv2 2/2] USB: host: ohci_at91: Stop/start USB PLL for all sleep modes

[Boris Brezillon]

On Sat, 17 Jan 2015 16:36:35 +0100
Sylvain Rochet <sylvain.rochet@finsecur.com> wrote:

> Disable/unprepare clocks without testing the sleep target_state, removed
> the at91_suspend_entering_slow_clock() call (which is only a
> target_state == PM_SUSPEND_MEM).
> 
> Other kind of suspend now benefit from the power save induced by this
> PLL deactivation. The resume penalty is about 500 us, which is not
> negligible but acceptable considering the amount of power we are saving.
> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
>  drivers/usb/host/ohci-at91.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index dc9e4e6..f0da734 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -49,6 +49,8 @@ extern int usb_disabled(void);
>  
>  static void at91_start_clock(void)
>  {
> +	if (clocked)
> +		return;
>  	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>  		clk_set_rate(uclk, 48000000);
>  		clk_prepare_enable(uclk);
> @@ -61,6 +63,8 @@ static void at91_start_clock(void)
>  
>  static void at91_stop_clock(void)
>  {
> +	if (!clocked)
> +		return;
>  	clk_disable_unprepare(fclk);
>  	clk_disable_unprepare(iclk);
>  	clk_disable_unprepare(hclk);
> @@ -615,16 +619,14 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
>  	 *
>  	 * REVISIT: some boards will be able to turn VBUS off...
>  	 */
> -	if (at91_suspend_entering_slow_clock()) {
> -		ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
> -		ohci->hc_control &= OHCI_CTRL_RWC;
> -		ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
> -		ohci->rh_state = OHCI_RH_HALTED;
> -
> -		/* flush the writes */
> -		(void) ohci_readl (ohci, &ohci->regs->control);
> -		at91_stop_clock();
> -	}
> +	ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
> +	ohci->hc_control &= OHCI_CTRL_RWC;
> +	ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
> +	ohci->rh_state = OHCI_RH_HALTED;
> +
> +	/* flush the writes */
> +	(void) ohci_readl (ohci, &ohci->regs->control);
> +	at91_stop_clock();
>  
>  	return ret;
>  }
> @@ -636,8 +638,7 @@ static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
>  	if (device_may_wakeup(&pdev->dev))
>  		disable_irq_wake(hcd->irq);
>  
> -	if (!clocked)
> -		at91_start_clock();
> +	at91_start_clock();
>  
>  	ohci_resume(hcd, false);
>  	return 0;



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCHv2 2/2] USB: host: ohci_at91: Stop/start USB PLL for all sleep modes

[Alexandre Belloni]

On 17/01/2015 at 16:36:35 +0100, Sylvain Rochet wrote :
> Disable/unprepare clocks without testing the sleep target_state, removed
> the at91_suspend_entering_slow_clock() call (which is only a
> target_state == PM_SUSPEND_MEM).
> 
> Other kind of suspend now benefit from the power save induced by this
> PLL deactivation. The resume penalty is about 500 us, which is not
> negligible but acceptable considering the amount of power we are saving.
> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  drivers/usb/host/ohci-at91.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index dc9e4e6..f0da734 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -49,6 +49,8 @@ extern int usb_disabled(void);
>  
>  static void at91_start_clock(void)
>  {
> +	if (clocked)
> +		return;
>  	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>  		clk_set_rate(uclk, 48000000);
>  		clk_prepare_enable(uclk);
> @@ -61,6 +63,8 @@ static void at91_start_clock(void)
>  
>  static void at91_stop_clock(void)
>  {
> +	if (!clocked)
> +		return;
>  	clk_disable_unprepare(fclk);
>  	clk_disable_unprepare(iclk);
>  	clk_disable_unprepare(hclk);
> @@ -615,16 +619,14 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
>  	 *
>  	 * REVISIT: some boards will be able to turn VBUS off...
>  	 */
> -	if (at91_suspend_entering_slow_clock()) {
> -		ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
> -		ohci->hc_control &= OHCI_CTRL_RWC;
> -		ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
> -		ohci->rh_state = OHCI_RH_HALTED;
> -
> -		/* flush the writes */
> -		(void) ohci_readl (ohci, &ohci->regs->control);
> -		at91_stop_clock();
> -	}
> +	ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
> +	ohci->hc_control &= OHCI_CTRL_RWC;
> +	ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
> +	ohci->rh_state = OHCI_RH_HALTED;
> +
> +	/* flush the writes */
> +	(void) ohci_readl (ohci, &ohci->regs->control);
> +	at91_stop_clock();
>  
>  	return ret;
>  }
> @@ -636,8 +638,7 @@ static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
>  	if (device_may_wakeup(&pdev->dev))
>  		disable_irq_wake(hcd->irq);
>  
> -	if (!clocked)
> -		at91_start_clock();
> +	at91_start_clock();
>  
>  	ohci_resume(hcd, false);
>  	return 0;
> -- 
> 2.1.4
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCHv2 1/2] USB: host: ehci_atmel: Add suspend/resume support

[Boris Brezillon]

On Sat, 17 Jan 2015 16:36:34 +0100
Sylvain Rochet <sylvain.rochet@finsecur.com> wrote:

> This patch add suspend/resume support for Atmel EHCI, mostly
> about disabling and unpreparing clocks so USB PLL is stopped
> before entering sleep state.
> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
>  drivers/usb/host/ehci-atmel.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 56a8850..2e0043b 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
> @@ -37,6 +37,8 @@ static int clocked;
>  
>  static void atmel_start_clock(void)
>  {
> +	if (clocked)
> +		return;
>  	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
>  		clk_set_rate(uclk, 48000000);
>  		clk_prepare_enable(uclk);
> @@ -48,6 +50,8 @@ static void atmel_start_clock(void)
>  
>  static void atmel_stop_clock(void)
>  {
> +	if (!clocked)
> +		return;
>  	clk_disable_unprepare(fclk);
>  	clk_disable_unprepare(iclk);
>  	if (IS_ENABLED(CONFIG_COMMON_CLK))
> @@ -174,6 +178,32 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
> +{
> +	struct usb_hcd *hcd = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = ehci_suspend(hcd, false);
> +	if (ret)
> +		return ret;
> +
> +	atmel_stop_clock();
> +	return 0;
> +}
> +
> +static int ehci_atmel_drv_resume(struct platform_device *pdev)
> +{
> +	struct usb_hcd *hcd = platform_get_drvdata(pdev);
> +
> +	atmel_start_clock();
> +	return ehci_resume(hcd, false);
> +}
> +#else
> +#define ehci_atmel_drv_suspend NULL
> +#define ehci_atmel_drv_resume  NULL
> +#endif
> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id atmel_ehci_dt_ids[] = {
>  	{ .compatible = "atmel,at91sam9g45-ehci" },
> @@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
>  	.probe		= ehci_atmel_drv_probe,
>  	.remove		= ehci_atmel_drv_remove,
>  	.shutdown	= usb_hcd_platform_shutdown,
> +	.suspend	= ehci_atmel_drv_suspend,
> +	.resume		= ehci_atmel_drv_resume,
>  	.driver		= {
>  		.name	= "atmel-ehci",
>  		.of_match_table	= of_match_ptr(atmel_ehci_dt_ids),



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Sylvain Rochet]

Hello Boris,


On Sat, Jan 17, 2015 at 01:58:57PM +0100, Boris Brezillon wrote:
> On Sat, 17 Jan 2015 11:43:00 +0100 Sylvain Rochet <sylvain.rochet@finsecur.com> wrote:
> > 
> > I agree, but it depends on what we mean with standby vs mem, there 
> > should be a difference between the two sleep mode.
> 
> AFAIU PM_SUSPEND_MEM mode only implies putting the SDRAM in
> self-refresh mode to avoid waking the processor up for periodic SDRAM
> bank refresh.
> IMO this should not impact other peripherals behavior (BTW I haven't
> found any USB driver testing for this suspend state before disabling
> their clks).

This is my understanding as well.


> And what if we decide to implement runtime PM for this peripheral ?
> Shouldn't we disable these clks (which are one of the main source of
> power consumption of this IP) ?

Of course we should, disengaging clocks and stopping unused PLL and 
crystals is almost the only way to save power.


Should I have added the "Acked-by: Alexandre Belloni 
<alexandre.belloni@free-electrons.com>" on 1/2 ?


> > This behavior follows what the Atmel OHCI driver is currently doing.
> 
> Yes, but it doesn't mean we should do the same ;-), especially since
> we're trying to remove this at91_suspend_entering_slow_clock function.

Arrg, this is not what I meant ;-), anyway, proposing as well a change 
which remove the at91_suspend_entering_slow_clock() from OHCI.

I am not so happy with the global clocked boolean, but I guess this is 
necessary if a suspended device gets removed.


> > > Is there such a big penalty when resuming the device if the PLL and
> > > peripheral clocks are disabled ?
> > 
> > There is a penalty, starting up a PLL takes about 500 ?s, however I 
> > can't decide if this is a small or a big penalty.
> 
> That's indeed not negligible, but when one enters suspend, I guess it
> does expect some latency to resume from the suspended state.

Overall resume of all drivers takes about 300 ms, whichever chosen 
suspend mode. Suspend to ram with slow clock mode enabled adds about 15 ms
of resume time.

As a side note, unfortunately AT91 slow clock mode only work on "recent" 
boards with Wenyou changes on linux4sam/linux-at91/linux-3.10-at91 
branch and is why I am still stuck to this branch (and the HLCDC FB 
driver, but this is going to be merged soon from what I saw, YAY !).


Sylvain

[PATCH 1/3] USB: host: ehci_atmel: Move global variables to private struct

[Sylvain Rochet]

This patch move Atmel EHCI global variables (clocks ptr and clocked
boolean) to private struct atmel_ehci, appended at the end of the parent
struct usb_hcd.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 75 +++++++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 2e0043b..a47fe3f 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -30,45 +30,62 @@ static const char hcd_name[] = "ehci-atmel";
 static struct hc_driver __read_mostly ehci_atmel_hc_driver;
 
 /* interface and function clocks */
-static struct clk *iclk, *fclk, *uclk;
-static int clocked;
+struct atmel_ehci {
+	struct ehci_hcd	ehci;
+
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	bool clocked;
+};
 
 /*-------------------------------------------------------------------------*/
 
-static void atmel_start_clock(void)
+static inline struct atmel_ehci *hcd_to_atmel_ehci(struct usb_hcd *hcd)
+{
+	return (struct atmel_ehci *) hcd->hcd_priv;
+}
+
+static void atmel_start_clock(struct atmel_ehci *atmel_ehci)
 {
-	if (clocked)
+	if (atmel_ehci->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(atmel_ehci->uclk, 48000000);
+		clk_prepare_enable(atmel_ehci->uclk);
 	}
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(atmel_ehci->iclk);
+	clk_prepare_enable(atmel_ehci->fclk);
+	atmel_ehci->clocked = true;
 }
 
-static void atmel_stop_clock(void)
+static void atmel_stop_clock(struct atmel_ehci *atmel_ehci)
 {
-	if (!clocked)
+	if (!atmel_ehci->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
+	clk_disable_unprepare(atmel_ehci->fclk);
+	clk_disable_unprepare(atmel_ehci->iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(atmel_ehci->uclk);
+	atmel_ehci->clocked = false;
 }
 
 static void atmel_start_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci *atmel_ehci = hcd_to_atmel_ehci(hcd);
+
 	dev_dbg(&pdev->dev, "start\n");
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 }
 
 static void atmel_stop_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci *atmel_ehci = hcd_to_atmel_ehci(hcd);
+
 	dev_dbg(&pdev->dev, "stop\n");
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -79,6 +96,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	const struct hc_driver *driver = &ehci_atmel_hc_driver;
 	struct resource *res;
 	struct ehci_hcd *ehci;
+	struct atmel_ehci *atmel_ehci;
 	int irq;
 	int retval;
 
@@ -109,6 +127,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 		retval = -ENOMEM;
 		goto fail_create_hcd;
 	}
+	atmel_ehci = hcd_to_atmel_ehci(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -120,23 +139,23 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(&pdev->dev, "ehci_clk");
-	if (IS_ERR(iclk)) {
+	atmel_ehci->iclk = devm_clk_get(&pdev->dev, "ehci_clk");
+	if (IS_ERR(atmel_ehci->iclk)) {
 		dev_err(&pdev->dev, "Error getting interface clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
-	fclk = devm_clk_get(&pdev->dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	atmel_ehci->fclk = devm_clk_get(&pdev->dev, "uhpck");
+	if (IS_ERR(atmel_ehci->fclk)) {
 		dev_err(&pdev->dev, "Error getting function clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(&pdev->dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		atmel_ehci->uclk = devm_clk_get(&pdev->dev, "usb_clk");
+		if (IS_ERR(atmel_ehci->uclk)) {
 			dev_err(&pdev->dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(atmel_ehci->uclk);
 			goto fail_request_resource;
 		}
 	}
@@ -173,7 +192,6 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	usb_put_hcd(hcd);
 
 	atmel_stop_ehci(pdev);
-	fclk = iclk = NULL;
 
 	return 0;
 }
@@ -182,21 +200,23 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci *atmel_ehci = hcd_to_atmel_ehci(hcd);
 	int ret;
 
 	ret = ehci_suspend(hcd, false);
 	if (ret)
 		return ret;
 
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 	return 0;
 }
 
 static int ehci_atmel_drv_resume(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci *atmel_ehci = hcd_to_atmel_ehci(hcd);
 
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 	return ehci_resume(hcd, false);
 }
 #else
@@ -232,6 +252,7 @@ static int __init ehci_atmel_init(void)
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
 	ehci_init_driver(&ehci_atmel_hc_driver, NULL);
+	ehci_atmel_hc_driver.hcd_priv_size = sizeof(struct atmel_ehci);
 	return platform_driver_register(&ehci_atmel_driver);
 }
 module_init(ehci_atmel_init);
-- 
2.1.4

[PATCH 2/3] USB: host: ohci_at91: Move global variables to private struct

[Sylvain Rochet]

This patch move Atmel OHCI global variables (clocks ptr and clocked
boolean) to private struct ohci_at91, appended at the end of the parent
struct usb_hcd.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 85 +++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index f0da734..ca53f8a 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -33,7 +33,15 @@
 		for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++)
 
 /* interface, function and usb clocks; sometimes also an AHB clock */
-static struct clk *iclk, *fclk, *uclk, *hclk;
+struct ohci_at91 {
+	struct ohci_hcd	ohci;
+
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	struct clk *hclk;
+	bool clocked;
+};
 /* interface and function clocks; sometimes also an AHB clock */
 
 #define DRIVER_DESC "OHCI Atmel driver"
@@ -41,49 +49,54 @@ static struct clk *iclk, *fclk, *uclk, *hclk;
 static const char hcd_name[] = "ohci-atmel";
 
 static struct hc_driver __read_mostly ohci_at91_hc_driver;
-static int clocked;
 
 extern int usb_disabled(void);
 
 /*-------------------------------------------------------------------------*/
 
-static void at91_start_clock(void)
+static inline struct ohci_at91 *hcd_to_ohci_at91(struct usb_hcd *hcd)
+{
+	return (struct ohci_at91 *) hcd->hcd_priv;
+}
+
+static void at91_start_clock(struct ohci_at91 *ohci_at91)
 {
-	if (clocked)
+	if (ohci_at91->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(ohci_at91->uclk, 48000000);
+		clk_prepare_enable(ohci_at91->uclk);
 	}
-	clk_prepare_enable(hclk);
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(ohci_at91->hclk);
+	clk_prepare_enable(ohci_at91->iclk);
+	clk_prepare_enable(ohci_at91->fclk);
+	ohci_at91->clocked = true;
 }
 
-static void at91_stop_clock(void)
+static void at91_stop_clock(struct ohci_at91 *ohci_at91)
 {
-	if (!clocked)
+	if (!ohci_at91->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
-	clk_disable_unprepare(hclk);
+	clk_disable_unprepare(ohci_at91->fclk);
+	clk_disable_unprepare(ohci_at91->iclk);
+	clk_disable_unprepare(ohci_at91->hclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(ohci_at91->uclk);
+	ohci_at91->clocked = false;
 }
 
 static void at91_start_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91 *ohci_at91 = hcd_to_ohci_at91(hcd);
 
 	dev_dbg(&pdev->dev, "start\n");
 
 	/*
 	 * Start the USB clocks.
 	 */
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	/*
 	 * The USB host controller must remain in reset.
@@ -95,6 +108,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91 *ohci_at91 = hcd_to_ohci_at91(hcd);
 
 	dev_dbg(&pdev->dev, "stop\n");
 
@@ -106,7 +120,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 	/*
 	 * Stop the USB clocks.
 	 */
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 }
 
 
@@ -133,6 +147,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct ohci_hcd *ohci;
 	int retval;
 	struct usb_hcd *hcd = NULL;
+	struct ohci_at91 *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	int irq;
@@ -146,6 +161,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd = usb_create_hcd(driver, dev, "at91");
 	if (!hcd)
 		return -ENOMEM;
+	ohci_at91 = hcd_to_ohci_at91(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(dev, res);
@@ -156,29 +172,29 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(dev, "ohci_clk");
-	if (IS_ERR(iclk)) {
+	ohci_at91->iclk = devm_clk_get(dev, "ohci_clk");
+	if (IS_ERR(ohci_at91->iclk)) {
 		dev_err(dev, "failed to get ohci_clk\n");
-		retval = PTR_ERR(iclk);
+		retval = PTR_ERR(ohci_at91->iclk);
 		goto err;
 	}
-	fclk = devm_clk_get(dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	ohci_at91->fclk = devm_clk_get(dev, "uhpck");
+	if (IS_ERR(ohci_at91->fclk)) {
 		dev_err(dev, "failed to get uhpck\n");
-		retval = PTR_ERR(fclk);
+		retval = PTR_ERR(ohci_at91->fclk);
 		goto err;
 	}
-	hclk = devm_clk_get(dev, "hclk");
-	if (IS_ERR(hclk)) {
+	ohci_at91->hclk = devm_clk_get(dev, "hclk");
+	if (IS_ERR(ohci_at91->hclk)) {
 		dev_err(dev, "failed to get hclk\n");
-		retval = PTR_ERR(hclk);
+		retval = PTR_ERR(ohci_at91->hclk);
 		goto err;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		ohci_at91->uclk = devm_clk_get(dev, "usb_clk");
+		if (IS_ERR(ohci_at91->uclk)) {
 			dev_err(dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(ohci_at91->uclk);
 			goto err;
 		}
 	}
@@ -601,6 +617,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 {
 	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
+	struct ohci_at91 *ohci_at91 = hcd_to_ohci_at91(hcd);
 	bool		do_wakeup = device_may_wakeup(&pdev->dev);
 	int		ret;
 
@@ -626,7 +643,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 
 	/* flush the writes */
 	(void) ohci_readl (ohci, &ohci->regs->control);
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 
 	return ret;
 }
@@ -634,11 +651,12 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
 {
 	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct ohci_at91 *ohci_at91 = hcd_to_ohci_at91(hcd);
 
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(hcd->irq);
 
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	ohci_resume(hcd, false);
 	return 0;
@@ -667,6 +685,7 @@ static int __init ohci_at91_init(void)
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
 	ohci_init_driver(&ohci_at91_hc_driver, NULL);
+	ohci_at91_hc_driver.hcd_priv_size = sizeof(struct ohci_at91);
 
 	/*
 	 * The Atmel HW has some unusual quirks, which require Atmel-specific
-- 
2.1.4

[PATCH 3/3] USB: host: ohci_at91: usb_hcd_at91_probe(), remove useless stack initialisation

[Sylvain Rochet]

struct usb_hcd *hcd = NULL;
...
hcd = usb_create_hcd(driver, dev, "at91");

This patch remove *hcd useless initialisation to NULL;

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index ca53f8a..10c9d76 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -146,7 +146,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct at91_usbh_data *board;
 	struct ohci_hcd *ohci;
 	int retval;
-	struct usb_hcd *hcd = NULL;
+	struct usb_hcd *hcd;
 	struct ohci_at91 *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
-- 
2.1.4

[PATCH 1/3] USB: host: ehci_atmel: Move global variables to private struct

[Boris Brezillon]

Hi Sylvain,

On Sat, 17 Jan 2015 19:23:31 +0100
Sylvain Rochet <sylvain.rochet@finsecur.com> wrote:

> This patch move Atmel EHCI global variables (clocks ptr and clocked
> boolean) to private struct atmel_ehci, appended at the end of the parent
> struct usb_hcd.

Maybe you should add a cover letter to clearly state that this series
depends on another one: [1].

Anyway, thanks for removing these global variables.

> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>

To the whole series:

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

[1]https://www.mail-archive.com/linux-usb at vger.kernel.org/msg54675.html


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 1/3] USB: host: ehci_atmel: Move global variables to private struct

[Alan Stern]

On Sat, 17 Jan 2015, Sylvain Rochet wrote:

> This patch move Atmel EHCI global variables (clocks ptr and clocked
> boolean) to private struct atmel_ehci, appended at the end of the parent
> struct usb_hcd.
> 
> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> ---
>  drivers/usb/host/ehci-atmel.c | 75 +++++++++++++++++++++++++++----------------
>  1 file changed, 48 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 2e0043b..a47fe3f 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
> @@ -30,45 +30,62 @@ static const char hcd_name[] = "ehci-atmel";
>  static struct hc_driver __read_mostly ehci_atmel_hc_driver;
>  
>  /* interface and function clocks */
> -static struct clk *iclk, *fclk, *uclk;
> -static int clocked;
> +struct atmel_ehci {
> +	struct ehci_hcd	ehci;
> +
> +	struct clk *iclk;
> +	struct clk *fclk;
> +	struct clk *uclk;
> +	bool clocked;
> +};
>  
>  /*-------------------------------------------------------------------------*/
>  
> -static void atmel_start_clock(void)
> +static inline struct atmel_ehci *hcd_to_atmel_ehci(struct usb_hcd *hcd)
> +{
> +	return (struct atmel_ehci *) hcd->hcd_priv;
> +}

This is not the right way to do it.  For an example of the right way, 
see the ehci-platform.c file.  Your private structure is stored in 
ehci->priv, and its size is specified by the .extra_priv_size member of 
an ehci_driver_overrides structure passed to ehci_init_driver().

Nevertheless, I approve the idea of getting rid of global variables.

Alan Stern

[PATCHv3 0/5] USB: host: Atmel OHCI and EHCI drivers improvements

[Sylvain Rochet]

Sylvain Rochet (5):
  USB: host: ehci_atmel: Add suspend/resume support
  USB: host: ohci_at91: Stop/start USB PLL for all sleep modes
  USB: host: ehci_atmel: Move global variables to private struct
  USB: host: ohci_at91: Move global variables to private struct
  USB: host: ohci_at91: usb_hcd_at91_probe(), remove useless stack
    initialisation

 drivers/usb/host/ehci-atmel.c | 102 +++++++++++++++++++++++++++++++----------
 drivers/usb/host/ohci-at91.c  | 104 +++++++++++++++++++++++++-----------------
 2 files changed, 138 insertions(+), 68 deletions(-)

-- 
2.1.4

[PATCHv3 1/5] USB: host: ehci_atmel: Add suspend/resume support

[Sylvain Rochet]

This patch add suspend/resume support for Atmel EHCI, mostly
about disabling and unpreparing clocks so USB PLL is stopped
before entering sleep state.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 56a8850..2e0043b 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -37,6 +37,8 @@ static int clocked;
 
 static void atmel_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -48,6 +50,8 @@ static void atmel_start_clock(void)
 
 static void atmel_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
@@ -174,6 +178,32 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = ehci_suspend(hcd, false);
+	if (ret)
+		return ret;
+
+	atmel_stop_clock();
+	return 0;
+}
+
+static int ehci_atmel_drv_resume(struct platform_device *pdev)
+{
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+
+	atmel_start_clock();
+	return ehci_resume(hcd, false);
+}
+#else
+#define ehci_atmel_drv_suspend NULL
+#define ehci_atmel_drv_resume  NULL
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id atmel_ehci_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9g45-ehci" },
@@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
 	.probe		= ehci_atmel_drv_probe,
 	.remove		= ehci_atmel_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
+	.suspend	= ehci_atmel_drv_suspend,
+	.resume		= ehci_atmel_drv_resume,
 	.driver		= {
 		.name	= "atmel-ehci",
 		.of_match_table	= of_match_ptr(atmel_ehci_dt_ids),
-- 
2.1.4

[PATCHv3 1/5] USB: host: ehci_atmel: Add suspend/resume support

[Sergei Shtylyov]

Hello.

On 01/18/2015 12:25 AM, Sylvain Rochet wrote:

    There's little inconsistency in your patch subjects: you're using '_' but 
the files you're modifying are named using '-'...

> This patch add suspend/resume support for Atmel EHCI, mostly
> about disabling and unpreparing clocks so USB PLL is stopped
> before entering sleep state.

> Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
> ---
>   drivers/usb/host/ehci-atmel.c | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)

> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 56a8850..2e0043b 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
[...]
> @@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
>   	.probe		= ehci_atmel_drv_probe,
>   	.remove		= ehci_atmel_drv_remove,
>   	.shutdown	= usb_hcd_platform_shutdown,
> +	.suspend	= ehci_atmel_drv_suspend,
> +	.resume		= ehci_atmel_drv_resume,

    I think you should use 'struct dev_pm_ops' now.

WBR, Sergei

[PATCHv3 1/5] USB: host: ehci_atmel: Add suspend/resume support

[Sylvain Rochet]

Hi Sergei,


On Sun, Jan 18, 2015 at 01:22:38AM +0300, Sergei Shtylyov wrote:
> 
>    There's little inconsistency in your patch subjects: you're using
> '_' but the files you're modifying are named using '-'...

Indeed.


> >@@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
> >  	.probe		= ehci_atmel_drv_probe,
> >  	.remove		= ehci_atmel_drv_remove,
> >  	.shutdown	= usb_hcd_platform_shutdown,
> >+	.suspend	= ehci_atmel_drv_suspend,
> >+	.resume		= ehci_atmel_drv_resume,
> 
>    I think you should use 'struct dev_pm_ops' now.

This way ?

static int ehci_atmel_drv_suspend(struct device *dev)
{
	struct usb_hcd *hcd = dev_get_drvdata(dev);
(...)


static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend, ehci_atmel_drv_resume);

(...)
	.driver         = {
		.pm     = &ehci_atmel_pm_ops,
	}
(...)


Should I send a v4 or can I send this change separately on top of the 
previous change ?


Sylvain

[PATCHv3 1/5] USB: host: ehci_atmel: Add suspend/resume support

[Boris Brezillon]

Hi Sylvain,

On Sat, 17 Jan 2015 23:49:00 +0100
Sylvain Rochet <sylvain.rochet@finsecur.com> wrote:

> Hi Sergei,
> 
> 
> On Sun, Jan 18, 2015 at 01:22:38AM +0300, Sergei Shtylyov wrote:
> > 
> >    There's little inconsistency in your patch subjects: you're using
> > '_' but the files you're modifying are named using '-'...
> 
> Indeed.
> 
> 
> > >@@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
> > >  	.probe		= ehci_atmel_drv_probe,
> > >  	.remove		= ehci_atmel_drv_remove,
> > >  	.shutdown	= usb_hcd_platform_shutdown,
> > >+	.suspend	= ehci_atmel_drv_suspend,
> > >+	.resume		= ehci_atmel_drv_resume,
> > 
> >    I think you should use 'struct dev_pm_ops' now.
> 
> This way ?
> 
> static int ehci_atmel_drv_suspend(struct device *dev)
> {
> 	struct usb_hcd *hcd = dev_get_drvdata(dev);
> (...)
> 
> 
> static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend, ehci_atmel_drv_resume);
> 
> (...)
> 	.driver         = {
> 		.pm     = &ehci_atmel_pm_ops,
> 	}
> (...)
> 
> 
> Should I send a v4 or can I send this change separately on top of the 
> previous change ?

I think it's better to send a v4 reworking this patch (you'll have to
change your commit subject anyway ;-)).

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCHv3 1/5] USB: host: ehci_atmel: Add suspend/resume support

[Sergei Shtylyov]

Hello.

On 01/18/2015 01:49 AM, Sylvain Rochet wrote:

>>     There's little inconsistency in your patch subjects: you're using
>> '_' but the files you're modifying are named using '-'...

> Indeed.

>>> @@ -187,6 +217,8 @@ static struct platform_driver ehci_atmel_driver = {
>>>   	.probe		= ehci_atmel_drv_probe,
>>>   	.remove		= ehci_atmel_drv_remove,
>>>   	.shutdown	= usb_hcd_platform_shutdown,
>>> +	.suspend	= ehci_atmel_drv_suspend,
>>> +	.resume		= ehci_atmel_drv_resume,
>>
>>     I think you should use 'struct dev_pm_ops' now.

    I'm not sure but perhaps scripts/checkpatch.pl would complain about the 
old-style PM methods... should check.

> This way ?

> static int ehci_atmel_drv_suspend(struct device *dev)
> {
> 	struct usb_hcd *hcd = dev_get_drvdata(dev);
> (...)
>
>
> static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend, ehci_atmel_drv_resume);
>
> (...)
> 	.driver         = {
> 		.pm     = &ehci_atmel_pm_ops,
> 	}
> (...)

    Yes, probably.

> Should I send a v4 or can I send this change separately on top of the
> previous change ?

    v4 please.

> Sylvain

WBR, Sergei

[PATCHv3 2/5] USB: host: ohci_at91: Stop/start USB PLL for all sleep modes

[Sylvain Rochet]

Disable/unprepare clocks without testing the sleep target_state, removed
the at91_suspend_entering_slow_clock() call (which is only a
target_state == PM_SUSPEND_MEM).

Other kind of suspend now benefit from the power save induced by this
PLL deactivation. The resume penalty is about 500 us, which is not
negligible but acceptable considering the amount of power we are saving.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/usb/host/ohci-at91.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index dc9e4e6..f0da734 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -49,6 +49,8 @@ extern int usb_disabled(void);
 
 static void at91_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -61,6 +63,8 @@ static void at91_start_clock(void)
 
 static void at91_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	clk_disable_unprepare(hclk);
@@ -615,16 +619,14 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 	 *
 	 * REVISIT: some boards will be able to turn VBUS off...
 	 */
-	if (at91_suspend_entering_slow_clock()) {
-		ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
-		ohci->hc_control &= OHCI_CTRL_RWC;
-		ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
-		ohci->rh_state = OHCI_RH_HALTED;
-
-		/* flush the writes */
-		(void) ohci_readl (ohci, &ohci->regs->control);
-		at91_stop_clock();
-	}
+	ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
+	ohci->hc_control &= OHCI_CTRL_RWC;
+	ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
+	ohci->rh_state = OHCI_RH_HALTED;
+
+	/* flush the writes */
+	(void) ohci_readl (ohci, &ohci->regs->control);
+	at91_stop_clock();
 
 	return ret;
 }
@@ -636,8 +638,7 @@ static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(hcd->irq);
 
-	if (!clocked)
-		at91_start_clock();
+	at91_start_clock();
 
 	ohci_resume(hcd, false);
 	return 0;
-- 
2.1.4

[PATCHv3 3/5] USB: host: ehci_atmel: Move global variables to private struct

[Sylvain Rochet]

This patch move Atmel EHCI global variables (clocks ptr and clocked
boolean) to private struct atmel_ehci_priv, stored in ehci->priv.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 78 +++++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 2e0043b..eb0924f 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -27,48 +27,65 @@
 #define DRIVER_DESC "EHCI Atmel driver"
 
 static const char hcd_name[] = "ehci-atmel";
-static struct hc_driver __read_mostly ehci_atmel_hc_driver;
 
 /* interface and function clocks */
-static struct clk *iclk, *fclk, *uclk;
-static int clocked;
+#define hcd_to_atmel_ehci_priv(h) ((struct atmel_ehci_priv *)hcd_to_ehci(h)->priv)
+
+struct atmel_ehci_priv {
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	bool clocked;
+};
+
+static struct hc_driver __read_mostly ehci_atmel_hc_driver;
+
+static const struct ehci_driver_overrides ehci_atmel_drv_overrides __initconst = {
+	.extra_priv_size = sizeof(struct atmel_ehci_priv),
+};
 
 /*-------------------------------------------------------------------------*/
 
-static void atmel_start_clock(void)
+static void atmel_start_clock(struct atmel_ehci_priv *atmel_ehci)
 {
-	if (clocked)
+	if (atmel_ehci->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(atmel_ehci->uclk, 48000000);
+		clk_prepare_enable(atmel_ehci->uclk);
 	}
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(atmel_ehci->iclk);
+	clk_prepare_enable(atmel_ehci->fclk);
+	atmel_ehci->clocked = true;
 }
 
-static void atmel_stop_clock(void)
+static void atmel_stop_clock(struct atmel_ehci_priv *atmel_ehci)
 {
-	if (!clocked)
+	if (!atmel_ehci->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
+	clk_disable_unprepare(atmel_ehci->fclk);
+	clk_disable_unprepare(atmel_ehci->iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(atmel_ehci->uclk);
+	atmel_ehci->clocked = false;
 }
 
 static void atmel_start_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
+
 	dev_dbg(&pdev->dev, "start\n");
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 }
 
 static void atmel_stop_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
+
 	dev_dbg(&pdev->dev, "stop\n");
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -79,6 +96,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	const struct hc_driver *driver = &ehci_atmel_hc_driver;
 	struct resource *res;
 	struct ehci_hcd *ehci;
+	struct atmel_ehci_priv *atmel_ehci;
 	int irq;
 	int retval;
 
@@ -109,6 +127,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 		retval = -ENOMEM;
 		goto fail_create_hcd;
 	}
+	atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -120,23 +139,23 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(&pdev->dev, "ehci_clk");
-	if (IS_ERR(iclk)) {
+	atmel_ehci->iclk = devm_clk_get(&pdev->dev, "ehci_clk");
+	if (IS_ERR(atmel_ehci->iclk)) {
 		dev_err(&pdev->dev, "Error getting interface clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
-	fclk = devm_clk_get(&pdev->dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	atmel_ehci->fclk = devm_clk_get(&pdev->dev, "uhpck");
+	if (IS_ERR(atmel_ehci->fclk)) {
 		dev_err(&pdev->dev, "Error getting function clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(&pdev->dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		atmel_ehci->uclk = devm_clk_get(&pdev->dev, "usb_clk");
+		if (IS_ERR(atmel_ehci->uclk)) {
 			dev_err(&pdev->dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(atmel_ehci->uclk);
 			goto fail_request_resource;
 		}
 	}
@@ -173,7 +192,6 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	usb_put_hcd(hcd);
 
 	atmel_stop_ehci(pdev);
-	fclk = iclk = NULL;
 
 	return 0;
 }
@@ -182,21 +200,23 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 static int ehci_atmel_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 	int ret;
 
 	ret = ehci_suspend(hcd, false);
 	if (ret)
 		return ret;
 
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 	return 0;
 }
 
 static int ehci_atmel_drv_resume(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 	return ehci_resume(hcd, false);
 }
 #else
@@ -231,7 +251,7 @@ static int __init ehci_atmel_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-	ehci_init_driver(&ehci_atmel_hc_driver, NULL);
+	ehci_init_driver(&ehci_atmel_hc_driver, &ehci_atmel_drv_overrides);
 	return platform_driver_register(&ehci_atmel_driver);
 }
 module_init(ehci_atmel_init);
-- 
2.1.4

[PATCHv3 4/5] USB: host: ohci_at91: Move global variables to private struct

[Sylvain Rochet]

This patch move AT91 OHCI global variables (clocks ptr and clocked
boolean) to private struct ohci_at91_priv, stored in ohci->priv.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 85 ++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index f0da734..ab39b94 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -33,7 +33,15 @@
 		for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++)
 
 /* interface, function and usb clocks; sometimes also an AHB clock */
-static struct clk *iclk, *fclk, *uclk, *hclk;
+#define hcd_to_ohci_at91_priv(h) ((struct ohci_at91_priv *)hcd_to_ohci(h)->priv)
+
+struct ohci_at91_priv {
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	struct clk *hclk;
+	bool clocked;
+};
 /* interface and function clocks; sometimes also an AHB clock */
 
 #define DRIVER_DESC "OHCI Atmel driver"
@@ -41,49 +49,53 @@ static struct clk *iclk, *fclk, *uclk, *hclk;
 static const char hcd_name[] = "ohci-atmel";
 
 static struct hc_driver __read_mostly ohci_at91_hc_driver;
-static int clocked;
+
+static const struct ohci_driver_overrides ohci_at91_drv_overrides __initconst = {
+	.extra_priv_size = sizeof(struct ohci_at91_priv),
+};
 
 extern int usb_disabled(void);
 
 /*-------------------------------------------------------------------------*/
 
-static void at91_start_clock(void)
+static void at91_start_clock(struct ohci_at91_priv *ohci_at91)
 {
-	if (clocked)
+	if (ohci_at91->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(ohci_at91->uclk, 48000000);
+		clk_prepare_enable(ohci_at91->uclk);
 	}
-	clk_prepare_enable(hclk);
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(ohci_at91->hclk);
+	clk_prepare_enable(ohci_at91->iclk);
+	clk_prepare_enable(ohci_at91->fclk);
+	ohci_at91->clocked = true;
 }
 
-static void at91_stop_clock(void)
+static void at91_stop_clock(struct ohci_at91_priv *ohci_at91)
 {
-	if (!clocked)
+	if (!ohci_at91->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
-	clk_disable_unprepare(hclk);
+	clk_disable_unprepare(ohci_at91->fclk);
+	clk_disable_unprepare(ohci_at91->iclk);
+	clk_disable_unprepare(ohci_at91->hclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(ohci_at91->uclk);
+	ohci_at91->clocked = false;
 }
 
 static void at91_start_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	dev_dbg(&pdev->dev, "start\n");
 
 	/*
 	 * Start the USB clocks.
 	 */
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	/*
 	 * The USB host controller must remain in reset.
@@ -95,6 +107,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	dev_dbg(&pdev->dev, "stop\n");
 
@@ -106,7 +119,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 	/*
 	 * Stop the USB clocks.
 	 */
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 }
 
 
@@ -133,6 +146,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct ohci_hcd *ohci;
 	int retval;
 	struct usb_hcd *hcd = NULL;
+	struct ohci_at91_priv *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	int irq;
@@ -146,6 +160,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd = usb_create_hcd(driver, dev, "at91");
 	if (!hcd)
 		return -ENOMEM;
+	ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(dev, res);
@@ -156,29 +171,29 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(dev, "ohci_clk");
-	if (IS_ERR(iclk)) {
+	ohci_at91->iclk = devm_clk_get(dev, "ohci_clk");
+	if (IS_ERR(ohci_at91->iclk)) {
 		dev_err(dev, "failed to get ohci_clk\n");
-		retval = PTR_ERR(iclk);
+		retval = PTR_ERR(ohci_at91->iclk);
 		goto err;
 	}
-	fclk = devm_clk_get(dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	ohci_at91->fclk = devm_clk_get(dev, "uhpck");
+	if (IS_ERR(ohci_at91->fclk)) {
 		dev_err(dev, "failed to get uhpck\n");
-		retval = PTR_ERR(fclk);
+		retval = PTR_ERR(ohci_at91->fclk);
 		goto err;
 	}
-	hclk = devm_clk_get(dev, "hclk");
-	if (IS_ERR(hclk)) {
+	ohci_at91->hclk = devm_clk_get(dev, "hclk");
+	if (IS_ERR(ohci_at91->hclk)) {
 		dev_err(dev, "failed to get hclk\n");
-		retval = PTR_ERR(hclk);
+		retval = PTR_ERR(ohci_at91->hclk);
 		goto err;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		ohci_at91->uclk = devm_clk_get(dev, "usb_clk");
+		if (IS_ERR(ohci_at91->uclk)) {
 			dev_err(dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(ohci_at91->uclk);
 			goto err;
 		}
 	}
@@ -601,6 +616,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 {
 	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 	bool		do_wakeup = device_may_wakeup(&pdev->dev);
 	int		ret;
 
@@ -626,7 +642,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 
 	/* flush the writes */
 	(void) ohci_readl (ohci, &ohci->regs->control);
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 
 	return ret;
 }
@@ -634,11 +650,12 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
 {
 	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(hcd->irq);
 
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	ohci_resume(hcd, false);
 	return 0;
@@ -666,7 +683,7 @@ static int __init ohci_at91_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-	ohci_init_driver(&ohci_at91_hc_driver, NULL);
+	ohci_init_driver(&ohci_at91_hc_driver, &ohci_at91_drv_overrides);
 
 	/*
 	 * The Atmel HW has some unusual quirks, which require Atmel-specific
-- 
2.1.4

[PATCHv3 5/5] USB: host: ohci_at91: usb_hcd_at91_probe(), remove useless stack initialisation

[Sylvain Rochet]

struct usb_hcd *hcd = NULL;
...
hcd = usb_create_hcd(driver, dev, "at91");

This patch remove *hcd useless initialisation

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index ab39b94..7e37657 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -145,7 +145,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct at91_usbh_data *board;
 	struct ohci_hcd *ohci;
 	int retval;
-	struct usb_hcd *hcd = NULL;
+	struct usb_hcd *hcd;
 	struct ohci_at91_priv *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
-- 
2.1.4

[PATCHv3 0/5] USB: host: Atmel OHCI and EHCI drivers improvements

[Alan Stern]

On Sat, 17 Jan 2015, Sylvain Rochet wrote:

> Sylvain Rochet (5):
>   USB: host: ehci_atmel: Add suspend/resume support
>   USB: host: ohci_at91: Stop/start USB PLL for all sleep modes
>   USB: host: ehci_atmel: Move global variables to private struct
>   USB: host: ohci_at91: Move global variables to private struct
>   USB: host: ohci_at91: usb_hcd_at91_probe(), remove useless stack
>     initialisation
> 
>  drivers/usb/host/ehci-atmel.c | 102 +++++++++++++++++++++++++++++++----------
>  drivers/usb/host/ohci-at91.c  | 104 +++++++++++++++++++++++++-----------------
>  2 files changed, 138 insertions(+), 68 deletions(-)

These patches look pretty good to me.  Have you run them through 
checkpatch.pl?

Alan Stern

[PATCHv4 0/6] USB: host: Atmel OHCI and EHCI drivers improvements

[Sylvain Rochet]

Suspend/resume support for EHCI.
struct dev_pm_ops for OHCI.
PLL stop for all sleep modes for OHCI.
Removed global variables from both.
Removed at91_suspend_entering_slow_clock() from both.

Changes since v3:
  * Using struct dev_pm_ops instead of static struct platform_driver
    resume and suspend bindings for both EHCI and OHCI
  * Fixed inconsistency in patch subjects, _ intead of - for file names
  * Patch cleaning with the help of checkpatch.pl, fixed lines over
    80 characters

Changes since v2:
  * Added patchs from an other submission, because this series
    depended on this one
    * EHCI: Move global variables to private struct
    * OHCI: Move global variables to private struct
  * Using ohci->priv and ehci->priv instead of hcd->hcd_priv,
    which were not the right way to do that

Changes since v1:
  * Don't use at91_suspend_entering_slow_clock() on EHCI,
    we are trying to get read of this of this function
  * Removed at91_suspend_entering_slow_clock() from OHCI

Sylvain Rochet (6):
  USB: host: ehci-atmel: Add suspend/resume support
  USB: host: ohci-at91: Use struct dev_pm_ops instead of struct
    platform_driver
  USB: host: ohci-at91: Stop/start USB PLL for all sleep modes
  USB: host: ehci-atmel: Move global variables to private struct
  USB: host: ohci-at91: Move global variables to private struct
  USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
    initialisation

 drivers/usb/host/ehci-atmel.c | 102 +++++++++++++++++++++++++---------
 drivers/usb/host/ohci-at91.c  | 126 ++++++++++++++++++++++++------------------
 2 files changed, 149 insertions(+), 79 deletions(-)

-- 
2.1.4

[PATCHv4 1/6] USB: host: ehci-atmel: Add suspend/resume support

[Sylvain Rochet]

This patch add suspend/resume support for Atmel EHCI, mostly
about disabling and unpreparing clocks so USB PLL is stopped
before entering sleep state.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 56a8850..5a15e3d 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -37,6 +37,8 @@ static int clocked;
 
 static void atmel_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -48,6 +50,8 @@ static void atmel_start_clock(void)
 
 static void atmel_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
@@ -174,6 +178,29 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ehci_atmel_drv_suspend(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ehci_suspend(hcd, false);
+	if (ret)
+		return ret;
+
+	atmel_stop_clock();
+	return 0;
+}
+
+static int ehci_atmel_drv_resume(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+
+	atmel_start_clock();
+	return ehci_resume(hcd, false);
+}
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id atmel_ehci_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9g45-ehci" },
@@ -183,12 +210,16 @@ static const struct of_device_id atmel_ehci_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, atmel_ehci_dt_ids);
 #endif
 
+static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend,
+					ehci_atmel_drv_resume);
+
 static struct platform_driver ehci_atmel_driver = {
 	.probe		= ehci_atmel_drv_probe,
 	.remove		= ehci_atmel_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver		= {
 		.name	= "atmel-ehci",
+		.pm	= &ehci_atmel_pm_ops,
 		.of_match_table	= of_match_ptr(atmel_ehci_dt_ids),
 	},
 };
-- 
2.1.4

[PATCHv4 2/6] USB: host: ohci-at91: Use struct dev_pm_ops instead of struct platform_driver

[Sylvain Rochet]

This patch replace struct platform_driver.{resume,suspend} PM bindings
to a new struct dev_pm_ops.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index dc9e4e6..65e7836 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -593,11 +593,11 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM
 
 static int
-ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
+ohci_hcd_at91_drv_suspend(struct device *dev)
 {
-	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
-	bool		do_wakeup = device_may_wakeup(&pdev->dev);
+	bool		do_wakeup = device_may_wakeup(dev);
 	int		ret;
 
 	if (do_wakeup)
@@ -629,11 +629,11 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 	return ret;
 }
 
-static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
+static int ohci_hcd_at91_drv_resume(struct device *dev)
 {
-	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(dev))
 		disable_irq_wake(hcd->irq);
 
 	if (!clocked)
@@ -642,19 +642,18 @@ static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
 	ohci_resume(hcd, false);
 	return 0;
 }
-#else
-#define ohci_hcd_at91_drv_suspend NULL
-#define ohci_hcd_at91_drv_resume  NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(ohci_hcd_at91_pm_ops, ohci_hcd_at91_drv_suspend,
+					ohci_hcd_at91_drv_resume);
+
 static struct platform_driver ohci_hcd_at91_driver = {
 	.probe		= ohci_hcd_at91_drv_probe,
 	.remove		= ohci_hcd_at91_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
-	.suspend	= ohci_hcd_at91_drv_suspend,
-	.resume		= ohci_hcd_at91_drv_resume,
 	.driver		= {
 		.name	= "at91_ohci",
+		.pm	= &ohci_hcd_at91_pm_ops,
 		.of_match_table	= of_match_ptr(at91_ohci_dt_ids),
 	},
 };
-- 
2.1.4

[PATCHv4 3/6] USB: host: ohci-at91: Stop/start USB PLL for all sleep modes

[Sylvain Rochet]

Disable/unprepare clocks without testing the sleep target_state, removed
the at91_suspend_entering_slow_clock() call (which is only a
target_state == PM_SUSPEND_MEM).

Other kind of suspend now benefit from the power save induced by this
PLL deactivation. The resume penalty is about 500 us, which is not
negligible but acceptable considering the amount of power we are saving.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
Reported-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/usb/host/ohci-at91.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 65e7836..79e343e 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -49,6 +49,8 @@ extern int usb_disabled(void);
 
 static void at91_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -61,6 +63,8 @@ static void at91_start_clock(void)
 
 static void at91_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	clk_disable_unprepare(hclk);
@@ -615,16 +619,14 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 	 *
 	 * REVISIT: some boards will be able to turn VBUS off...
 	 */
-	if (at91_suspend_entering_slow_clock()) {
-		ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
-		ohci->hc_control &= OHCI_CTRL_RWC;
-		ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
-		ohci->rh_state = OHCI_RH_HALTED;
-
-		/* flush the writes */
-		(void) ohci_readl (ohci, &ohci->regs->control);
-		at91_stop_clock();
-	}
+	ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
+	ohci->hc_control &= OHCI_CTRL_RWC;
+	ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
+	ohci->rh_state = OHCI_RH_HALTED;
+
+	/* flush the writes */
+	(void) ohci_readl (ohci, &ohci->regs->control);
+	at91_stop_clock();
 
 	return ret;
 }
@@ -636,8 +638,7 @@ static int ohci_hcd_at91_drv_resume(struct device *dev)
 	if (device_may_wakeup(dev))
 		disable_irq_wake(hcd->irq);
 
-	if (!clocked)
-		at91_start_clock();
+	at91_start_clock();
 
 	ohci_resume(hcd, false);
 	return 0;
-- 
2.1.4

[PATCHv4 3/6] USB: host: ohci-at91: Stop/start USB PLL for all sleep modes

[Sylvain Rochet]

Hello,

On Sun, Jan 18, 2015 at 08:36:03PM +0100, Sylvain Rochet wrote:
> Disable/unprepare clocks without testing the sleep target_state, removed
> the at91_suspend_entering_slow_clock() call (which is only a
> target_state == PM_SUSPEND_MEM).
> 
> Other kind of suspend now benefit from the power save induced by this
> PLL deactivation. The resume penalty is about 500 us, which is not
> negligible but acceptable considering the amount of power we are saving.

Well, this patch actually lights up a previous issue on wake up support.

This device needs to be continuously clocked to provide wake up support, 
previously, if STANDBY target were chosen the device were 
enable_irq_wake-prepared and clock still active and if MEM target were 
chosen the device were also enable_irq_wake-prepared but not clocked 
anymore, which is wrong, but not so wrong.

This patch actually breaks wake up support for STANDBY sleep target. I 
am going to propose a v5 which reinitiate at91_suspend_entering_slow_clock()
and fix wake up support on this driver.

Sylvain

[PATCHv5 0/6] USB: host: Atmel OHCI and EHCI drivers improvements

[Sylvain Rochet]

USB: host: Atmel OHCI and EHCI drivers improvements

Suspend/resume support for EHCI.
struct dev_pm_ops for OHCI.
Removed global variables from both.
Fixed OHCI wake up support for STANDBY(wake-up enabled) and MEM(wake-up 
disabled) sleep targets.

Changes since v4:
  * Re-add at91_suspend_entering_slow_clock() to OHCI, we can't naively
    remove this one, this device needs to be continuously clocked to
    provide wake up support.
    The removal of at91_suspend_entering_slow_clock() actually lighted up
    an issue on wake up support, which is now fixed.

Changes since v3:
  * Using struct dev_pm_ops instead of static struct platform_driver
    resume and suspend bindings for both EHCI and OHCI
  * Fixed inconsistency in patch subjects, _ intead of - for file names
  * Patch cleaning with the help of checkpatch.pl, fixed lines over
    80 characters

Changes since v2:
  * Added patchs from an other submission, because this series
    depended on this one
    * EHCI: Move global variables to private struct
    * OHCI: Move global variables to private struct
  * Using ohci->priv and ehci->priv instead of hcd->hcd_priv,
    which were not the right way to do that

Changes since v1:
  * Don't use at91_suspend_entering_slow_clock() on EHCI,
    we are trying to get read of this of this function
  * Removed at91_suspend_entering_slow_clock() from OHCI

Sylvain Rochet (6):
  USB: host: ehci-atmel: Add suspend/resume support
  USB: host: ohci-at91: Use struct dev_pm_ops instead of struct
    platform_driver
  USB: host: ehci-atmel: Move global variables to private struct
  USB: host: ohci-at91: Fix wake-up support
  USB: host: ohci-at91: Move global variables to private struct
  USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
    initialisation

 drivers/usb/host/ehci-atmel.c | 102 ++++++++++++++++++++++++++---------
 drivers/usb/host/ohci-at91.c  | 120 +++++++++++++++++++++++++-----------------
 2 files changed, 150 insertions(+), 72 deletions(-)

-- 
2.1.4

[PATCHv5 1/6] USB: host: ehci-atmel: Add suspend/resume support

[Sylvain Rochet]

This patch add suspend/resume support for Atmel EHCI, mostly
about disabling and unpreparing clocks so USB PLL is stopped
before entering sleep state.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 56a8850..5a15e3d 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -37,6 +37,8 @@ static int clocked;
 
 static void atmel_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -48,6 +50,8 @@ static void atmel_start_clock(void)
 
 static void atmel_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
@@ -174,6 +178,29 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ehci_atmel_drv_suspend(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	int ret;
+
+	ret = ehci_suspend(hcd, false);
+	if (ret)
+		return ret;
+
+	atmel_stop_clock();
+	return 0;
+}
+
+static int ehci_atmel_drv_resume(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+
+	atmel_start_clock();
+	return ehci_resume(hcd, false);
+}
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id atmel_ehci_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9g45-ehci" },
@@ -183,12 +210,16 @@ static const struct of_device_id atmel_ehci_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, atmel_ehci_dt_ids);
 #endif
 
+static SIMPLE_DEV_PM_OPS(ehci_atmel_pm_ops, ehci_atmel_drv_suspend,
+					ehci_atmel_drv_resume);
+
 static struct platform_driver ehci_atmel_driver = {
 	.probe		= ehci_atmel_drv_probe,
 	.remove		= ehci_atmel_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver		= {
 		.name	= "atmel-ehci",
+		.pm	= &ehci_atmel_pm_ops,
 		.of_match_table	= of_match_ptr(atmel_ehci_dt_ids),
 	},
 };
-- 
2.1.4

[PATCHv5 2/6] USB: host: ohci-at91: Use struct dev_pm_ops instead of struct platform_driver

[Sylvain Rochet]

This patch replace struct platform_driver.{resume,suspend} PM bindings
to a new struct dev_pm_ops.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index dc9e4e6..65e7836 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -593,11 +593,11 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM
 
 static int
-ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
+ohci_hcd_at91_drv_suspend(struct device *dev)
 {
-	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
-	bool		do_wakeup = device_may_wakeup(&pdev->dev);
+	bool		do_wakeup = device_may_wakeup(dev);
 	int		ret;
 
 	if (do_wakeup)
@@ -629,11 +629,11 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, pm_message_t mesg)
 	return ret;
 }
 
-static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
+static int ohci_hcd_at91_drv_resume(struct device *dev)
 {
-	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(dev))
 		disable_irq_wake(hcd->irq);
 
 	if (!clocked)
@@ -642,19 +642,18 @@ static int ohci_hcd_at91_drv_resume(struct platform_device *pdev)
 	ohci_resume(hcd, false);
 	return 0;
 }
-#else
-#define ohci_hcd_at91_drv_suspend NULL
-#define ohci_hcd_at91_drv_resume  NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(ohci_hcd_at91_pm_ops, ohci_hcd_at91_drv_suspend,
+					ohci_hcd_at91_drv_resume);
+
 static struct platform_driver ohci_hcd_at91_driver = {
 	.probe		= ohci_hcd_at91_drv_probe,
 	.remove		= ohci_hcd_at91_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
-	.suspend	= ohci_hcd_at91_drv_suspend,
-	.resume		= ohci_hcd_at91_drv_resume,
 	.driver		= {
 		.name	= "at91_ohci",
+		.pm	= &ohci_hcd_at91_pm_ops,
 		.of_match_table	= of_match_ptr(at91_ohci_dt_ids),
 	},
 };
-- 
2.1.4

[PATCHv5 3/6] USB: host: ehci-atmel: Move global variables to private struct

[Sylvain Rochet]

This patch move Atmel EHCI global variables (clocks ptr and clocked
boolean) to private struct atmel_ehci_priv, stored in ehci->priv.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 79 +++++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 5a15e3d..663f790 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -27,48 +27,66 @@
 #define DRIVER_DESC "EHCI Atmel driver"
 
 static const char hcd_name[] = "ehci-atmel";
-static struct hc_driver __read_mostly ehci_atmel_hc_driver;
 
 /* interface and function clocks */
-static struct clk *iclk, *fclk, *uclk;
-static int clocked;
+#define hcd_to_atmel_ehci_priv(h) \
+	((struct atmel_ehci_priv *)hcd_to_ehci(h)->priv)
+
+struct atmel_ehci_priv {
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	bool clocked;
+};
+
+static struct hc_driver __read_mostly ehci_atmel_hc_driver;
+
+static const struct ehci_driver_overrides ehci_atmel_drv_overrides __initconst = {
+	.extra_priv_size = sizeof(struct atmel_ehci_priv),
+};
 
 /*-------------------------------------------------------------------------*/
 
-static void atmel_start_clock(void)
+static void atmel_start_clock(struct atmel_ehci_priv *atmel_ehci)
 {
-	if (clocked)
+	if (atmel_ehci->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(atmel_ehci->uclk, 48000000);
+		clk_prepare_enable(atmel_ehci->uclk);
 	}
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(atmel_ehci->iclk);
+	clk_prepare_enable(atmel_ehci->fclk);
+	atmel_ehci->clocked = true;
 }
 
-static void atmel_stop_clock(void)
+static void atmel_stop_clock(struct atmel_ehci_priv *atmel_ehci)
 {
-	if (!clocked)
+	if (!atmel_ehci->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
+	clk_disable_unprepare(atmel_ehci->fclk);
+	clk_disable_unprepare(atmel_ehci->iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(atmel_ehci->uclk);
+	atmel_ehci->clocked = false;
 }
 
 static void atmel_start_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
+
 	dev_dbg(&pdev->dev, "start\n");
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 }
 
 static void atmel_stop_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
+
 	dev_dbg(&pdev->dev, "stop\n");
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -79,6 +97,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	const struct hc_driver *driver = &ehci_atmel_hc_driver;
 	struct resource *res;
 	struct ehci_hcd *ehci;
+	struct atmel_ehci_priv *atmel_ehci;
 	int irq;
 	int retval;
 
@@ -109,6 +128,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 		retval = -ENOMEM;
 		goto fail_create_hcd;
 	}
+	atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -120,23 +140,23 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(&pdev->dev, "ehci_clk");
-	if (IS_ERR(iclk)) {
+	atmel_ehci->iclk = devm_clk_get(&pdev->dev, "ehci_clk");
+	if (IS_ERR(atmel_ehci->iclk)) {
 		dev_err(&pdev->dev, "Error getting interface clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
-	fclk = devm_clk_get(&pdev->dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	atmel_ehci->fclk = devm_clk_get(&pdev->dev, "uhpck");
+	if (IS_ERR(atmel_ehci->fclk)) {
 		dev_err(&pdev->dev, "Error getting function clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(&pdev->dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		atmel_ehci->uclk = devm_clk_get(&pdev->dev, "usb_clk");
+		if (IS_ERR(atmel_ehci->uclk)) {
 			dev_err(&pdev->dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(atmel_ehci->uclk);
 			goto fail_request_resource;
 		}
 	}
@@ -173,7 +193,6 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	usb_put_hcd(hcd);
 
 	atmel_stop_ehci(pdev);
-	fclk = iclk = NULL;
 
 	return 0;
 }
@@ -182,21 +201,23 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 static int ehci_atmel_drv_suspend(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 	int ret;
 
 	ret = ehci_suspend(hcd, false);
 	if (ret)
 		return ret;
 
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 	return 0;
 }
 
 static int ehci_atmel_drv_resume(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 	return ehci_resume(hcd, false);
 }
 #endif
@@ -230,7 +251,7 @@ static int __init ehci_atmel_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-	ehci_init_driver(&ehci_atmel_hc_driver, NULL);
+	ehci_init_driver(&ehci_atmel_hc_driver, &ehci_atmel_drv_overrides);
 	return platform_driver_register(&ehci_atmel_driver);
 }
 module_init(ehci_atmel_init);
-- 
2.1.4

[PATCHv5 4/6] USB: host: ohci-at91: Fix wake-up support

[Sylvain Rochet]

This device needs to be continuously clocked to provide wake up support,
previously, if STANDBY target were chosen the device were
enable_irq_wake()-prepared and clock still active and if MEM target were
chosen the device were also enable_irq_wake()-prepared but not clocked
anymore, which is wrong.

Now, if STANDBY target is chosen the device is still clocked with wake
up support enabled, which were the previous default and if MEM target is
chosen the device is declocked with wake up support disabled.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 65e7836..2738352 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -49,6 +49,8 @@ extern int usb_disabled(void);
 
 static void at91_start_clock(void)
 {
+	if (clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(uclk, 48000000);
 		clk_prepare_enable(uclk);
@@ -61,6 +63,8 @@ static void at91_start_clock(void)
 
 static void at91_stop_clock(void)
 {
+	if (!clocked)
+		return;
 	clk_disable_unprepare(fclk);
 	clk_disable_unprepare(iclk);
 	clk_disable_unprepare(hclk);
@@ -597,15 +601,20 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
-	bool		do_wakeup = device_may_wakeup(dev);
+	bool		do_wakeup;
 	int		ret;
 
+	if (at91_suspend_entering_slow_clock())
+		device_init_wakeup(dev, 0);
+
+	do_wakeup = device_may_wakeup(dev);
 	if (do_wakeup)
 		enable_irq_wake(hcd->irq);
 
 	ret = ohci_suspend(hcd, do_wakeup);
 	if (ret) {
-		disable_irq_wake(hcd->irq);
+		if (do_wakeup)
+			disable_irq_wake(hcd->irq);
 		return ret;
 	}
 	/*
@@ -615,7 +624,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 	 *
 	 * REVISIT: some boards will be able to turn VBUS off...
 	 */
-	if (at91_suspend_entering_slow_clock()) {
+	if (!do_wakeup) {
 		ohci->hc_control = ohci_readl(ohci, &ohci->regs->control);
 		ohci->hc_control &= OHCI_CTRL_RWC;
 		ohci_writel(ohci, ohci->hc_control, &ohci->regs->control);
@@ -635,9 +644,9 @@ static int ohci_hcd_at91_drv_resume(struct device *dev)
 
 	if (device_may_wakeup(dev))
 		disable_irq_wake(hcd->irq);
+	device_init_wakeup(dev, 1);
 
-	if (!clocked)
-		at91_start_clock();
+	at91_start_clock();
 
 	ohci_resume(hcd, false);
 	return 0;
-- 
2.1.4

[PATCHv5 5/6] USB: host: ohci-at91: Move global variables to private struct

[Sylvain Rochet]

This patch move AT91 OHCI global variables (clocks ptr and clocked
boolean) to private struct ohci_at91_priv, stored in ohci->priv.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 86 ++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 2738352..e1d0a75 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -33,7 +33,16 @@
 		for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++)
 
 /* interface, function and usb clocks; sometimes also an AHB clock */
-static struct clk *iclk, *fclk, *uclk, *hclk;
+#define hcd_to_ohci_at91_priv(h) \
+	((struct ohci_at91_priv *)hcd_to_ohci(h)->priv)
+
+struct ohci_at91_priv {
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	struct clk *hclk;
+	bool clocked;
+};
 /* interface and function clocks; sometimes also an AHB clock */
 
 #define DRIVER_DESC "OHCI Atmel driver"
@@ -41,49 +50,53 @@ static struct clk *iclk, *fclk, *uclk, *hclk;
 static const char hcd_name[] = "ohci-atmel";
 
 static struct hc_driver __read_mostly ohci_at91_hc_driver;
-static int clocked;
+
+static const struct ohci_driver_overrides ohci_at91_drv_overrides __initconst = {
+	.extra_priv_size = sizeof(struct ohci_at91_priv),
+};
 
 extern int usb_disabled(void);
 
 /*-------------------------------------------------------------------------*/
 
-static void at91_start_clock(void)
+static void at91_start_clock(struct ohci_at91_priv *ohci_at91)
 {
-	if (clocked)
+	if (ohci_at91->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(ohci_at91->uclk, 48000000);
+		clk_prepare_enable(ohci_at91->uclk);
 	}
-	clk_prepare_enable(hclk);
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(ohci_at91->hclk);
+	clk_prepare_enable(ohci_at91->iclk);
+	clk_prepare_enable(ohci_at91->fclk);
+	ohci_at91->clocked = true;
 }
 
-static void at91_stop_clock(void)
+static void at91_stop_clock(struct ohci_at91_priv *ohci_at91)
 {
-	if (!clocked)
+	if (!ohci_at91->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
-	clk_disable_unprepare(hclk);
+	clk_disable_unprepare(ohci_at91->fclk);
+	clk_disable_unprepare(ohci_at91->iclk);
+	clk_disable_unprepare(ohci_at91->hclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(ohci_at91->uclk);
+	ohci_at91->clocked = false;
 }
 
 static void at91_start_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	dev_dbg(&pdev->dev, "start\n");
 
 	/*
 	 * Start the USB clocks.
 	 */
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	/*
 	 * The USB host controller must remain in reset.
@@ -95,6 +108,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	dev_dbg(&pdev->dev, "stop\n");
 
@@ -106,7 +120,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 	/*
 	 * Stop the USB clocks.
 	 */
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 }
 
 
@@ -133,6 +147,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct ohci_hcd *ohci;
 	int retval;
 	struct usb_hcd *hcd = NULL;
+	struct ohci_at91_priv *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	int irq;
@@ -146,6 +161,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd = usb_create_hcd(driver, dev, "at91");
 	if (!hcd)
 		return -ENOMEM;
+	ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(dev, res);
@@ -156,29 +172,29 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(dev, "ohci_clk");
-	if (IS_ERR(iclk)) {
+	ohci_at91->iclk = devm_clk_get(dev, "ohci_clk");
+	if (IS_ERR(ohci_at91->iclk)) {
 		dev_err(dev, "failed to get ohci_clk\n");
-		retval = PTR_ERR(iclk);
+		retval = PTR_ERR(ohci_at91->iclk);
 		goto err;
 	}
-	fclk = devm_clk_get(dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	ohci_at91->fclk = devm_clk_get(dev, "uhpck");
+	if (IS_ERR(ohci_at91->fclk)) {
 		dev_err(dev, "failed to get uhpck\n");
-		retval = PTR_ERR(fclk);
+		retval = PTR_ERR(ohci_at91->fclk);
 		goto err;
 	}
-	hclk = devm_clk_get(dev, "hclk");
-	if (IS_ERR(hclk)) {
+	ohci_at91->hclk = devm_clk_get(dev, "hclk");
+	if (IS_ERR(ohci_at91->hclk)) {
 		dev_err(dev, "failed to get hclk\n");
-		retval = PTR_ERR(hclk);
+		retval = PTR_ERR(ohci_at91->hclk);
 		goto err;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		ohci_at91->uclk = devm_clk_get(dev, "usb_clk");
+		if (IS_ERR(ohci_at91->uclk)) {
 			dev_err(dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(ohci_at91->uclk);
 			goto err;
 		}
 	}
@@ -601,6 +617,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 	bool		do_wakeup;
 	int		ret;
 
@@ -632,7 +649,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 
 		/* flush the writes */
 		(void) ohci_readl (ohci, &ohci->regs->control);
-		at91_stop_clock();
+		at91_stop_clock(ohci_at91);
 	}
 
 	return ret;
@@ -641,12 +658,13 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 static int ohci_hcd_at91_drv_resume(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	if (device_may_wakeup(dev))
 		disable_irq_wake(hcd->irq);
 	device_init_wakeup(dev, 1);
 
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	ohci_resume(hcd, false);
 	return 0;
@@ -673,7 +691,7 @@ static int __init ohci_at91_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-	ohci_init_driver(&ohci_at91_hc_driver, NULL);
+	ohci_init_driver(&ohci_at91_hc_driver, &ohci_at91_drv_overrides);
 
 	/*
 	 * The Atmel HW has some unusual quirks, which require Atmel-specific
-- 
2.1.4

[PATCHv5 6/6] USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack initialisation

[Sylvain Rochet]

struct usb_hcd *hcd = NULL;
...
hcd = usb_create_hcd(driver, dev, "at91");

This patch remove *hcd useless initialisation

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index e1d0a75..bb53231 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -146,7 +146,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct at91_usbh_data *board;
 	struct ohci_hcd *ohci;
 	int retval;
-	struct usb_hcd *hcd = NULL;
+	struct usb_hcd *hcd;
 	struct ohci_at91_priv *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
-- 
2.1.4

[PATCHv5 0/6] USB: host: Atmel OHCI and EHCI drivers improvements

[Nicolas Ferre]

Le 18/01/2015 23:25, Sylvain Rochet a ?crit :
> USB: host: Atmel OHCI and EHCI drivers improvements
> 
> Suspend/resume support for EHCI.
> struct dev_pm_ops for OHCI.
> Removed global variables from both.
> Fixed OHCI wake up support for STANDBY(wake-up enabled) and MEM(wake-up 
> disabled) sleep targets.

I'm okay with the whole series:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Alan,
It seems that Boris and Alexandre also add their tag to the series
already. Do you want us to collect them in a new series sent to you?

Sylvain,
Thanks a lot for this nice series built at a high pace ;-)
Even if I'm not sure to keep the separation between "slow clock" PM and
"normal" PM on AT91 but more likely to use the "slow clock" for all the
PM modes, let's go forward with this step for now. I mean, we will
certainly rework this at91_suspend_entering_slow_clock() aspect in the
future.

Bye,

> Changes since v4:
>   * Re-add at91_suspend_entering_slow_clock() to OHCI, we can't naively
>     remove this one, this device needs to be continuously clocked to
>     provide wake up support.
>     The removal of at91_suspend_entering_slow_clock() actually lighted up
>     an issue on wake up support, which is now fixed.
> 
> Changes since v3:
>   * Using struct dev_pm_ops instead of static struct platform_driver
>     resume and suspend bindings for both EHCI and OHCI
>   * Fixed inconsistency in patch subjects, _ intead of - for file names
>   * Patch cleaning with the help of checkpatch.pl, fixed lines over
>     80 characters
> 
> Changes since v2:
>   * Added patchs from an other submission, because this series
>     depended on this one
>     * EHCI: Move global variables to private struct
>     * OHCI: Move global variables to private struct
>   * Using ohci->priv and ehci->priv instead of hcd->hcd_priv,
>     which were not the right way to do that
> 
> Changes since v1:
>   * Don't use at91_suspend_entering_slow_clock() on EHCI,
>     we are trying to get read of this of this function
>   * Removed at91_suspend_entering_slow_clock() from OHCI
> 
> Sylvain Rochet (6):
>   USB: host: ehci-atmel: Add suspend/resume support
>   USB: host: ohci-at91: Use struct dev_pm_ops instead of struct
>     platform_driver
>   USB: host: ehci-atmel: Move global variables to private struct
>   USB: host: ohci-at91: Fix wake-up support
>   USB: host: ohci-at91: Move global variables to private struct
>   USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
>     initialisation
> 
>  drivers/usb/host/ehci-atmel.c | 102 ++++++++++++++++++++++++++---------
>  drivers/usb/host/ohci-at91.c  | 120 +++++++++++++++++++++++++-----------------
>  2 files changed, 150 insertions(+), 72 deletions(-)
> 


-- 
Nicolas Ferre

[PATCHv5 0/6] USB: host: Atmel OHCI and EHCI drivers improvements

[Sylvain Rochet]

Hello Nicolas,


On Mon, Jan 19, 2015 at 02:34:20PM +0100, Nicolas Ferre wrote:
> Le 18/01/2015 23:25, Sylvain Rochet a ?crit :
> > USB: host: Atmel OHCI and EHCI drivers improvements
> > 
> > Suspend/resume support for EHCI.
> > struct dev_pm_ops for OHCI.
> > Removed global variables from both.
> > Fixed OHCI wake up support for STANDBY(wake-up enabled) and MEM(wake-up 
> > disabled) sleep targets.
> 
> I'm okay with the whole series:
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Oops, you missed that, but there is a v6 I sent this morning, which 
fixes a small mistake I made in v5.


> Alan, It seems that Boris and Alexandre also add their tag to the 
> series already. Do you want us to collect them in a new series sent to 
> you?
> 
> Sylvain,
> Thanks a lot for this nice series built at a high pace ;-)

Yeah, I have the feeling I messed up a "little" :-)

Anyway, v6 is fine, I hope.


> Even if I'm not sure to keep the separation between "slow clock" PM and
> "normal" PM on AT91 but more likely to use the "slow clock" for all the
> PM modes, let's go forward with this step for now. I mean, we will
> certainly rework this at91_suspend_entering_slow_clock() aspect in the
> future.

I agree, I discussed privately with Boris about that, this require a 
larger rework, lets only fix the OHCI wakeup bug for now.


Sylvain

[PATCHv5 0/6] USB: host: Atmel OHCI and EHCI drivers improvements

[Alan Stern]

On Mon, 19 Jan 2015, Nicolas Ferre wrote:

> Alan,
> It seems that Boris and Alexandre also add their tag to the series
> already. Do you want us to collect them in a new series sent to you?

Sylvain has already sent a new series, v6.  I suppose it would make 
life a little easier for Greg KH (who will do the actual merge) if the 
patches are resent with all the relevant tags, after I have a chance to 
look through them in detail.

Alan Stern

[PATCHv4 4/6] USB: host: ehci-atmel: Move global variables to private struct

[Sylvain Rochet]

This patch move Atmel EHCI global variables (clocks ptr and clocked
boolean) to private struct atmel_ehci_priv, stored in ehci->priv.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ehci-atmel.c | 79 +++++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 5a15e3d..663f790 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -27,48 +27,66 @@
 #define DRIVER_DESC "EHCI Atmel driver"
 
 static const char hcd_name[] = "ehci-atmel";
-static struct hc_driver __read_mostly ehci_atmel_hc_driver;
 
 /* interface and function clocks */
-static struct clk *iclk, *fclk, *uclk;
-static int clocked;
+#define hcd_to_atmel_ehci_priv(h) \
+	((struct atmel_ehci_priv *)hcd_to_ehci(h)->priv)
+
+struct atmel_ehci_priv {
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	bool clocked;
+};
+
+static struct hc_driver __read_mostly ehci_atmel_hc_driver;
+
+static const struct ehci_driver_overrides ehci_atmel_drv_overrides __initconst = {
+	.extra_priv_size = sizeof(struct atmel_ehci_priv),
+};
 
 /*-------------------------------------------------------------------------*/
 
-static void atmel_start_clock(void)
+static void atmel_start_clock(struct atmel_ehci_priv *atmel_ehci)
 {
-	if (clocked)
+	if (atmel_ehci->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(atmel_ehci->uclk, 48000000);
+		clk_prepare_enable(atmel_ehci->uclk);
 	}
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(atmel_ehci->iclk);
+	clk_prepare_enable(atmel_ehci->fclk);
+	atmel_ehci->clocked = true;
 }
 
-static void atmel_stop_clock(void)
+static void atmel_stop_clock(struct atmel_ehci_priv *atmel_ehci)
 {
-	if (!clocked)
+	if (!atmel_ehci->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
+	clk_disable_unprepare(atmel_ehci->fclk);
+	clk_disable_unprepare(atmel_ehci->iclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(atmel_ehci->uclk);
+	atmel_ehci->clocked = false;
 }
 
 static void atmel_start_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
+
 	dev_dbg(&pdev->dev, "start\n");
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 }
 
 static void atmel_stop_ehci(struct platform_device *pdev)
 {
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
+
 	dev_dbg(&pdev->dev, "stop\n");
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -79,6 +97,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	const struct hc_driver *driver = &ehci_atmel_hc_driver;
 	struct resource *res;
 	struct ehci_hcd *ehci;
+	struct atmel_ehci_priv *atmel_ehci;
 	int irq;
 	int retval;
 
@@ -109,6 +128,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 		retval = -ENOMEM;
 		goto fail_create_hcd;
 	}
+	atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -120,23 +140,23 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(&pdev->dev, "ehci_clk");
-	if (IS_ERR(iclk)) {
+	atmel_ehci->iclk = devm_clk_get(&pdev->dev, "ehci_clk");
+	if (IS_ERR(atmel_ehci->iclk)) {
 		dev_err(&pdev->dev, "Error getting interface clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
-	fclk = devm_clk_get(&pdev->dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	atmel_ehci->fclk = devm_clk_get(&pdev->dev, "uhpck");
+	if (IS_ERR(atmel_ehci->fclk)) {
 		dev_err(&pdev->dev, "Error getting function clock\n");
 		retval = -ENOENT;
 		goto fail_request_resource;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(&pdev->dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		atmel_ehci->uclk = devm_clk_get(&pdev->dev, "usb_clk");
+		if (IS_ERR(atmel_ehci->uclk)) {
 			dev_err(&pdev->dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(atmel_ehci->uclk);
 			goto fail_request_resource;
 		}
 	}
@@ -173,7 +193,6 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 	usb_put_hcd(hcd);
 
 	atmel_stop_ehci(pdev);
-	fclk = iclk = NULL;
 
 	return 0;
 }
@@ -182,21 +201,23 @@ static int ehci_atmel_drv_remove(struct platform_device *pdev)
 static int ehci_atmel_drv_suspend(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 	int ret;
 
 	ret = ehci_suspend(hcd, false);
 	if (ret)
 		return ret;
 
-	atmel_stop_clock();
+	atmel_stop_clock(atmel_ehci);
 	return 0;
 }
 
 static int ehci_atmel_drv_resume(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd);
 
-	atmel_start_clock();
+	atmel_start_clock(atmel_ehci);
 	return ehci_resume(hcd, false);
 }
 #endif
@@ -230,7 +251,7 @@ static int __init ehci_atmel_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-	ehci_init_driver(&ehci_atmel_hc_driver, NULL);
+	ehci_init_driver(&ehci_atmel_hc_driver, &ehci_atmel_drv_overrides);
 	return platform_driver_register(&ehci_atmel_driver);
 }
 module_init(ehci_atmel_init);
-- 
2.1.4

[PATCHv4 5/6] USB: host: ohci-at91: Move global variables to private struct

[Sylvain Rochet]

This patch move AT91 OHCI global variables (clocks ptr and clocked
boolean) to private struct ohci_at91_priv, stored in ohci->priv.

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 86 ++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 79e343e..2e50d3b 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -33,7 +33,16 @@
 		for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++)
 
 /* interface, function and usb clocks; sometimes also an AHB clock */
-static struct clk *iclk, *fclk, *uclk, *hclk;
+#define hcd_to_ohci_at91_priv(h) \
+	((struct ohci_at91_priv *)hcd_to_ohci(h)->priv)
+
+struct ohci_at91_priv {
+	struct clk *iclk;
+	struct clk *fclk;
+	struct clk *uclk;
+	struct clk *hclk;
+	bool clocked;
+};
 /* interface and function clocks; sometimes also an AHB clock */
 
 #define DRIVER_DESC "OHCI Atmel driver"
@@ -41,49 +50,53 @@ static struct clk *iclk, *fclk, *uclk, *hclk;
 static const char hcd_name[] = "ohci-atmel";
 
 static struct hc_driver __read_mostly ohci_at91_hc_driver;
-static int clocked;
+
+static const struct ohci_driver_overrides ohci_at91_drv_overrides __initconst = {
+	.extra_priv_size = sizeof(struct ohci_at91_priv),
+};
 
 extern int usb_disabled(void);
 
 /*-------------------------------------------------------------------------*/
 
-static void at91_start_clock(void)
+static void at91_start_clock(struct ohci_at91_priv *ohci_at91)
 {
-	if (clocked)
+	if (ohci_at91->clocked)
 		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		clk_set_rate(uclk, 48000000);
-		clk_prepare_enable(uclk);
+		clk_set_rate(ohci_at91->uclk, 48000000);
+		clk_prepare_enable(ohci_at91->uclk);
 	}
-	clk_prepare_enable(hclk);
-	clk_prepare_enable(iclk);
-	clk_prepare_enable(fclk);
-	clocked = 1;
+	clk_prepare_enable(ohci_at91->hclk);
+	clk_prepare_enable(ohci_at91->iclk);
+	clk_prepare_enable(ohci_at91->fclk);
+	ohci_at91->clocked = true;
 }
 
-static void at91_stop_clock(void)
+static void at91_stop_clock(struct ohci_at91_priv *ohci_at91)
 {
-	if (!clocked)
+	if (!ohci_at91->clocked)
 		return;
-	clk_disable_unprepare(fclk);
-	clk_disable_unprepare(iclk);
-	clk_disable_unprepare(hclk);
+	clk_disable_unprepare(ohci_at91->fclk);
+	clk_disable_unprepare(ohci_at91->iclk);
+	clk_disable_unprepare(ohci_at91->hclk);
 	if (IS_ENABLED(CONFIG_COMMON_CLK))
-		clk_disable_unprepare(uclk);
-	clocked = 0;
+		clk_disable_unprepare(ohci_at91->uclk);
+	ohci_at91->clocked = false;
 }
 
 static void at91_start_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	dev_dbg(&pdev->dev, "start\n");
 
 	/*
 	 * Start the USB clocks.
 	 */
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	/*
 	 * The USB host controller must remain in reset.
@@ -95,6 +108,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_regs __iomem *regs = hcd->regs;
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	dev_dbg(&pdev->dev, "stop\n");
 
@@ -106,7 +120,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 	/*
 	 * Stop the USB clocks.
 	 */
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 }
 
 
@@ -133,6 +147,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct ohci_hcd *ohci;
 	int retval;
 	struct usb_hcd *hcd = NULL;
+	struct ohci_at91_priv *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	int irq;
@@ -146,6 +161,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd = usb_create_hcd(driver, dev, "at91");
 	if (!hcd)
 		return -ENOMEM;
+	ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hcd->regs = devm_ioremap_resource(dev, res);
@@ -156,29 +172,29 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
 
-	iclk = devm_clk_get(dev, "ohci_clk");
-	if (IS_ERR(iclk)) {
+	ohci_at91->iclk = devm_clk_get(dev, "ohci_clk");
+	if (IS_ERR(ohci_at91->iclk)) {
 		dev_err(dev, "failed to get ohci_clk\n");
-		retval = PTR_ERR(iclk);
+		retval = PTR_ERR(ohci_at91->iclk);
 		goto err;
 	}
-	fclk = devm_clk_get(dev, "uhpck");
-	if (IS_ERR(fclk)) {
+	ohci_at91->fclk = devm_clk_get(dev, "uhpck");
+	if (IS_ERR(ohci_at91->fclk)) {
 		dev_err(dev, "failed to get uhpck\n");
-		retval = PTR_ERR(fclk);
+		retval = PTR_ERR(ohci_at91->fclk);
 		goto err;
 	}
-	hclk = devm_clk_get(dev, "hclk");
-	if (IS_ERR(hclk)) {
+	ohci_at91->hclk = devm_clk_get(dev, "hclk");
+	if (IS_ERR(ohci_at91->hclk)) {
 		dev_err(dev, "failed to get hclk\n");
-		retval = PTR_ERR(hclk);
+		retval = PTR_ERR(ohci_at91->hclk);
 		goto err;
 	}
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
-		uclk = devm_clk_get(dev, "usb_clk");
-		if (IS_ERR(uclk)) {
+		ohci_at91->uclk = devm_clk_get(dev, "usb_clk");
+		if (IS_ERR(ohci_at91->uclk)) {
 			dev_err(dev, "failed to get uclk\n");
-			retval = PTR_ERR(uclk);
+			retval = PTR_ERR(ohci_at91->uclk);
 			goto err;
 		}
 	}
@@ -601,6 +617,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 	bool		do_wakeup = device_may_wakeup(dev);
 	int		ret;
 
@@ -626,7 +643,7 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 
 	/* flush the writes */
 	(void) ohci_readl (ohci, &ohci->regs->control);
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 
 	return ret;
 }
@@ -634,11 +651,12 @@ ohci_hcd_at91_drv_suspend(struct device *dev)
 static int ohci_hcd_at91_drv_resume(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
+	struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd);
 
 	if (device_may_wakeup(dev))
 		disable_irq_wake(hcd->irq);
 
-	at91_start_clock();
+	at91_start_clock(ohci_at91);
 
 	ohci_resume(hcd, false);
 	return 0;
@@ -665,7 +683,7 @@ static int __init ohci_at91_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
-	ohci_init_driver(&ohci_at91_hc_driver, NULL);
+	ohci_init_driver(&ohci_at91_hc_driver, &ohci_at91_drv_overrides);
 
 	/*
 	 * The Atmel HW has some unusual quirks, which require Atmel-specific
-- 
2.1.4

[PATCHv4 6/6] USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack initialisation

[Sylvain Rochet]

struct usb_hcd *hcd = NULL;
...
hcd = usb_create_hcd(driver, dev, "at91");

This patch remove *hcd useless initialisation

Signed-off-by: Sylvain Rochet <sylvain.rochet@finsecur.com>
---
 drivers/usb/host/ohci-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 2e50d3b..8edc5df 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -146,7 +146,7 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver,
 	struct at91_usbh_data *board;
 	struct ohci_hcd *ohci;
 	int retval;
-	struct usb_hcd *hcd = NULL;
+	struct usb_hcd *hcd;
 	struct ohci_at91_priv *ohci_at91;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
-- 
2.1.4

[PATCHv3 0/5] USB: host: Atmel OHCI and EHCI drivers improvements

[Sylvain Rochet]

Hello Alan,

On Sun, Jan 18, 2015 at 12:20:49PM -0500, Alan Stern wrote:
> On Sat, 17 Jan 2015, Sylvain Rochet wrote:
> 
> > Sylvain Rochet (5):
> >   USB: host: ehci_atmel: Add suspend/resume support
> >   USB: host: ohci_at91: Stop/start USB PLL for all sleep modes
> >   USB: host: ehci_atmel: Move global variables to private struct
> >   USB: host: ohci_at91: Move global variables to private struct
> >   USB: host: ohci_at91: usb_hcd_at91_probe(), remove useless stack
> >     initialisation
> > 
> >  drivers/usb/host/ehci-atmel.c | 102 +++++++++++++++++++++++++++++++----------
> >  drivers/usb/host/ohci-at91.c  | 104 +++++++++++++++++++++++++-----------------
> >  2 files changed, 138 insertions(+), 68 deletions(-)
> 
> These patches look pretty good to me.  Have you run them through 
> checkpatch.pl?

Just did, fixed some lines over 80 characters + Sergei suggestion about 
using struct dev_pm_ops instead of struct platform_driver.{resume,suspend}.

Sylvain

[PATCH 1/3] USB: host: ehci_atmel: Move global variables to private struct

[Sylvain Rochet]

Hello Alan,

On Sat, Jan 17, 2015 at 03:30:45PM -0500, Alan Stern wrote:
> 
> This is not the right way to do it.  For an example of the right way, 
> see the ehci-platform.c file.  Your private structure is stored in 
> ehci->priv, and its size is specified by the .extra_priv_size member of 
> an ehci_driver_overrides structure passed to ehci_init_driver().

Dammit? reworked this way, thanks for the heads-up !

Sylvain

[PATCH] USB: host: ehci_atmel: Add suspend/resume support

[Alexandre Belloni]

On 17/01/2015 at 17:03:04 +0100, Sylvain Rochet wrote :
> Should I have added the "Acked-by: Alexandre Belloni 
> <alexandre.belloni@free-electrons.com>" on 1/2 ?
> 

Yes, you could. Just a small remark, you send both patches as separate
in v1, you should have kept them separate so it is easier to understand
what v2 is relating to. Also, I don't think you need to use
--in-reply-to when sending v2.

Finally, please including a changelog after the '---' marker in each patch
or in the cover letter.

Those are simply small things and your work is great.

> As a side note, unfortunately AT91 slow clock mode only work on "recent" 
> boards with Wenyou changes on linux4sam/linux-at91/linux-3.10-at91 
> branch and is why I am still stuck to this branch (and the HLCDC FB 
> driver, but this is going to be merged soon from what I saw, YAY !).
> 

Work is in progress to get that in 3.20 or 3.21.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com