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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
this should be patch one so it can be backported to stable kernels.
--
balbi
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 <[email protected]>
>
> 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
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 <[email protected]>
> >
> > 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
> From: Sudip Mukherjee [mailto:[email protected]]
> 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 <[email protected]>
> >
> > 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