[PATCH v3 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing

[PATCH v3 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing

[Krishna Chaitanya Chundru]

The T_POWER_ON indicates the time (in μs) that a Port requires the port
on the opposite side of Link to wait in L1.2.Exit after sampling CLKREQ#
asserted before actively driving the interface. This value is used by
the ASPM driver to compute the LTR_L1.2_THRESHOLD.

Currently, qcom root port exposes T_POWER_ON value of zero in the L1SS
capability registers, leading to incorrect LTR_L1.2_THRESHOLD calculations,
which can result in improper L1.2 exit behavior and can trigger AER's.

In this series, qcom controller drivers read the devicetree property
"t-power-on" which got merged recently[1], and use that value to over
write default/wrong value.

To convert T_POWER_ON in to T_POWER_ON_SCALE & T_POWER_ON_VALUE created
a pcie_encode_t_power_on() helper in aspm.c and also created
dw_pcie_program_t_power_on() helper for other drivers to use these
helpers.

Link [1]: https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v3:
- move pcie_encode_t_power_on() include/linux/pci.h to
  drivers/pci/pci.h (Bjorn).
- couple of changes in commit text and variable name like t_power_on (Bjorn).
- remove return 0 from qcom_pcie_configure_ports (Bjorn).
- used FIELD_MODIFY instead of FIELD_PREP (Bjorn).
- Link to v2: https://lore.kernel.org/r/20260223-t_power_on_fux-v2-0-20c921262709@oss.qualcomm.com

Changes in v2:
- Instead of hard coding the values in the driver, created a devicetree
  property "t-power-on" to program it (Bjorn & Mani).
- Link to v1: https://lore.kernel.org/r/20251104-t_power_on_fux-v1-1-eb5916e47fd7@oss.qualcomm.com

---
Krishna Chaitanya Chundru (3):
      PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields
      PCI: dwc: Add helper to Program T_POWER_ON
      PCI: qcom: Program T_POWER_ON

 drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c       | 13 +++++++++
 drivers/pci/pci.h                            |  2 ++
 drivers/pci/pcie/aspm.c                      | 43 ++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20251104-t_power_on_fux-70dc68377941

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

[PATCH v3 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields

[Krishna Chaitanya Chundru]

Add a shared helper to encode the PCIe L1 PM Substates T_POWER_ON
parameter into the T_POWER_ON_Scale and T_POWER_ON_Value fields.

This helper can be used by the controller drivers to change the
default/wrong value of T_POWER_ON in L1ss capability register to
avoid incorrect calculation of LTR_L1.2_THRESHOLD value.

The helper converts a T_POWER_ON time specified in microseconds into
the appropriate scale/value encoding defined by the PCIe spec r7.0,
sec 7.8.3.2. Values that exceed the maximum encodable range are clamped
to the largest representable encoding.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/pci.h       |  2 ++
 drivers/pci/pcie/aspm.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 13d998fbacce6698514d92500dfea03cc562cdc2..48964602d802e114a6a2481df3fb75d9e178a31b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1105,6 +1105,7 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked);
 void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
 void pci_configure_ltr(struct pci_dev *pdev);
 void pci_bridge_reconfigure_ltr(struct pci_dev *pdev);
+void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value);
 #else
 static inline void pcie_aspm_remove_cap(struct pci_dev *pdev, u32 lnkcap) { }
 static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
@@ -1113,6 +1114,7 @@ static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked)
 static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
 static inline void pci_configure_ltr(struct pci_dev *pdev) { }
 static inline void pci_bridge_reconfigure_ltr(struct pci_dev *pdev) { }
+static inline void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value) { }
 #endif
 
 #ifdef CONFIG_PCIE_ECRC
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 21f5d23e0b61bd7e1163cc869fe9356d1ab87b34..d7f9ae9e48c25dbc2d9b4887e2f74623688098e0 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -525,6 +525,49 @@ static u32 calc_l12_pwron(struct pci_dev *pdev, u32 scale, u32 val)
 	return 0;
 }
 
