usb: uhci: Add clk support to uhci-platform

Hello!

Quoted 89 lines · Benjamin Herrenschmidt

Quoted 89 lines

Quoted 89 lines

The Aspeed SoCs use uhci-platform. With the new dynamic clock control framework, the corresponding IP block clock must be properly enabled.

This is a simplified variant of what ehci-platform does, it looks for *one* clock attached to the device, and if it's there, enables it.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

Acked-by Alan Stern <[email protected]>

---

v4. Remove unnecessary NULL checks

v3. Fix tab vs. spaces and probe error path

v2. Don't forget to git add latest changes :-) This adds the
part where we turn the clock off when removing the driver.

---
   drivers/usb/host/uhci-hcd.h      |  3 +++
   drivers/usb/host/uhci-platform.c | 20 ++++++++++++++++++--
   2 files changed, 21 insertions(+), 2 deletions(-)

drivers/usb/host/uhci-hcd.h +3 -0

index f1cc47292a59..7f9f33c8c232 100644
@@ -4,6 +4,7 @@
 
   #include <linux/list.h>
   #include <linux/usb.h>
+#include <linux/clk.h>
 
   #define usb_packetid(pipe)	(usb_pipein(pipe) ? USB_PID_IN : USB_PID_OUT)
   #define PIPE_DEVEP_MASK		0x0007ff00

@@ -447,6 +448,8 @@ struct uhci_hcd {
   	int total_load;				/* Sum of array values */
   	short load[MAX_PHASE];			/* Periodic allocations */
 
+	struct clk *clk;			/* (optional) clock source */
+
   	/* Reset host controller */
   	void	(*reset_hc) (struct uhci_hcd *uhci);
   	int	(*check_and_reset_hc) (struct uhci_hcd *uhci);

drivers/usb/host/uhci-platform.c +15 -2

index 6cb16d4b2257..55c2d99bf7ec 100644
@@ -89,6 +89,8 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
   	if (!hcd)
   		return -ENOMEM;
 
+	uhci = hcd_to_uhci(hcd);
+
   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
   	if (IS_ERR(hcd->regs)) {

@@ -98,8 +100,6 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
   	hcd->rsrc_start = res->start;
   	hcd->rsrc_len = resource_size(res);
 
-	uhci = hcd_to_uhci(hcd);
-
   	uhci->regs = hcd->regs;
 
   	/* Grab some things from the device-tree */

@@ -119,6 +119,19 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
   				 "Enabled Aspeed implementation workarounds\n");
   		}
   	}
+
+	/* Get and enable clock if any specified */
+	uhci->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(uhci->clk)) {
+		ret = PTR_ERR(uhci->clk);
+		goto err_rmr;
+	}
+	ret = clk_prepare_enable(uhci->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Error couldn't enable clock (%d)\n", ret);
+		goto err_rmr;
+	}
+
   	ret = usb_add_hcd(hcd, pdev->resource[1].start, IRQF_SHARED);
   	if (ret)
   		goto err_rmr;

@@ -127,6 +140,7 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
   	return 0;
 

Quoted 7 lines

Quoted 5 lines

drivers/usb/host/uhci-platform.c static int uhci_hcd_platform_probe(struct platform_device *pdev) · line 143

Quoted 2 lines

drivers/usb/host/uhci-platform.c +1 -0

   err_rmr:
+	clk_disable_unprepare(uhci->clk);

Really? You haven't enabled the clock before either *goto* above, why
disable it here?

If clk is NULL or an error, disable will do nothing. The only case

The rule of thumb was that you don't pass the error pointers to APIs but
indeed I'm now seeing the NULL check extended to the error pointers too...

Quoted 2 lines

would be if enable at failed but then disable will do nothing in that case.

Are you sure there'll be no WARN_ON() triggered by clk_disable() with the
enable count == 0?

Quoted 1 line

The above change was done at the suggestion of Alan. If you really

I'd probably missed his mail.

Quoted 2 lines

prefer I could add a clk_put and clearing of "clk" in the error path of clk_prepare_enable, but that's about it.