musb: sunxi: Do not allocate musb struct multiple times

PATCH New Hans de Goede 4 participants 10 messages Apr 2, 2016 – Apr 6, 2016

Subsystems (2)

USB Arm/Allwinner sunXi SoC support

lore

Share & tools

lore

Hans de Goede

author

2016-04-02 18:46 UTC #0 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

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

---
 drivers/usb/musb-new/sunxi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/usb/musb-new/sunxi.c +7 -4

index be1d2ec..3081afc 100644
@@ -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");

Signature · 1 line

2.7.2

Hans de Goede

author

2016-04-05 09:00 UTC #1 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

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

Quoted 44 lines · Hans de Goede

Signed-off-by Hans de Goede <[email protected]>

---
  drivers/usb/musb-new/sunxi.c | 11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)

drivers/usb/musb-new/sunxi.c +7 -4

index be1d2ec..3081afc 100644
@@ -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");

Marek Vasut

2016-04-05 23:12 UTC #2 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

Quoted 10 lines · 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/

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

Best regards,
Marek Vasut

Hans de Goede

author

2016-04-06 12:21 UTC #3 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

Hi,

Quoted 10 lines · Marek Vasut

Quoted 9 lines · 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:

Quoted 4 lines

Quoted 1 line
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

Marek Vasut

2016-04-06 12:34 UTC #4 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

Quoted 25 lines · Hans de Goede

Hi,
Quoted 10 lines · Marek Vasut
Quoted 9 lines · 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:
Quoted 4 lines
Quoted 1 line
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:

Quoted 8 lines

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

Hans de Goede

author

2016-04-02 18:46 UTC #5 lore raw

PATCH 2/3 musb: Properly call musb_stop() on probe failure

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

---
 drivers/usb/musb-new/musb_uboot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/musb-new/musb_uboot.c +3 -1

index 233a0e4..6ce528c 100644
@@ -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;

Signature · 1 line

2.7.2

Peter Korsgaard

2016-04-02 18:57 UTC #6 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

In reply to Hans de Goede

Quoted 7 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hans" == Hans de Goede <[email protected]> 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.

Signature · 1 line

Bye, Peter Korsgaard

Hans de Goede

author

2016-04-02 21:39 UTC #7 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

Hi,

Quoted 9 lines · Peter Korsgaard

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hans" == Hans de Goede <[email protected]> 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

Ian Campbell

2016-04-05 09:37 UTC #8 lore raw

PATCH 1/3 musb: sunxi: Do not allocate musb struct multiple times

Quoted 20 lines · Hans de Goede

Hi,
Quoted 17 lines · Peter Korsgaard
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
"Hans" == Hans de Goede <[email protected]> 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.

Quoted 2 lines

Quoted 1 line
s/safe/save/ (twice).
Thanks, fixed in my tree.

Acked-by Ian Campbell <[email protected]>

Hans de Goede

author

2016-04-02 18:46 UTC #9 lore raw

PATCH 3/3 dm: usb: Do not reprobe usb hosts on "usb tree" command

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

---
 cmd/usb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

cmd/usb.c +5 -2

index 9ed5dc6..8741088 100644
@@ -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);

Signature · 1 line

2.7.2