On Mon, Oct 02, 2017 at 05:49:52PM +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> On Mon, Oct 02, 2017 at 03:41:42PM +0000, Kershner, David A wrote:
> > Hey Greg, we think the code for visorbus is ready to be moved out
> > of staging, can you review it to see if we have missed anything?
>
> I think your html email got rejected by the list :(
>
> > The files include:
> > /drivers/staging/unisys/visorbus/
> > /drivers/staging/unisys/include/visorchannel.h
> > /drivers/staging/unisys/include/visorbus.h
> >
> > The directories staging/drivers/unisys/visornic, visorhba, and visorinput
> > are not part of the review as well as the file
> > drivers/staging/unisys/include/iochannel.h.
> >
> > We currently have 5 checkpatch.pl warnings that I know about:
> > - 3 CHECKs in visorchannel that are due to a MACRO that gets passed a FIELD
> > instead of just a variable
> > - 2 WARNINGS dealing with char * becoming static const
> >
> >
> >
> > Dan Carpenter found some extra parenthesis errors that I will address as
> > well as look through the code to fix, but I would like to ask for the review
> > while we are working on that.
>
> Sure, I'll look at doing it in a week or so when I catch up with other
> patches in my queue.
>
> Everyone else is also welcome to do review :)
cppcheck emits a few style warnings, nothing super important but FWIW
here is a patch
---
drivers/staging/unisys/visorbus/visorchannel.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index aae16073ba03..2c1beddfee29 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -131,12 +131,13 @@ int visorchannel_write(struct visorchannel *channel, ulong offset, void *dest,
ulong nbytes)
{
size_t chdr_size = sizeof(struct channel_header);
- size_t copy_size;
if (offset + nbytes > channel->nbytes)
return -EIO;
if (offset < chdr_size) {
+ size_t copy_size;
+
copy_size = min(chdr_size - offset, nbytes);
memcpy(((char *)(&channel->chan_hdr)) + offset,
dest, copy_size);
@@ -260,9 +261,10 @@ int visorchannel_signalremove(struct visorchannel *channel, u32 queue,
void *msg)
{
int rc;
- unsigned long flags;
if (channel->needs_lock) {
+ unsigned long flags;
+
spin_lock_irqsave(&channel->remove_lock, flags);
rc = signalremove_inner(channel, queue, msg);
spin_unlock_irqrestore(&channel->remove_lock, flags);
@@ -428,9 +430,10 @@ int visorchannel_signalinsert(struct visorchannel *channel, u32 queue,
void *msg)
{
int rc;
- unsigned long flags;
if (channel->needs_lock) {
+ unsigned long flags;
+
spin_lock_irqsave(&channel->insert_lock, flags);
rc = signalinsert_inner(channel, queue, msg);
spin_unlock_irqrestore(&channel->insert_lock, flags);
--
2.7.4
thanks,
Tobin.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
