| Message ID | 20230502193140.1062470-3-ajayagarwal@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | ASPM: aspm_disable/default state handling fixes | expand |
On 5/2/23 12:31 PM, Ajay Agarwal wrote: > Currently the core driver sets ASPM_STATE_L1 as well as I think you can use the term ASPM driver uniformly. > ASPM_STATE_L1SS when the caller wants to enable just L1.0. L1? > This is incorrect. Fix this by setting the ASPM_STATE_L1 bit > only when the caller wishes to enable L1.0. > > Signed-off-by: Ajay Agarwal <ajayagarwal@google.com> > --- Otherwise, looks fine. Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > Changelog since v1: > - Break down the L1 and L1ss handling into separate patches > > 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 5765b226102a..4ad0bf5d5838 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -1170,8 +1170,7 @@ int pci_enable_link_state(struct pci_dev *pdev, int state) > if (state & PCIE_LINK_STATE_L0S) > link->aspm_default |= ASPM_STATE_L0S; > if (state & PCIE_LINK_STATE_L1) > - /* L1 PM substates require L1 */ > - link->aspm_default |= ASPM_STATE_L1 | ASPM_STATE_L1SS; > + link->aspm_default |= ASPM_STATE_L1; > if (state & PCIE_LINK_STATE_L1_1) > link->aspm_default |= ASPM_STATE_L1_1; > if (state & PCIE_LINK_STATE_L1_2)
On Tue, May 02, 2023 at 06:17:31PM -0700, Sathyanarayanan Kuppuswamy wrote: > > > On 5/2/23 12:31 PM, Ajay Agarwal wrote: > > Currently the core driver sets ASPM_STATE_L1 as well as > > I think you can use the term ASPM driver uniformly. > Ack. Will change in the next version. > > ASPM_STATE_L1SS when the caller wants to enable just L1.0. > > L1? > Ack. Will change in the next version. > > This is incorrect. Fix this by setting the ASPM_STATE_L1 bit > > only when the caller wishes to enable L1.0. > > > > Signed-off-by: Ajay Agarwal <ajayagarwal@google.com> > > --- > > Otherwise, looks fine. > > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > > Changelog since v1: > > - Break down the L1 and L1ss handling into separate patches > > > > 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 5765b226102a..4ad0bf5d5838 100644 > > --- a/drivers/pci/pcie/aspm.c > > +++ b/drivers/pci/pcie/aspm.c > > @@ -1170,8 +1170,7 @@ int pci_enable_link_state(struct pci_dev *pdev, int state) > > if (state & PCIE_LINK_STATE_L0S) > > link->aspm_default |= ASPM_STATE_L0S; > > if (state & PCIE_LINK_STATE_L1) > > - /* L1 PM substates require L1 */ > > - link->aspm_default |= ASPM_STATE_L1 | ASPM_STATE_L1SS; > > + link->aspm_default |= ASPM_STATE_L1; > > if (state & PCIE_LINK_STATE_L1_1) > > link->aspm_default |= ASPM_STATE_L1_1; > > if (state & PCIE_LINK_STATE_L1_2) > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 5765b226102a..4ad0bf5d5838 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -1170,8 +1170,7 @@ int pci_enable_link_state(struct pci_dev *pdev, int state) if (state & PCIE_LINK_STATE_L0S) link->aspm_default |= ASPM_STATE_L0S; if (state & PCIE_LINK_STATE_L1) - /* L1 PM substates require L1 */ - link->aspm_default |= ASPM_STATE_L1 | ASPM_STATE_L1SS; + link->aspm_default |= ASPM_STATE_L1; if (state & PCIE_LINK_STATE_L1_1) link->aspm_default |= ASPM_STATE_L1_1; if (state & PCIE_LINK_STATE_L1_2)
Currently the core driver sets ASPM_STATE_L1 as well as ASPM_STATE_L1SS when the caller wants to enable just L1.0. This is incorrect. Fix this by setting the ASPM_STATE_L1 bit only when the caller wishes to enable L1.0. Signed-off-by: Ajay Agarwal <ajayagarwal@google.com> --- Changelog since v1: - Break down the L1 and L1ss handling into separate patches drivers/pci/pcie/aspm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)