Attention is currently required from: Matt DeVillier, Paul Menzel, Sean Rhodes.
Angel Pons has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email )
Change subject: i2c/drivers/generic: Add support for including a CDM ......................................................................
Patch Set 9:
(3 comments)
File src/drivers/i2c/generic/chip.h:
https://review.coreboot.org/c/coreboot/+/81409/comment/7e68993a_7e6ab8e8?usp... : PS9, Line 98: bool has_cdm; : int cdm_index; If a zero value is not valid, the boolean isn't needed:
```suggestion enum { CDM_NOT_PRESENT = 0, CDM_ROT_90 = 1, CDM_ROT_180 = 2, CDM_ROT_270 = 3, CDM_ROT_0 = 4, CDM_ROT_90_INVERT = 5, CDM_ROT_180_INVERT = 6, CDM_ROT_270_INVERT = 7, CDM_ROT_0_INVERT = 8, } cdm_index; ```
The acpigen magic would be skipped if `cdm_index` equals `CDM_NOT_PRESENT` (the default for unspecified devicetree settings)
File src/drivers/i2c/generic/generic.c:
https://review.coreboot.org/c/coreboot/+/81409/comment/5ba1298b_f98b407d?usp... : PS9, Line 170: acpigen_write_method("_CDM", 1); Could this be a Name instead? Or does it need to be a Method?
https://review.coreboot.org/c/coreboot/+/81409/comment/576bebdb_c8adbbae?usp... : PS9, Line 171: + Any reason not to use a bitwise OR?