Change in coreboot[master]: soc/intel/skylake: Configure L1 substates for PCH root ports

[Benjamin Doron (Code Review)]

Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39538 )

Change subject: soc/intel/skylake: Configure L1 substates for PCH root ports
......................................................................

Patch Set 25:
> Patch Set 25: Code-Review+1
> > Patch Set 25:
> > > Patch Set 25:
> > > > Patch Set 25: Code-Review+1
> > > >
> > > > LGTM. Are there any interdependencies with any of the other PCIe PM-related settings?
> > >
> > > Perhaps that "PcieRpAspm" doesn't disable L1 for a given root port?
> >
> > should coreboot automatically disable L1SS on a given port if PcieRpAspm is disabled for that port? Or are we just depending on the board config to be sane?
>
> pciexp_config_L1_sub_state() checks for the capabilities. If we tell FSP
> that it's not capable, the root port's capability regs should reflect
> that (I never tested it, though).
>
> One dependency for L1 substates is clock PM (i.e. a usable clockreq
> signal). There is no capability for that. It's currently only
> configurable globally via Kconfig (cf. CB:41696).

Do you think that L1 substates should be guarded by that Kconfig?