+/**
+ * pcie_encode_t_power_on - Encode T_POWER_ON into scale and value fields
+ * @t_power_on_us: T_POWER_ON time in microseconds
+ * @scale: Encoded T_POWER_ON_Scale (0..2)
+ * @value: Encoded T_POWER_ON_Value
+ *
+ * T_POWER_ON is encoded as:
+ *   T_POWER_ON(us) = scale_unit(us) * value
+ *
+ * where scale_unit is selected by @scale:
+ *   0: 2us
+ *   1: 10us
+ *   2: 100us
+ *
+ * If @t_power_on_us exceeds the maximum representable value, the result
+ * is clamped to the largest encodable T_POWER_ON.
+ *
+ * See PCIe r7.0, sec 7.8.3.2.
+ */
+void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value)
+{
+	u8 maxv = FIELD_MAX(PCI_L1SS_CTL2_T_PWR_ON_VALUE);
+
+	/*
+	 * T_POWER_ON_Value ("value") is a 5-bit field with max
+	 * value of 31.
+	 */
+	if (t_power_on_us <= 2 * maxv) {
+		*scale = 0; /* Value times 2us */
+		*value = DIV_ROUND_UP(t_power_on_us, 2);
+	} else if (t_power_on_us <= 10 * maxv) {
+		*scale = 1; /* Value times 10us */
+		*value = DIV_ROUND_UP(t_power_on_us, 10);
+	} else if (t_power_on_us <= 100 * maxv) {
+		*scale = 2; /* value times 100us */
+		*value = DIV_ROUND_UP(t_power_on_us, 100);
+	} else {
+		*scale = 2;
+		*value = maxv;
+	}
+}
+EXPORT_SYMBOL(pcie_encode_t_power_on);
+
 /*
  * Encode an LTR_L1.2_THRESHOLD value for the L1 PM Substates Control 1
  * register.  Ports enter L1.2 when the most recent LTR value is greater

-- 
2.34.1

[PATCH v3 2/3] PCI: dwc: Add helper to Program T_POWER_ON

[Krishna Chaitanya Chundru]

The T_POWER_ON indicates the time (in μs) that a Port requires the port
on the opposite side of Link to wait in L1.2.Exit after sampling CLKREQ#
asserted before actively driving the interface. This value is used by
the ASPM driver to compute the LTR_L1.2_THRESHOLD.

Currently, some controllers exposes T_POWER_ON value of zero in the L1SS
capability registers, leading to incorrect LTR_L1.2_THRESHOLD calculations,
which can result in improper L1.2 exit behavior and if AER happens to be
supported and enabled, the error may be *reported* via AER.

Add a helper to override T_POWER_ON value by the DWC controller drivers.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++++++++++++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 5741c09dde7f40487c6da6dfd66f7c8d96a74259..6289329ef2b2a4ac9264d1c6cb5ea4e88c261634 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -1249,6 +1249,34 @@ void dw_pcie_hide_unsupported_l1ss(struct dw_pcie *pci)
 	dw_pcie_writel_dbi(pci, l1ss + PCI_L1SS_CAP, l1ss_cap);
 }
 
+/* TODO: Need to handle multi root ports */
+void dw_pcie_program_t_power_on(struct dw_pcie *pci, u16 t_power_on)
+{
+	u8 scale, value;
+	u16 offset;
+	u32 val;
+
+	if (!t_power_on)
+		return;
+
+	offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
+	if (!offset)
+		return;
+
+	pcie_encode_t_power_on(t_power_on, &scale, &value);
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	val = readl(pci->dbi_base + offset + PCI_L1SS_CAP);
+	val &= ~(PCI_L1SS_CAP_P_PWR_ON_SCALE | PCI_L1SS_CAP_P_PWR_ON_VALUE);
+	FIELD_MODIFY(PCI_L1SS_CAP_P_PWR_ON_SCALE, &val, scale);
+	FIELD_MODIFY(PCI_L1SS_CAP_P_PWR_ON_VALUE, &val, value);
+
+	writel(val, pci->dbi_base + offset + PCI_L1SS_CAP);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+}
+
 void dw_pcie_setup(struct dw_pcie *pci)
 {
 	u32 val;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ae6389dd9caa5c27690f998d58729130ea863984..da67beece3f11e33d9a1937fa23d443feea3bbc7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -602,6 +602,7 @@ int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
 				u8 bar, size_t size);
 void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index);
 void dw_pcie_hide_unsupported_l1ss(struct dw_pcie *pci);
+void dw_pcie_program_t_power_on(struct dw_pcie *pci, u16 t_power_on);
 void dw_pcie_setup(struct dw_pcie *pci);
 void dw_pcie_iatu_detect(struct dw_pcie *pci);
 int dw_pcie_edma_detect(struct dw_pcie *pci);

-- 
2.34.1

[PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Krishna Chaitanya Chundru]

Some platforms have incorrect T_POWER_ON value programmed in hardware.
Generally these will be corrected by bootloaders, but not all targets
support bootloaders to program correct values due to that
LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
can result in improper L1.2 exit behavior and if AER happens to be
supported and enabled, the error may be *reported* via AER.

Parse "t-power-on-us" property from each root port node and program them
as part of host initialization using dw_pcie_program_t_power_on() before
link training.

This property in added to the dtschema here[1].

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Link[1]: https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/
---
 drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -269,6 +269,7 @@ struct qcom_pcie_perst {
 struct qcom_pcie_port {
 	struct list_head list;
 	struct phy *phy;
+	u32 l1ss_t_power_on;
 	struct list_head perst;
 };
 
@@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct qcom_pcie *pcie)
 	return 0;
 }
 
