RE: [EXT] Re: [PATCH RESEND 3/5] USB: serial: cp210x: Added comments to CRTSCT flag code.

[Konstantin Shkolnyy]

> -----Original Message-----
> From: Johan Hovold [mailto:jhovold@xxxxxxxxx] On Behalf Of Johan Hovold
> Sent: Tuesday, April 26, 2016 02:26
> To: Konstantin Shkolnyy
> Cc: Johan Hovold; Konstantin Shkolnyy; linux-usb@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [EXT] Re: [PATCH RESEND 3/5] USB: serial: cp210x: Added
> comments to CRTSCT flag code.
> 
> On Mon, Apr 25, 2016 at 06:09:01PM +0000, Konstantin Shkolnyy wrote:
> > I was planning to define all these bits in a separate future patch.
> > Would you rather prefer the magic numbers defined before fixing the
> bugs?
> 
> Fixing the RTS bug (patch 1), which is the only "real" bug, should be
> done before adding defines, and fixing and cleaning up the rest.
> 
> > I guess I can do that. Is something like this acceptable?
> >
> > /* CP210X_GET_FLOW/CP210X_SET_FLOW read/write these 0x10 bytes */
> > struct cp210x_flow_ctl {
> > 	u8	SERIAL_DTR_MASK        : 2; /* byte 0 */
> > 	u8	                       : 1;
> > 	u8	SERIAL_CTS_HANDSHAKE   : 1;
> > 	u8	SERIAL_DSR_HANDSHAKE   : 1;
> > 	u8	SERIAL_DCD_HANDSHAKE   : 1;
> > 	u8	SERIAL_DSR_SENSITIVITY : 1;
> > 	u8	                       : 1;
> > 	u8; /* byte 1 */
> > 	u8; /* byte 2 */
> > 	u8; /* byte 3 */
> > 	u8	SERIAL_AUTO_TRANSMIT   : 1; /* byte 4 */
> > 	u8	SERIAL_AUTO_RECEIVE    : 1;
> > 	u8	SERIAL_ERROR_CHAR      : 1;
> > 	u8	SERIAL_NULL_STRIPPING  : 1;
> > 	u8	SERIAL_BREAK_CHAR      : 1;
> > 	u8	                       : 1;
> > 	u8	SERIAL_RTS_MASK        : 2;
> > 	u8; /* byte 5 */
> > 	u8; /* byte 6 */
> > 	u8	                       : 7; /* byte 7 */
> > 	u8	SERIAL_XOFF_CONTINUE   : 1;
> > 	__le32	ulXonLimit;
> > 	__le32	ulXoffLimit;
> > } __packed;
> 
> No, shouldn't rely on the layout of bitfields. Define masks and shifts
> as needed and the message structure as
> 
> struct cp210x_flow_ctl {
> 	__le32	ulControlHandshake;
> 	__le32	ulFlowReplace;
>  	__le32	ulXonLimit;
>  	__le32	ulXoffLimit;
> };
> 
> that is, as per AN571.
> 

OK, from searching www I see that bitfields have bad reputation for unclear reasons, so I guess it's now easier to avoid them.
But doing it like you suggest, instead of splitting it to bytes, would complicate the code with endian conversions.
Is there a reason for this other than making it identical to the spec?\