mbox series

[v4,0/4] PCI/ASPM: add sysfs attributes for controlling ASPM

Message ID 3797de51-0135-07b6-9566-a1ce8cf3f24e@gmail.com
Headers show
Series PCI/ASPM: add sysfs attributes for controlling ASPM | expand

Message

Heiner Kallweit Aug. 24, 2019, 3:39 p.m. UTC
Background of this extension is a problem with the r8169 network driver.
Several combinations of board chipsets and network chip versions have
problems if ASPM is enabled, therefore we have to disable ASPM per
default. However especially on notebooks ASPM can provide significant
power-saving, therefore we want to give users the option to enable
ASPM. With the new sysfs attributes users can control which ASPM
link-states are disabled.

v2:
- use a dedicated sysfs attribute per link state
- allow separate control of ASPM and PCI PM L1 sub-states

v3:
- patch 3: statically allocate the attribute group
- patch 3: replace snprintf with printf
- add patch 4

v4:
- patch 3: add call to sysfs_update_group because is_visible callback
           returns false always at file creation time
- patch 3: simplify code a little

Heiner Kallweit (4):
  PCI/ASPM: add L1 sub-state support to pci_disable_link_state
  PCI/ASPM: allow to re-enable Clock PM
  PCI/ASPM: add sysfs attributes for controlling ASPM link states
  PCI/ASPM: remove Kconfig option PCIEASPM_DEBUG and related code

 Documentation/ABI/testing/sysfs-bus-pci |  13 ++
 drivers/pci/pci-sysfs.c                 |  10 +-
 drivers/pci/pci.h                       |  12 +-
 drivers/pci/pcie/Kconfig                |   7 -
 drivers/pci/pcie/aspm.c                 | 236 ++++++++++++++++--------
 include/linux/pci-aspm.h                |  10 +-
 6 files changed, 195 insertions(+), 93 deletions(-)

Comments

Bjorn Helgaas Aug. 27, 2019, 11:35 p.m. UTC | #1
On Sat, Aug 24, 2019 at 05:39:37PM +0200, Heiner Kallweit wrote:
> Background of this extension is a problem with the r8169 network driver.
> Several combinations of board chipsets and network chip versions have
> problems if ASPM is enabled, therefore we have to disable ASPM per
> default. However especially on notebooks ASPM can provide significant
> power-saving, therefore we want to give users the option to enable
> ASPM. With the new sysfs attributes users can control which ASPM
> link-states are disabled.
> 
> v2:
> - use a dedicated sysfs attribute per link state
> - allow separate control of ASPM and PCI PM L1 sub-states
> 
> v3:
> - patch 3: statically allocate the attribute group
> - patch 3: replace snprintf with printf
> - add patch 4
> 
> v4:
> - patch 3: add call to sysfs_update_group because is_visible callback
>            returns false always at file creation time
> - patch 3: simplify code a little
> 
> Heiner Kallweit (4):
>   PCI/ASPM: add L1 sub-state support to pci_disable_link_state
>   PCI/ASPM: allow to re-enable Clock PM
>   PCI/ASPM: add sysfs attributes for controlling ASPM link states
>   PCI/ASPM: remove Kconfig option PCIEASPM_DEBUG and related code
> 
>  Documentation/ABI/testing/sysfs-bus-pci |  13 ++
>  drivers/pci/pci-sysfs.c                 |  10 +-
>  drivers/pci/pci.h                       |  12 +-
>  drivers/pci/pcie/Kconfig                |   7 -
>  drivers/pci/pcie/aspm.c                 | 236 ++++++++++++++++--------
>  include/linux/pci-aspm.h                |  10 +-
>  6 files changed, 195 insertions(+), 93 deletions(-)

I can fix this if you don't get to it, but this doesn't apply cleanly
to either my "master" branch (v5.3-rc1) or my "next" branch.  I always
prefer series based on my "master" branch when possible.

