USB: host: Atmel OHCI and EHCI drivers improvements

PATCH Superseded Sylvain Rochet 4 participants 10 messages Jan 19, 2015 – Jan 20, 2015

Subsystems (4)

USB Arm/Microchip (AT91) SoC support USB ehci driver USB ohci driver

lore

Share & tools

lore

Sylvain Rochet

author

2015-01-19 11:06 UTC #0 lore raw

v6 cover USB: host: Atmel OHCI and EHCI drivers improvements

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 v5:
* Don't overwrite device wakeup flag with device_init_wakeup(), now using a private wakeup bool instead.

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: Move global variables to private struct
USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
initialisation
USB: host: ohci-at91: Fix wake-up support

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

Signature · 1 line

2.1.4

Sylvain Rochet

author

2015-01-19 11:06 UTC #1 lore raw

PATCH v6 1/6 USB: host: ehci-atmel: Add suspend/resume support

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

---
 drivers/usb/host/ehci-atmel.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

drivers/usb/host/ehci-atmel.c +31 -0

index 56a8850..5a15e3d 100644
@@ -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),
 	},
 };

Signature · 1 line

2.1.4

Sylvain Rochet

author

2015-01-19 11:06 UTC #2 lore raw

PATCH v6 2/6 USB: host: ohci-at91: Use struct dev_pm_ops instead of struct platform_driver

In reply to Sylvain Rochet

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

Signed-off-by Sylvain Rochet <[email protected]>

---
 drivers/usb/host/ohci-at91.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

drivers/usb/host/ohci-at91.c +10 ~~-11~~

index dc9e4e6..65e7836 100644
@@ -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),
 	},
 };

Signature · 1 line

2.1.4

Sylvain Rochet

author

2015-01-19 11:06 UTC #3 lore raw

PATCH v6 3/6 USB: host: ehci-atmel: Move global variables to private struct

In reply to 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 <[email protected]>

---
 drivers/usb/host/ehci-atmel.c | 79 +++++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 29 deletions(-)

drivers/usb/host/ehci-atmel.c +50 ~~-29~~

index 5a15e3d..663f790 100644
@@ -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);

Signature · 1 line

2.1.4

Sylvain Rochet

author

2015-01-19 11:06 UTC #4 lore raw

PATCH v6 4/6 USB: host: ohci-at91: Move global variables to private struct

In reply to 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 <[email protected]>

---
 drivers/usb/host/ohci-at91.c | 84 +++++++++++++++++++++++++++-----------------
 1 file changed, 51 insertions(+), 33 deletions(-)

drivers/usb/host/ohci-at91.c +51 ~~-33~~

index 65e7836..c810917 100644
@@ -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,45 +50,49 @@ 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 (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)
 {
-	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.

@@ -91,6 +104,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");

@@ -102,7 +116,7 @@ static void at91_stop_hc(struct platform_device *pdev)
 	/*
 	 * Stop the USB clocks.
 	 */
-	at91_stop_clock();
+	at91_stop_clock(ohci_at91);
 }

@@ -129,6 +143,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;

@@ -142,6 +157,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);

@@ -152,29 +168,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;
 		}
 	}

@@ -597,6 +613,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;

@@ -623,7 +640,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;

@@ -632,12 +649,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);
 
-	if (!clocked)
-		at91_start_clock();
+	if (!ohci_at91->clocked)
+		at91_start_clock(ohci_at91);
 
 	ohci_resume(hcd, false);
 	return 0;

@@ -664,7 +682,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

Signature · 1 line

2.1.4

Sylvain Rochet

author

2015-01-19 11:06 UTC #5 lore raw

PATCH v6 5/6 USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack initialisation

In reply to 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 <[email protected]>

---
 drivers/usb/host/ohci-at91.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/ohci-at91.c +1 -1

index c810917..970a776 100644
@@ -142,7 +142,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;

Signature · 1 line

2.1.4

Sylvain Rochet

author

2015-01-19 11:06 UTC #6 lore raw

PATCH v6 6/6 USB: host: ohci-at91: Fix wake-up support

In reply to 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 <[email protected]>

---
 drivers/usb/host/ohci-at91.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

drivers/usb/host/ohci-at91.c +19 -8

index 970a776..1fc5610 100644
@@ -42,6 +42,7 @@ struct ohci_at91_priv {
 	struct clk *uclk;
 	struct clk *hclk;
 	bool clocked;
+	bool wakeup;		/* Saved wake-up state for resume */
 };
 /* interface and function clocks; sometimes also an AHB clock */

