Re: [U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls
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 GoedeLGTM 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls
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 GoedeLGTM 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] sunxi: musb: Re-init musb controller on repeated probe calls
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--- 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 */ 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(_plat, NULL, - (void *)SUNXI_USB0_BASE); - } - - host->host = sunxi_musb; + host->host = musb_init_controller(_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(>ahb_gate0, 1 << AHB_GATE_OFFSET_USB0); + free(host->host); + host->host = NULL; + return 0; } -- 2.9.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot