Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35762 )
Change subject: sb/intel/ibexpeak: Implement USB current settings ......................................................................
Patch Set 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35762/9/src/mainboard/packardbell/m... File src/mainboard/packardbell/ms2290/romstage.c:
https://review.coreboot.org/c/coreboot/+/35762/9/src/mainboard/packardbell/m... PS9, Line 90: early_usb_init(mainboard_usb_ports);
Pass ARRAY_SIZE() as call argument, test for equals TOTAL_USB_Ports? Same for snb/ivy?
Later on this array is declared extern with a fixed size and used in the common romstage, which would defeat the purpose of this? Same holds true for snb/ivy. Should the fixed arraysize be dropped? That would also drop the compiletime warning for excess elements.
https://review.coreboot.org/c/coreboot/+/35762/8/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/early_usb.c:
https://review.coreboot.org/c/coreboot/+/35762/8/src/southbridge/intel/ibexp... PS8, Line 29: const u16 currents[] = { 0xf57, 0xf5f, 0x753, 0x75f, : 0x14b, 0x74b, 0x557, 0x757, : 0x55f, 0x54b
please use the 96-character line length limit :D
Humans count to 4 easily. I temporarily did that to port the RCBA setting to the new format :)
https://review.coreboot.org/c/coreboot/+/35762/8/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/pch.h:
https://review.coreboot.org/c/coreboot/+/35762/8/src/southbridge/intel/ibexp... PS8, Line 72: current_lookup_idx
Any clue as to what these mean?
They are related to the topology of the USB ports, but without more complete documentation a lookup table with a handy index enum holding the value's name is the best we can do.