On Mon, Jun 14, 2021 at 05:59:35PM +0300, Mika Westerberg wrote:
> Hi Greg,
>
> On Mon, Jun 14, 2021 at 04:06:30PM +0200, Greg KH wrote:
> > On Mon, Jun 14, 2021 at 04:52:10PM +0300, Gil Fine wrote:
> > > DROM for USB4 host/device has a shorter header than Thunderbolt DROM
> > > header. This patch addresses host/device with USB4 DROM (According to spec:
> > > Universal Serial Bus 4 (USB4) Device ROM Specification, Rev 1.0, Feb-2021).
> > >
> > > Signed-off-by: Gil Fine <gil.fine@xxxxxxxxx>
> > > ---
> > > drivers/thunderbolt/eeprom.c | 19 +++++++++++--------
> > > 1 file changed, 11 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
> > > index 46d0906a3070..f9d26bd4f486 100644
> > > --- a/drivers/thunderbolt/eeprom.c
> > > +++ b/drivers/thunderbolt/eeprom.c
> > > @@ -214,7 +214,10 @@ static u32 tb_crc32(void *data, size_t len)
> > > return ~__crc32c_le(~0, data, len);
> > > }
> > >
> > > -#define TB_DROM_DATA_START 13
> > > +#define TB_DROM_DATA_START 13
> > > +#define TB_DROM_HEADER_LENGTH 22
> > > +/* BYTES 16-21 - nonexistent in USB4 DROM */
> > > +#define TB_DROM_USB4_HEADER_LENGTH 16
> > > struct tb_drom_header {
> > > /* BYTE 0 */
> > > u8 uid_crc8; /* checksum for uid */
> > > @@ -224,9 +227,9 @@ struct tb_drom_header {
> > > u32 data_crc32; /* checksum for data_len bytes starting at byte 13 */
> > > /* BYTE 13 */
> > > u8 device_rom_revision; /* should be <= 1 */
> > > - u16 data_len:10;
> > > - u8 __unknown1:6;
> > > - /* BYTES 16-21 */
> > > + u16 data_len:12;
> > > + u8 reserved:4;
> > > + /* BYTES 16-21 - Only for TBT DROM, nonexistent in USB4 DROM */
> >
> > What is the odds the above does not work properly for big endian
> > systems?
>
> If you mean the bitfields, we have been trying to get rid of them. Any
> new code is expected not to introduce new structures like this but it
> has been OK for existing structures (for now).
Ok, as long as you all realize this is broken :)
> > And why put the comment after the area and not before?
>
> The gap is there after the "reserved" field.
Ah, I read the patch wrong, sorry, my fault.
greg k-h