Bjorn
Heiner Kallweit Aug. 28, 2019, 5:40 a.m. UTC | #2
On 28.08.2019 01:35, Bjorn Helgaas wrote:
> On Sat, Aug 24, 2019 at 05:39:37PM +0200, Heiner Kallweit wrote:
>> Background of this extension is a problem with the r8169 network driver.
>> Several combinations of board chipsets and network chip versions have
>> problems if ASPM is enabled, therefore we have to disable ASPM per
>> default. However especially on notebooks ASPM can provide significant
>> power-saving, therefore we want to give users the option to enable
>> ASPM. With the new sysfs attributes users can control which ASPM
>> link-states are disabled.
>>
>> v2:
>> - use a dedicated sysfs attribute per link state
>> - allow separate control of ASPM and PCI PM L1 sub-states
>>
>> v3:
>> - patch 3: statically allocate the attribute group
>> - patch 3: replace snprintf with printf
>> - add patch 4
>>
>> v4:
>> - patch 3: add call to sysfs_update_group because is_visible callback
>>            returns false always at file creation time
>> - patch 3: simplify code a little
>>
>> Heiner Kallweit (4):
>>   PCI/ASPM: add L1 sub-state support to pci_disable_link_state
>>   PCI/ASPM: allow to re-enable Clock PM
>>   PCI/ASPM: add sysfs attributes for controlling ASPM link states
>>   PCI/ASPM: remove Kconfig option PCIEASPM_DEBUG and related code
>>
>>  Documentation/ABI/testing/sysfs-bus-pci |  13 ++
>>  drivers/pci/pci-sysfs.c                 |  10 +-
>>  drivers/pci/pci.h                       |  12 +-
>>  drivers/pci/pcie/Kconfig                |   7 -
>>  drivers/pci/pcie/aspm.c                 | 236 ++++++++++++++++--------
>>  include/linux/pci-aspm.h                |  10 +-
>>  6 files changed, 195 insertions(+), 93 deletions(-)
> 
> I can fix this if you don't get to it, but this doesn't apply cleanly
> to either my "master" branch (v5.3-rc1) or my "next" branch.  I always
> prefer series based on my "master" branch when possible.
> 
> Bjorn
> 
I based it on top of linux-next, can rebase it to your master branch.

Heiner
Heiner Kallweit Aug. 28, 2019, 5:48 a.m. UTC | #3
On 28.08.2019 07:40, Heiner Kallweit wrote:
> On 28.08.2019 01:35, Bjorn Helgaas wrote:
>> On Sat, Aug 24, 2019 at 05:39:37PM +0200, Heiner Kallweit wrote:
>>> Background of this extension is a problem with the r8169 network driver.
>>> Several combinations of board chipsets and network chip versions have
>>> problems if ASPM is enabled, therefore we have to disable ASPM per
>>> default. However especially on notebooks ASPM can provide significant
>>> power-saving, therefore we want to give users the option to enable
>>> ASPM. With the new sysfs attributes users can control which ASPM
>>> link-states are disabled.
>>>
>>> v2:
>>> - use a dedicated sysfs attribute per link state
>>> - allow separate control of ASPM and PCI PM L1 sub-states
>>>
>>> v3:
>>> - patch 3: statically allocate the attribute group
>>> - patch 3: replace snprintf with printf
>>> - add patch 4
>>>
>>> v4:
>>> - patch 3: add call to sysfs_update_group because is_visible callback
>>>            returns false always at file creation time
>>> - patch 3: simplify code a little
>>>
>>> Heiner Kallweit (4):
>>>   PCI/ASPM: add L1 sub-state support to pci_disable_link_state
>>>   PCI/ASPM: allow to re-enable Clock PM
>>>   PCI/ASPM: add sysfs attributes for controlling ASPM link states
>>>   PCI/ASPM: remove Kconfig option PCIEASPM_DEBUG and related code
>>>
>>>  Documentation/ABI/testing/sysfs-bus-pci |  13 ++
>>>  drivers/pci/pci-sysfs.c                 |  10 +-
>>>  drivers/pci/pci.h                       |  12 +-
>>>  drivers/pci/pcie/Kconfig                |   7 -
>>>  drivers/pci/pcie/aspm.c                 | 236 ++++++++++++++++--------
>>>  include/linux/pci-aspm.h                |  10 +-
>>>  6 files changed, 195 insertions(+), 93 deletions(-)
>>
>> I can fix this if you don't get to it, but this doesn't apply cleanly
>> to either my "master" branch (v5.3-rc1) or my "next" branch.  I always
>> prefer series based on my "master" branch when possible.
>>
>> Bjorn
>>
> I based it on top of linux-next, can rebase it to your master branch.
> 
> Heiner
> 
Ah, one more point:
This series has a dependency on Mika Westerberg's
"PCI: Make pcie_downstream_port() available outside of access.c"
that is sitting in your inbox. How do you want to deal with this?

