* [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
@ 2025-10-23 2:51 Shawn Lin
2025-10-23 2:51 ` [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Shawn Lin @ 2025-10-23 2:51 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, Niklas Cassel, linux-pci, Shawn Lin
L1 PM Substates for RC mode require support in the dw-rockchip driver
including 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.
For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
Meanwhile, for the EP mode, we haven't prepared enough to actually support
L1 PM Substates yet. So disable it now until proper support is added later.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v3:
- rephrease the changelog
- use FIELD_PREP_WM16
- rename to rockchip_pcie_configure_l1sub
- disable L1ss for EP mode
Changes in v2:
- drop of_pci_clkreq_presnt API
- drop dependency of Niklas's patch
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 3e2752c..25d2474 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -62,6 +62,12 @@
/* Interrupt Mask Register Related to Miscellaneous Operation */
#define PCIE_CLIENT_INTR_MASK_MISC 0x24
+/* Power Management Control Register */
+#define PCIE_CLIENT_POWER_CON 0x2c
+#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
+#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
+#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
+
/* Hot Reset Control Register */
#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
#define PCIE_LTSSM_APP_DLY2_EN BIT(1)
@@ -85,6 +91,7 @@ struct rockchip_pcie {
struct regulator *vpcie3v3;
struct irq_domain *irq_domain;
const struct rockchip_pcie_of_data *data;
+ bool supports_clkreq;
};
struct rockchip_pcie_of_data {
@@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
}
+/*
+ * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
+ * needed to support L1 substates. Currently, just enable L1 substates for RC
+ * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
+ * For EP mode, there are more things should be done to actually save power in
+ * L1 substates, so disable L1 substates until there is proper support.
+ */
+static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
+{
+ struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
+ u32 cap, l1subcap;
+
+ /* Enable L1 substates if CLKREQ# is properly connected */
+ if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
+ rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
+ return;
+ }
+
+ /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
+ rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
+ PCIE_CLIENT_POWER_CON);
+ 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);
+ }
+}
+
static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
{
u32 cap, lnkcap;
@@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
rockchip);
+ rockchip_pcie_configure_l1sub(pci);
rockchip_pcie_enable_l0s(pci);
return 0;
@@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
enum pci_barno bar;
+ rockchip_pcie_configure_l1sub(pci);
rockchip_pcie_enable_l0s(pci);
rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
@@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
"failed to get reset lines\n");
+ rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
+ "supports-clkreq");
+
return 0;
}
--
2.7.4
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1
2025-10-23 2:51 [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Shawn Lin
@ 2025-10-23 2:51 ` Shawn Lin
2025-10-23 5:06 ` Manivannan Sadhasivam
2025-10-23 5:05 ` [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Manivannan Sadhasivam
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Shawn Lin @ 2025-10-23 2:51 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, Niklas Cassel, linux-pci, Shawn Lin
Add supports-clkreq and pinmux for PCIe ASPM L1 substates.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Hans Zhang <hans.zhang@cixtech.com>
---
Changes in v3:
- add Hans' tag
Changes in v2: None
arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
index ff1ba5e..c9d284c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
@@ -522,6 +522,7 @@
pinctrl-names = "default";
pinctrl-0 = <&pcie2_0_rst>, <&pcie2_0_wake>, <&pcie2_0_clkreq>, <&wifi_host_wake_irq>;
reset-gpios = <&gpio4 RK_PA5 GPIO_ACTIVE_HIGH>;
+ supports-clkreq;
vpcie3v3-supply = <&vcc3v3_wlan>;
status = "okay";
@@ -545,7 +546,8 @@
&pcie2x1l1 {
reset-gpios = <&gpio4 RK_PA2 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
- pinctrl-0 = <&pcie2_1_rst>, <&rtl8111_isolate>;
+ pinctrl-0 = <&pcie2_1_rst>, <&rtl8111_isolate>, <&pcie30x1m1_1_clkreqn>;
+ supports-clkreq;
status = "okay";
};
@@ -555,7 +557,8 @@
&pcie3x4 {
pinctrl-names = "default";
- pinctrl-0 = <&pcie3_reset>;
+ pinctrl-0 = <&pcie3_reset>, <&pcie30x4m1_clkreqn>;
+ supports-clkreq;
reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>;
vpcie3v3-supply = <&vcc3v3_pcie30>;
status = "okay";
--
2.7.4
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 2:51 [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Shawn Lin
2025-10-23 2:51 ` [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
@ 2025-10-23 5:05 ` Manivannan Sadhasivam
2025-10-23 6:26 ` Shawn Lin
2025-10-23 9:49 ` Diederik de Haas
2025-10-23 15:46 ` Frank Li
3 siblings, 1 reply; 14+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-23 5:05 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Bjorn Helgaas, linux-rockchip, Niklas Cassel,
linux-pci
On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
> L1 PM Substates for RC mode require support in the dw-rockchip driver
> including 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.
>
> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>
> Meanwhile, for the EP mode, we haven't prepared enough to actually support
> L1 PM Substates yet. So disable it now until proper support is added later.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
> Changes in v3:
> - rephrease the changelog
> - use FIELD_PREP_WM16
> - rename to rockchip_pcie_configure_l1sub
> - disable L1ss for EP mode
>
> Changes in v2:
> - drop of_pci_clkreq_presnt API
> - drop dependency of Niklas's patch
>
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 3e2752c..25d2474 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -62,6 +62,12 @@
> /* Interrupt Mask Register Related to Miscellaneous Operation */
> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>
> +/* Power Management Control Register */
> +#define PCIE_CLIENT_POWER_CON 0x2c
> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
> +
> /* Hot Reset Control Register */
> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> @@ -85,6 +91,7 @@ struct rockchip_pcie {
> struct regulator *vpcie3v3;
> struct irq_domain *irq_domain;
> const struct rockchip_pcie_of_data *data;
> + bool supports_clkreq;
> };
>
> struct rockchip_pcie_of_data {
> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
> }
>
> +/*
> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
> + * needed to support L1 substates. Currently, just enable L1 substates for RC
> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
> + * For EP mode, there are more things should be done to actually save power in
> + * L1 substates, so disable L1 substates until there is proper support.
> + */
> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
> +{
> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> + u32 cap, l1subcap;
> +
> + /* Enable L1 substates if CLKREQ# is properly connected */
> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
> + return;
> + }
> +
> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> + PCIE_CLIENT_POWER_CON);
> + 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);
> + }
> +}
> +
> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> {
> u32 cap, lnkcap;
> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> rockchip);
>
> + rockchip_pcie_configure_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
>
> return 0;
> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> enum pci_barno bar;
>
> + rockchip_pcie_configure_l1sub(pci);
Didn't you agree to drop the EP change?
- Mani
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1
2025-10-23 2:51 ` [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
@ 2025-10-23 5:06 ` Manivannan Sadhasivam
0 siblings, 0 replies; 14+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-23 5:06 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Bjorn Helgaas, linux-rockchip, Niklas Cassel,
linux-pci
On Thu, Oct 23, 2025 at 10:51:23AM +0800, Shawn Lin wrote:
> Add supports-clkreq and pinmux for PCIe ASPM L1 substates.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> Reviewed-by: Hans Zhang <hans.zhang@cixtech.com>
> ---
>
> Changes in v3:
> - add Hans' tag
>
> Changes in v2: None
>
> arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
> index ff1ba5e..c9d284c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
> @@ -522,6 +522,7 @@
> pinctrl-names = "default";
> pinctrl-0 = <&pcie2_0_rst>, <&pcie2_0_wake>, <&pcie2_0_clkreq>, <&wifi_host_wake_irq>;
> reset-gpios = <&gpio4 RK_PA5 GPIO_ACTIVE_HIGH>;
> + supports-clkreq;
> vpcie3v3-supply = <&vcc3v3_wlan>;
> status = "okay";
>
> @@ -545,7 +546,8 @@
> &pcie2x1l1 {
> reset-gpios = <&gpio4 RK_PA2 GPIO_ACTIVE_HIGH>;
> pinctrl-names = "default";
> - pinctrl-0 = <&pcie2_1_rst>, <&rtl8111_isolate>;
> + pinctrl-0 = <&pcie2_1_rst>, <&rtl8111_isolate>, <&pcie30x1m1_1_clkreqn>;
> + supports-clkreq;
> status = "okay";
> };
>
> @@ -555,7 +557,8 @@
>
> &pcie3x4 {
> pinctrl-names = "default";
> - pinctrl-0 = <&pcie3_reset>;
> + pinctrl-0 = <&pcie3_reset>, <&pcie30x4m1_clkreqn>;
> + supports-clkreq;
> reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>;
> vpcie3v3-supply = <&vcc3v3_pcie30>;
> status = "okay";
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 5:05 ` [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Manivannan Sadhasivam
@ 2025-10-23 6:26 ` Shawn Lin
2025-10-23 6:50 ` Manivannan Sadhasivam
0 siblings, 1 reply; 14+ messages in thread
From: Shawn Lin @ 2025-10-23 6:26 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: shawn.lin, Heiko Stuebner, Bjorn Helgaas, linux-rockchip,
Niklas Cassel, linux-pci
Hi Mani
在 2025/10/23 星期四 13:05, Manivannan Sadhasivam 写道:
> On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
>> L1 PM Substates for RC mode require support in the dw-rockchip driver
>> including 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.
>>
>> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
>> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>>
>> Meanwhile, for the EP mode, we haven't prepared enough to actually support
>> L1 PM Substates yet. So disable it now until proper support is added later.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v3:
>> - rephrease the changelog
>> - use FIELD_PREP_WM16
>> - rename to rockchip_pcie_configure_l1sub
>> - disable L1ss for EP mode
>>
>> Changes in v2:
>> - drop of_pci_clkreq_presnt API
>> - drop dependency of Niklas's patch
>>
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> index 3e2752c..25d2474 100644
>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> @@ -62,6 +62,12 @@
>> /* Interrupt Mask Register Related to Miscellaneous Operation */
>> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>>
>> +/* Power Management Control Register */
>> +#define PCIE_CLIENT_POWER_CON 0x2c
>> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
>> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
>> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
>> +
>> /* Hot Reset Control Register */
>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>> @@ -85,6 +91,7 @@ struct rockchip_pcie {
>> struct regulator *vpcie3v3;
>> struct irq_domain *irq_domain;
>> const struct rockchip_pcie_of_data *data;
>> + bool supports_clkreq;
>> };
>>
>> struct rockchip_pcie_of_data {
>> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
>> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
>> }
>>
>> +/*
>> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
>> + * needed to support L1 substates. Currently, just enable L1 substates for RC
>> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
>> + * For EP mode, there are more things should be done to actually save power in
>> + * L1 substates, so disable L1 substates until there is proper support.
>> + */
>> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
>> +{
>> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>> + u32 cap, l1subcap;
>> +
>> + /* Enable L1 substates if CLKREQ# is properly connected */
>> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
>> + return;
>> + }
>> +
>> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
>> + PCIE_CLIENT_POWER_CON);
>> + 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);
>> + }
>> +}
>> +
>> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>> {
>> u32 cap, lnkcap;
>> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
>> rockchip);
>>
>> + rockchip_pcie_configure_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>>
>> return 0;
>> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> enum pci_barno bar;
>>
>> + rockchip_pcie_configure_l1sub(pci);
>
> Didn't you agree to drop the EP change?
My previous understanding was that you wanted me not to rely on
supports-clkreq to decide whether to enable the EP's support for L1 PM
Substates. Therefore, my patch did the same thing as Niklas' patch,
which is to temporarily disable L1 PM Substates in EP mode. So shold I
keep the EP change?
>
> - Mani
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 6:26 ` Shawn Lin
@ 2025-10-23 6:50 ` Manivannan Sadhasivam
0 siblings, 0 replies; 14+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-23 6:50 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Bjorn Helgaas, linux-rockchip, Niklas Cassel,
linux-pci
On Thu, Oct 23, 2025 at 02:26:14PM +0800, Shawn Lin wrote:
> Hi Mani
>
> 在 2025/10/23 星期四 13:05, Manivannan Sadhasivam 写道:
> > On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
> > > L1 PM Substates for RC mode require support in the dw-rockchip driver
> > > including 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.
> > >
> > > For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
> > > Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
> > >
> > > Meanwhile, for the EP mode, we haven't prepared enough to actually support
> > > L1 PM Substates yet. So disable it now until proper support is added later.
> > >
> > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > >
> > > ---
> > >
> > > Changes in v3:
> > > - rephrease the changelog
> > > - use FIELD_PREP_WM16
> > > - rename to rockchip_pcie_configure_l1sub
> > > - disable L1ss for EP mode
> > >
> > > Changes in v2:
> > > - drop of_pci_clkreq_presnt API
> > > - drop dependency of Niklas's patch
> > >
> > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
> > > 1 file changed, 43 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > index 3e2752c..25d2474 100644
> > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > @@ -62,6 +62,12 @@
> > > /* Interrupt Mask Register Related to Miscellaneous Operation */
> > > #define PCIE_CLIENT_INTR_MASK_MISC 0x24
> > > +/* Power Management Control Register */
> > > +#define PCIE_CLIENT_POWER_CON 0x2c
> > > +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
> > > +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
> > > +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
> > > +
> > > /* Hot Reset Control Register */
> > > #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> > > #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> > > @@ -85,6 +91,7 @@ struct rockchip_pcie {
> > > struct regulator *vpcie3v3;
> > > struct irq_domain *irq_domain;
> > > const struct rockchip_pcie_of_data *data;
> > > + bool supports_clkreq;
> > > };
> > > struct rockchip_pcie_of_data {
> > > @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> > > return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
> > > }
> > > +/*
> > > + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
> > > + * needed to support L1 substates. Currently, just enable L1 substates for RC
> > > + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
> > > + * For EP mode, there are more things should be done to actually save power in
> > > + * L1 substates, so disable L1 substates until there is proper support.
> > > + */
> > > +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
> > > +{
> > > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> > > + u32 cap, l1subcap;
> > > +
> > > + /* Enable L1 substates if CLKREQ# is properly connected */
> > > + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
> > > + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
> > > + return;
> > > + }
> > > +
> > > + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
> > > + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> > > + PCIE_CLIENT_POWER_CON);
> > > + 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);
> > > + }
> > > +}
> > > +
> > > static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> > > {
> > > u32 cap, lnkcap;
> > > @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> > > irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> > > rockchip);
> > > + rockchip_pcie_configure_l1sub(pci);
> > > rockchip_pcie_enable_l0s(pci);
> > > return 0;
> > > @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > enum pci_barno bar;
> > > + rockchip_pcie_configure_l1sub(pci);
> >
> > Didn't you agree to drop the EP change?
>
> My previous understanding was that you wanted me not to rely on
> supports-clkreq to decide whether to enable the EP's support for L1 PM
> Substates. Therefore, my patch did the same thing as Niklas' patch, which is
> to temporarily disable L1 PM Substates in EP mode. So shold I
> keep the EP change?
>
Maybe I got confused a bit. You can keep it disabled as-is. Please ignore my
above comment.
- Mani
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 2:51 [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Shawn Lin
2025-10-23 2:51 ` [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
2025-10-23 5:05 ` [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Manivannan Sadhasivam
@ 2025-10-23 9:49 ` Diederik de Haas
2025-10-23 10:22 ` Shawn Lin
2025-10-23 15:46 ` Frank Li
3 siblings, 1 reply; 14+ messages in thread
From: Diederik de Haas @ 2025-10-23 9:49 UTC (permalink / raw)
To: Shawn Lin, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, Niklas Cassel, linux-pci
On Thu Oct 23, 2025 at 4:51 AM CEST, Shawn Lin wrote:
> L1 PM Substates for RC mode require support in the dw-rockchip driver
> including 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.
>
> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>
> Meanwhile, for the EP mode, we haven't prepared enough to actually support
> L1 PM Substates yet. So disable it now until proper support is added later.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
> Changes in v3:
> - rephrease the changelog
> - use FIELD_PREP_WM16
> - rename to rockchip_pcie_configure_l1sub
> - disable L1ss for EP mode
>
> Changes in v2:
> - drop of_pci_clkreq_presnt API
> - drop dependency of Niklas's patch
>
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 3e2752c..25d2474 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -62,6 +62,12 @@
> /* Interrupt Mask Register Related to Miscellaneous Operation */
> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>
> +/* Power Management Control Register */
> +#define PCIE_CLIENT_POWER_CON 0x2c
> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
> +
> /* Hot Reset Control Register */
> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> @@ -85,6 +91,7 @@ struct rockchip_pcie {
> struct regulator *vpcie3v3;
> struct irq_domain *irq_domain;
> const struct rockchip_pcie_of_data *data;
> + bool supports_clkreq;
> };
>
> struct rockchip_pcie_of_data {
> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
> }
>
> +/*
> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
> + * needed to support L1 substates. Currently, just enable L1 substates for RC
> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
> + * For EP mode, there are more things should be done to actually save power in
"For EP mode, more things should be done ..." or
"For EP mode, there are more things that should be done ..."
Q: Is this patch set to fix the problem I and others reported wrt ASPM?
Because there's Niklas' patch which has been accepted, there's another
patch by Bjorn which initially didn't land on the linux-rockchip ML and
there's this patch set.
To me, those *seem* all 3 different solutions to the same issue, but
it's quite possible I'm wrong in that. Apparently to try Bjorn's patch,
one should not have Niklas' patch applied. And I have no idea what, if
any, the relationship is of this patch set with those others.
Some clarification would be helpful for n00bs like me.
Cheers,
Diederik
> + * L1 substates, so disable L1 substates until there is proper support.
> + */
> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
> +{
> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> + u32 cap, l1subcap;
> +
> + /* Enable L1 substates if CLKREQ# is properly connected */
> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
> + return;
> + }
> +
> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> + PCIE_CLIENT_POWER_CON);
> + 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);
> + }
> +}
> +
> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> {
> u32 cap, lnkcap;
> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> rockchip);
>
> + rockchip_pcie_configure_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
>
> return 0;
> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> enum pci_barno bar;
>
> + rockchip_pcie_configure_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>
> @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
> return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
> "failed to get reset lines\n");
>
> + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
> + "supports-clkreq");
> +
> return 0;
> }
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 9:49 ` Diederik de Haas
@ 2025-10-23 10:22 ` Shawn Lin
0 siblings, 0 replies; 14+ messages in thread
From: Shawn Lin @ 2025-10-23 10:22 UTC (permalink / raw)
To: Diederik de Haas
Cc: shawn.lin, linux-rockchip, Niklas Cassel, linux-pci,
Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
在 2025/10/23 星期四 17:49, Diederik de Haas 写道:
> On Thu Oct 23, 2025 at 4:51 AM CEST, Shawn Lin wrote:
>> L1 PM Substates for RC mode require support in the dw-rockchip driver
>> including 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.
>>
>> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
>> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>>
>> Meanwhile, for the EP mode, we haven't prepared enough to actually support
>> L1 PM Substates yet. So disable it now until proper support is added later.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v3:
>> - rephrease the changelog
>> - use FIELD_PREP_WM16
>> - rename to rockchip_pcie_configure_l1sub
>> - disable L1ss for EP mode
>>
>> Changes in v2:
>> - drop of_pci_clkreq_presnt API
>> - drop dependency of Niklas's patch
>>
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> index 3e2752c..25d2474 100644
>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> @@ -62,6 +62,12 @@
>> /* Interrupt Mask Register Related to Miscellaneous Operation */
>> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>>
>> +/* Power Management Control Register */
>> +#define PCIE_CLIENT_POWER_CON 0x2c
>> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
>> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
>> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
>> +
>> /* Hot Reset Control Register */
>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>> @@ -85,6 +91,7 @@ struct rockchip_pcie {
>> struct regulator *vpcie3v3;
>> struct irq_domain *irq_domain;
>> const struct rockchip_pcie_of_data *data;
>> + bool supports_clkreq;
>> };
>>
>> struct rockchip_pcie_of_data {
>> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
>> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
>> }
>>
>> +/*
>> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
>> + * needed to support L1 substates. Currently, just enable L1 substates for RC
>> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
>> + * For EP mode, there are more things should be done to actually save power in
>
> "For EP mode, more things should be done ..." or
> "For EP mode, there are more things that should be done ..."
>
> Q: Is this patch set to fix the problem I and others reported wrt ASPM?
>
> Because there's Niklas' patch which has been accepted, there's another
> patch by Bjorn which initially didn't land on the linux-rockchip ML and
> there's this patch set.
> To me, those *seem* all 3 different solutions to the same issue, but
> it's quite possible I'm wrong in that. Apparently to try Bjorn's patch,
> one should not have Niklas' patch applied. And I have no idea what, if
> any, the relationship is of this patch set with those others.
>
> Some clarification would be helpful for n00bs like me.
I think Bejorn's patch is for v6.18 to not enable L1ss support on
different platforms. And Niklas' patch is for v6.19 to explicitly
disable L1ss on Rockchip platforms, because even with Bejorn's
patch, someone could still enable L1ss and make dw-rockchip broken.
This $subject patch just try to implement L1ss support on Rockchip's
platform for v6.19 cycle too. And by previous discussion, we will
replace Niklas' patch with this one. Hope it explains the situation
clearly.
>
> Cheers,
> Diederik
>
>> + * L1 substates, so disable L1 substates until there is proper support.
>> + */
>> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
>> +{
>> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>> + u32 cap, l1subcap;
>> +
>> + /* Enable L1 substates if CLKREQ# is properly connected */
>> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
>> + return;
>> + }
>> +
>> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
>> + PCIE_CLIENT_POWER_CON);
>> + 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);
>> + }
>> +}
>> +
>> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>> {
>> u32 cap, lnkcap;
>> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
>> rockchip);
>>
>> + rockchip_pcie_configure_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>>
>> return 0;
>> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> enum pci_barno bar;
>>
>> + rockchip_pcie_configure_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>>
>> @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
>> return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
>> "failed to get reset lines\n");
>>
>> + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
>> + "supports-clkreq");
>> +
>> return 0;
>> }
>>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 2:51 [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Shawn Lin
` (2 preceding siblings ...)
2025-10-23 9:49 ` Diederik de Haas
@ 2025-10-23 15:46 ` Frank Li
2025-10-24 0:43 ` Shawn Lin
3 siblings, 1 reply; 14+ messages in thread
From: Frank Li @ 2025-10-23 15:46 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
linux-rockchip, Niklas Cassel, linux-pci
On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
> L1 PM Substates for RC mode require support in the dw-rockchip driver
> including 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.
>
> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>
> Meanwhile, for the EP mode, we haven't prepared enough to actually support
> L1 PM Substates yet. So disable it now until proper support is added later.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
> Changes in v3:
> - rephrease the changelog
> - use FIELD_PREP_WM16
> - rename to rockchip_pcie_configure_l1sub
> - disable L1ss for EP mode
>
> Changes in v2:
> - drop of_pci_clkreq_presnt API
> - drop dependency of Niklas's patch
>
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 3e2752c..25d2474 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -62,6 +62,12 @@
> /* Interrupt Mask Register Related to Miscellaneous Operation */
> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>
> +/* Power Management Control Register */
> +#define PCIE_CLIENT_POWER_CON 0x2c
> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
> +
> /* Hot Reset Control Register */
> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> @@ -85,6 +91,7 @@ struct rockchip_pcie {
> struct regulator *vpcie3v3;
> struct irq_domain *irq_domain;
> const struct rockchip_pcie_of_data *data;
> + bool supports_clkreq;
> };
>
> struct rockchip_pcie_of_data {
> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
> }
>
> +/*
> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
> + * needed to support L1 substates. Currently, just enable L1 substates for RC
> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
> + * For EP mode, there are more things should be done to actually save power in
> + * L1 substates, so disable L1 substates until there is proper support.
> + */
> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
> +{
> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> + u32 cap, l1subcap;
> +
> + /* Enable L1 substates if CLKREQ# is properly connected */
> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
> + return;
> + }
> +
> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> + PCIE_CLIENT_POWER_CON);
Looks like you force pull down clkreq should be enough, needn't disable
L1SS. when RC force clkreq is low, Ref CLK always on even if L1SS enabled.
Of course it depend on hardware implementation, But I think FULL_DOWN have
high priority to force clkreq to low then PCI_L1SS control.
Frank
> + 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);
> + }
> +}
> +
> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> {
> u32 cap, lnkcap;
> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> rockchip);
>
> + rockchip_pcie_configure_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
>
> return 0;
> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> enum pci_barno bar;
>
> + rockchip_pcie_configure_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>
> @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
> return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
> "failed to get reset lines\n");
>
> + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
> + "supports-clkreq");
> +
> return 0;
> }
>
> --
> 2.7.4
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-23 15:46 ` Frank Li
@ 2025-10-24 0:43 ` Shawn Lin
2025-10-24 3:09 ` Frank Li
0 siblings, 1 reply; 14+ messages in thread
From: Shawn Lin @ 2025-10-24 0:43 UTC (permalink / raw)
To: Frank Li
Cc: shawn.lin, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
linux-rockchip, Niklas Cassel, linux-pci
在 2025/10/23 星期四 23:46, Frank Li 写道:
> On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
>> L1 PM Substates for RC mode require support in the dw-rockchip driver
>> including 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.
>>
>> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
>> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>>
>> Meanwhile, for the EP mode, we haven't prepared enough to actually support
>> L1 PM Substates yet. So disable it now until proper support is added later.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v3:
>> - rephrease the changelog
>> - use FIELD_PREP_WM16
>> - rename to rockchip_pcie_configure_l1sub
>> - disable L1ss for EP mode
>>
>> Changes in v2:
>> - drop of_pci_clkreq_presnt API
>> - drop dependency of Niklas's patch
>>
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> index 3e2752c..25d2474 100644
>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> @@ -62,6 +62,12 @@
>> /* Interrupt Mask Register Related to Miscellaneous Operation */
>> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>>
>> +/* Power Management Control Register */
>> +#define PCIE_CLIENT_POWER_CON 0x2c
>> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
>> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
>> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
>> +
>> /* Hot Reset Control Register */
>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>> @@ -85,6 +91,7 @@ struct rockchip_pcie {
>> struct regulator *vpcie3v3;
>> struct irq_domain *irq_domain;
>> const struct rockchip_pcie_of_data *data;
>> + bool supports_clkreq;
>> };
>>
>> struct rockchip_pcie_of_data {
>> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
>> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
>> }
>>
>> +/*
>> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
>> + * needed to support L1 substates. Currently, just enable L1 substates for RC
>> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
>> + * For EP mode, there are more things should be done to actually save power in
>> + * L1 substates, so disable L1 substates until there is proper support.
>> + */
>> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
>> +{
>> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>> + u32 cap, l1subcap;
>> +
>> + /* Enable L1 substates if CLKREQ# is properly connected */
>> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
>> + return;
>> + }
>> +
>> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
>> + PCIE_CLIENT_POWER_CON);
>
> Looks like you force pull down clkreq should be enough, needn't disable
> L1SS. when RC force clkreq is low, Ref CLK always on even if L1SS enabled.
>
> Of course it depend on hardware implementation, But I think FULL_DOWN have
> high priority to force clkreq to low then PCI_L1SS control.
>
Hi Frank,
Thanks for your review. TBH, the basic idea here I think is not to
advertise a capability if the HW as whole hasn't been well prepared to
support it yet. So I'd prefer to keep it as-is.
> Frank
>
>> + 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);
>> + }
>> +}
>> +
>> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>> {
>> u32 cap, lnkcap;
>> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
>> rockchip);
>>
>> + rockchip_pcie_configure_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>>
>> return 0;
>> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> enum pci_barno bar;
>>
>> + rockchip_pcie_configure_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>>
>> @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
>> return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
>> "failed to get reset lines\n");
>>
>> + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
>> + "supports-clkreq");
>> +
>> return 0;
>> }
>>
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-24 0:43 ` Shawn Lin
@ 2025-10-24 3:09 ` Frank Li
2025-10-24 3:19 ` Shawn Lin
0 siblings, 1 reply; 14+ messages in thread
From: Frank Li @ 2025-10-24 3:09 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
linux-rockchip, Niklas Cassel, linux-pci
On Fri, Oct 24, 2025 at 08:43:28AM +0800, Shawn Lin wrote:
> 在 2025/10/23 星期四 23:46, Frank Li 写道:
> > On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
> > > L1 PM Substates for RC mode require support in the dw-rockchip driver
> > > including 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.
> > >
> > > For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
> > > Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
> > >
> > > Meanwhile, for the EP mode, we haven't prepared enough to actually support
> > > L1 PM Substates yet. So disable it now until proper support is added later.
> > >
> > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > >
> > > ---
> > >
> > > Changes in v3:
> > > - rephrease the changelog
> > > - use FIELD_PREP_WM16
> > > - rename to rockchip_pcie_configure_l1sub
> > > - disable L1ss for EP mode
> > >
> > > Changes in v2:
> > > - drop of_pci_clkreq_presnt API
> > > - drop dependency of Niklas's patch
> > >
> > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
> > > 1 file changed, 43 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > index 3e2752c..25d2474 100644
> > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > @@ -62,6 +62,12 @@
> > > /* Interrupt Mask Register Related to Miscellaneous Operation */
> > > #define PCIE_CLIENT_INTR_MASK_MISC 0x24
> > >
> > > +/* Power Management Control Register */
> > > +#define PCIE_CLIENT_POWER_CON 0x2c
> > > +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
> > > +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
> > > +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
> > > +
> > > /* Hot Reset Control Register */
> > > #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> > > #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> > > @@ -85,6 +91,7 @@ struct rockchip_pcie {
> > > struct regulator *vpcie3v3;
> > > struct irq_domain *irq_domain;
> > > const struct rockchip_pcie_of_data *data;
> > > + bool supports_clkreq;
> > > };
> > >
> > > struct rockchip_pcie_of_data {
> > > @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> > > return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
> > > }
> > >
> > > +/*
> > > + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
> > > + * needed to support L1 substates. Currently, just enable L1 substates for RC
> > > + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
> > > + * For EP mode, there are more things should be done to actually save power in
> > > + * L1 substates, so disable L1 substates until there is proper support.
> > > + */
> > > +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
> > > +{
> > > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> > > + u32 cap, l1subcap;
> > > +
> > > + /* Enable L1 substates if CLKREQ# is properly connected */
> > > + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
> > > + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
> > > + return;
> > > + }
> > > +
> > > + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
> > > + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> > > + PCIE_CLIENT_POWER_CON);
> >
> > Looks like you force pull down clkreq should be enough, needn't disable
> > L1SS. when RC force clkreq is low, Ref CLK always on even if L1SS enabled.
> >
> > Of course it depend on hardware implementation, But I think FULL_DOWN have
> > high priority to force clkreq to low then PCI_L1SS control.
> >
>
> Hi Frank,
>
> Thanks for your review. TBH, the basic idea here I think is not to
> advertise a capability if the HW as whole hasn't been well prepared to
> support it yet. So I'd prefer to keep it as-is.
>
If that, I prefer do it at dwc common driver or provide helper function to
avoid other vendor to copy same logic.
Frank
> > Frank
> >
> > > + 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);
> > > + }
> > > +}
> > > +
> > > static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> > > {
> > > u32 cap, lnkcap;
> > > @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> > > irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> > > rockchip);
> > >
> > > + rockchip_pcie_configure_l1sub(pci);
> > > rockchip_pcie_enable_l0s(pci);
> > >
> > > return 0;
> > > @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > enum pci_barno bar;
> > >
> > > + rockchip_pcie_configure_l1sub(pci);
> > > rockchip_pcie_enable_l0s(pci);
> > > rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
> > >
> > > @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
> > > return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
> > > "failed to get reset lines\n");
> > >
> > > + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
> > > + "supports-clkreq");
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.7.4
> > >
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> >
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-24 3:09 ` Frank Li
@ 2025-10-24 3:19 ` Shawn Lin
2025-10-24 3:40 ` Frank Li
0 siblings, 1 reply; 14+ messages in thread
From: Shawn Lin @ 2025-10-24 3:19 UTC (permalink / raw)
To: Frank Li
Cc: shawn.lin, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
linux-rockchip, Niklas Cassel, linux-pci
在 2025/10/24 星期五 11:09, Frank Li 写道:
> On Fri, Oct 24, 2025 at 08:43:28AM +0800, Shawn Lin wrote:
>> 在 2025/10/23 星期四 23:46, Frank Li 写道:
>>> On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
>>>> L1 PM Substates for RC mode require support in the dw-rockchip driver
>>>> including 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.
>>>>
>>>> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
>>>> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>>>>
>>>> Meanwhile, for the EP mode, we haven't prepared enough to actually support
>>>> L1 PM Substates yet. So disable it now until proper support is added later.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>
>>>> ---
>>>>
>>>> Changes in v3:
>>>> - rephrease the changelog
>>>> - use FIELD_PREP_WM16
>>>> - rename to rockchip_pcie_configure_l1sub
>>>> - disable L1ss for EP mode
>>>>
>>>> Changes in v2:
>>>> - drop of_pci_clkreq_presnt API
>>>> - drop dependency of Niklas's patch
>>>>
>>>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
>>>> 1 file changed, 43 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>>>> index 3e2752c..25d2474 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>>>> @@ -62,6 +62,12 @@
>>>> /* Interrupt Mask Register Related to Miscellaneous Operation */
>>>> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>>>>
>>>> +/* Power Management Control Register */
>>>> +#define PCIE_CLIENT_POWER_CON 0x2c
>>>> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
>>>> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
>>>> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
>>>> +
>>>> /* Hot Reset Control Register */
>>>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>>>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>>>> @@ -85,6 +91,7 @@ struct rockchip_pcie {
>>>> struct regulator *vpcie3v3;
>>>> struct irq_domain *irq_domain;
>>>> const struct rockchip_pcie_of_data *data;
>>>> + bool supports_clkreq;
>>>> };
>>>>
>>>> struct rockchip_pcie_of_data {
>>>> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
>>>> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
>>>> }
>>>>
>>>> +/*
>>>> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
>>>> + * needed to support L1 substates. Currently, just enable L1 substates for RC
>>>> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
>>>> + * For EP mode, there are more things should be done to actually save power in
>>>> + * L1 substates, so disable L1 substates until there is proper support.
>>>> + */
>>>> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
>>>> +{
>>>> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>>>> + u32 cap, l1subcap;
>>>> +
>>>> + /* Enable L1 substates if CLKREQ# is properly connected */
>>>> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
>>>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
>>>> + return;
>>>> + }
>>>> +
>>>> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
>>>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
>>>> + PCIE_CLIENT_POWER_CON);
>>>
>>> Looks like you force pull down clkreq should be enough, needn't disable
>>> L1SS. when RC force clkreq is low, Ref CLK always on even if L1SS enabled.
>>>
>>> Of course it depend on hardware implementation, But I think FULL_DOWN have
>>> high priority to force clkreq to low then PCI_L1SS control.
>>>
>>
>> Hi Frank,
>>
>> Thanks for your review. TBH, the basic idea here I think is not to
>> advertise a capability if the HW as whole hasn't been well prepared to
>> support it yet. So I'd prefer to keep it as-is.
>>
>
> If that, I prefer do it at dwc common driver or provide helper function to
> avoid other vendor to copy same logic.
Right, definitely we could improve it once another driver need it. :)
>
> Frank
>
>>> Frank
>>>
>>>> + 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);
>>>> + }
>>>> +}
>>>> +
>>>> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>>>> {
>>>> u32 cap, lnkcap;
>>>> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>>>> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
>>>> rockchip);
>>>>
>>>> + rockchip_pcie_configure_l1sub(pci);
>>>> rockchip_pcie_enable_l0s(pci);
>>>>
>>>> return 0;
>>>> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
>>>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>> enum pci_barno bar;
>>>>
>>>> + rockchip_pcie_configure_l1sub(pci);
>>>> rockchip_pcie_enable_l0s(pci);
>>>> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>>>>
>>>> @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
>>>> return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
>>>> "failed to get reset lines\n");
>>>>
>>>> + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
>>>> + "supports-clkreq");
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> --
>>>> 2.7.4
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-rockchip mailing list
>>>> Linux-rockchip@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-24 3:19 ` Shawn Lin
@ 2025-10-24 3:40 ` Frank Li
2025-10-24 3:51 ` Shawn Lin
0 siblings, 1 reply; 14+ messages in thread
From: Frank Li @ 2025-10-24 3:40 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
linux-rockchip, Niklas Cassel, linux-pci
On Fri, Oct 24, 2025 at 11:19:45AM +0800, Shawn Lin wrote:
> 在 2025/10/24 星期五 11:09, Frank Li 写道:
> > On Fri, Oct 24, 2025 at 08:43:28AM +0800, Shawn Lin wrote:
> > > 在 2025/10/23 星期四 23:46, Frank Li 写道:
> > > > On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
> > > > > L1 PM Substates for RC mode require support in the dw-rockchip driver
> > > > > including 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.
> > > > >
> > > > > For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
> > > > > Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
> > > > >
> > > > > Meanwhile, for the EP mode, we haven't prepared enough to actually support
> > > > > L1 PM Substates yet. So disable it now until proper support is added later.
> > > > >
> > > > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v3:
> > > > > - rephrease the changelog
> > > > > - use FIELD_PREP_WM16
> > > > > - rename to rockchip_pcie_configure_l1sub
> > > > > - disable L1ss for EP mode
> > > > >
> > > > > Changes in v2:
> > > > > - drop of_pci_clkreq_presnt API
> > > > > - drop dependency of Niklas's patch
> > > > >
> > > > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
> > > > > 1 file changed, 43 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > > > index 3e2752c..25d2474 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > > > @@ -62,6 +62,12 @@
> > > > > /* Interrupt Mask Register Related to Miscellaneous Operation */
> > > > > #define PCIE_CLIENT_INTR_MASK_MISC 0x24
> > > > >
> > > > > +/* Power Management Control Register */
> > > > > +#define PCIE_CLIENT_POWER_CON 0x2c
> > > > > +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
> > > > > +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
> > > > > +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
> > > > > +
> > > > > /* Hot Reset Control Register */
> > > > > #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> > > > > #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> > > > > @@ -85,6 +91,7 @@ struct rockchip_pcie {
> > > > > struct regulator *vpcie3v3;
> > > > > struct irq_domain *irq_domain;
> > > > > const struct rockchip_pcie_of_data *data;
> > > > > + bool supports_clkreq;
> > > > > };
> > > > >
> > > > > struct rockchip_pcie_of_data {
> > > > > @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> > > > > return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
> > > > > }
> > > > >
> > > > > +/*
> > > > > + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
> > > > > + * needed to support L1 substates. Currently, just enable L1 substates for RC
> > > > > + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
> > > > > + * For EP mode, there are more things should be done to actually save power in
> > > > > + * L1 substates, so disable L1 substates until there is proper support.
> > > > > + */
> > > > > +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
> > > > > +{
> > > > > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> > > > > + u32 cap, l1subcap;
> > > > > +
> > > > > + /* Enable L1 substates if CLKREQ# is properly connected */
> > > > > + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
> > > > > + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
> > > > > + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> > > > > + PCIE_CLIENT_POWER_CON);
> > > >
> > > > Looks like you force pull down clkreq should be enough, needn't disable
> > > > L1SS. when RC force clkreq is low, Ref CLK always on even if L1SS enabled.
> > > >
> > > > Of course it depend on hardware implementation, But I think FULL_DOWN have
> > > > high priority to force clkreq to low then PCI_L1SS control.
> > > >
> > >
> > > Hi Frank,
> > >
> > > Thanks for your review. TBH, the basic idea here I think is not to
> > > advertise a capability if the HW as whole hasn't been well prepared to
> > > support it yet. So I'd prefer to keep it as-is.
> > >
> >
> > If that, I prefer do it at dwc common driver or provide helper function to
> > avoid other vendor to copy same logic.
>
> Right, definitely we could improve it once another driver need it. :)
Basic pci-imx6 do similar things at
https://lore.kernel.org/imx/20251015030428.2980427-1-hongxing.zhu@nxp.com/
Just have not clean l1subcap.
Frank
>
> >
> > Frank
> >
> > > > Frank
> > > >
> > > > > + 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);
> > > > > + }
> > > > > +}
> > > > > +
> > > > > static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> > > > > {
> > > > > u32 cap, lnkcap;
> > > > > @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> > > > > irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> > > > > rockchip);
> > > > >
> > > > > + rockchip_pcie_configure_l1sub(pci);
> > > > > rockchip_pcie_enable_l0s(pci);
> > > > >
> > > > > return 0;
> > > > > @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > enum pci_barno bar;
> > > > >
> > > > > + rockchip_pcie_configure_l1sub(pci);
> > > > > rockchip_pcie_enable_l0s(pci);
> > > > > rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
> > > > >
> > > > > @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
> > > > > return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
> > > > > "failed to get reset lines\n");
> > > > >
> > > > > + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
> > > > > + "supports-clkreq");
> > > > > +
> > > > > return 0;
> > > > > }
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > Linux-rockchip mailing list
> > > > > Linux-rockchip@lists.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> > > >
> > >
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > Linux-rockchip@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support
2025-10-24 3:40 ` Frank Li
@ 2025-10-24 3:51 ` Shawn Lin
0 siblings, 0 replies; 14+ messages in thread
From: Shawn Lin @ 2025-10-24 3:51 UTC (permalink / raw)
To: Frank Li, Richard Zhu
Cc: shawn.lin, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
linux-rockchip, Niklas Cassel, linux-pci
+ Richard
在 2025/10/24 星期五 11:40, Frank Li 写道:
> On Fri, Oct 24, 2025 at 11:19:45AM +0800, Shawn Lin wrote:
>> 在 2025/10/24 星期五 11:09, Frank Li 写道:
>>> On Fri, Oct 24, 2025 at 08:43:28AM +0800, Shawn Lin wrote:
>>>> 在 2025/10/23 星期四 23:46, Frank Li 写道:
>>>>> On Thu, Oct 23, 2025 at 10:51:22AM +0800, Shawn Lin wrote:
>>>>>> L1 PM Substates for RC mode require support in the dw-rockchip driver
>>>>>> including 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.
>>>>>>
>>>>>> For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1
>>>>>> Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
>>>>>>
>>>>>> Meanwhile, for the EP mode, we haven't prepared enough to actually support
>>>>>> L1 PM Substates yet. So disable it now until proper support is added later.
>>>>>>
>>>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Changes in v3:
>>>>>> - rephrease the changelog
>>>>>> - use FIELD_PREP_WM16
>>>>>> - rename to rockchip_pcie_configure_l1sub
>>>>>> - disable L1ss for EP mode
>>>>>>
>>>>>> Changes in v2:
>>>>>> - drop of_pci_clkreq_presnt API
>>>>>> - drop dependency of Niklas's patch
>>>>>>
>>>>>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 43 +++++++++++++++++++++++++++
>>>>>> 1 file changed, 43 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>>>>>> index 3e2752c..25d2474 100644
>>>>>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>>>>>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>>>>>> @@ -62,6 +62,12 @@
>>>>>> /* Interrupt Mask Register Related to Miscellaneous Operation */
>>>>>> #define PCIE_CLIENT_INTR_MASK_MISC 0x24
>>>>>>
>>>>>> +/* Power Management Control Register */
>>>>>> +#define PCIE_CLIENT_POWER_CON 0x2c
>>>>>> +#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
>>>>>> +#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
>>>>>> +#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
>>>>>> +
>>>>>> /* Hot Reset Control Register */
>>>>>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>>>>>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>>>>>> @@ -85,6 +91,7 @@ struct rockchip_pcie {
>>>>>> struct regulator *vpcie3v3;
>>>>>> struct irq_domain *irq_domain;
>>>>>> const struct rockchip_pcie_of_data *data;
>>>>>> + bool supports_clkreq;
>>>>>> };
>>>>>>
>>>>>> struct rockchip_pcie_of_data {
>>>>>> @@ -200,6 +207,37 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
>>>>>> return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
>>>>>> }
>>>>>>
>>>>>> +/*
>>>>>> + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
>>>>>> + * needed to support L1 substates. Currently, just enable L1 substates for RC
>>>>>> + * mode if CLKREQ# is properly connected and supports-clkreq is present in DT.
>>>>>> + * For EP mode, there are more things should be done to actually save power in
>>>>>> + * L1 substates, so disable L1 substates until there is proper support.
>>>>>> + */
>>>>>> +static void rockchip_pcie_configure_l1sub(struct dw_pcie *pci)
>>>>>> +{
>>>>>> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>>>>>> + u32 cap, l1subcap;
>>>>>> +
>>>>>> + /* Enable L1 substates if CLKREQ# is properly connected */
>>>>>> + if (rockchip->supports_clkreq && rockchip->data->mode == DW_PCIE_RC_TYPE ) {
>>>>>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER_CON);
>>>>>> + return;
>>>>>> + }
>>>>>> +
>>>>>> + /* Otherwise, pull down CLKREQ# and disable L1 PM substates */
>>>>>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
>>>>>> + PCIE_CLIENT_POWER_CON);
>>>>>
>>>>> Looks like you force pull down clkreq should be enough, needn't disable
>>>>> L1SS. when RC force clkreq is low, Ref CLK always on even if L1SS enabled.
>>>>>
>>>>> Of course it depend on hardware implementation, But I think FULL_DOWN have
>>>>> high priority to force clkreq to low then PCI_L1SS control.
>>>>>
>>>>
>>>> Hi Frank,
>>>>
>>>> Thanks for your review. TBH, the basic idea here I think is not to
>>>> advertise a capability if the HW as whole hasn't been well prepared to
>>>> support it yet. So I'd prefer to keep it as-is.
>>>>
>>>
>>> If that, I prefer do it at dwc common driver or provide helper function to
>>> avoid other vendor to copy same logic.
>>
>> Right, definitely we could improve it once another driver need it. :)
>
> Basic pci-imx6 do similar things at
>
> https://lore.kernel.org/imx/20251015030428.2980427-1-hongxing.zhu@nxp.com/
>
> Just have not clean l1subcap.
I went through the series before, didn't find it clean l1subcap, so I
didn't add this to the dwc core since there is only one user. I could
respin a v4 to address your concern here.
Hi Richard,
Will you mind respining your series based on the
nearly-coming v4 which provide a API to clean l1subcap? If so,
I'll do that ASAP.
>
> Frank
>>
>>>
>>> Frank
>>>
>>>>> Frank
>>>>>
>>>>>> + 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);
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
>>>>>> {
>>>>>> u32 cap, lnkcap;
>>>>>> @@ -264,6 +302,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>>>>>> irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
>>>>>> rockchip);
>>>>>>
>>>>>> + rockchip_pcie_configure_l1sub(pci);
>>>>>> rockchip_pcie_enable_l0s(pci);
>>>>>>
>>>>>> return 0;
>>>>>> @@ -301,6 +340,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
>>>>>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>>> enum pci_barno bar;
>>>>>>
>>>>>> + rockchip_pcie_configure_l1sub(pci);
>>>>>> rockchip_pcie_enable_l0s(pci);
>>>>>> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>>>>>>
>>>>>> @@ -412,6 +452,9 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
>>>>>> return dev_err_probe(&pdev->dev, PTR_ERR(rockchip->rst),
>>>>>> "failed to get reset lines\n");
>>>>>>
>>>>>> + rockchip->supports_clkreq = of_property_read_bool(pdev->dev.of_node,
>>>>>> + "supports-clkreq");
>>>>>> +
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Linux-rockchip mailing list
>>>>>> Linux-rockchip@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-rockchip mailing list
>>>> Linux-rockchip@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>>
>>> _______________________________________________
>>> Linux-rockchip mailing list
>>> Linux-rockchip@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-10-24 3:52 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-23 2:51 [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Shawn Lin
2025-10-23 2:51 ` [PATCH v3 2/2] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
2025-10-23 5:06 ` Manivannan Sadhasivam
2025-10-23 5:05 ` [PATCH v3 1/2] PCI: dw-rockchip: Configure L1sub support Manivannan Sadhasivam
2025-10-23 6:26 ` Shawn Lin
2025-10-23 6:50 ` Manivannan Sadhasivam
2025-10-23 9:49 ` Diederik de Haas
2025-10-23 10:22 ` Shawn Lin
2025-10-23 15:46 ` Frank Li
2025-10-24 0:43 ` Shawn Lin
2025-10-24 3:09 ` Frank Li
2025-10-24 3:19 ` Shawn Lin
2025-10-24 3:40 ` Frank Li
2025-10-24 3:51 ` Shawn Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox