[U-Boot] [PATCH 1/3] musb: sunxi: Do not allocate musb struct multiple times

[U-Boot] [PATCH 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Hans de Goede]

The probe function of the musb host driver can be called multiple
times. The code assumes that it can safe the pointer to the allocated
musb struct in the driver model priv_auto_alloc data, but this data
gets free-ed on a probe failure or on removal, so we must safe the
pointer elsewhere.

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

diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index be1d2ec..3081afc 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -201,6 +201,7 @@ 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)
 {
@@ -320,13 +321,15 @@ int musb_usb_probe(struct udevice *dev)
 
 	priv->desc_before_addr = true;
 
-	if (!host->host) {
-		host->host = musb_init_controller(&musb_plat, NULL,
+	if (!sunxi_musb) {
+		sunxi_musb = musb_init_controller(&musb_plat, NULL,
 						  (void *)SUNXI_USB0_BASE);
-		if (!host->host)
-			return -EIO;
 	}
 
+	host->host = sunxi_musb;
+	if (!host->host)
+		return -EIO;
+
 	ret = musb_lowlevel_init(host);
 	if (ret == 0)
 		printf("MUSB OTG\n");
-- 
2.7.2

[U-Boot] [PATCH 2/3] musb: Properly call musb_stop() on probe failure

[Hans de Goede]

musb_lowlevelinit(): if no device is plugged in / detected call
musb_stop() to undo the preceding musb_start() call.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/musb-new/musb_uboot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 233a0e4..6ce528c 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -237,8 +237,10 @@ int musb_lowlevel_init(struct musb_host_data *host)
 		if (musb_readb(mbase, MUSB_DEVCTL) & MUSB_DEVCTL_HM)
 			break;
 	} while (get_timer(0) < timeout);
-	if (get_timer(0) >= timeout)
+	if (get_timer(0) >= timeout) {
+		musb_stop(host->host);
 		return -ENODEV;
+	}
 
 	_musb_reset_root_port(host, NULL);
 	host->host->is_active = 1;
-- 
2.7.2

[U-Boot] [PATCH 3/3] dm: usb: Do not reprobe usb hosts on "usb tree" command

[Hans de Goede]

Some usb hosts may have failed to probe on "usb start", i.e. an otg
host without an otg-host cable plugged in.

"usb tree" would cause the probe method of these hosts to get called
again, something which should only happen on "usb reset".

This commit fixes this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 cmd/usb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/cmd/usb.c b/cmd/usb.c
index 9ed5dc6..8741088 100644
--- a/cmd/usb.c
+++ b/cmd/usb.c
@@ -442,12 +442,15 @@ void usb_show_tree(void)
 #ifdef CONFIG_DM_USB
 	struct udevice *bus;
 
-	for (uclass_first_device(UCLASS_USB, &bus);
+	for (uclass_find_first_device(UCLASS_USB, &bus);
 		bus;
-		uclass_next_device(&bus)) {
+		uclass_find_next_device(&bus)) {
 		struct usb_device *udev;
 		struct udevice *dev;
 
+		if (!device_active(bus))
+			continue;
+
 		device_find_first_child(bus, &dev);
 		if (dev && device_active(dev)) {
 			udev = dev_get_parent_priv(dev);
-- 
2.7.2

[U-Boot] [PATCH 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Peter Korsgaard]

>>>>> "Hans" == Hans de Goede <hdegoede@redhat.com> writes:

 > The probe function of the musb host driver can be called multiple
 > times. The code assumes that it can safe the pointer to the allocated
 > musb struct in the driver model priv_auto_alloc data, but this data
 > gets free-ed on a probe failure or on removal, so we must safe the
 > pointer elsewhere.

s/safe/save/ (twice).

Otherwise it looks sensibe to me.

-- 
Bye, Peter Korsgaard

[U-Boot] [PATCH 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Hans de Goede]

Hi,

On 04/02/2016 08:57 PM, Peter Korsgaard wrote:
>>>>>> "Hans" == Hans de Goede <hdegoede@redhat.com> writes:
>
>   > The probe function of the musb host driver can be called multiple
>   > times. The code assumes that it can safe the pointer to the allocated
>   > musb struct in the driver model priv_auto_alloc data, but this data
>   > gets free-ed on a probe failure or on removal, so we must safe the
>   > pointer elsewhere.
>
> s/safe/save/ (twice).

Thanks, fixed in my tree.

Regards,

Hansd

[U-Boot] [PATCH 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Ian Campbell]

On Sat, 2016-04-02 at 23:39 +0200, Hans de Goede wrote:
> Hi,
> 
> On 04/02/2016 08:57 PM, Peter Korsgaard wrote:
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > "Hans" == Hans de Goede <hdegoede@redhat.com> writes:
> > ? > The probe function of the musb host driver can be called
> > multiple
> > ? > times. The code assumes that it can safe the pointer to the
> > allocated
> > ? > musb struct in the driver model priv_auto_alloc data, but this
> > data
> > ? > gets free-ed on a probe failure or on removal, so we must safe
> > the
> > ? > pointer elsewhere.
> > 
> > s/safe/save/ (twice).
> Thanks, fixed in my tree.

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

[U-Boot] [U-Boot, 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Hans de Goede]

Marek, Simon,

Any news on this patch-set ?

Patch 1 is sunxi specific, but patch 2 is a generic musb patch
and patch 3 (of which I've send a v2 fixing some compiler warnings)
is a generic (dm) usb patch, so my plan was for these 3 patches
to go upstream through Marek's usb tree, after a review of:

https://patchwork.ozlabs.org/patch/605491/

by Simon.

Regards,

Hans


On 02-04-16 20:46, Hans de Goede wrote:
> The probe function of the musb host driver can be called multiple
> times. The code assumes that it can safe the pointer to the allocated
> musb struct in the driver model priv_auto_alloc data, but this data
> gets free-ed on a probe failure or on removal, so we must safe the
> pointer elsewhere.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/usb/musb-new/sunxi.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
> index be1d2ec..3081afc 100644
> --- a/drivers/usb/musb-new/sunxi.c
> +++ b/drivers/usb/musb-new/sunxi.c
> @@ -201,6 +201,7 @@ 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)
>   {
> @@ -320,13 +321,15 @@ int musb_usb_probe(struct udevice *dev)
>
>   	priv->desc_before_addr = true;
>
> -	if (!host->host) {
> -		host->host = musb_init_controller(&musb_plat, NULL,
> +	if (!sunxi_musb) {
> +		sunxi_musb = musb_init_controller(&musb_plat, NULL,
>   						  (void *)SUNXI_USB0_BASE);
> -		if (!host->host)
> -			return -EIO;
>   	}
>
> +	host->host = sunxi_musb;
> +	if (!host->host)
> +		return -EIO;
> +
>   	ret = musb_lowlevel_init(host);
>   	if (ret == 0)
>   		printf("MUSB OTG\n");
>

[U-Boot] [U-Boot, 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Marek Vasut]

On 04/05/2016 11:00 AM, Hans de Goede wrote:
> Marek, Simon,
> 
> Any news on this patch-set ?
> 
> Patch 1 is sunxi specific, but patch 2 is a generic musb patch
> and patch 3 (of which I've send a v2 fixing some compiler warnings)
> is a generic (dm) usb patch, so my plan was for these 3 patches
> to go upstream through Marek's usb tree, after a review of:
> 
> https://patchwork.ozlabs.org/patch/605491/

Applied all three to u-boot-usb/master . Sorry for the delay, I was
snowed under with work.

Best regards,
Marek Vasut

[U-Boot] [U-Boot, 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Hans de Goede]

Hi,

On 06-04-16 01:12, Marek Vasut wrote:
> On 04/05/2016 11:00 AM, Hans de Goede wrote:
>> Marek, Simon,
>>
>> Any news on this patch-set ?
>>
>> Patch 1 is sunxi specific, but patch 2 is a generic musb patch
>> and patch 3 (of which I've send a v2 fixing some compiler warnings)
>> is a generic (dm) usb patch, so my plan was for these 3 patches
>> to go upstream through Marek's usb tree, after a review of:
>>
>> https://patchwork.ozlabs.org/patch/605491/
>
> Applied all three to u-boot-usb/master . Sorry for the delay, I was
> snowed under with work.

Thanks, but it looks like you've picked up v1 of the 3th patch:

"dm: usb: Do not reprobe usb hosts on "usb tree" command"

As mentioned before I've posted a v2 of this one, which fixes a
compiler warning:

https://patchwork.ozlabs.org/patch/605491/

Can you please pick up that one instead. And can you also do
s/safe/save/ in the commit msg of:

"musb: sunxi: Do not allocate musb struct multiple times"

I messed up my spelling there :)

Regards,

Hans

[U-Boot] [U-Boot, 1/3] musb: sunxi: Do not allocate musb struct multiple times

[Marek Vasut]

On 04/06/2016 02:21 PM, Hans de Goede wrote:
> Hi,
> 
> On 06-04-16 01:12, Marek Vasut wrote:
>> On 04/05/2016 11:00 AM, Hans de Goede wrote:
>>> Marek, Simon,
>>>
>>> Any news on this patch-set ?
>>>
>>> Patch 1 is sunxi specific, but patch 2 is a generic musb patch
>>> and patch 3 (of which I've send a v2 fixing some compiler warnings)
>>> is a generic (dm) usb patch, so my plan was for these 3 patches
>>> to go upstream through Marek's usb tree, after a review of:
>>>
>>> https://patchwork.ozlabs.org/patch/605491/
>>
>> Applied all three to u-boot-usb/master . Sorry for the delay, I was
>> snowed under with work.
> 
> Thanks, but it looks like you've picked up v1 of the 3th patch:
> 
> "dm: usb: Do not reprobe usb hosts on "usb tree" command"
> 
> As mentioned before I've posted a v2 of this one, which fixes a
> compiler warning:
> 
> https://patchwork.ozlabs.org/patch/605491/
> 
> Can you please pick up that one instead. And can you also do
> s/safe/save/ in the commit msg of:
> 
> "musb: sunxi: Do not allocate musb struct multiple times"
> 
> I messed up my spelling there :)

Done and done.

Best regards,
Marek Vasut