> Comment style should be
>
> /*
> * ...
> */
I tried sticking to the style of the rest of the code but I'll keep that
in mind from now on. Wouldn't that be something checkpatch.pl could check?
> But I'm not sure you need to be that verbose in this case. You can
> probably just drop the comment.
Will do.
> Please move this termios update to the default-case in the switch below.
> It's ok to do some redundant processing even the resulting data bits
> settings remain unchanged.
I moved it here, because I'd have to replicate the switch to handle
old_termios->c_cflag instead of cflag:
switch (cflag & CSIZE) {
case CS7:
urb_value |= 7;
dev_dbg(ddev, "Setting CS7\n");
break;
case CS8:
urb_value |= 8;
dev_dbg(ddev, "Setting CS8\n");
break;
default:
dev_err(ddev, "CSIZE was set but not CS7-CS8\n");
if (!old_termios) {
termios->c_cflag = (termios->c_cflag & ~CSIZE) | CS8;
urb_value |= 8;
dev_dbg(ddev, "Setting CS8\n");
} else {
termios->c_cflag = (termios->c_cflag & ~CSIZE) |
(old_termios->c_cflag & CSIZE);
switch (old_termios->c_cflag & CSIZE) {
case CS7:
urb_value |= 7;
dev_dbg(ddev, "Setting CS7\n");
break;
case CS8:
default:
urb_value |= 8;
dev_dbg(ddev, "Setting CS8\n");
break;
}
}
break;
}
Not sure if that's really an enhancement.
>
> Doesn't this all mean that we've been trying to set 0 data bits
> (whatever that would mean) if CS5 or CS6 was requested this far, as the
> least significant byte of urb_value would then have been zero?
Yes, that's correct. I have no protocol documentation at hand to see what
that means but it clearly doesn't switch to 0 data bits :).
> Care to send a v2?
>
> Please include a "[PATCH v2]" subject-prefix when you resend. Sending as
> a reply to this mail is also a good idea if possible, in order to keep
> everything in one mail thread. The git send-email command has an
> --in-reply-to switch if you want to use that.
I'll give it a try.
-Colin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
