> -----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?\ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
