Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33190 )
Change subject: soc/intel/icelake: Replace PCI device LPC to ESPI as per EDS ......................................................................
Patch Set 11:
Patch Set 10: Code-Review+1
Patch Set 10:
Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 7:
> Patch Set 7: > > > Patch Set 6: > > > > (5 comments) > > > > > Patch Set 6: > > > > > > (5 comments) > > > > > > I am curious about the motivation behind the change. > > was intended to rename all soc/mb LPC reference to eSPI. > > > > > It doesn't look like you have gotten rid of all the lpc references anyways? > > I believe anything that might touches common code, i have kept as is else i guess i have rename everything now in newer patchset > > Hi Furquan, Any comments ?
I am just wondering if it makes sense to update the common code to ensure that it uses either:
- Callback to get the right device -- LPC/ESPI
[Subrata] Are you suggesting to add "_espi" in common lpc.c and other functions ?
- Have macros that define the right device and can be used by any ASL code
[Subrata] I guess there is only one ASL where it still name as LPCB because chromeec.asl might looking at device by same name. I'm not sure if i can touch that code to look for any other name as well based on some Kconfig ?
With this, SoC specific code can provide the right callback/macros and we don't have to carry a mix of LPC and eSPI within that code. What do you think?
- No, I was suggesting adding a callback soc_get_lpc_espi_dev() that will return appropriate device LPC or ESPI depending upon the SoC implementation. Then the SoC doesn't need to provide PCH_DEV_LPC.
[Subrata] working on it.
- Humm, yeah it might get tricky based on the way it is being used. I was thinking on the lines of how DPTF_CPU_DEVICE is defined. But, it might not work well here.
[Subrata] DPTF_CPU_DEVICE is just inside device scope unlike "LPCB" used on many ASL files inside function calling hence it might be mess :(
Hi Furquan, can you please help to review
mb/google/{hatch, sarien, arcada}: Make HECI1 chip config disable soc/intel/{cml, whl}: Add option to skip HECI disable in SMM soc/intel/icelake: Replace PCI device LPC to ESPI as per EDS
Hi Subrata,
I looked at the latest patchset and compared it with patchset #6 and I think I like #6 more than this -- we are anyways not able to get rid of lpc references completely not even in the SoC specific code and the extra API call seems to just unnecessarily add more code which is probably not required. Since we anyways have to live with LPCB device and lpc calls, we can just use #define PCH_DEV_LPC PCH_DEV_ESPI like you had before and go ahead with that. Sorry about the back and forth.
Done