[PATCH] usb: max3421: unlock on error in max3421_hub_control()

[PATCH] usb: max3421: unlock on error in max3421_hub_control()

[Dan Carpenter]

We can't return directly in max3421_hub_control(), we have to unlock
first.

Fixes: 721fdc83b31b ("usb: max3421: Add devicetree support")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 928a5aabee02..46a86a7f3360 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1704,7 +1704,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 	pdata = spi->dev.platform_data;
 	if (!pdata) {
 		dev_err(&spi->dev, "Device platform data is missing\n");
-		return -EFAULT;
+		retval = -EFAULT;
+		goto unlock;
 	}
 
 	switch (type_req) {
@@ -1787,6 +1788,7 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 		retval = -EPIPE;
 	}
 
+unlock:
 	spin_unlock_irqrestore(&max3421_hcd->lock, flags);
 	return retval;
 }

Re: [PATCH] usb: max3421: unlock on error in max3421_hub_control()

[Jules Maselbas]

Hi,

> We can't return directly in max3421_hub_control(), we have to unlock
> first.
Yes you are right.
I think testing if pdata is null is not necessary as driver_probe will fail if
the device do not have a platform_data.

Jules.

> Fixes: 721fdc83b31b ("usb: max3421: Add devicetree support")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 928a5aabee02..46a86a7f3360 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -1704,7 +1704,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16
> value, u16 index,
> 	pdata = spi->dev.platform_data;
> 	if (!pdata) {
> 		dev_err(&spi->dev, "Device platform data is missing\n");
> -		return -EFAULT;
> +		retval = -EFAULT;
> +		goto unlock;
> 	}
> 
> 	switch (type_req) {
> @@ -1787,6 +1788,7 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16
> value, u16 index,
> 		retval = -EPIPE;
> 	}
> 
> +unlock:
> 	spin_unlock_irqrestore(&max3421_hcd->lock, flags);
> 	return retval;
>  }

Re: [PATCH] usb: max3421: unlock on error in max3421_hub_control()

[Dan Carpenter]

On Fri, Oct 27, 2017 at 11:06:36AM +0200, Jules Maselbas wrote:
> Hi,
> 
> > We can't return directly in max3421_hub_control(), we have to unlock
> > first.
> Yes you are right.
> I think testing if pdata is null is not necessary as driver_probe will fail if
> the device do not have a platform_data.
> 

In that case, let's drop my patch.  Could you send a patch for which
removes the NULL check and add a Reported-by: for me?

regards,
dan carpenter

Re: [PATCH] usb: max3421: unlock on error in max3421_hub_control()

[Jules Maselbas]

> On Fri, Oct 27, 2017 at 11:06:36AM +0200, Jules Maselbas wrote:
>> Hi,
>> 
>> > We can't return directly in max3421_hub_control(), we have to unlock
>> > first.
>> Yes you are right.
>> I think testing if pdata is null is not necessary as driver_probe will fail if
>> the device do not have a platform_data.
>> 
> 
> In that case, let's drop my patch.  Could you send a patch for which
> removes the NULL check and add a Reported-by: for me?
It is already in the v2 (and v3) of my initial patch.
https://www.spinics.net/lists/devicetree/msg199901.html

Should I make a brand new patch (not another version)?

> 
> regards,
> dan carpenter

Re: [PATCH] usb: max3421: unlock on error in max3421_hub_control()

[Dan Carpenter]

On Fri, Oct 27, 2017 at 02:08:38PM +0200, Jules Maselbas wrote:
> 
> > On Fri, Oct 27, 2017 at 11:06:36AM +0200, Jules Maselbas wrote:
> >> Hi,
> >> 
> >> > We can't return directly in max3421_hub_control(), we have to unlock
> >> > first.
> >> Yes you are right.
> >> I think testing if pdata is null is not necessary as driver_probe will fail if
> >> the device do not have a platform_data.
> >> 
> > 
> > In that case, let's drop my patch.  Could you send a patch for which
> > removes the NULL check and add a Reported-by: for me?
> It is already in the v2 (and v3) of my initial patch.
> https://www.spinics.net/lists/devicetree/msg199901.html
> 
> Should I make a brand new patch (not another version)?

Yeah.  Greg never rebases once the tree has been shared with the public
so you'll need to send a new patch instead of a v3.

regards,
dan carpenter