[U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls

[U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls

[Hans de Goede]

With sunxi-musb musb_lowlevel_init() can fail when a charger; or no cable
is plugged into the otg port.

To avoid leaking the struct musb allocated by musb_init_controller()
on repeated musb_usb_probe() calls, we were caching its result.
But musb_init_controller() does more, such as calling sunxi_musb_init()
which enables the clocks.

Not calling sunxi_musb_init() causes the musb controller to stop working
after a "usb reset" since that calls musb_usb_remove() which disables the
clocks.

This commit fixes this by removing the caching of the struct returned
from musb_init_controller(), it replaces this by free-ing the allocated
memory in musb_usb_remove() and calling musb_usb_remove() on
musb_usb_probe() errors to ensure proper cleanup.

While at it also make musb_usb_probe() and musb_usb_remove() static.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/sunxi.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index dece781..469377f 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -201,7 +201,6 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
 
 /* musb_core does not call enable / disable in a balanced manner <sigh> */
 static bool enabled = false;
-static struct musb *sunxi_musb;
 
 static int sunxi_musb_enable(struct musb *musb)
 {
@@ -309,7 +308,9 @@ static struct musb_hdrc_platform_data musb_plat = {
 };
 
 #ifdef CONFIG_USB_MUSB_HOST
-int musb_usb_probe(struct udevice *dev)
+static int musb_usb_remove(struct udevice *dev);
+
+static int musb_usb_probe(struct udevice *dev)
 {
 	struct musb_host_data *host = dev_get_priv(dev);
 	struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
@@ -317,23 +318,21 @@ int musb_usb_probe(struct udevice *dev)
 
 	priv->desc_before_addr = true;
 
-	if (!sunxi_musb) {
-		sunxi_musb = musb_init_controller(&musb_plat, NULL,
-						  (void *)SUNXI_USB0_BASE);
-	}
-
-	host->host = sunxi_musb;
+	host->host = musb_init_controller(&musb_plat, NULL,
+					  (void *)SUNXI_USB0_BASE);
 	if (!host->host)
 		return -EIO;
 
 	ret = musb_lowlevel_init(host);
 	if (ret == 0)
 		printf("MUSB OTG\n");
+	else
+		musb_usb_remove(dev);
 
 	return ret;
 }
 
-int musb_usb_remove(struct udevice *dev)
+static int musb_usb_remove(struct udevice *dev)
 {
 	struct musb_host_data *host = dev_get_priv(dev);
 	struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
@@ -346,6 +345,9 @@ int musb_usb_remove(struct udevice *dev)
 #endif
 	clrbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_USB0);
 
+	free(host->host);
+	host->host = NULL;
+
 	return 0;
 }
 
-- 
2.9.3

[U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls

[Ian Campbell]

On Sun, 2016-09-18 at 16:53 +0200, Hans de Goede wrote:
> With sunxi-musb musb_lowlevel_init() can fail when a charger; or no
> cable
> is plugged into the otg port.
> 
> To avoid leaking the struct musb allocated by musb_init_controller()
> on repeated musb_usb_probe() calls, we were caching its result.
> But musb_init_controller() does more, such as calling
> sunxi_musb_init()
> which enables the clocks.
> 
> Not calling sunxi_musb_init() causes the musb controller to stop
> working
> after a "usb reset" since that calls musb_usb_remove() which disables
> the
> clocks.
> 
> This commit fixes this by removing the caching of the struct returned
> from musb_init_controller(), it replaces this by free-ing the
> allocated
> memory in musb_usb_remove() and calling musb_usb_remove() on
> musb_usb_probe() errors to ensure proper cleanup.
> 
> While at it also make musb_usb_probe() and musb_usb_remove() static.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

LGTM in so far as it seems like a more normal way to do things, but
maybe an Ack from a USB-in-uboot-savvy person might be nice?

Anyway: Acked-by: Ian Campbell <ijc@hellion.org.uk>

[U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls

[Hans de Goede]

Hi,

On 18-09-16 17:00, Ian Campbell wrote:
> On Sun, 2016-09-18 at 16:53 +0200, Hans de Goede wrote:
>> With sunxi-musb musb_lowlevel_init() can fail when a charger; or no
>> cable
>> is plugged into the otg port.
>>
>> To avoid leaking the struct musb allocated by musb_init_controller()
>> on repeated musb_usb_probe() calls, we were caching its result.
>> But musb_init_controller() does more, such as calling
>> sunxi_musb_init()
>> which enables the clocks.
>>
>> Not calling sunxi_musb_init() causes the musb controller to stop
>> working
>> after a "usb reset" since that calls musb_usb_remove() which disables
>> the
>> clocks.
>>
>> This commit fixes this by removing the caching of the struct returned
>> from musb_init_controller(), it replaces this by free-ing the
>> allocated
>> memory in musb_usb_remove() and calling musb_usb_remove() on
>> musb_usb_probe() errors to ensure proper cleanup.
>>
>> While at it also make musb_usb_probe() and musb_usb_remove() static.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> LGTM in so far as it seems like a more normal way to do things,

Yeah my original hack here was bad, and something which I should
never have done, ain't hindsight a wonderful thing :)

Regards,

Hans