[+cc Greg, Rajat] On Thu, May 23, 2019 at 10:05:35PM +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 > attribute users can control which ASPM link-states are enabled/disabled. > > Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx> > --- > Documentation/ABI/testing/sysfs-bus-pci | 13 ++ > drivers/pci/pci.h | 8 +- > drivers/pci/pcie/aspm.c | 180 +++++++++++++++++++++++- > 3 files changed, 193 insertions(+), 8 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci > index 8bfee557e..38fe358de 100644 > --- a/Documentation/ABI/testing/sysfs-bus-pci > +++ b/Documentation/ABI/testing/sysfs-bus-pci > @@ -347,3 +347,16 @@ Description: > If the device has any Peer-to-Peer memory registered, this > file contains a '1' if the memory has been published for > use outside the driver that owns the device. > + > +What: /sys/bus/pci/devices/.../power/aspm_link_states > +Date: May 2019 > +Contact: Heiner Kallweit <hkallweit1@xxxxxxxxx> > +Description: > + If ASPM is supported for an endpoint, then this file can be > + used to enable / disable link states. A link state > + displayed in brackets is enabled, otherwise it's disabled. > + To control link states (case insensitive): > + +state : enables a supported state > + -state : disables a state > + none : disables all link states > + all : enables all supported link states IIUC this "aspm_link_states" file will contain things like this: L0S L1 L1.1 L1.2 # All states supported, all disabled [L0S] L1 # L0s enabled, L1 supported but disabled [L0S] [L1] # L0s and L1 enabled ... and the control is by writing things like this to it: +L1 # enables L1 +L1.1 # enables L1.1 -L0S # disables L0s I know this file structure is similar to protocol handling in drivers/media/rc/rc-main.c, but Documentation/filesystems/sysfs.txt suggests single values in a file, and Greg recently pointed out that we screwed up some PCI AER stats [1]. So I'm thinking maybe we should split this into several files, e.g., /sys/devices/pci*/.../power/aspm_l0s /sys/devices/pci*/.../power/aspm_l1 /sys/devices/pci*/.../power/aspm_l1.1 /sys/devices/pci*/.../power/aspm_l1.2 which would contain just 1/0 values, and we'd write 1/0 to enable/disable things. Since the L1 PM Substates control register has separate enable bits for PCI-PM L1.1 and L1.2, we might also want a way to manage those. Bjorn [1] https://lore.kernel.org/r/20190621072911.GA21600@xxxxxxxxx
