Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39850 )
Change subject: [WIP]/mb/google/hatch/vr/puff: Add psys_pmax calculation ......................................................................
Patch Set 4:
(12 comments)
https://review.coreboot.org/c/coreboot/+/39850/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39850/4//COMMIT_MSG@12 PS4, Line 12: dependnets
depends
Ack
https://review.coreboot.org/c/coreboot/+/39850/4//COMMIT_MSG@11 PS4, Line 11: maybe varies which : dependnets on power ratings
… can vary depending on the power ratings
Ack
https://review.coreboot.org/c/coreboot/+/39850/4//COMMIT_MSG@12 PS4, Line 12: infomration
information
Ack
https://review.coreboot.org/c/coreboot/+/39850/4//COMMIT_MSG@14 PS4, Line 14: pcode
What is that?
Pcode is like a microprocessor who manage the power like PL1, PL2, PsysPLx and etc.
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 87: * Given the hardware design in puff, the serial shunt resistor is 0.01ohm
Please add a dot/period at the end of sentences.
Ack
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 90: 15k
What is the unit *k*?
k is kilo, 1000. The unit here is ohms.
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 91: * Hence, Iinput (Amps) = 9.58A
Sorry my ignorance, I am no engineer, but I get 9.6 A. […]
Ack
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 98:
Please use exactly one space and no tab.
Ack
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 127: /
Please add spaces around the operator.
Ack
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 139: gpio_configure_pads. */
Fits on one line in 96 characters.
Ack
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 144: && !gpio_get(GPIO_DP_HPD)) {
One line? Only copied, so fix in a commit before or in a follow-up?
I like the original style, it's clear to me.
https://review.coreboot.org/c/coreboot/+/39850/4/src/mainboard/google/hatch/... PS4, Line 149: setup
set up
Ack