[coreboot-gerrit] Change in coreboot[master]: drivers/intel/usb4: Add driver for USB4 retimer device

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44918 )

Change subject: drivers/intel/usb4: Add driver for USB4 retimer device
......................................................................

Patch Set 12:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44918/12/src/drivers/intel/usb4/ret... 
File src/drivers/intel/usb4/retimer/retimer.c:

https://review.coreboot.org/c/coreboot/+/44918/12/src/drivers/intel/usb4/ret... 
PS12, Line 39: 0x0F
> When bit 0 is set, it means that "more bits follow":

Even in that case this will have to be 0x3 and not 0xF. There are 2 additional functions supported with revision 1 i.e. "Query force power enable state" and "Set force power state". So, this should be returning 0x3.
> Why not adjust the function so that accounting for this bit is unnecessary?

That is a good idea. I think that will help eliminate some of the confusion. Basically, the query function will have to be implemented by the generic helper and it will have to take in sets of functions for each revision. Anyways, I think that should be done as a separate change.