Re: [PATCH v4 06/11] PCI/ASPM: Clear aspm_disable as part of __pci_enable_link_state()

[Hans Zhang <18255117159@163.com>]

On 2025/7/13 01:02, Manivannan Sadhasivam wrote:
> On Sun, Jul 13, 2025 at 12:05:18AM GMT, Hans Zhang wrote:
>>
>>
>> On 2025/7/12 17:35, Manivannan Sadhasivam wrote:
>>>> We only have two callers of this (pcie-qcom.c and vmd.c, both in
>>>> drivers/pci/), so it's not clear to me that it needs to be in
>>>> include/linux/pci.h.
>>>>
>>>> I'm a little dubious about it in the first place since I don't think
>>>> drivers should be enabling ASPM states on their own, but pcie-qcom.c
>>>> and vmd.c are PCIe controller drivers, not PCI device drivers, so I
>>>> guess we can live with them for now.
>>>>
>>>> IMO the "someday" goal should be that we get rid of aspm_policy and
>>>> enable all the available power saving states by default.  We have
>>>> sysfs knobs that administrators can use if necessary, and drivers or
>>>> quirks can disable states if they need to work around hardware
>>>> defects.
>>>>
>>>
>>> Yeah, I think the default should be powersave and let the users disable it for
>>> performance if they want.
>>>
>>
>> Dear Bjorn and Mani,
>>
>> Perhaps I don't think so. At present, our company's testing team has tested
>> quite a few NVMe SSDS. As far as I can remember, the SSDS from two companies
>> have encountered problems and will hang directly when turned on. We have set
>> CONFIG_PCIEASPM_POWERSAVE=y by default. When encountering SSDS from these
>> two companies, we had to add "pcie_aspm.policy=default" in the cmdline, and
>> then the boot worked normally. Currently, we do not have a PCIe protocol
>> analyzer to analyze such issues. The current approach is to modify the
>> cmdline. So I can't prove whether it's a problem with the Root Port of our
>> SOC or the SSD device.
>>
>> Here I agree with Bjorn's statement that sometimes the EP is not necessarily
>> very standard and there are no hardware issues. Personally, I think the
>> default is default or performance. When users need to save power, they
>> should then decide whether to configure it as powersave or powersupersave.
>> Sometimes, if the EP device connected by the customer is perfect, they can
>> turn it on to save power. But if the EP is not perfect, at least they will
>> immediately know what caused the problem.
>>
> 
> We all agree that not all endpoints are standards compliant. So if they have any
> issues with ASPM, then ASPM for those devices should be disabled in the quirks
> or in the device driver.
> 
> That said, the change that Bjorn proposed is not going to happen in the
> immediate future.

Dear Mani,

Ok, I will keep an eye on the changes of ASPM.

Best regards,
Hans