On Tue, Nov 04, 2025 at 04:17:24PM -0600, Bjorn Helgaas wrote:
On Tue, Nov 04, 2025 at 08:58:02AM +0800, Shawn Lin wrote:
在 2025/11/04 星期二 5:32, Bjorn Helgaas 写道:
On Tue, Oct 28, 2025 at 02:02:18PM -0500, Bjorn Helgaas wrote:
On Fri, Oct 17, 2025 at 06:32:53PM +0200, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, namely: -Proper handling of the CLKREQ# sideband signal. (It is mostly handled by hardware, but software still needs to set the clkreq fields in the PCIE_CLIENT_POWER_CON register to match the hardware implementation.) -Program the frequency of the aux clock into the DSP_PCIE_PL_AUX_CLK_FREQ_OFF register. (During L1 substates the core_clk is turned off and the aux_clk is used instead.)
...
+static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci) +{
- u32 cap, l1subcap;
- cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
- if (cap) {
l1subcap = dw_pcie_readl_dbi(pci, cap + PCI_L1SS_CAP);l1subcap &= ~(PCI_L1SS_CAP_L1_PM_SS | PCI_L1SS_CAP_ASPM_L1_1 |PCI_L1SS_CAP_ASPM_L1_2 | PCI_L1SS_CAP_PCIPM_L1_1 |PCI_L1SS_CAP_PCIPM_L1_2);dw_pcie_writel_dbi(pci, cap + PCI_L1SS_CAP, l1subcap);- }
+}
I like this. But why should we do it just for dw-rockchip? Is there something special about dw-rockchip that makes this a problem? Maybe we should consider doing this in the dwc, cadence, mobiveil, and plda cores instead of trying to do it for every driver individually?
Advertising L1SS support via PCI_EXT_CAP_ID_L1SS means users can enable L1SS via CONFIG_PCIEASPM_POWER_SUPERSAVE=y or sysfs, and that seems likely to cause problems unless CLKREQ# is supported.
Any thoughts on this? There's nothing rockchip-specific in this patch. What I'm proposing is something like this:
I like your idea, though. But could it be another form of regression that we may breaks the platform which have already support L1SS properly? It's even harder to detect because a functional break is easier to notice than increased power consumption.
True, but I think it's unlikely because the PCI core never enabled L1SS (except for CONFIG_PCIEASPM_POWER_SUPERSAVE=y or sysfs, which I doubt anybody really uses).
Devicetree platforms that use L1SS should have explicit code to enable it, like qcom does, so we should be able to find them and make sure they do what's needed to prevent the regression.
Or maybe we could just export dw_pcie_clear_l1ss_advert() in dwc for host drivers to call it?
I don't like the idea of host drivers having to opt in for this because that requires changes to all of them, not just changes to drivers that have done the work to actually support L1SS.
Otherwise, we may have to introduce a flag for the controller drivers to opt-out of this disablement. Like, dw_pcie_rp::native_clkreq and bailing out early if this flag is set.
- Mani