Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47639 )
Change subject: drivers/i2c/sx9310: Refactor register struct ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47639/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47639/4//COMMIT_MSG@9 PS4, Line 9: Kernel driver sx93xx is looking for reg_name and data. sx9310 and sx9324 : have very different register name. In order to support more sx93xx serials, : refactor the register structure to be compatible for any register. I think it would be better to add a separate driver for sx932x in coreboot. I see that sx9310 kernel driver is very different in v4.4 and v5.4 in the chromium tree and looks like v5.4 for sx9310 expects very different properties and not register names. Not sure if anyone has looked into sx9310 driver for v5.4. But to keep things simpler, it might be better to just start with a separate driver in coreboot fro sx932x and later we can see if we want to combine them into one.
https://review.coreboot.org/c/coreboot/+/47639/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/hatch/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47639/4/src/mainboard/google/hatch/... PS4, Line 130: register "regs[0].reg_name" = ""reg_prox_ctrl0"" : register "regs[0].value" = "0x10" In my opinion, it's better to have regiter "reg_xxxx" = "vvvv". Makes it easier to read. Having a separate driver for sx923x will allow us to keep the same structure.