+static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list)
+		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
+}
+
 static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
 	dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
 
+	qcom_pcie_configure_ports(pcie);
+
 	qcom_pcie_perst_deassert(pcie);
 
 	if (pcie->cfg->ops->config_sid) {
@@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
 	if (ret)
 		return ret;
 
+	of_property_read_u32(node, "t-power-on-us", &port->l1ss_t_power_on);
+
 	port->phy = phy;
 	INIT_LIST_HEAD(&port->list);
 	list_add_tail(&port->list, &pcie->ports);

-- 
2.34.1

Re: [PATCH v3 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields

[Shawn Lin]

在 2026/03/11 星期三 20:40, Krishna Chaitanya Chundru 写道:
> Add a shared helper to encode the PCIe L1 PM Substates T_POWER_ON
> parameter into the T_POWER_ON_Scale and T_POWER_ON_Value fields.
> 
> This helper can be used by the controller drivers to change the
> default/wrong value of T_POWER_ON in L1ss capability register to
> avoid incorrect calculation of LTR_L1.2_THRESHOLD value.
> 
> The helper converts a T_POWER_ON time specified in microseconds into
> the appropriate scale/value encoding defined by the PCIe spec r7.0,
> sec 7.8.3.2. Values that exceed the maximum encodable range are clamped
> to the largest representable encoding.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

I've tested it on my end with different t_power_on values in DT, it
works as expected.

Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> ---
>   drivers/pci/pci.h       |  2 ++
>   drivers/pci/pcie/aspm.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 13d998fbacce6698514d92500dfea03cc562cdc2..48964602d802e114a6a2481df3fb75d9e178a31b 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -1105,6 +1105,7 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked);
>   void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
>   void pci_configure_ltr(struct pci_dev *pdev);
>   void pci_bridge_reconfigure_ltr(struct pci_dev *pdev);
> +void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value);
>   #else
>   static inline void pcie_aspm_remove_cap(struct pci_dev *pdev, u32 lnkcap) { }
>   static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
> @@ -1113,6 +1114,7 @@ static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked)
>   static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
>   static inline void pci_configure_ltr(struct pci_dev *pdev) { }
>   static inline void pci_bridge_reconfigure_ltr(struct pci_dev *pdev) { }
> +static inline void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value) { }
>   #endif
>   
>   #ifdef CONFIG_PCIE_ECRC
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 21f5d23e0b61bd7e1163cc869fe9356d1ab87b34..d7f9ae9e48c25dbc2d9b4887e2f74623688098e0 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -525,6 +525,49 @@ static u32 calc_l12_pwron(struct pci_dev *pdev, u32 scale, u32 val)
>   	return 0;
>   }
>   
> +/**
> + * pcie_encode_t_power_on - Encode T_POWER_ON into scale and value fields
> + * @t_power_on_us: T_POWER_ON time in microseconds
> + * @scale: Encoded T_POWER_ON_Scale (0..2)
> + * @value: Encoded T_POWER_ON_Value
> + *
> + * T_POWER_ON is encoded as:
> + *   T_POWER_ON(us) = scale_unit(us) * value
> + *
> + * where scale_unit is selected by @scale:
> + *   0: 2us
> + *   1: 10us
> + *   2: 100us
> + *
> + * If @t_power_on_us exceeds the maximum representable value, the result
> + * is clamped to the largest encodable T_POWER_ON.
> + *
> + * See PCIe r7.0, sec 7.8.3.2.
> + */
> +void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value)
> +{
> +	u8 maxv = FIELD_MAX(PCI_L1SS_CTL2_T_PWR_ON_VALUE);
> +
> +	/*
> +	 * T_POWER_ON_Value ("value") is a 5-bit field with max
> +	 * value of 31.
> +	 */
> +	if (t_power_on_us <= 2 * maxv) {
> +		*scale = 0; /* Value times 2us */
> +		*value = DIV_ROUND_UP(t_power_on_us, 2);
> +	} else if (t_power_on_us <= 10 * maxv) {
> +		*scale = 1; /* Value times 10us */
> +		*value = DIV_ROUND_UP(t_power_on_us, 10);
> +	} else if (t_power_on_us <= 100 * maxv) {
> +		*scale = 2; /* value times 100us */
> +		*value = DIV_ROUND_UP(t_power_on_us, 100);
> +	} else {
> +		*scale = 2;
> +		*value = maxv;
> +	}
> +}
> +EXPORT_SYMBOL(pcie_encode_t_power_on);
> +
>   /*
>    * Encode an LTR_L1.2_THRESHOLD value for the L1 PM Substates Control 1
>    * register.  Ports enter L1.2 when the most recent LTR value is greater
> 

Re: [PATCH v3 2/3] PCI: dwc: Add helper to Program T_POWER_ON

[Shawn Lin]

在 2026/03/11 星期三 20:40, Krishna Chaitanya Chundru 写道:
> The T_POWER_ON indicates the time (in μs) that a Port requires the port
> on the opposite side of Link to wait in L1.2.Exit after sampling CLKREQ#
> asserted before actively driving the interface. This value is used by
> the ASPM driver to compute the LTR_L1.2_THRESHOLD.
> 
> Currently, some controllers exposes T_POWER_ON value of zero in the L1SS
> capability registers, leading to incorrect LTR_L1.2_THRESHOLD calculations,
> which can result in improper L1.2 exit behavior and if AER happens to be
> supported and enabled, the error may be *reported* via AER.
> 
> Add a helper to override T_POWER_ON value by the DWC controller drivers.
> 

Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>   drivers/pci/controller/dwc/pcie-designware.c | 28 ++++++++++++++++++++++++++++
>   drivers/pci/controller/dwc/pcie-designware.h |  1 +
>   2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 5741c09dde7f40487c6da6dfd66f7c8d96a74259..6289329ef2b2a4ac9264d1c6cb5ea4e88c261634 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -1249,6 +1249,34 @@ void dw_pcie_hide_unsupported_l1ss(struct dw_pcie *pci)
>   	dw_pcie_writel_dbi(pci, l1ss + PCI_L1SS_CAP, l1ss_cap);
>   }
>   
> +/* TODO: Need to handle multi root ports */
> +void dw_pcie_program_t_power_on(struct dw_pcie *pci, u16 t_power_on)
> +{
> +	u8 scale, value;
> +	u16 offset;
> +	u32 val;
> +
> +	if (!t_power_on)
> +		return;
> +
> +	offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
> +	if (!offset)
> +		return;
> +
> +	pcie_encode_t_power_on(t_power_on, &scale, &value);
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +
> +	val = readl(pci->dbi_base + offset + PCI_L1SS_CAP);
> +	val &= ~(PCI_L1SS_CAP_P_PWR_ON_SCALE | PCI_L1SS_CAP_P_PWR_ON_VALUE);
> +	FIELD_MODIFY(PCI_L1SS_CAP_P_PWR_ON_SCALE, &val, scale);
> +	FIELD_MODIFY(PCI_L1SS_CAP_P_PWR_ON_VALUE, &val, value);
> +
> +	writel(val, pci->dbi_base + offset + PCI_L1SS_CAP);
> +
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
>   void dw_pcie_setup(struct dw_pcie *pci)
>   {
>   	u32 val;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ae6389dd9caa5c27690f998d58729130ea863984..da67beece3f11e33d9a1937fa23d443feea3bbc7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -602,6 +602,7 @@ int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>   				u8 bar, size_t size);
>   void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index);
>   void dw_pcie_hide_unsupported_l1ss(struct dw_pcie *pci);
> +void dw_pcie_program_t_power_on(struct dw_pcie *pci, u16 t_power_on);
>   void dw_pcie_setup(struct dw_pcie *pci);
>   void dw_pcie_iatu_detect(struct dw_pcie *pci);
>   int dw_pcie_edma_detect(struct dw_pcie *pci);
> 

