Attention is currently required from: Angel Pons, Elyes Haouas.
Máté Kukri has posted comments on this change by Máté Kukri. ( https://review.coreboot.org/c/coreboot/+/82053?usp=email )
Change subject: [WIP] OptiPlex 3050 Micro port ......................................................................
Patch Set 14:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/82053/comment/71c8617c_f25b7323?usp... : PS14, Line 7: [WIP]
Should this be removed before submitting?
USB ports are still TODO, and I would like to get late HWM init in too for fan control. Will remove the WIP when ready for submitting.
File src/mainboard/dell/optiplex_3050/cmos.layout:
https://review.coreboot.org/c/coreboot/+/82053/comment/252771e2_d764e124?usp... : PS14, Line 25: 416 128 r 0 vbnv
Does this board use vboot/ChromeOS?
I guess it can in theory, but I have no such intentions, so not against removing it.
File src/mainboard/dell/optiplex_3050/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/82053/comment/c9956ceb_f193376b?usp... : PS14, Line 13: register "PrimaryDisplay" = "Display_iGFX"
Is this needed? It's not wrong, though.
I suspect that it is not needed.
https://review.coreboot.org/c/coreboot/+/82053/comment/460ade89_5f57f6c0?usp... : PS14, Line 18: OC0
That's a lot of ports on OC0, it feels wrong
Yeah it is definitely wrong, I still need to fix the USB ports, that's just bad copypasta
https://review.coreboot.org/c/coreboot/+/82053/comment/53ad5cec_933618e5?usp... : PS14, Line 55: device ref pcie_rp21 on
Which device is this? Would be nice to add a comment
Will do, I think this is the M.2 SSD port if I remember correctly.
File src/mainboard/dell/optiplex_3050/ramstage.c:
https://review.coreboot.org/c/coreboot/+/82053/comment/b2b4775c_9ce9c6de?usp... : PS14, Line 11: static void init_mainboard(void *chip_info) : { : } : : struct chip_operations mainboard_ops = { : .init = init_mainboard, : };
If this does nothing, is it needed? Should GPIOs be configured in there?
Not needed, no.
File src/mainboard/dell/optiplex_3050/sch5555_ec.c:
https://review.coreboot.org/c/coreboot/+/82053/comment/6fa6b522_17f04b3f?usp... : PS14, Line 11: uint8_t tmp = inb(SCH555x_EMI_IOBASE + 1); : outb(tmp, SCH555x_EMI_IOBASE + 1);
not sure but maybe `pnp_unset_and_set_index` can be used ? […]
This isn't PNP, this is a dedicated I/O window for the EC that is mapped via a BAR in PNP config space.