[<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