* [PATCH 0/4] Add L1 substates support for Rockchip platforms
@ 2025-10-21 7:48 Shawn Lin
2025-10-21 7:48 ` [PATCH 1/4] PCI: of: Add of_pci_clkreq_present() Shawn Lin
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Shawn Lin @ 2025-10-21 7:48 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci,
Shawn Lin
This patch-set adds ASPM L1 substates support for Rockchip platform.
As supports-clkreq is used by some more host drivers, patch 1 creates
of_pci_clkreq_present() and patch 2 reuse it. Patch 3 enables or disables
L1 substates on Rockchip driver by checking if clkreq# is ready. Patch 4
enable L1 substates on RK3588-EVB1.
This series of patches is tested on RK3588-EVB1 with a NVMe connected
to the pcie3x4 slot(actually I tested several NVMes), A RTL8111 NIC card
connected pcie2x1l1 and a broadcom Wi-Fi connected to pcie2x1l0. All works
fine under L1 substates by checking the LTSSM.
Shawn Lin (1):
arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1
PCI: dw-rockchip: Add L1sub support
PCI: tegra194: Use of_pci_clkreq_present() instead
PCI: of: Add of_pci_clkreq_present()
arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-21 7:48 [PATCH 0/4] Add L1 substates support for Rockchip platforms Shawn Lin
@ 2025-10-21 7:48 ` Shawn Lin
2025-10-21 16:16 ` Frank Li
2025-10-22 10:02 ` Manivannan Sadhasivam
2025-10-21 7:48 ` [PATCH 2/4] PCI: tegra194: Use of_pci_clkreq_present() instead Shawn Lin
` (2 subsequent siblings)
3 siblings, 2 replies; 18+ messages in thread
From: Shawn Lin @ 2025-10-21 7:48 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci,
Shawn Lin
of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
is properly connected and could enable L1.1/L1.2 support.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/pci/of.c | 18 ++++++++++++++++++
drivers/pci/pci.h | 6 ++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..52c6d365083b 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
return 0;
}
EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
+
+/**
+ * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
+ * @np: Device tree node
+ *
+ * If the property is present, it means CLKREQ# is properly connected
+ * and the hardware is ready to support L1.1/L1.2
+ *
+ * Return: true if the property is available, false otherwise.
+ */
+bool of_pci_clkreq_present(struct device_node *np)
+{
+ if (!np)
+ return false;
+
+ return of_property_present(np, "supports-clkreq");
+}
+EXPORT_SYMBOL_GPL(of_pci_clkreq_present);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4492b809094b..2421e39e6e48 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1061,6 +1061,7 @@ bool of_pci_supply_present(struct device_node *np);
int of_pci_get_equalization_presets(struct device *dev,
struct pci_eq_presets *presets,
int num_lanes);
+bool of_pci_clkreq_present(struct device_node *np);
#else
static inline int
of_get_pci_domain_nr(struct device_node *node)
@@ -1106,6 +1107,11 @@ static inline bool of_pci_supply_present(struct device_node *np)
return false;
}
+static inline bool of_pci_clkreq_present(struct device_node *np)
+{
+ return false;
+}
+
static inline int of_pci_get_equalization_presets(struct device *dev,
struct pci_eq_presets *presets,
int num_lanes)
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] PCI: tegra194: Use of_pci_clkreq_present() instead
2025-10-21 7:48 [PATCH 0/4] Add L1 substates support for Rockchip platforms Shawn Lin
2025-10-21 7:48 ` [PATCH 1/4] PCI: of: Add of_pci_clkreq_present() Shawn Lin
@ 2025-10-21 7:48 ` Shawn Lin
2025-10-21 7:48 ` [PATCH 3/4] PCI: dw-rockchip: Add L1sub support Shawn Lin
2025-10-21 7:48 ` [PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
3 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2025-10-21 7:48 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci,
Shawn Lin
No functional changes intended.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/pci/controller/dwc/pcie-tegra194.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 10e74458e667..7a82a53e7c4d 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1161,8 +1161,7 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
"nvidia,enable-ext-refclk");
}
- pcie->supports_clkreq =
- of_property_read_bool(pcie->dev->of_node, "supports-clkreq");
+ pcie->supports_clkreq = of_pci_clkreq_present(pcie->dev->of_node);
pcie->enable_cdm_check =
of_property_read_bool(np, "snps,enable-cdm-check");
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] PCI: dw-rockchip: Add L1sub support
2025-10-21 7:48 [PATCH 0/4] Add L1 substates support for Rockchip platforms Shawn Lin
2025-10-21 7:48 ` [PATCH 1/4] PCI: of: Add of_pci_clkreq_present() Shawn Lin
2025-10-21 7:48 ` [PATCH 2/4] PCI: tegra194: Use of_pci_clkreq_present() instead Shawn Lin
@ 2025-10-21 7:48 ` Shawn Lin
2025-10-21 8:01 ` Hans Zhang
2025-10-21 7:48 ` [PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
3 siblings, 1 reply; 18+ messages in thread
From: Shawn Lin @ 2025-10-21 7:48 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci,
Shawn Lin
The driver should set app_clk_req_n(clkreq ready) of PCIE_CLIENT_POWER reg
to support L1sub. Otherwise, unset app_clk_req_n and pull down CLKREQ#.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 31 ++++++++++++++-----
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 87dd2dd188b4..8a52ff73ec46 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 0x2c
+#define PCIE_CLKREQ_READY 0x10001
+#define PCIE_CLKREQ_NOT_READY 0x10000
+#define PCIE_CLKREQ_PULL_DOWN 0x30001000
+
/* Hot Reset Control Register */
#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
#define PCIE_LTSSM_APP_DLY2_EN BIT(1)
@@ -84,6 +90,7 @@ struct rockchip_pcie {
struct gpio_desc *rst_gpio;
struct irq_domain *irq_domain;
const struct rockchip_pcie_of_data *data;
+ bool supports_clkreq;
};
struct rockchip_pcie_of_data {
@@ -199,15 +206,21 @@ 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, not a single rockchip platform
- * performs these steps, so disable L1 substates until there is proper support.
- */
-static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci)
+static void rockchip_pcie_enable_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) {
+ /* Ready to have reference clock removed */
+ rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER);
+ return;
+ }
+
+ /* Otherwise, pull down CLKREQ# and disable L1 substates */
+ rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
+ PCIE_CLIENT_POWER);
cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
if (cap) {
l1subcap = dw_pcie_readl_dbi(pci, cap + PCI_L1SS_CAP);
@@ -282,7 +295,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_disable_l1sub(pci);
+ rockchip_pcie_enable_l1sub(pci);
rockchip_pcie_enable_l0s(pci);
return 0;
@@ -320,7 +333,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_disable_l1sub(pci);
+ rockchip_pcie_enable_l1sub(pci);
rockchip_pcie_enable_l0s(pci);
rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
@@ -432,6 +445,8 @@ 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_pci_clkreq_present(pdev->dev.of_node);
+
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1
2025-10-21 7:48 [PATCH 0/4] Add L1 substates support for Rockchip platforms Shawn Lin
` (2 preceding siblings ...)
2025-10-21 7:48 ` [PATCH 3/4] PCI: dw-rockchip: Add L1sub support Shawn Lin
@ 2025-10-21 7:48 ` Shawn Lin
3 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2025-10-21 7:48 UTC (permalink / raw)
To: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: Thierry Reding, 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>
---
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 ff1ba5ed56ef..c9d284cb738b 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 @@ &pcie2x1l0 {
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 @@ wifi: wifi@0,0 {
&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 @@ &pcie30phy {
&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.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support
2025-10-21 7:48 ` [PATCH 3/4] PCI: dw-rockchip: Add L1sub support Shawn Lin
@ 2025-10-21 8:01 ` Hans Zhang
2025-10-21 8:42 ` Shawn Lin
0 siblings, 1 reply; 18+ messages in thread
From: Hans Zhang @ 2025-10-21 8:01 UTC (permalink / raw)
To: Shawn Lin, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
Cc: Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci
On 10/21/2025 3:48 PM, Shawn Lin wrote:
> EXTERNAL EMAIL
>
> The driver should set app_clk_req_n(clkreq ready) of PCIE_CLIENT_POWER reg
> to support L1sub. Otherwise, unset app_clk_req_n and pull down CLKREQ#.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 31 ++++++++++++++-----
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 87dd2dd188b4..8a52ff73ec46 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 0x2c
> +#define PCIE_CLKREQ_READY 0x10001
> +#define PCIE_CLKREQ_NOT_READY 0x10000
> +#define PCIE_CLKREQ_PULL_DOWN 0x30001000
> +
> /* Hot Reset Control Register */
> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> @@ -84,6 +90,7 @@ struct rockchip_pcie {
> struct gpio_desc *rst_gpio;
> struct irq_domain *irq_domain;
> const struct rockchip_pcie_of_data *data;
> + bool supports_clkreq;
> };
>
> struct rockchip_pcie_of_data {
> @@ -199,15 +206,21 @@ 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, not a single rockchip platform
> - * performs these steps, so disable L1 substates until there is proper support.
> - */
> -static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci)
Hi,
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dw-rockchip&id=40331c63e7901a2cc75ce6b5d24d50601efb833d
Mani has already placed this part in the above branch. Can it be removed?
Best regards,
Hans
> +static void rockchip_pcie_enable_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) {
> + /* Ready to have reference clock removed */
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY, PCIE_CLIENT_POWER);
> + return;
> + }
> +
> + /* Otherwise, pull down CLKREQ# and disable L1 substates */
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
> + PCIE_CLIENT_POWER);
> cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
> if (cap) {
> l1subcap = dw_pcie_readl_dbi(pci, cap + PCI_L1SS_CAP);
> @@ -282,7 +295,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_disable_l1sub(pci);
> + rockchip_pcie_enable_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
>
> return 0;
> @@ -320,7 +333,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_disable_l1sub(pci);
> + rockchip_pcie_enable_l1sub(pci);
> rockchip_pcie_enable_l0s(pci);
> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>
> @@ -432,6 +445,8 @@ 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_pci_clkreq_present(pdev->dev.of_node);
> +
> return 0;
> }
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support
2025-10-21 8:01 ` Hans Zhang
@ 2025-10-21 8:42 ` Shawn Lin
2025-10-21 9:03 ` Hans Zhang
2025-10-22 10:34 ` Manivannan Sadhasivam
0 siblings, 2 replies; 18+ messages in thread
From: Shawn Lin @ 2025-10-21 8:42 UTC (permalink / raw)
To: Hans Zhang
Cc: shawn.lin, Thierry Reding, linux-rockchip, Niklas Cassel,
linux-pci, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
在 2025/10/21 星期二 16:01, Hans Zhang 写道:
>
>
> On 10/21/2025 3:48 PM, Shawn Lin wrote:
>> EXTERNAL EMAIL
>>
>> The driver should set app_clk_req_n(clkreq ready) of PCIE_CLIENT_POWER
>> reg
>> to support L1sub. Otherwise, unset app_clk_req_n and pull down CLKREQ#.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 31 ++++++++++++++-----
>> 1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/
>> pci/controller/dwc/pcie-dw-rockchip.c
>> index 87dd2dd188b4..8a52ff73ec46 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 0x2c
>> +#define PCIE_CLKREQ_READY 0x10001
>> +#define PCIE_CLKREQ_NOT_READY 0x10000
>> +#define PCIE_CLKREQ_PULL_DOWN 0x30001000
>> +
>> /* Hot Reset Control Register */
>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>> @@ -84,6 +90,7 @@ struct rockchip_pcie {
>> struct gpio_desc *rst_gpio;
>> struct irq_domain *irq_domain;
>> const struct rockchip_pcie_of_data *data;
>> + bool supports_clkreq;
>> };
>>
>> struct rockchip_pcie_of_data {
>> @@ -199,15 +206,21 @@ 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, not a single rockchip
>> platform
>> - * performs these steps, so disable L1 substates until there is
>> proper support.
>> - */
>> -static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci)
>
> Hi,
>
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?
> h=controller/dw-rockchip&id=40331c63e7901a2cc75ce6b5d24d50601efb833d
>
> Mani has already placed this part in the above branch. Can it be removed?
>
I think it's better to apply the changes on top of Niklas's commit
rather than removing it, out of respect for Niklas's credit.
> Best regards,
> Hans
>
>
>> +static void rockchip_pcie_enable_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) {
>> + /* Ready to have reference clock removed */
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY,
>> PCIE_CLIENT_POWER);
>> + return;
>> + }
>> +
>> + /* Otherwise, pull down CLKREQ# and disable L1 substates */
>> + rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_PULL_DOWN |
>> PCIE_CLKREQ_NOT_READY,
>> + PCIE_CLIENT_POWER);
>> cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
>> if (cap) {
>> l1subcap = dw_pcie_readl_dbi(pci, cap + PCI_L1SS_CAP);
>> @@ -282,7 +295,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_disable_l1sub(pci);
>> + rockchip_pcie_enable_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>>
>> return 0;
>> @@ -320,7 +333,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_disable_l1sub(pci);
>> + rockchip_pcie_enable_l1sub(pci);
>> rockchip_pcie_enable_l0s(pci);
>> rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
>>
>> @@ -432,6 +445,8 @@ 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_pci_clkreq_present(pdev-
>> >dev.of_node);
>> +
>> return 0;
>> }
>>
>> --
>> 2.43.0
>>
>>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support
2025-10-21 8:42 ` Shawn Lin
@ 2025-10-21 9:03 ` Hans Zhang
2025-10-22 10:34 ` Manivannan Sadhasivam
1 sibling, 0 replies; 18+ messages in thread
From: Hans Zhang @ 2025-10-21 9:03 UTC (permalink / raw)
To: Shawn Lin
Cc: Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci,
Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas
On 10/21/2025 4:42 PM, Shawn Lin wrote:
>>>
>>> -/*
>>> - * 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, not a single rockchip
>>> platform
>>> - * performs these steps, so disable L1 substates until there is
>>> proper support.
>>> - */
>>> -static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci)
>>
>> Hi,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?
>> h=controller/dw-rockchip&id=40331c63e7901a2cc75ce6b5d24d50601efb833d
>>
>> Mani has already placed this part in the above branch. Can it be removed?
>>
>
> I think it's better to apply the changes on top of Niklas's commit
> rather than removing it, out of respect for Niklas's credit.
Hi Shawn,
Sorry, maybe I'm just looking at this issue from the perspective of
patch and understand what you mean.
Best regards,
Hans
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-21 7:48 ` [PATCH 1/4] PCI: of: Add of_pci_clkreq_present() Shawn Lin
@ 2025-10-21 16:16 ` Frank Li
2025-10-22 9:17 ` Shawn Lin
2025-10-22 10:02 ` Manivannan Sadhasivam
1 sibling, 1 reply; 18+ messages in thread
From: Frank Li @ 2025-10-21 16:16 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci
On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> is properly connected and could enable L1.1/L1.2 support.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> drivers/pci/of.c | 18 ++++++++++++++++++
> drivers/pci/pci.h | 6 ++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..52c6d365083b 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> return 0;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> +
> +/**
> + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
> + * @np: Device tree node
> + *
> + * If the property is present, it means CLKREQ# is properly connected
> + * and the hardware is ready to support L1.1/L1.2
> + *
> + * Return: true if the property is available, false otherwise.
> + */
> +bool of_pci_clkreq_present(struct device_node *np)
> +{
> + if (!np)
> + return false;
> +
> + return of_property_present(np, "supports-clkreq");
> +}
> +EXPORT_SYMBOL_GPL(of_pci_clkreq_present);
This helper function is quite small. I suggest direct put inline function
into pci.h to keep simple.
static inline bool of_pci_clkreq_present(struct device_node *np)
{
return np && of_property_present(np, "supports-clkreq");
}
Frank
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4492b809094b..2421e39e6e48 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -1061,6 +1061,7 @@ bool of_pci_supply_present(struct device_node *np);
> int of_pci_get_equalization_presets(struct device *dev,
> struct pci_eq_presets *presets,
> int num_lanes);
> +bool of_pci_clkreq_present(struct device_node *np);
> #else
> static inline int
> of_get_pci_domain_nr(struct device_node *node)
> @@ -1106,6 +1107,11 @@ static inline bool of_pci_supply_present(struct device_node *np)
> return false;
> }
>
> +static inline bool of_pci_clkreq_present(struct device_node *np)
> +{
> + return false;
> +}
> +
> static inline int of_pci_get_equalization_presets(struct device *dev,
> struct pci_eq_presets *presets,
> int num_lanes)
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-21 16:16 ` Frank Li
@ 2025-10-22 9:17 ` Shawn Lin
0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2025-10-22 9:17 UTC (permalink / raw)
To: Frank Li
Cc: shawn.lin, Heiko Stuebner, Manivannan Sadhasivam, Bjorn Helgaas,
Thierry Reding, linux-rockchip, Niklas Cassel, linux-pci
在 2025/10/22 星期三 0:16, Frank Li 写道:
> On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
>> of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
>> is properly connected and could enable L1.1/L1.2 support.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>> drivers/pci/of.c | 18 ++++++++++++++++++
>> drivers/pci/pci.h | 6 ++++++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 3579265f1198..52c6d365083b 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
>> +
>> +/**
>> + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
>> + * @np: Device tree node
>> + *
>> + * If the property is present, it means CLKREQ# is properly connected
>> + * and the hardware is ready to support L1.1/L1.2
>> + *
>> + * Return: true if the property is available, false otherwise.
>> + */
>> +bool of_pci_clkreq_present(struct device_node *np)
>> +{
>> + if (!np)
>> + return false;
>> +
>> + return of_property_present(np, "supports-clkreq");
>> +}
>> +EXPORT_SYMBOL_GPL(of_pci_clkreq_present);
>
> This helper function is quite small. I suggest direct put inline function
> into pci.h to keep simple.
Sounds reasonable, will wait for other comments and fix your comment
together in V2.
>
> static inline bool of_pci_clkreq_present(struct device_node *np)
> {
> return np && of_property_present(np, "supports-clkreq");
> }
>
> Frank
>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 4492b809094b..2421e39e6e48 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -1061,6 +1061,7 @@ bool of_pci_supply_present(struct device_node *np);
>> int of_pci_get_equalization_presets(struct device *dev,
>> struct pci_eq_presets *presets,
>> int num_lanes);
>> +bool of_pci_clkreq_present(struct device_node *np);
>> #else
>> static inline int
>> of_get_pci_domain_nr(struct device_node *node)
>> @@ -1106,6 +1107,11 @@ static inline bool of_pci_supply_present(struct device_node *np)
>> return false;
>> }
>>
>> +static inline bool of_pci_clkreq_present(struct device_node *np)
>> +{
>> + return false;
>> +}
>> +
>> static inline int of_pci_get_equalization_presets(struct device *dev,
>> struct pci_eq_presets *presets,
>> int num_lanes)
>> --
>> 2.43.0
>>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-21 7:48 ` [PATCH 1/4] PCI: of: Add of_pci_clkreq_present() Shawn Lin
2025-10-21 16:16 ` Frank Li
@ 2025-10-22 10:02 ` Manivannan Sadhasivam
2025-10-22 10:13 ` Shawn Lin
1 sibling, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-22 10:02 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Bjorn Helgaas, Thierry Reding, linux-rockchip,
Niklas Cassel, linux-pci
On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> is properly connected and could enable L1.1/L1.2 support.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> drivers/pci/of.c | 18 ++++++++++++++++++
> drivers/pci/pci.h | 6 ++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..52c6d365083b 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> return 0;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> +
> +/**
> + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
I don't see a benefit of this API, tbh. The API name creates an impression that
the API will check for the presence of CLKREQ# signal in DT, but it checks
for the presence of the 'supports-clkreq' property. Even though the presence of
the property implies that the CLKREQ# routing is available, I'd prefer to check
for the property explicitly instead of hiding it inside this API.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-22 10:02 ` Manivannan Sadhasivam
@ 2025-10-22 10:13 ` Shawn Lin
2025-10-22 10:29 ` Manivannan Sadhasivam
0 siblings, 1 reply; 18+ messages in thread
From: Shawn Lin @ 2025-10-22 10:13 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: shawn.lin, Heiko Stuebner, Bjorn Helgaas, Thierry Reding,
linux-rockchip, Niklas Cassel, linux-pci
Hi Mani
在 2025/10/22 星期三 18:02, Manivannan Sadhasivam 写道:
> On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
>> of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
>> is properly connected and could enable L1.1/L1.2 support.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>> drivers/pci/of.c | 18 ++++++++++++++++++
>> drivers/pci/pci.h | 6 ++++++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 3579265f1198..52c6d365083b 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
>> +
>> +/**
>> + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
>
> I don't see a benefit of this API, tbh. The API name creates an impression that
> the API will check for the presence of CLKREQ# signal in DT, but it checks
> for the presence of the 'supports-clkreq' property. Even though the presence of
> the property implies that the CLKREQ# routing is available, I'd prefer to check
> for the property explicitly instead of hiding it inside this API.
It makes sense.
Will the name of_pci_supports_clkreq_present() look good? Or we just
drop it and let host drivers to explicitly check supports-clkreq inside
their code?
>
> - Mani
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-22 10:13 ` Shawn Lin
@ 2025-10-22 10:29 ` Manivannan Sadhasivam
2025-10-22 16:22 ` Bjorn Helgaas
0 siblings, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-22 10:29 UTC (permalink / raw)
To: Shawn Lin
Cc: Heiko Stuebner, Bjorn Helgaas, Thierry Reding, linux-rockchip,
Niklas Cassel, linux-pci
On Wed, Oct 22, 2025 at 06:13:59PM +0800, Shawn Lin wrote:
> Hi Mani
>
> 在 2025/10/22 星期三 18:02, Manivannan Sadhasivam 写道:
> > On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> > > of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> > > is properly connected and could enable L1.1/L1.2 support.
> > >
> > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > ---
> > > drivers/pci/of.c | 18 ++++++++++++++++++
> > > drivers/pci/pci.h | 6 ++++++
> > > 2 files changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > index 3579265f1198..52c6d365083b 100644
> > > --- a/drivers/pci/of.c
> > > +++ b/drivers/pci/of.c
> > > @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> > > return 0;
> > > }
> > > EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> > > +
> > > +/**
> > > + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
> >
> > I don't see a benefit of this API, tbh. The API name creates an impression that
> > the API will check for the presence of CLKREQ# signal in DT, but it checks
> > for the presence of the 'supports-clkreq' property. Even though the presence of
> > the property implies that the CLKREQ# routing is available, I'd prefer to check
> > for the property explicitly instead of hiding it inside this API.
>
> It makes sense.
>
> Will the name of_pci_supports_clkreq_present() look good? Or we just
> drop it and let host drivers to explicitly check supports-clkreq inside
> their code?
>
I'd prefer to drop the API.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support
2025-10-21 8:42 ` Shawn Lin
2025-10-21 9:03 ` Hans Zhang
@ 2025-10-22 10:34 ` Manivannan Sadhasivam
1 sibling, 0 replies; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-22 10:34 UTC (permalink / raw)
To: Shawn Lin
Cc: Hans Zhang, Thierry Reding, linux-rockchip, Niklas Cassel,
linux-pci, Heiko Stuebner, Bjorn Helgaas
On Tue, Oct 21, 2025 at 04:42:52PM +0800, Shawn Lin wrote:
>
> 在 2025/10/21 星期二 16:01, Hans Zhang 写道:
> >
> >
> > On 10/21/2025 3:48 PM, Shawn Lin wrote:
> > > EXTERNAL EMAIL
> > >
> > > The driver should set app_clk_req_n(clkreq ready) of
> > > PCIE_CLIENT_POWER reg
> > > to support L1sub. Otherwise, unset app_clk_req_n and pull down CLKREQ#.
> > >
> > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 31 ++++++++++++++-----
> > > 1 file changed, 23 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > b/drivers/ pci/controller/dwc/pcie-dw-rockchip.c
> > > index 87dd2dd188b4..8a52ff73ec46 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 0x2c
> > > +#define PCIE_CLKREQ_READY 0x10001
> > > +#define PCIE_CLKREQ_NOT_READY 0x10000
> > > +#define PCIE_CLKREQ_PULL_DOWN 0x30001000
> > > +
> > > /* Hot Reset Control Register */
> > > #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> > > #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
> > > @@ -84,6 +90,7 @@ struct rockchip_pcie {
> > > struct gpio_desc *rst_gpio;
> > > struct irq_domain *irq_domain;
> > > const struct rockchip_pcie_of_data *data;
> > > + bool supports_clkreq;
> > > };
> > >
> > > struct rockchip_pcie_of_data {
> > > @@ -199,15 +206,21 @@ 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, not a single rockchip
> > > platform
> > > - * performs these steps, so disable L1 substates until there is
> > > proper support.
> > > - */
> > > -static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci)
> >
> > Hi,
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?
> > h=controller/dw-rockchip&id=40331c63e7901a2cc75ce6b5d24d50601efb833d
> >
> > Mani has already placed this part in the above branch. Can it be removed?
> >
>
> I think it's better to apply the changes on top of Niklas's commit rather
> than removing it, out of respect for Niklas's credit.
>
There is no point in removing a feature in one patch and adding it back in
another patch in the same release. Once this series materializes, I'll drop the
patch from Niklas.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-22 10:29 ` Manivannan Sadhasivam
@ 2025-10-22 16:22 ` Bjorn Helgaas
2025-10-22 17:35 ` Manivannan Sadhasivam
0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2025-10-22 16:22 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Shawn Lin, Heiko Stuebner, Bjorn Helgaas, Thierry Reding,
linux-rockchip, Niklas Cassel, linux-pci
On Wed, Oct 22, 2025 at 03:59:13PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 22, 2025 at 06:13:59PM +0800, Shawn Lin wrote:
> > 在 2025/10/22 星期三 18:02, Manivannan Sadhasivam 写道:
> > > On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> > > > of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> > > > is properly connected and could enable L1.1/L1.2 support.
> > > >
> > > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > ---
> > > > drivers/pci/of.c | 18 ++++++++++++++++++
> > > > drivers/pci/pci.h | 6 ++++++
> > > > 2 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > index 3579265f1198..52c6d365083b 100644
> > > > --- a/drivers/pci/of.c
> > > > +++ b/drivers/pci/of.c
> > > > @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> > > > return 0;
> > > > }
> > > > EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> > > > +
> > > > +/**
> > > > + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
> > >
> > > I don't see a benefit of this API, tbh. The API name creates an
> > > impression that the API will check for the presence of CLKREQ#
> > > signal in DT, but it checks for the presence of the
> > > 'supports-clkreq' property. Even though the presence of the
> > > property implies that the CLKREQ# routing is available, I'd
> > > prefer to check for the property explicitly instead of hiding it
> > > inside this API.
> >
> > It makes sense.
> >
> > Will the name of_pci_supports_clkreq_present() look good? Or we
> > just drop it and let host drivers to explicitly check
> > supports-clkreq inside their code?
>
> I'd prefer to drop the API.
An API might help with consistency across DT bindings. We don't want
drivers to pick their own names for 'supports-clkreq'.
But the pci-bus-common schema already includes 'supports-clkreq', and
that's probably enough:
https://github.com/devicetree-org/dt-schema/blob/979fea8f3c18/dtschema/schemas/pci/pci-bus-common.yaml#L155
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-22 16:22 ` Bjorn Helgaas
@ 2025-10-22 17:35 ` Manivannan Sadhasivam
2025-10-22 18:14 ` Bjorn Helgaas
0 siblings, 1 reply; 18+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-22 17:35 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Shawn Lin, Heiko Stuebner, Bjorn Helgaas, Thierry Reding,
linux-rockchip, Niklas Cassel, linux-pci
On Wed, Oct 22, 2025 at 11:22:29AM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 22, 2025 at 03:59:13PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Oct 22, 2025 at 06:13:59PM +0800, Shawn Lin wrote:
> > > 在 2025/10/22 星期三 18:02, Manivannan Sadhasivam 写道:
> > > > On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> > > > > of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> > > > > is properly connected and could enable L1.1/L1.2 support.
> > > > >
> > > > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > > ---
> > > > > drivers/pci/of.c | 18 ++++++++++++++++++
> > > > > drivers/pci/pci.h | 6 ++++++
> > > > > 2 files changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > > index 3579265f1198..52c6d365083b 100644
> > > > > --- a/drivers/pci/of.c
> > > > > +++ b/drivers/pci/of.c
> > > > > @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> > > > > return 0;
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> > > > > +
> > > > > +/**
> > > > > + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
> > > >
> > > > I don't see a benefit of this API, tbh. The API name creates an
> > > > impression that the API will check for the presence of CLKREQ#
> > > > signal in DT, but it checks for the presence of the
> > > > 'supports-clkreq' property. Even though the presence of the
> > > > property implies that the CLKREQ# routing is available, I'd
> > > > prefer to check for the property explicitly instead of hiding it
> > > > inside this API.
> > >
> > > It makes sense.
> > >
> > > Will the name of_pci_supports_clkreq_present() look good? Or we
> > > just drop it and let host drivers to explicitly check
> > > supports-clkreq inside their code?
> >
> > I'd prefer to drop the API.
>
> An API might help with consistency across DT bindings. We don't want
> drivers to pick their own names for 'supports-clkreq'.
>
I don't know what you mean by 'drivers to pick their own names'. With or without
this API, the drivers have to use a local variable:
- pcie->supports_clkreq =
- of_property_read_bool(pcie->dev->of_node, "supports-clkreq");
+ pcie->supports_clkreq = of_pci_clkreq_present(pcie->dev->of_node);
What this API does is, just hiding the 'supports-clkreq' property, nothing more.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()
2025-10-22 17:35 ` Manivannan Sadhasivam
@ 2025-10-22 18:14 ` Bjorn Helgaas
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-10-22 18:14 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Shawn Lin, Heiko Stuebner, Bjorn Helgaas, Thierry Reding,
linux-rockchip, Niklas Cassel, linux-pci
On Wed, Oct 22, 2025 at 11:05:50PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 22, 2025 at 11:22:29AM -0500, Bjorn Helgaas wrote:
> > On Wed, Oct 22, 2025 at 03:59:13PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Oct 22, 2025 at 06:13:59PM +0800, Shawn Lin wrote:
> > > > 在 2025/10/22 星期三 18:02, Manivannan Sadhasivam 写道:
> > > > > On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> > > > > > of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> > > > > > is properly connected and could enable L1.1/L1.2 support.
> > > > > >
> > > > > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > > > ---
> > > > > > drivers/pci/of.c | 18 ++++++++++++++++++
> > > > > > drivers/pci/pci.h | 6 ++++++
> > > > > > 2 files changed, 24 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > > > index 3579265f1198..52c6d365083b 100644
> > > > > > --- a/drivers/pci/of.c
> > > > > > +++ b/drivers/pci/of.c
> > > > > > @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> > > > > > return 0;
> > > > > > }
> > > > > > EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> > > > > > +
> > > > > > +/**
> > > > > > + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
> > > > >
> > > > > I don't see a benefit of this API, tbh. The API name creates an
> > > > > impression that the API will check for the presence of CLKREQ#
> > > > > signal in DT, but it checks for the presence of the
> > > > > 'supports-clkreq' property. Even though the presence of the
> > > > > property implies that the CLKREQ# routing is available, I'd
> > > > > prefer to check for the property explicitly instead of hiding it
> > > > > inside this API.
> > > >
> > > > It makes sense.
> > > >
> > > > Will the name of_pci_supports_clkreq_present() look good? Or we
> > > > just drop it and let host drivers to explicitly check
> > > > supports-clkreq inside their code?
> > >
> > > I'd prefer to drop the API.
> >
> > An API might help with consistency across DT bindings. We don't want
> > drivers to pick their own names for 'supports-clkreq'.
>
> I don't know what you mean by 'drivers to pick their own names'.
> With or without this API, the drivers have to use a local variable:
I just meant different names for the 'supports-clkreq' DT property.
But the schema I mentioned should be enough for that.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1
2025-11-11 22:16 [PATCH 0/4] PCI: dwc: Advertise L1 PM Substates only if driver requests it Bjorn Helgaas
@ 2025-11-11 22:16 ` Bjorn Helgaas
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2025-11-11 22:16 UTC (permalink / raw)
To: Niklas Cassel, Shawn Lin
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Heiko Stuebner,
Kever Yang, Simon Xue, Damien Le Moal, Dragan Simic,
FUKAUMI Naoki, Diederik de Haas, Richard Zhu, Frank Li,
Lucas Stach, Shawn Guo, Sascha Hauer, Fabio Estevam, Conor Dooley,
Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter, Hans Zhang,
linux-tegra, linux-pci, linux-arm-kernel, linux-rockchip, kernel,
Bjorn Helgaas
From: Shawn Lin <shawn.lin@rock-chips.com>
Add supports-clkreq and pinmux for PCIe ASPM L1 substates.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Hans Zhang <hans.zhang@cixtech.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Link: https://patch.msgid.link/1761187883-150120-2-git-send-email-shawn.lin@rock-chips.com
---
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 ff1ba5ed56ef..c9d284cb738b 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 @@ &pcie2x1l0 {
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 @@ wifi: wifi@0,0 {
&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 @@ &pcie30phy {
&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.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-11-11 22:16 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-21 7:48 [PATCH 0/4] Add L1 substates support for Rockchip platforms Shawn Lin
2025-10-21 7:48 ` [PATCH 1/4] PCI: of: Add of_pci_clkreq_present() Shawn Lin
2025-10-21 16:16 ` Frank Li
2025-10-22 9:17 ` Shawn Lin
2025-10-22 10:02 ` Manivannan Sadhasivam
2025-10-22 10:13 ` Shawn Lin
2025-10-22 10:29 ` Manivannan Sadhasivam
2025-10-22 16:22 ` Bjorn Helgaas
2025-10-22 17:35 ` Manivannan Sadhasivam
2025-10-22 18:14 ` Bjorn Helgaas
2025-10-21 7:48 ` [PATCH 2/4] PCI: tegra194: Use of_pci_clkreq_present() instead Shawn Lin
2025-10-21 7:48 ` [PATCH 3/4] PCI: dw-rockchip: Add L1sub support Shawn Lin
2025-10-21 8:01 ` Hans Zhang
2025-10-21 8:42 ` Shawn Lin
2025-10-21 9:03 ` Hans Zhang
2025-10-22 10:34 ` Manivannan Sadhasivam
2025-10-21 7:48 ` [PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Shawn Lin
-- strict thread matches above, loose matches on Subject: below --
2025-11-11 22:16 [PATCH 0/4] PCI: dwc: Advertise L1 PM Substates only if driver requests it Bjorn Helgaas
2025-11-11 22:16 ` [PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1 Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).