@@ -61,6 +62,8 @@ extern int usb_disabled(void);
 
 static void at91_start_clock(struct ohci_at91_priv *ohci_at91)
 {
+	if (ohci_at91->clocked)
+		return;
 	if (IS_ENABLED(CONFIG_COMMON_CLK)) {
 		clk_set_rate(ohci_at91->uclk, 48000000);
 		clk_prepare_enable(ohci_at91->uclk);

@@ -73,6 +76,8 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91)
 
 static void at91_stop_clock(struct ohci_at91_priv *ohci_at91)
 {
+	if (!ohci_at91->clocked)
+		return;
 	clk_disable_unprepare(ohci_at91->fclk);
 	clk_disable_unprepare(ohci_at91->iclk);
 	clk_disable_unprepare(ohci_at91->hclk);

@@ -614,15 +619,22 @@ 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;
 
-	if (do_wakeup)
+	/*
+	 * Disable wakeup if we are going to sleep with slow clock mode
+	 * enabled.
+	 */
+	ohci_at91->wakeup = device_may_wakeup(dev)
+			&& !at91_suspend_entering_slow_clock();
+
+	if (ohci_at91->wakeup)
 		enable_irq_wake(hcd->irq);
 
-	ret = ohci_suspend(hcd, do_wakeup);
+	ret = ohci_suspend(hcd, ohci_at91->wakeup);
 	if (ret) {
-		disable_irq_wake(hcd->irq);
+		if (ohci_at91->wakeup)
+			disable_irq_wake(hcd->irq);
 		return ret;
 	}
 	/*

@@ -632,7 +644,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 (!ohci_at91->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);

@@ -651,11 +663,10 @@ 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))
+	if (ohci_at91->wakeup)
 		disable_irq_wake(hcd->irq);
 
-	if (!ohci_at91->clocked)
-		at91_start_clock(ohci_at91);
+	at91_start_clock(ohci_at91);
 
 	ohci_resume(hcd, false);
 	return 0;

Signature · 1 line

2.1.4

Alan Stern

maintainer

2015-01-19 16:55 UTC #7 lore raw

v6 cover USB: host: Atmel OHCI and EHCI drivers improvements

In reply to Sylvain Rochet

Quoted 49 lines · 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 v5:
* Don't overwrite device wakeup flag with device_init_wakeup(), now using a private wakeup bool instead.
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: Move global variables to private struct
USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
initialisation
USB: host: ohci-at91: Fix wake-up support

Quoted 3 lines

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

For all 6 patches:

Acked-by Alan Stern <[email protected]>

Alan Stern

Boris Brezillon

2015-01-20 10:51 UTC #8 lore raw

v6 cover USB: host: Atmel OHCI and EHCI drivers improvements

In reply to Sylvain Rochet

Quoted 7 lines · On Mon, 19 Jan 2015 12:06:01 +0100 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.

To the whole series:

Acked-by Boris Brezillon <[email protected]>

Quoted 46 lines

Changes since v5:
* Don't overwrite device wakeup flag with device_init_wakeup(), now using a private wakeup bool instead.
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: Move global variables to private struct
USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
initialisation
USB: host: ohci-at91: Fix wake-up support
 drivers/usb/host/ehci-atmel.c | 102 +++++++++++++++++++++++++---------
 drivers/usb/host/ohci-at91.c  | 126 ++++++++++++++++++++++++++----------------
 2 files changed, 154 insertions(+), 74 deletions(-)

Signature · 3 lines

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

Nicolas Ferre

2015-01-20 13:01 UTC #9 lore raw

v6 cover USB: host: Atmel OHCI and EHCI drivers improvements

In reply to Sylvain Rochet

Le 19/01/2015 12:06, Sylvain Rochet a ?crit :

Quoted 49 lines

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 v5:
* Don't overwrite device wakeup flag with device_init_wakeup(), now using a private wakeup bool instead.
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: Move global variables to private struct
USB: host: ohci-at91: usb_hcd_at91_probe(), remove useless stack
initialisation
USB: host: ohci-at91: Fix wake-up support

Quoted 3 lines

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

As already mentioned for the v5, I'm okay with the whole series:

Acked-by Nicolas Ferre <[email protected]>

So Sylvain, as said by Alan, I think you can now collect all the "Acked-by" tags (Alex had also sent some if I recall well) and re-send a clean v7 series to Greg KH.

BTW, for rewriting this series' logs, I suggest you to use:
"git filter-branch -f --msg-filter ..." ;-)

Bye,

Signature · 1 line

Nicolas Ferre