Re: [PATCH] thunderbolt: Fix DROM handling for USB4 DROM

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Mon, Jun 14, 2021 at 05:21:06PM +0200, Greg KH wrote:
> 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@intel.com>
> > > > ---
> > > >  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 :)

Yes we do :) This is one of the things that need to be sorted out once
we have all the "features" in place.