[PATCH 0/4] Add L1 substates support for Rockchip platforms

[PATCH 0/4] Add L1 substates support for Rockchip platforms

[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

[PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[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

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

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

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
>

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Shawn Lin]

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

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

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

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Shawn Lin]

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
> 

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Manivannan Sadhasivam]

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

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Bjorn Helgaas]

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

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Manivannan Sadhasivam]

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

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Bjorn Helgaas]

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.

[PATCH 2/4] PCI: tegra194: Use of_pci_clkreq_present() instead

[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

[PATCH 3/4] PCI: dw-rockchip: Add L1sub support

[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

Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support

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

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

Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support

[Shawn Lin]

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

Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support

[Hans Zhang]

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

Re: [PATCH 3/4] PCI: dw-rockchip: Add L1sub support

[Manivannan Sadhasivam]

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

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 4/4] arm64: dts: rockchip: Add PCIe clkreq stuff for RK3588 EVB1

[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