Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Alan Huang. Rory Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59531 )
Change subject: drivers/net/r8168: Modify to support RTL8125 LEDs ......................................................................
Patch Set 5:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59531/comment/62dd639b_ab57988f PS2, Line 9: The Realtek RTL8125 has four registers for each led, : therefore add customized_mleds[] to support register array.
Please reflow for 75 characters per line.
Ack
https://review.coreboot.org/c/coreboot/+/59531/comment/febce706_17d735c3 PS2, Line 7: drivers/net/r8168: Add customized_mleds to support RTL8125 : : The Realtek RTL8125 has four registers for each led, : therefore add customized_mleds[] to support register array. : : BUG=b:193750191 : TEST=emerge-brask coreboot chromeos-bootimage
Please remove the indentation. […]
Ack
https://review.coreboot.org/c/coreboot/+/59531/comment/82b73186_2400dfcc PS2, Line 13: TEST=emerge-brask coreboot chromeos-bootimage
How did you test this exactly?
Modify overridetree.cb to verify LEDs' settings. https://review.coreboot.org/c/coreboot/+/60298
File src/drivers/net/r8168.c:
https://review.coreboot.org/c/coreboot/+/59531/comment/8545c5b8_3eb1f11f PS1, Line 256: printk(BIOS_DEBUG, "r8168: Customized LED 0x%x\n", config->customized_mleds[i]);
line over 96 characters
Fixed.
File src/drivers/net/r8168.c:
https://review.coreboot.org/c/coreboot/+/59531/comment/d0ad869f_1f7d11ff PS2, Line 32: #define CMD_LEDSEL[4] {0x18, 0x86, 0x84, 0x96}
Where did these IO port numbers come from? Do they come from the PCI enumeration?
These come from the RTL8125 series datasheet 5.1 LED register definition.
File src/include/device/pci_ids.h:
PS2:
nit: please break out PCI ID updates to a separate change
Done