diff mbox

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

Message ID 1459622771-12704-1-git-send-email-hdegoede@redhat.com
State Accepted
Commit 38b4a3e14397582549b3bb1b301fd9b5c7fc89d2
Delegated to: Marek Vasut
Headers show

Commit Message

Hans de Goede April 2, 2016, 6:46 p.m. UTC
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(-)

Comments

Peter Korsgaard April 2, 2016, 6:57 p.m. UTC | #1
>>>>> "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.
Hans de Goede April 2, 2016, 9:39 p.m. UTC | #2
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
Hans de Goede April 5, 2016, 9 a.m. UTC | #3
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");
>
Ian Campbell April 5, 2016, 9:37 a.m. UTC | #4
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>
Marek Vasut April 5, 2016, 11:12 p.m. UTC | #5
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
Hans de Goede April 6, 2016, 12:21 p.m. UTC | #6
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
Marek Vasut April 6, 2016, 12:34 p.m. UTC | #7
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
diff mbox

Patch

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");