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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>
> 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
This isn't obvious to me as I'm not that familiar with the PCIe subsystem.
- Why do we need more changes on the downstream end?
- Which locking is affected?
> window of opportunity here where we can tweak the sysfs structure
> before we're committed to supporting something forever.
>
> Bjorn
>
Heiner
