Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30957 )
Change subject: superio/ite: Add and use it8528e ......................................................................
Patch Set 4:
(2 comments)
Patch Set 2:
Patch Set 2:
Yeah, the whole IT85xx line seems to be ECs. We have some in `ec/<possible firmware vendor>/it85*`. Their PnP interface seems to be independent from the programmable part, though. So maybe treating the interfaces separately would be feasible in this case.
We can't share the EC firmware interface parts ofc, but we could share the PnP definitions. Though, if this is going to be another driver that doesn't make this distinction (and gets firmware interface parts added later), it should probably reside in `ec/`.
Ok, yes, it seems to make sense to split those parts then.
Oh, I didn't know that there's IT85xx support already.
The EC is mostly independent of the SuperIO configuration as it has it's own address space. Looks like a good idea to place it in ec/
https://review.coreboot.org/#/c/30957/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30957/2//COMMIT_MSG@9 PS2, Line 9: ITE8528E
IT8528E
Done
https://review.coreboot.org/#/c/30957/2/src/superio/ite/it8528e/superio.c File src/superio/ite/it8528e/superio.c:
https://review.coreboot.org/#/c/30957/2/src/superio/ite/it8528e/superio.c@53 PS2, Line 53:
indent with one tab and not one space? same below
Done