public inbox for linux-rockchip@lists.infradead.org
 help / color / mirror / Atom feed
From: Shawn Lin <shawn.lin@rock-chips.com>
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: shawn.lin@rock-chips.com,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com>,
	"Bjorn Helgaas" <helgaas@kernel.org>,
	"Niklas Cassel" <cassel@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	"David E. Box" <david.e.box@linux.intel.com>,
	"Kai-Heng Feng" <kai.heng.feng@canonical.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Chia-Lin Kao" <acelan.kao@canonical.com>,
	"Dragan Simic" <dsimic@manjaro.org>,
	linux-rockchip@lists.infradead.org, regressions@lists.linux.dev,
	"FUKAUMI Naoki" <naoki@radxa.com>
Subject: Re: [PATCH v2 1/2] PCI/ASPM: Override the ASPM and Clock PM states set by BIOS for devicetree platforms
Date: Fri, 17 Oct 2025 20:19:11 +0800	[thread overview]
Message-ID: <e53bd057-0bdc-4f40-acff-eba904b0a728@rock-chips.com> (raw)
In-Reply-To: <22owgu6qb34bh47cevupnwnvwwfhtn4lwfav6fuxfydaiujw6y@oeh3q2u4wo2h>

在 2025/10/17 星期五 18:04, Manivannan Sadhasivam 写道:
> On Fri, Oct 17, 2025 at 05:47:44PM +0800, Shawn Lin wrote:
>> Hi Mani and Bjorn
>>
>> 在 2025/10/17 星期五 11:36, Manivannan Sadhasivam 写道:
>>> On Wed, Oct 15, 2025 at 06:30:54PM -0500, Bjorn Helgaas wrote:
>>>> On Wed, Oct 15, 2025 at 09:00:41PM +0800, Shawn Lin wrote:
>>>>> ...
>>>>
>>>>> For now, this is a acceptable option if default ASPM policy enable
>>>>> L1ss w/o checking if the HW could supports it... But how about
>>>>> adding supports-clkreq stuff to upstream host driver directly? That
>>>>> would help folks enable L1ss if the HW is ready and they just need
>>>>> adding property to the DT.
>>>>> ...
>>>>
>>>>> The L1ss support is quite strict and need several steps to check, so we
>>>>> didn't add supports-clkreq for them unless the HW is ready to go...
>>>>>
>>>>> For adding supports of L1ss,
>>>>> [1] the HW should support CLKREQ#, expecially for PCIe3.0 case on Rockchip
>>>>> SoCs , since both  CLKREQ# of RC and EP should connect to the
>>>>> 100MHz crystal generator's enable pin, as L1.2 need to disable refclk as
>>>>> well. If the enable pin is high active, the HW even need a invertor....
>>>>>
>>>>> [2] define proper clkreq iomux to pinctrl of pcie node
>>>>> [3] make sure the devices work fine with L1ss.(It's hard to check the slot
>>>>> case with random devices in the wild )
>>>>> [4] add supports-clkreq to the DT and enable
>>>>> CONFIG_PCIEASPM_POWER_SUPERSAVE
>>>>
>>>> I don't understand the details of the supports-clkreq issue.
>>>>
>>>> If we need to add supports-clkreq to devicetree, I want to understand
>>>> why we need it there when we don't seem to need it for ACPI systems.
>>>>
>>>> Generally the OS relies on what the hardware advertises, e.g., in Link
>>>> Capabilities and the L1 PM Substates Capability, and what is available
>>>> from firmware, e.g., the ACPI _DSM for Latency Tolerance Reporting.
>>>>
>>>> On the ACPI side, I don't think we get any specific information about
>>>> CLKREQ#.  Can somebody explain why we do need it on the devicetree
>>>> side?
>>>>
>>>
>>> I think there is a disconnect between enabling L1ss CAP and CLKREQ#
>>> availability.. When L1ss CAP is enabled for the Root Port in the hardware, there
>>> is no guarantee that CLKREQ# is also available. If CLKREQ# is not available,
>>> then if L1ss is enabled by the OS, it is not possible to exit the L1ss states
>>> (assuming that L1ss is entered due to CLKREQ# in deassert (default) state).
>>>
>>> Yes, there seems to be no standard way to know CLKREQ# presence in ACPI, but
>>> in devicetree, we have this 'supports-clkreq' property to tell the OS that
>>> CLKREQ# is available in the platform. But unfortunately, this property is not
>>> widely used by the devicetrees out there. So we cannot use it in generic
>>> pci/aspm.c driver.
>>>
>>> We can certainly rely on the BIOS to enable L1ss as the fw developers would
>>> have the knowledge of the CLKREQ# availability. But BIOS is not a thing on
>>> mobile and embedded platforms where L1ss would come handy.
>>>
>>> What I would suggest is, the host controller drivers (mostly for devicetree
>>> platforms) should enable L1ss CAP for the Root Port only if they know that
>>> CLKREQ# is available. They can either rely on the 'supports-clkreq' property or
>>> some platform specific knowledge (for instance, on all Qcom platforms, we
>>> certainly know that CLKREQ# is available, but we don't set the DT property).
>>
>> While we're on the topic of ASPM, may I ask a silly question?
>> I saw the ASPM would only be configured once the function driver calling
>> pci_enable_device. So if the modular driver hasn't been insmoded, the
>> link will be in L0 even though there is no transcation on-going. What is
>> the intention behind it?
>>
> 
> I don't see where ASPM is configured during pci_enable_device(). It is currently
> configured for all devices during pci_scan_slot().

This is the dump_stack() where I observed. If I compile NVMe driver as a
module and never insmod it, the link is always in L0, namely ASPM
Disabled.

[    0.747508] pci 0000:01:00.0: ASPM: DT platform, enabling L0s-up 
L0s-dw L1 ASPM-L1.1 ASPM-L1.2 PCI-PM-L1.1 PCI-PM-L1.2
[    0.748509] pci 0000:01:00.0: ASPM: DT platform, enabling ClockPM
[    0.749145] pci 0000:01:00.0: BAR 0 [mem 0xf0200000-0xf0203fff 
64bit]: assigned
[    0.750430] nvme nvme0: pci function 0000:01:00.0
[    0.750850] CPU: 7 UID: 0 PID: 120 Comm: kworker/u32:7 Not tainted 
6.18.0-rc1-00019-gb00f931c2b43-dirty #19 PREEMPT
[    0.750854] Hardware name: Rockchip RK3588 EVB1 V10 Board (DT)
[    0.750856] Workqueue: async async_run_entry_fn
[    0.750863] Call trace:
[    0.750864]  show_stack+0x18/0x24 (C)
[    0.750869]  dump_stack_lvl+0x40/0x84
[    0.750873]  dump_stack+0x18/0x24
[    0.750876]  pcie_config_aspm_link+0x40/0x2dc
[    0.750882]  pcie_aspm_powersave_config_link+0x68/0x104
[    0.750885]  do_pci_enable_device+0x80/0x170
[    0.750890]  pci_enable_device_flags+0x1b8/0x220
[    0.750894]  pci_enable_device_mem+0x14/0x20
[    0.750898]  nvme_pci_enable+0x3c/0x66c
[    0.750904]  nvme_probe+0x5f8/0x7d4
[    0.750908]  pci_device_probe+0x1dc/0x2ac
[    0.750912]  really_probe+0x138/0x2d8
[    0.750915]  __driver_probe_device+0xa0/0x128
[    0.750918]  driver_probe_device+0x3c/0x1f8
[    0.750921]  __device_attach_driver+0x118/0x140
[    0.750924]  bus_for_each_drv+0xf4/0x14c
[    0.750927]  __device_attach_async_helper+0x78/0xd0
[    0.750930]  async_run_entry_fn+0x24/0xdc
[    0.750933]  process_scheduled_works+0x194/0x2c4
[    0.750937]  worker_thread+0x28c/0x394
[    0.750939]  kthread+0x1bc/0x200
[    0.750943]  ret_from_fork+0x10/0x20
[    0.750947] pcie_config_aspm_link 989
[    0.753436] ehci-platform fc800000.usb: USB 2.0 started, EHCI 1.00
[    0.753620] pcie_config_aspm_link 997 state 0x7c
[    0.754204] hub 4-0:1.0: USB hub found
[    0.754279] pcie_config_aspm_link 1004 state 0x7c
[    0.754281] pcie_config_aspm_link 1008
[    0.754741] hub 4-0:1.0: 1 port detected
[    0.755153] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    0.769741] hub 1-0:1.0: USB hub found
[    0.770084] hub 1-0:1.0: 1 port detected
[    0.801662] hub 3-0:1.0: USB hub found
[    0.802007] hub 3-0:1.0: 1 port detected
[    0.806393] nvme nvme0: D3 entry latency set to 10 seconds
[    0.810188] nvme nvme0: 8/0/0 default/read/poll queues


> 
> - Mani
> 


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  reply	other threads:[~2025-10-17 12:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250922-pci-dt-aspm-v2-0-2a65cf84e326@oss.qualcomm.com>
     [not found] ` <20250922-pci-dt-aspm-v2-1-2a65cf84e326@oss.qualcomm.com>
2025-10-14 16:30   ` [PATCH v2 1/2] PCI/ASPM: Override the ASPM and Clock PM states set by BIOS for devicetree platforms FUKAUMI Naoki
2025-10-14 18:49     ` Bjorn Helgaas
2025-10-14 23:33       ` Dragan Simic
2025-10-15  6:22         ` Manivannan Sadhasivam
2025-10-15 11:23           ` Diederik de Haas
2025-10-23 18:57           ` Dragan Simic
2025-10-15  6:26       ` Manivannan Sadhasivam
2025-10-15  7:13         ` FUKAUMI Naoki
2025-10-15  7:50           ` Manivannan Sadhasivam
2025-10-15  9:11             ` Shawn Lin
2025-10-15  9:43               ` Manivannan Sadhasivam
2025-10-15  9:46               ` Niklas Cassel
2025-10-15 10:33                 ` Manivannan Sadhasivam
2025-10-15 12:17                   ` Niklas Cassel
2025-10-15 13:00                     ` Shawn Lin
2025-10-15 15:23                       ` Niklas Cassel
2025-10-15 23:30                       ` Bjorn Helgaas
2025-10-16  6:46                         ` Hongxing Zhu
2025-10-17  3:36                         ` Manivannan Sadhasivam
2025-10-17  9:47                           ` Shawn Lin
2025-10-17 10:04                             ` Manivannan Sadhasivam
2025-10-17 12:19                               ` Shawn Lin [this message]
2025-10-17 12:54                                 ` Manivannan Sadhasivam
2025-10-17 13:45                                   ` Bjorn Helgaas
2025-10-31  6:21                                     ` Manivannan Sadhasivam
2025-10-15 12:26       ` Diederik de Haas
2025-10-15 22:50         ` Bjorn Helgaas
2025-10-16 17:38           ` Diederik de Haas
2025-10-30 22:14       ` Bjorn Helgaas
2025-10-30 22:16         ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e53bd057-0bdc-4f40-acff-eba904b0a728@rock-chips.com \
    --to=shawn.lin@rock-chips.com \
    --cc=acelan.kao@canonical.com \
    --cc=bhelgaas@google.com \
    --cc=cassel@kernel.org \
    --cc=david.e.box@linux.intel.com \
    --cc=dsimic@manjaro.org \
    --cc=helgaas@kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=kai.heng.feng@canonical.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=manivannan.sadhasivam@oss.qualcomm.com \
    --cc=naoki@radxa.com \
    --cc=rafael@kernel.org \
    --cc=regressions@lists.linux.dev \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox