On 5/2/23 12:31 PM, Ajay Agarwal wrote: > Currently the aspm driver sets ASPM_STATE_L1 as well as > ASPM_STATE_L1SS bits in aspm_disable when the caller disables L1. > pcie_config_aspm_link takes care that L1ss ASPM is not enabled > if L1 is disabled. ASPM_STATE_L1SS bits do not need to be > explicitly set. The sysfs node store() function, which also > modifies the aspm_disable value, does not set these bits either > when only L1 ASPM is disabled by the user. > > Disable ASPM_STATE_L1 only when the caller disables L1 ASPM. Maybe you can add something like, No functional changes intended. Otherwise, looks good. Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> > > Signed-off-by: Ajay Agarwal <ajayagarwal@xxxxxxxxxx> > --- > Changelog since v1: > - Better commit message > > drivers/pci/pcie/aspm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 66d7514ca111..5765b226102a 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -1095,8 +1095,7 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem) > if (state & PCIE_LINK_STATE_L0S) > link->aspm_disable |= ASPM_STATE_L0S; > if (state & PCIE_LINK_STATE_L1) > - /* L1 PM substates require L1 */ > - link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS; > + link->aspm_disable |= ASPM_STATE_L1; > if (state & PCIE_LINK_STATE_L1_1) > link->aspm_disable |= ASPM_STATE_L1_1; > if (state & PCIE_LINK_STATE_L1_2) -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
