lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <B08A289028EE2F0D+c2d1f2e5-d7c6-497a-82bd-92ae477e1016@radxa.com>
Date: Thu, 23 Oct 2025 14:01:59 +0900
From: FUKAUMI Naoki <naoki@...xa.com>
To: Bjorn Helgaas <helgaas@...nel.org>,
 Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
Cc: Christian Zigotzky <chzigotzky@...osoft.de>, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
 linux-rockchip@...ts.infradead.org
Subject: Re: [RESEND] Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for
 devicetree platforms

On 10/23/25 13:25, FUKAUMI Naoki wrote:
> # Fixes the ML address for linux-rockchip
> # Please resend the original patch to linux-rockchip@...ts.infradead.org

and linuxppc-dev@...ts.ozlabs.org?

> Hi Bjorn,
> 
> On 10/23/25 04:13, Bjorn Helgaas wrote:
>> Christian, Naoki, any chance you could test this patch on top of
>> v6.18-rc1 to see whether it resolves the problem you reported?
>>
>> I'd like to verify that it works before merging it.
> 
> I'll be testing now. May I test on v6.18-rc2 without the following patch?
> 
>   "PCI: dw-rockchip: Prevent advertising L1 Substates support"
> 
> Best regards,
> 
> -- 
> FUKAUMI Naoki
> Radxa Computer (Shenzhen) Co., Ltd.
> 
>> On Mon, Oct 20, 2025 at 05:12:07PM -0500, Bjorn Helgaas wrote:
>>> From: Bjorn Helgaas <bhelgaas@...gle.com>
>>>
>>> f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for 
>>> devicetree
>>> platforms") enabled Clock Power Management and L1 Substates, but that
>>> caused regressions because these features depend on CLKREQ#, and not all
>>> devices and form factors support it.
>>>
>>> Enable only ASPM L0s and L1, and only when both ends of the link 
>>> advertise
>>> support for them.
>>>
>>> Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states 
>>> for devicetree platforms")
>>> Reported-by: Christian Zigotzky <chzigotzky@...osoft.de>
>>> Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045- 
>>> a1b04908051a@...osoft.de/
>>> Reported-by: FUKAUMI Naoki <naoki@...xa.com>
>>> Link: https://lore.kernel.org/ 
>>> r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@...xa.com/

and https://lore.kernel.org/all/DDIW7ZP5K1VR.2I7VW56B9CZLF@cknow-tech.com/

maybe https://lore.kernel.org/all/20251015101304.3ec03e6b@bootlin.com/
(then +Cc: linux-arm-kernel@...ts.infradead.org?)

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>>> ---
>>>
>>> Mani, not sure what you think we should do here.  Here's a stab at it 
>>> as a
>>> strawman and in case anybody can test it.
>>>
>>> Not sure about the message log message.  Maybe OK for testing, but 
>>> might be
>>> overly verbose ultimately.
>>>
>>> ---
>>>   drivers/pci/pcie/aspm.c | 34 +++++++++-------------------------
>>>   1 file changed, 9 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>>> index 7cc8281e7011..dbc74cc85bcb 100644
>>> --- a/drivers/pci/pcie/aspm.c
>>> +++ b/drivers/pci/pcie/aspm.c
>>> @@ -243,8 +243,7 @@ struct pcie_link_state {
>>>       /* Clock PM state */
>>>       u32 clkpm_capable:1;        /* Clock PM capable? */
>>>       u32 clkpm_enabled:1;        /* Current Clock PM state */
>>> -    u32 clkpm_default:1;        /* Default Clock PM state by BIOS or
>>> -                       override */
>>> +    u32 clkpm_default:1;        /* Default Clock PM state by BIOS */
>>>       u32 clkpm_disable:1;        /* Clock PM disabled */
>>>   };
>>> @@ -376,18 +375,6 @@ static void pcie_set_clkpm(struct 
>>> pcie_link_state *link, int enable)
>>>       pcie_set_clkpm_nocheck(link, enable);
>>>   }
>>> -static void pcie_clkpm_override_default_link_state(struct 
>>> pcie_link_state *link,
>>> -                           int enabled)
>>> -{
>>> -    struct pci_dev *pdev = link->downstream;
>>> -
>>> -    /* For devicetree platforms, enable ClockPM by default */
>>> -    if (of_have_populated_dt() && !enabled) {
>>> -        link->clkpm_default = 1;
>>> -        pci_info(pdev, "ASPM: DT platform, enabling ClockPM\n");
>>> -    }
>>> -}
>>> -
>>>   static void pcie_clkpm_cap_init(struct pcie_link_state *link, int 
>>> blacklist)
>>>   {
>>>       int capable = 1, enabled = 1;
>>> @@ -410,7 +397,6 @@ static void pcie_clkpm_cap_init(struct 
>>> pcie_link_state *link, int blacklist)
>>>       }
>>>       link->clkpm_enabled = enabled;
>>>       link->clkpm_default = enabled;
>>> -    pcie_clkpm_override_default_link_state(link, enabled);
>>>       link->clkpm_capable = capable;
>>>       link->clkpm_disable = blacklist ? 1 : 0;
>>>   }
>>> @@ -811,19 +797,17 @@ static void 
>>> pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>>>       struct pci_dev *pdev = link->downstream;
>>>       u32 override;
>>> -    /* For devicetree platforms, enable all ASPM states by default */
>>> +    /* For devicetree platforms, enable L0s and L1 by default */
>>>       if (of_have_populated_dt()) {
>>> -        link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
>>> +        if (link->aspm_support & PCIE_LINK_STATE_L0S)
>>> +            link->aspm_default |= PCIE_LINK_STATE_L0S;
>>> +        if (link->aspm_support & PCIE_LINK_STATE_L1)
>>> +            link->aspm_default |= PCIE_LINK_STATE_L1;
>>>           override = link->aspm_default & ~link->aspm_enabled;
>>>           if (override)
>>> -            pci_info(pdev, "ASPM: DT platform, 
>>> enabling%s%s%s%s%s%s%s\n",
>>> -                 FLAG(override, L0S_UP, " L0s-up"),
>>> -                 FLAG(override, L0S_DW, " L0s-dw"),
>>> -                 FLAG(override, L1, " L1"),
>>> -                 FLAG(override, L1_1, " ASPM-L1.1"),
>>> -                 FLAG(override, L1_2, " ASPM-L1.2"),
>>> -                 FLAG(override, L1_1_PCIPM, " PCI-PM-L1.1"),
>>> -                 FLAG(override, L1_2_PCIPM, " PCI-PM-L1.2"));
>>> +            pci_info(pdev, "ASPM: DT platform, enabling%s%s\n",
>>> +                 FLAG(override, L0S, " L0s"),
>>> +                 FLAG(override, L1, " L1"));
>>>       }
>>>   }
>>> -- 
>>> 2.43.0
>>>
>>
> 
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