Overall this is a great refactor. It's really easy to read.
Patch set 2:Code-Review +1
9 comments:
File src/soc/amd/common/block/include/amdblocks/espi.h:
VALUE?
Patch Set #2, Line 38: enum espi_io_mode {
nit: Move the enum declarations after the register declaration. This way all the macros are in one block and it's easier to visually see they are all related.
File src/soc/amd/common/block/lpc/espi_util.c:
How about cmd_type so it looks closer to enum espi_cmd_type.
Can we return an error?
Can we print something out saying that we are falling back to a different mode. Otherwise someone might assume we are running in quad, but actually running in dual.
Patch Set #2, Line 621: ntentional fall-through
Same as above
Patch Set #2, Line 749: espi_send_pltrst_deassert
Can you move this to the caller. Seems like an undocummented side effect
nit: space
/* Enable subtractive decode if configured */
global_ctrl_reg = espi_read32(ESPI_GLOBAL_CONTROL_1);
if (cfg->subtractive_decode) {
global_ctrl_reg &= ~ESPI_SUB_DECODE_SLV_MASK;
global_ctrl_reg |= ESPI_SUB_DECODE_EN;
} else {
global_ctrl_reg &= ~ESPI_SUB_DECODE_EN;
}
espi_write32(ESPI_GLOBAL_CONTROL_1, global_ctrl_reg);
Move this into espi_setup_subtractive_decode so it matches the rest of the function.
To view, visit change 41272. To unsubscribe, or for help writing mail filters, visit settings.