Attention is currently required from: Alexander Couzens, Cleyton Silva, Keith Hui.
Angel Pons has posted comments on this change by Cleyton Silva. ( https://review.coreboot.org/c/coreboot/+/83670?usp=email )
Change subject: mb/lenovo: Add IH61M mainboard ......................................................................
Patch Set 5: Code-Review+1
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83670/comment/90f0bccf_f24e9b16?usp... : PS1, Line 13: 2MB+4MB
./util/ifdtool -f rom.layout bkp-lenovo-h61.rom […]
This doesn't show the flash chip order, though. Can you please show the output of `./util/ifdtool -d rom.layout bkp-lenovo-h61.rom`?
https://review.coreboot.org/c/coreboot/+/83670/comment/295f9197_c6df43c0?usp... : PS1, Line 43: - Internal flashing : - External flashing (in-circuit)
Yes, vendor firmware restricts internal flashing! […]
Nice! Isn't it convenient that UEFI code reuse leads to exploit reuse? 😄
The main reason why there's a flashing instructions page for the ThinkPads is because there's several models that can use this approach.
For your board, I believe you can document the flashing process in your mainboard's doc page (which you'd need to create), maybe linking to the Lenovo instructions if they're similar enough (mention what needs to be done differently on your mainboard page).
If the ThinkPad instructions are generic enough, maybe they could be moved to a mainboard-agnostic place (where the flashing firmware instructions are). But it would be best to handle documentation changes in a separate commit.
File src/mainboard/lenovo/ih61m/Kconfig:
https://review.coreboot.org/c/coreboot/+/83670/comment/cb42d950_3ede97da?usp... : PS4, Line 23: config MAINBOARD_PART_NUMBER : default "IH61M Ver:1.0" if BOARD_LENOVO_IH61M_V1 : default "IH61M Ver:4.2" if BOARD_LENOVO_IH61M_V4 Would it make more sense to set `MAINBOARD_VERSION` instead?
```suggestion config MAINBOARD_PART_NUMBER default "IH61M"
config MAINBOARD_VERSION default "Ver:1.0" if BOARD_LENOVO_IH61M_V1 default "Ver:4.2" if BOARD_LENOVO_IH61M_V4 ```
File src/mainboard/lenovo/ih61m/early_init.c:
https://review.coreboot.org/c/coreboot/+/83670/comment/11fecf2a_2e901de7?usp... : PS4, Line 27: pnp_set_logical_device(SERIAL_DEV); This shouldn't be needed, `nuvoton_enable_serial` already switches to `SERIAL_DEV`