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

it wouldn't hurt to add 'CONFIG(PCIEXP_L1_SUB_STATE) &&' to the front of the if statement

That's not the config that I meant. As Nico said, coreboot can enable L1 substates if the FSP says that it's capable. We can use this UPD to get finer-grained control over which devices have L1SS enabled. This may be useful if, for example, only one devices misbehaves with L1SS. In other words, as I understand it, "PCIEXP_L1_SUB_STATE" depends on the FSP UPD (per root-port).

One dependency for L1 substates is clock PM

Should L1SS be guarded by PCIEXP_CLK_PM?

It's all a bit foggy as these Kconfigs don't directly reflect
hardware state. PCIEXP_CLK_PM makes us enable a feature in
the downstream device of a link. This needs a working clock
request signal. Same with L1SS (but this also checks support
in the upstream device). But L1SS doesn't depend on the other
feature. What we'd probably need would be a Kconfig that says
ASSUME_CLK_REQ_SIGNALS, that would be something to depend on.
But I don't think it's worth the trouble. Also, would be much
nicer to directly tag links in the devicetree to have clock
request signals.

Now if FSP does the right thing if we say a port supports
L1SS but no ASPM is a different story. But I don't expect
any trouble if it doesn't filter the case, L1SS would just
not work, I guess.

Btw. there won't be a +2 from me as long as there is the
confusing CamelCase thing (same option name as FSP but with
different encoding). But I'm not blocking it.

View Change

To view, visit change 39538. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I36150858485715016158595c832c142b0582ddb8
Gerrit-Change-Number: 39538
Gerrit-PatchSet: 25
Gerrit-Owner: Benjamin Doron <benjamin.doron00@gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00@gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Comment-Date: Thu, 08 Oct 2020 20:57:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment