Re: [PATCH RFC 3/3] PCI/ASPM: add sysfs attribute for controlling ASPM

[Heiner Kallweit <hkallweit1@gmail.com>]

On 30.04.2019 19:53, Bjorn Helgaas wrote:
> On Sat, Apr 13, 2019 at 11:12:41AM +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 disabled.
> 
>> +void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev);
>> +void pcie_aspm_remove_sysfs_dev_files(struct pci_dev *pdev);
>>  #else
>>  static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
>>  static inline void pcie_aspm_exit_link_state(struct pci_dev *pdev) { }
>>  static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev) { }
>>  static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
>> -#endif
>> -
>> -#ifdef CONFIG_PCIEASPM_DEBUG
>> -void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev);
>> -void pcie_aspm_remove_sysfs_dev_files(struct pci_dev *pdev);
>> -#else
> 
> I like the idea of exposing these sysfs control files all the time,
> instead of only when CONFIG_PCIEASPM_DEBUG=y, but I think when we do
> that, we should put the files at the downstream end of the link (e.g.,
> an endpoint) instead of at the upstream end (e.g., a root port or
> switch downstream port).  We had some conversation about this here:
> 
> https://lore.kernel.org/lkml/20180727202619.GD173328@bhelgaas-glaptop.roam.corp.google.com
> 
> Doing it at the downstream end would require more changes, of course,
> and probably raises some locking issues, but I think we have a small
> window of opportunity here where we can tweak the sysfs structure
> before we're committed to supporting something forever.
> 
Thanks for the feedback. I'll check the mentioned conversation and try
to get the control moved to the endpoint.

> Bjorn
> 
Heiner