Heiner
Bjorn Helgaas Aug. 28, 2019, 1:06 p.m. UTC | #4
On Wed, Aug 28, 2019 at 07:48:18AM +0200, Heiner Kallweit wrote:
> On 28.08.2019 07:40, Heiner Kallweit wrote:
> > On 28.08.2019 01:35, Bjorn Helgaas wrote:
> >> On Sat, Aug 24, 2019 at 05:39:37PM +0200, Heiner Kallweit wrote:
> >>> Background of this extension is a problem with the r8169 network driver.
> >>> Several combinations of board chipsets and network chip versions have
> >>> problems if ASPM is enabled, therefore we have to disable ASPM per
> >>> default. However especially on notebooks ASPM can provide significant
> >>> power-saving, therefore we want to give users the option to enable
> >>> ASPM. With the new sysfs attributes users can control which ASPM
> >>> link-states are disabled.
> >>>
> >>> v2:
> >>> - use a dedicated sysfs attribute per link state
> >>> - allow separate control of ASPM and PCI PM L1 sub-states
> >>>
> >>> v3:
> >>> - patch 3: statically allocate the attribute group
> >>> - patch 3: replace snprintf with printf
> >>> - add patch 4
> >>>
> >>> v4:
> >>> - patch 3: add call to sysfs_update_group because is_visible callback
> >>>            returns false always at file creation time
> >>> - patch 3: simplify code a little
> >>>
> >>> Heiner Kallweit (4):
> >>>   PCI/ASPM: add L1 sub-state support to pci_disable_link_state
> >>>   PCI/ASPM: allow to re-enable Clock PM
> >>>   PCI/ASPM: add sysfs attributes for controlling ASPM link states
> >>>   PCI/ASPM: remove Kconfig option PCIEASPM_DEBUG and related code
> >>>
> >>>  Documentation/ABI/testing/sysfs-bus-pci |  13 ++
> >>>  drivers/pci/pci-sysfs.c                 |  10 +-
> >>>  drivers/pci/pci.h                       |  12 +-
> >>>  drivers/pci/pcie/Kconfig                |   7 -
> >>>  drivers/pci/pcie/aspm.c                 | 236 ++++++++++++++++--------
> >>>  include/linux/pci-aspm.h                |  10 +-
> >>>  6 files changed, 195 insertions(+), 93 deletions(-)
> >>
> >> I can fix this if you don't get to it, but this doesn't apply cleanly
> >> to either my "master" branch (v5.3-rc1) or my "next" branch.  I always
> >> prefer series based on my "master" branch when possible.
> >>
> > I based it on top of linux-next, can rebase it to your master branch.

That'd be great.  linux-next is a moving target, so I never apply
patches based directly on that.

> Ah, one more point:
> This series has a dependency on Mika Westerberg's
> "PCI: Make pcie_downstream_port() available outside of access.c"
> that is sitting in your inbox. How do you want to deal with this?

Just mention the dependency in the cover letter and I'll take care of
it.

Thanks!