linux-kernel - Re: [PATCH] USB: serial: cp210x: Adding tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 16 Oct 2015 14:39:48 +0200
From:	Johan Hovold <johan@...nel.org>
To:	Konstantin Shkolnyy <konstantin.shkolnyy@...il.com>
Cc:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	johan@...nel.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: serial: cp210x: Adding tx_empty() to avoid cp2108
 failure

On Fri, Oct 16, 2015 at 07:35:02AM -0500, Konstantin Shkolnyy wrote:
> Hello,
> 
> On Fri, Oct 16, 2015 at 6:19 AM, Sergei Shtylyov
> <sergei.shtylyov@...entembedded.com> wrote:
>  [...]
> >>
> >> @@ -249,6 +251,16 @@ static struct usb_serial_driver * const
> >> serial_drivers[] = {
> >>   #define CP210X_GET_CHARS      0x0E
> >>   #define CP210X_GET_PROPS      0x0F
> >>   #define CP210X_GET_COMM_STATUS        0x10
> >> +/* Data returned by CP210X_GET_COMM_STATUS -- h/w doc says it's 0x13
> >> bytes */
> >> +struct cp210x_comm_status {
> >> +       u32     errors;
> >> +       u32     hold_reasons;
> >> +       u32     amount_in_in_queue;
> >> +       u32     amount_in_out_queue;
> >> +       u8      eof_received;
> >> +       u8      wait_for_immediate;
> >> +       u8      reserved;
> >> +};
> >
> >
> >    Please don't declare structures amidst of the command #define's.
> >
> [...]
> 
> I agree with all suggestions except this one. I find it very
> convenient, when reading code, to have the command code and its data
> declared in the same place.

No, I agree with Sergei on this. Please place the struct after the
request defines (just like the various request values after are defined
after the request list).

I'll try to provide some more feedback on your patches shortly. Sorry
for the delay.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/