Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45578 )
Change subject: mb/hp: Add HP EliteBook Folio 9480m ......................................................................
Patch Set 8: Code-Review+1
(7 comments)
https://review.coreboot.org/c/coreboot/+/45578/8/Documentation/mainboard/hp/... File Documentation/mainboard/hp/folio_9480m.md:
https://review.coreboot.org/c/coreboot/+/45578/8/Documentation/mainboard/hp/... PS8, Line 40: develop development?
https://review.coreboot.org/c/coreboot/+/45578/8/Documentation/mainboard/hp/... PS8, Line 60: 00600000:00bfffff bios nit: order the layout entries so that they are contiguous
https://review.coreboot.org/c/coreboot/+/45578/8/Documentation/mainboard/hp/... PS8, Line 63: res1 what does this stand for? maybe use `reserved`?
https://review.coreboot.org/c/coreboot/+/45578/8/Documentation/mainboard/hp/... PS8, Line 93: is lost does it fall off the board? or is it that it's not visible after resume?
https://review.coreboot.org/c/coreboot/+/45578/8/Documentation/mainboard/hp/... PS8, Line 94: are lost same
https://review.coreboot.org/c/coreboot/+/45578/8/src/mainboard/hp/folio_9480... File src/mainboard/hp/folio_9480m/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/45578/8/src/mainboard/hp/folio_9480... PS8, Line 16: register "c1_acpower" = "1" : register "c1_battery" = "1" : register "c2_acpower" = "3" : register "c2_battery" = "3" : register "c3_acpower" = "5" : register "c3_battery" = "5" could you please try using `2`, `3` and `9` instead, like google/slippy and google/beltino do? I want to drop these settings from the devicetree and I need all Haswell boards to use the same sets of values ([1, 3, 5] for traditional, [2, 3, 9] for ULT)
https://review.coreboot.org/c/coreboot/+/45578/8/src/mainboard/hp/folio_9480... PS8, Line 39: sata_port0_gen3_dtle port0 is not in the port map