Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35271 )
Change subject: soc/amd/common/lpc: Add SuperIO decode function ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/lpc.h:
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/in... PS2, Line 78: #define LPC_SELECT_SIO_4E4F 1
Add a comment explaining why these 2 defines are Boolean
I don't feel like a selection from among two addresses lends itself very well to the bool idea of true and false. I want the mb consumer of this only to consider using the right define, not get caught up in whether it's true or false.
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/in... PS2, Line 78: 1
true
Same as above.
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/in... PS2, Line 79: 0
false
Same as above.
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/lp... File src/soc/amd/common/block/lpc/lpc_util.c:
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/lp... PS2, Line 168: addr
Maybe for clarity, is_it_uart1
I don't think that adds much clarity, given what I have below. The bool already should enforce effectively only one of two choices.
https://review.coreboot.org/c/coreboot/+/35271/2/src/soc/amd/common/block/lp... PS2, Line 174: LPC_SELECT_SIO_2E2F
Making the input a bool, the check against a value is no longer needed and makes no sense. […]
I don't feel like a selection of addresses lends itself well to true/false.