[PATCH v2 1/2] usb: dwc2: gadget: modify return statement

[PATCH v2 1/2] usb: dwc2: gadget: modify return statement

[Sudip Mukherjee]

modified the function to have a single return statement at the end
instead of multiple return statement in the middle of the function
to improve the readability of the code.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/dwc2/gadget.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856f..af5517f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2471,7 +2471,8 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 	dir_in = (desc->bEndpointAddress & USB_ENDPOINT_DIR_MASK) ? 1 : 0;
 	if (dir_in != hs_ep->dir_in) {
 		dev_err(hsotg->dev, "%s: direction mismatch!\n", __func__);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto error1;
 	}
 
 	mps = usb_endpoint_maxp(desc);
@@ -2561,8 +2562,10 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 			hs_ep->fifo_size = val;
 			break;
 		}
-		if (i == 8)
-			return -ENOMEM;
+		if (i == 8) {
+			ret = -ENOMEM;
+			goto error1;
+		}
 	}
 
 	/* for non control endpoints, set PID to D0 */
@@ -2580,6 +2583,8 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 	s3c_hsotg_ctrl_epint(hsotg, index, dir_in, 1);
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
+
+error1:
 	return ret;
 }
 
-- 
1.8.1.2

[PATCH v2 2/2] usb: dwc2: gadget: sparse warning of context imbalance

[Sudip Mukherjee]

sparse was giving the following warning:
        warning: context imbalance in 's3c_hsotg_ep_enable'
	        - different lock contexts for basic block

we were returning ENOMEM while still holding the spinlock.
The sparse warning was fixed by releasing the spinlock before return.

This patch depends on the previous patch of the series.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/usb/dwc2/gadget.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index af5517f..05a9a23 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2564,7 +2564,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 		}
 		if (i == 8) {
 			ret = -ENOMEM;
-			goto error1;
+			goto error2;
 		}
 	}
 
@@ -2582,6 +2582,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 	/* enable the endpoint interrupt */
 	s3c_hsotg_ctrl_epint(hsotg, index, dir_in, 1);
 
+error2:
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
 error1:
-- 
1.8.1.2

Re: [PATCH v2 2/2] usb: dwc2: gadget: sparse warning of context imbalance

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]

HI,

On Thu, Oct 16, 2014 at 01:11:10PM +0530, Sudip Mukherjee wrote:
> sparse was giving the following warning:
>         warning: context imbalance in 's3c_hsotg_ep_enable'
> 	        - different lock contexts for basic block
> 
> we were returning ENOMEM while still holding the spinlock.
> The sparse warning was fixed by releasing the spinlock before return.
> 
> This patch depends on the previous patch of the series.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

this should be patch one so it can be backported to stable kernels.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/2] usb: dwc2: gadget: sparse warning of context imbalance

[Sudip Mukherjee]

On Thu, Oct 16, 2014 at 08:21:55AM -0500, Felipe Balbi wrote:
> HI,
> 
> On Thu, Oct 16, 2014 at 01:11:10PM +0530, Sudip Mukherjee wrote:
> > sparse was giving the following warning:
> >         warning: context imbalance in 's3c_hsotg_ep_enable'
> > 	        - different lock contexts for basic block
> > 
> > we were returning ENOMEM while still holding the spinlock.
> > The sparse warning was fixed by releasing the spinlock before return.
> > 
> > This patch depends on the previous patch of the series.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> 
> this should be patch one so it can be backported to stable kernels.
> 
my v1 patch fixed only this , while reviewing that one Paul Zimmerman suggested to rewrite the return statements.
so this v2 series had the rewrite and the spinlock error fix.
now if this is to be made the patch one then it will be a duplicate of my v1 followed by another patch for return statements. 
should i do that ?

thanks
sudip

> -- 
> balbi

Re: [PATCH v2 2/2] usb: dwc2: gadget: sparse warning of context imbalance

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On Thu, Oct 16, 2014 at 08:22:08PM +0530, Sudip Mukherjee wrote:
> On Thu, Oct 16, 2014 at 08:21:55AM -0500, Felipe Balbi wrote:
> > HI,
> > 
> > On Thu, Oct 16, 2014 at 01:11:10PM +0530, Sudip Mukherjee wrote:
> > > sparse was giving the following warning:
> > >         warning: context imbalance in 's3c_hsotg_ep_enable'
> > > 	        - different lock contexts for basic block
> > > 
> > > we were returning ENOMEM while still holding the spinlock.
> > > The sparse warning was fixed by releasing the spinlock before return.
> > > 
> > > This patch depends on the previous patch of the series.
> > > 
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > 
> > this should be patch one so it can be backported to stable kernels.
> > 
> my v1 patch fixed only this , while reviewing that one Paul Zimmerman
> suggested to rewrite the return statements.
> so this v2 series had the rewrite and the spinlock error fix.
> now if this is to be made the patch one then it will be a duplicate of
> my v1 followed by another patch for return statements. 
> should i do that ?

Paul has the final word on this driver, if he already asked you to
change, I withdraw my comment ;-)

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH v2 2/2] usb: dwc2: gadget: sparse warning of context imbalance

[Paul Zimmerman]

> From: Sudip Mukherjee [mailto:sudipm.mukherjee@gmail.com]
> Sent: Thursday, October 16, 2014 7:52 AM
> 
> On Thu, Oct 16, 2014 at 08:21:55AM -0500, Felipe Balbi wrote:
> > HI,
> >
> > On Thu, Oct 16, 2014 at 01:11:10PM +0530, Sudip Mukherjee wrote:
> > > sparse was giving the following warning:
> > >         warning: context imbalance in 's3c_hsotg_ep_enable'
> > > 	        - different lock contexts for basic block
> > >
> > > we were returning ENOMEM while still holding the spinlock.
> > > The sparse warning was fixed by releasing the spinlock before return.
> > >
> > > This patch depends on the previous patch of the series.
> > >
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> >
> > this should be patch one so it can be backported to stable kernels.
> >
> my v1 patch fixed only this , while reviewing that one Paul Zimmerman suggested to rewrite the return
> statements.
> so this v2 series had the rewrite and the spinlock error fix.
> now if this is to be made the patch one then it will be a duplicate of my v1 followed by another patch
> for return statements.
> should i do that ?

Hi Sudip,

Please make the first patch like I showed in my previous reply. Then we
can mark that one for stable to fix the bug. Then make a second patch to
change the other error path.

-- 
Paul