Re: [PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Shawn Lin]

在 2026/03/11 星期三 20:40, Krishna Chaitanya Chundru 写道:
> Some platforms have incorrect T_POWER_ON value programmed in hardware.
> Generally these will be corrected by bootloaders, but not all targets
> support bootloaders to program correct values due to that
> LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
> can result in improper L1.2 exit behavior and if AER happens to be
> supported and enabled, the error may be *reported* via AER.
> 
> Parse "t-power-on-us" property from each root port node and program them
> as part of host initialization using dw_pcie_program_t_power_on() before
> link training.
> 
> This property in added to the dtschema here[1].
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Link[1]: https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -269,6 +269,7 @@ struct qcom_pcie_perst {
>   struct qcom_pcie_port {
>   	struct list_head list;
>   	struct phy *phy;
> +	u32 l1ss_t_power_on;

u16?

>   	struct list_head perst;
>   };
>   
> @@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct qcom_pcie *pcie)
>   	return 0;
>   }
>   
> +static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_port *port;
> +
> +	list_for_each_entry(port, &pcie->ports, list)
> +		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
> +}
> +
>   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>   {
>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>   	dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
>   	dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
>   
> +	qcom_pcie_configure_ports(pcie);
> +
>   	qcom_pcie_perst_deassert(pcie);
>   
>   	if (pcie->cfg->ops->config_sid) {
> @@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
>   	if (ret)
>   		return ret;
>   
> +	of_property_read_u32(node, "t-power-on-us", &port->l1ss_t_power_on);

Ditto?

> +
>   	port->phy = phy;
>   	INIT_LIST_HEAD(&port->list);
>   	list_add_tail(&port->list, &pcie->ports);
> 

Re: [PATCH v3 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields

[Manivannan Sadhasivam]

On Wed, Mar 11, 2026 at 06:10:38PM +0530, Krishna Chaitanya Chundru wrote:
> Add a shared helper to encode the PCIe L1 PM Substates T_POWER_ON
> parameter into the T_POWER_ON_Scale and T_POWER_ON_Value fields.
> 
> This helper can be used by the controller drivers to change the
> default/wrong value of T_POWER_ON in L1ss capability register to
> avoid incorrect calculation of LTR_L1.2_THRESHOLD value.
> 
> The helper converts a T_POWER_ON time specified in microseconds into
> the appropriate scale/value encoding defined by the PCIe spec r7.0,
> sec 7.8.3.2. Values that exceed the maximum encodable range are clamped
> to the largest representable encoding.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/pci.h       |  2 ++
>  drivers/pci/pcie/aspm.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 13d998fbacce6698514d92500dfea03cc562cdc2..48964602d802e114a6a2481df3fb75d9e178a31b 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -1105,6 +1105,7 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked);
>  void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
>  void pci_configure_ltr(struct pci_dev *pdev);
>  void pci_bridge_reconfigure_ltr(struct pci_dev *pdev);
> +void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value);
>  #else
>  static inline void pcie_aspm_remove_cap(struct pci_dev *pdev, u32 lnkcap) { }
>  static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
> @@ -1113,6 +1114,7 @@ static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked)
>  static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
>  static inline void pci_configure_ltr(struct pci_dev *pdev) { }
>  static inline void pci_bridge_reconfigure_ltr(struct pci_dev *pdev) { }
> +static inline void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value) { }
>  #endif
>  
>  #ifdef CONFIG_PCIE_ECRC
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 21f5d23e0b61bd7e1163cc869fe9356d1ab87b34..d7f9ae9e48c25dbc2d9b4887e2f74623688098e0 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -525,6 +525,49 @@ static u32 calc_l12_pwron(struct pci_dev *pdev, u32 scale, u32 val)
>  	return 0;
>  }
>  
> +/**
> + * pcie_encode_t_power_on - Encode T_POWER_ON into scale and value fields
> + * @t_power_on_us: T_POWER_ON time in microseconds
> + * @scale: Encoded T_POWER_ON_Scale (0..2)
> + * @value: Encoded T_POWER_ON_Value

Nit:

s/T_POWER_ON_Scale/T_POWER_ON Scale
s/T_POWER_ON_Value/T_POWER_ON Value

Same everywhere.

> + *
> + * T_POWER_ON is encoded as:
> + *   T_POWER_ON(us) = scale_unit(us) * value
> + *
> + * where scale_unit is selected by @scale:
> + *   0: 2us
> + *   1: 10us
> + *   2: 100us
> + *
> + * If @t_power_on_us exceeds the maximum representable value, the result
> + * is clamped to the largest encodable T_POWER_ON.
> + *
> + * See PCIe r7.0, sec 7.8.3.2.
> + */
> +void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value)
> +{
> +	u8 maxv = FIELD_MAX(PCI_L1SS_CTL2_T_PWR_ON_VALUE);

Since this API is for changing the 'capabilities register', you should use
PCI_L1SS_CAP_P_PWR_ON_VALUE.

> +
> +	/*
> +	 * T_POWER_ON_Value ("value") is a 5-bit field with max
> +	 * value of 31.

Use max 80 columns for comments.

- Mani

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

Re: [PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Manivannan Sadhasivam]

On Wed, Mar 11, 2026 at 06:10:40PM +0530, Krishna Chaitanya Chundru wrote:
> Some platforms have incorrect T_POWER_ON value programmed in hardware.
> Generally these will be corrected by bootloaders, but not all targets
> support bootloaders to program correct values due to that
> LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
> can result in improper L1.2 exit behavior and if AER happens to be
> supported and enabled, the error may be *reported* via AER.
> 
> Parse "t-power-on-us" property from each root port node and program them
> as part of host initialization using dw_pcie_program_t_power_on() before
> link training.
> 
> This property in added to the dtschema here[1].
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Link[1]: https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -269,6 +269,7 @@ struct qcom_pcie_perst {
>  struct qcom_pcie_port {
>  	struct list_head list;
>  	struct phy *phy;
> +	u32 l1ss_t_power_on;
>  	struct list_head perst;
>  };
>  
> @@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct qcom_pcie *pcie)
>  	return 0;
>  }
>  
> +static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_port *port;
> +
> +	list_for_each_entry(port, &pcie->ports, list)
> +		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
> +}
> +
>  static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>  	dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
>  	dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
>  
> +	qcom_pcie_configure_ports(pcie);
> +
>  	qcom_pcie_perst_deassert(pcie);
>  
>  	if (pcie->cfg->ops->config_sid) {
> @@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
>  	if (ret)
>  		return ret;
>  
> +	of_property_read_u32(node, "t-power-on-us", &port->l1ss_t_power_on);
> +

Since this is going to be common for all controller drivers, it'd be good to
move this inside the dwc API.

- Mani

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

Re: [PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Krishna Chaitanya Chundru]

On 3/16/2026 10:23 AM, Manivannan Sadhasivam wrote:
> On Wed, Mar 11, 2026 at 06:10:40PM +0530, Krishna Chaitanya Chundru wrote:
>> Some platforms have incorrect T_POWER_ON value programmed in hardware.
>> Generally these will be corrected by bootloaders, but not all targets
>> support bootloaders to program correct values due to that
>> LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
>> can result in improper L1.2 exit behavior and if AER happens to be
>> supported and enabled, the error may be *reported* via AER.
>>
>> Parse "t-power-on-us" property from each root port node and program them
>> as part of host initialization using dw_pcie_program_t_power_on() before
>> link training.
>>
>> This property in added to the dtschema here[1].
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Link[1]: https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -269,6 +269,7 @@ struct qcom_pcie_perst {
>>   struct qcom_pcie_port {
>>   	struct list_head list;
>>   	struct phy *phy;
>> +	u32 l1ss_t_power_on;
>>   	struct list_head perst;
>>   };
>>   
>> @@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct qcom_pcie *pcie)
>>   	return 0;
>>   }
>>   
>> +static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
>> +{
>> +	struct qcom_pcie_port *port;
>> +
>> +	list_for_each_entry(port, &pcie->ports, list)
>> +		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
>> +}
>> +
>>   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>   {
>>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> @@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>   	dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
>>   	dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
>>   
>> +	qcom_pcie_configure_ports(pcie);
>> +
>>   	qcom_pcie_perst_deassert(pcie);
>>   
>>   	if (pcie->cfg->ops->config_sid) {
>> @@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
>>   	if (ret)
>>   		return ret;
>>   
>> +	of_property_read_u32(node, "t-power-on-us", &port->l1ss_t_power_on);
>> +
> Since this is going to be common for all controller drivers, it'd be good to
> move this inside the dwc API.
As this per root port, we can summit changes to merge first so that we 
can move this to
dwc.

- Krishna Chaitanya.
>
> - Mani
>

Re: [PATCH v3 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields

[Krishna Chaitanya Chundru]

On 3/16/2026 10:21 AM, Manivannan Sadhasivam wrote:
> On Wed, Mar 11, 2026 at 06:10:38PM +0530, Krishna Chaitanya Chundru wrote:
>> Add a shared helper to encode the PCIe L1 PM Substates T_POWER_ON
>> parameter into the T_POWER_ON_Scale and T_POWER_ON_Value fields.
>>
>> This helper can be used by the controller drivers to change the
>> default/wrong value of T_POWER_ON in L1ss capability register to
>> avoid incorrect calculation of LTR_L1.2_THRESHOLD value.
>>
>> The helper converts a T_POWER_ON time specified in microseconds into
>> the appropriate scale/value encoding defined by the PCIe spec r7.0,
>> sec 7.8.3.2. Values that exceed the maximum encodable range are clamped
>> to the largest representable encoding.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   drivers/pci/pci.h       |  2 ++
>>   drivers/pci/pcie/aspm.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 13d998fbacce6698514d92500dfea03cc562cdc2..48964602d802e114a6a2481df3fb75d9e178a31b 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -1105,6 +1105,7 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked);
>>   void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
>>   void pci_configure_ltr(struct pci_dev *pdev);
>>   void pci_bridge_reconfigure_ltr(struct pci_dev *pdev);
>> +void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value);
>>   #else
>>   static inline void pcie_aspm_remove_cap(struct pci_dev *pdev, u32 lnkcap) { }
>>   static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
>> @@ -1113,6 +1114,7 @@ static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev, bool locked)
>>   static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
>>   static inline void pci_configure_ltr(struct pci_dev *pdev) { }
>>   static inline void pci_bridge_reconfigure_ltr(struct pci_dev *pdev) { }
>> +static inline void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value) { }
>>   #endif
>>   
>>   #ifdef CONFIG_PCIE_ECRC
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index 21f5d23e0b61bd7e1163cc869fe9356d1ab87b34..d7f9ae9e48c25dbc2d9b4887e2f74623688098e0 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -525,6 +525,49 @@ static u32 calc_l12_pwron(struct pci_dev *pdev, u32 scale, u32 val)
>>   	return 0;
>>   }
>>   
>> +/**
>> + * pcie_encode_t_power_on - Encode T_POWER_ON into scale and value fields
>> + * @t_power_on_us: T_POWER_ON time in microseconds
>> + * @scale: Encoded T_POWER_ON_Scale (0..2)
>> + * @value: Encoded T_POWER_ON_Value
> Nit:
>
> s/T_POWER_ON_Scale/T_POWER_ON Scale
> s/T_POWER_ON_Value/T_POWER_ON Value
>
> Same everywhere.
>
>> + *
>> + * T_POWER_ON is encoded as:
>> + *   T_POWER_ON(us) = scale_unit(us) * value
>> + *
>> + * where scale_unit is selected by @scale:
>> + *   0: 2us
>> + *   1: 10us
>> + *   2: 100us
>> + *
>> + * If @t_power_on_us exceeds the maximum representable value, the result
>> + * is clamped to the largest encodable T_POWER_ON.
>> + *
>> + * See PCIe r7.0, sec 7.8.3.2.
>> + */
>> +void pcie_encode_t_power_on(u16 t_power_on_us, u8 *scale, u8 *value)
>> +{
>> +	u8 maxv = FIELD_MAX(PCI_L1SS_CTL2_T_PWR_ON_VALUE);
> Since this API is for changing the 'capabilities register', you should use
> PCI_L1SS_CAP_P_PWR_ON_VALUE.
Ack.

- Krishna Chaitanya.
>> +	/*
>> +	 * T_POWER_ON_Value ("value") is a 5-bit field with max
>> +	 * value of 31.
> Use max 80 columns for comments.
>
> - Mani
>

Re: [PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Manivannan Sadhasivam]

On Mon, Mar 16, 2026 at 10:35:50AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/16/2026 10:23 AM, Manivannan Sadhasivam wrote:
> > On Wed, Mar 11, 2026 at 06:10:40PM +0530, Krishna Chaitanya Chundru wrote:
> > > Some platforms have incorrect T_POWER_ON value programmed in hardware.
> > > Generally these will be corrected by bootloaders, but not all targets
> > > support bootloaders to program correct values due to that
> > > LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
> > > can result in improper L1.2 exit behavior and if AER happens to be
> > > supported and enabled, the error may be *reported* via AER.
> > > 
> > > Parse "t-power-on-us" property from each root port node and program them
> > > as part of host initialization using dw_pcie_program_t_power_on() before
> > > link training.
> > > 
> > > This property in added to the dtschema here[1].
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > Link[1]: https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -269,6 +269,7 @@ struct qcom_pcie_perst {
> > >   struct qcom_pcie_port {
> > >   	struct list_head list;
> > >   	struct phy *phy;
> > > +	u32 l1ss_t_power_on;
> > >   	struct list_head perst;
> > >   };
> > > @@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct qcom_pcie *pcie)
> > >   	return 0;
> > >   }
> > > +static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
> > > +{
> > > +	struct qcom_pcie_port *port;
> > > +
> > > +	list_for_each_entry(port, &pcie->ports, list)
> > > +		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
> > > +}
> > > +
> > >   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > >   {
> > >   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > @@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > >   	dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
> > >   	dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
> > > +	qcom_pcie_configure_ports(pcie);
> > > +
> > >   	qcom_pcie_perst_deassert(pcie);
> > >   	if (pcie->cfg->ops->config_sid) {
> > > @@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
> > >   	if (ret)
> > >   		return ret;
> > > +	of_property_read_u32(node, "t-power-on-us", &port->l1ss_t_power_on);
> > > +
> > Since this is going to be common for all controller drivers, it'd be good to
> > move this inside the dwc API.
> As this per root port, we can summit changes to merge first so that we can
> move this to
> dwc.
> 

Fair. Atleast add a TODO here.

- Mani

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

Re: [PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Krishna Chaitanya Chundru]

On 3/13/2026 8:19 AM, Shawn Lin wrote:
> 在 2026/03/11 星期三 20:40, Krishna Chaitanya Chundru 写道:
>> Some platforms have incorrect T_POWER_ON value programmed in hardware.
>> Generally these will be corrected by bootloaders, but not all targets
>> support bootloaders to program correct values due to that
>> LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
>> can result in improper L1.2 exit behavior and if AER happens to be
>> supported and enabled, the error may be *reported* via AER.
>>
>> Parse "t-power-on-us" property from each root port node and program them
>> as part of host initialization using dw_pcie_program_t_power_on() before
>> link training.
>>
>> This property in added to the dtschema here[1].
>>
>> Signed-off-by: Krishna Chaitanya Chundru 
>> <krishna.chundru@oss.qualcomm.com>
>> Link[1]: 
>> https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c 
>> b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 
>> 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 
>> 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -269,6 +269,7 @@ struct qcom_pcie_perst {
>>   struct qcom_pcie_port {
>>       struct list_head list;
>>       struct phy *phy;
>> +    u32 l1ss_t_power_on;
>
> u16?
>
>>       struct list_head perst;
>>   };
>>   @@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct 
>> qcom_pcie *pcie)
>>       return 0;
>>   }
>>   +static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
>> +{
>> +    struct qcom_pcie_port *port;
>> +
>> +    list_for_each_entry(port, &pcie->ports, list)
>> +        dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
>> +}
>> +
>>   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>   {
>>       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> @@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct 
>> dw_pcie_rp *pp)
>>       dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
>>       dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
>>   +    qcom_pcie_configure_ports(pcie);
>> +
>>       qcom_pcie_perst_deassert(pcie);
>>         if (pcie->cfg->ops->config_sid) {
>> @@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct 
>> qcom_pcie *pcie, struct device_node *node
>>       if (ret)
>>           return ret;
>>   +    of_property_read_u32(node, "t-power-on-us", 
>> &port->l1ss_t_power_on);
>
> Ditto?
the property has -us and doesn't have $ref to indicate if it is u32 or 
u16, by default it is u32 property only.
can we can't keep it as u16 even if have always less than u16.

- Krishna Chaitanya.
>> +
>>       port->phy = phy;
>>       INIT_LIST_HEAD(&port->list);
>>       list_add_tail(&port->list, &pcie->ports);
>>

Re: [PATCH v3 3/3] PCI: qcom: Program T_POWER_ON

[Shawn Lin]

在 2026/03/30 星期一 16:16, Krishna Chaitanya Chundru 写道:
> 
> 
> On 3/13/2026 8:19 AM, Shawn Lin wrote:
>> 在 2026/03/11 星期三 20:40, Krishna Chaitanya Chundru 写道:
>>> Some platforms have incorrect T_POWER_ON value programmed in hardware.
>>> Generally these will be corrected by bootloaders, but not all targets
>>> support bootloaders to program correct values due to that
>>> LTR_L1.2_THRESHOLD value calculated by aspm driver can be wrong, which
>>> can result in improper L1.2 exit behavior and if AER happens to be
>>> supported and enabled, the error may be *reported* via AER.
>>>
>>> Parse "t-power-on-us" property from each root port node and program them
>>> as part of host initialization using dw_pcie_program_t_power_on() before
>>> link training.
>>>
>>> This property in added to the dtschema here[1].
>>>
>>> Signed-off-by: Krishna Chaitanya Chundru 
>>> <krishna.chundru@oss.qualcomm.com>
>>> Link[1]: 
>>> https://lore.kernel.org/all/20260205093346.667898-1-krishna.chundru@oss.qualcomm.com/ 
>>>
>>> ---
>>>   drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c 
>>> b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index 
>>> 67a16af69ddc75fca1b123e70715e692a91a9135..63d5628e988d3c07f42099c0d86e6b80fd54ce32 
>>> 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -269,6 +269,7 @@ struct qcom_pcie_perst {
>>>   struct qcom_pcie_port {
>>>       struct list_head list;
>>>       struct phy *phy;
>>> +    u32 l1ss_t_power_on;
>>
>> u16?
>>
>>>       struct list_head perst;
>>>   };
>>>   @@ -1283,6 +1284,14 @@ static int qcom_pcie_phy_power_on(struct 
>>> qcom_pcie *pcie)
>>>       return 0;
>>>   }
>>>   +static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
>>> +{
>>> +    struct qcom_pcie_port *port;
>>> +
>>> +    list_for_each_entry(port, &pcie->ports, list)
>>> +        dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
>>> +}
>>> +
>>>   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>   {
>>>       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>> @@ -1317,6 +1326,8 @@ static int qcom_pcie_host_init(struct 
>>> dw_pcie_rp *pp)
>>>       dw_pcie_remove_capability(pcie->pci, PCI_CAP_ID_MSIX);
>>>       dw_pcie_remove_ext_capability(pcie->pci, PCI_EXT_CAP_ID_DPC);
>>>   +    qcom_pcie_configure_ports(pcie);
>>> +
>>>       qcom_pcie_perst_deassert(pcie);
>>>         if (pcie->cfg->ops->config_sid) {
>>> @@ -1759,6 +1770,8 @@ static int qcom_pcie_parse_port(struct 
>>> qcom_pcie *pcie, struct device_node *node
>>>       if (ret)
>>>           return ret;
>>>   +    of_property_read_u32(node, "t-power-on-us", 
>>> &port->l1ss_t_power_on);
>>
>> Ditto?
> the property has -us and doesn't have $ref to indicate if it is u32 or 
> u16, by default it is u32 property only.
> can we can't keep it as u16 even if have always less than u16.

It came from dw_pcie_program_t_power_on(u16 t_power_on) which
take u16, so the value will be truncated anyway. Well, it's up to you,
just a bypass comment. :)

> 
> - Krishna Chaitanya.
>>> +
>>>       port->phy = phy;
>>>       INIT_LIST_HEAD(&port->list);
>>>       list_add_tail(&port->list, &pcie->ports);
>>>
> 
>