| Message ID | 1373533573-12272-9-git-send-email-ming.lei@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. On 11-07-2013 13:05, Ming Lei wrote: > Complete() will be run with interrupt enabled, so change to > spin_lock_irqsave(). > Cc: Juergen Stuber <starblue@users.sourceforge.net> > Signed-off-by: Ming Lei <ming.lei@canonical.com> > --- > drivers/usb/misc/legousbtower.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c > index 8089479..4044989 100644 > --- a/drivers/usb/misc/legousbtower.c > +++ b/drivers/usb/misc/legousbtower.c > @@ -771,6 +771,7 @@ static void tower_interrupt_in_callback (struct urb *urb) > struct lego_usb_tower *dev = urb->context; > int status = urb->status; > int retval; > + unsigned long flags; > > dbg(4, "%s: enter, status %d", __func__, status); > > @@ -788,7 +789,7 @@ static void tower_interrupt_in_callback (struct urb *urb) > } > > if (urb->actual_length > 0) { > - spin_lock (&dev->read_buffer_lock); > + spin_lock_irqsave (&dev->read_buffer_lock, flags); > if (dev->read_buffer_length + urb->actual_length < read_buffer_size) { > memcpy (dev->read_buffer + dev->read_buffer_length, > dev->interrupt_in_buffer, > @@ -799,7 +800,7 @@ static void tower_interrupt_in_callback (struct urb *urb) > } else { > printk(KERN_WARNING "%s: read_buffer overflow, %d bytes dropped", __func__, urb->actual_length); > } > - spin_unlock (&dev->read_buffer_lock); > + spin_unlock_irqrestore (&dev->read_buffer_lock, flags); > } I don't think this patch passes checkpatch.pl. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 11 July 2013 16:18:17 Sergei Shtylyov wrote:
> I don't think this patch passes checkpatch.pl.
This series is a mechanical replacement in dozens of drivers.
We cannot demand nice formatting. If you want to do something
productive, check the locking in the driver.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 11, 2013 at 8:18 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Hello. > > > On 11-07-2013 13:05, Ming Lei wrote: > >> Complete() will be run with interrupt enabled, so change to >> spin_lock_irqsave(). > > >> Cc: Juergen Stuber <starblue@users.sourceforge.net> >> Signed-off-by: Ming Lei <ming.lei@canonical.com> >> --- >> drivers/usb/misc/legousbtower.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) > > >> diff --git a/drivers/usb/misc/legousbtower.c >> b/drivers/usb/misc/legousbtower.c >> index 8089479..4044989 100644 >> --- a/drivers/usb/misc/legousbtower.c >> +++ b/drivers/usb/misc/legousbtower.c >> @@ -771,6 +771,7 @@ static void tower_interrupt_in_callback (struct urb >> *urb) >> struct lego_usb_tower *dev = urb->context; >> int status = urb->status; >> int retval; >> + unsigned long flags; >> >> dbg(4, "%s: enter, status %d", __func__, status); >> >> @@ -788,7 +789,7 @@ static void tower_interrupt_in_callback (struct urb >> *urb) >> } >> >> if (urb->actual_length > 0) { >> - spin_lock (&dev->read_buffer_lock); >> + spin_lock_irqsave (&dev->read_buffer_lock, flags); >> if (dev->read_buffer_length + urb->actual_length < >> read_buffer_size) { >> memcpy (dev->read_buffer + >> dev->read_buffer_length, >> dev->interrupt_in_buffer, >> @@ -799,7 +800,7 @@ static void tower_interrupt_in_callback (struct urb >> *urb) >> } else { >> printk(KERN_WARNING "%s: read_buffer overflow, %d >> bytes dropped", __func__, urb->actual_length); >> } >> - spin_unlock (&dev->read_buffer_lock); >> + spin_unlock_irqrestore (&dev->read_buffer_lock, flags); >> } > > > I don't think this patch passes checkpatch.pl. No errors reported from checkpatch.pl, only warnings which isn't introduced by this patch. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 11-07-2013 16:36, Oliver Neukum wrote: >> I don't think this patch passes checkpatch.pl. > This series is a mechanical replacement in dozens of drivers. That mechanicity shows too much in some patches. > We cannot demand nice formatting. If you want to do something > productive, check the locking in the driver. I'm not paid for it and don't have time to do it for free. > Regards > Oliver WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c index 8089479..4044989 100644 --- a/drivers/usb/misc/legousbtower.c +++ b/drivers/usb/misc/legousbtower.c @@ -771,6 +771,7 @@ static void tower_interrupt_in_callback (struct urb *urb) struct lego_usb_tower *dev = urb->context; int status = urb->status; int retval; + unsigned long flags; dbg(4, "%s: enter, status %d", __func__, status); @@ -788,7 +789,7 @@ static void tower_interrupt_in_callback (struct urb *urb) } if (urb->actual_length > 0) { - spin_lock (&dev->read_buffer_lock); + spin_lock_irqsave (&dev->read_buffer_lock, flags); if (dev->read_buffer_length + urb->actual_length < read_buffer_size) { memcpy (dev->read_buffer + dev->read_buffer_length, dev->interrupt_in_buffer, @@ -799,7 +800,7 @@ static void tower_interrupt_in_callback (struct urb *urb) } else { printk(KERN_WARNING "%s: read_buffer overflow, %d bytes dropped", __func__, urb->actual_length); } - spin_unlock (&dev->read_buffer_lock); + spin_unlock_irqrestore (&dev->read_buffer_lock, flags); } resubmit:
Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Juergen Stuber <starblue@users.sourceforge.net> Signed-off-by: Ming Lei <ming.lei@canonical.com> --- drivers/usb/misc/legousbtower.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)