* [PATCH v5 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing
@ 2026-04-28 8:37 Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields Krishna Chaitanya Chundru
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-04-28 8:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Jingoo Han
Cc: linux-pci, linux-arm-msm, linux-kernel, mayank.rana,
quic_vbadigan, Krishna Chaitanya Chundru, Shawn Lin
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 v5:
- Initialize *scale & *value to zero incase of ASPM is disabled pointed
by sashiko.
- Use dwc readl & writel API's instead of direct readl & writel pointed
by sashiko
- couple of nits (Mani).
- Link to v4: https://lore.kernel.org/r/20260404-t_power_on_fux-v4-0-2891391177f4@oss.qualcomm.com
Changes in v4:
- calculate maxv from PCI_L1SS_CTL2_T_PWR_ON_VALUE to PCI_L1SS_CAP_P_PWR_ON_VALUE (Mani).
- added a todo to move the reading the devicetree from qcom driver to
dwc once multi root port parsing support is added (Mani).
- Link to v3: https://lore.kernel.org/r/20260311-t_power_on_fux-v3-0-9b1f1d09c6f3@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
To: Bjorn Helgaas <bhelgaas@google.com>
To: Jingoo Han <jingoohan1@gmail.com>
To: Manivannan Sadhasivam <mani@kernel.org>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
To: Krzysztof Wilczyński <kwilczynski@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
---
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 | 14 ++++++++++
drivers/pci/pci.h | 6 +++++
drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++++++++++
5 files changed, 89 insertions(+)
---
base-commit: 3b3bea6d4b9c162f9e555905d96b8c1da67ecd5b
change-id: 20251104-t_power_on_fux-70dc68377941
Best regards,
--
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields
2026-04-28 8:37 [PATCH v5 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing Krishna Chaitanya Chundru
@ 2026-04-28 8:37 ` Krishna Chaitanya Chundru
2026-04-28 9:27 ` Ilpo Järvinen
2026-04-28 8:37 ` [PATCH v5 2/3] PCI: dwc: Add helper to Program T_POWER_ON Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 3/3] PCI: qcom: " Krishna Chaitanya Chundru
2 siblings, 1 reply; 6+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-04-28 8:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Jingoo Han
Cc: linux-pci, linux-arm-msm, linux-kernel, mayank.rana,
quic_vbadigan, Krishna Chaitanya Chundru, Shawn Lin
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.
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/pci.h | 6 ++++++
drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4a14f88e543a..c379befe1ebe 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1110,6 +1110,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) { }
@@ -1118,6 +1119,11 @@ 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)
+{
+ *scale = 0;
+ *value = 0;
+}
#endif
#ifdef CONFIG_PCIE_ECRC
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 925373b98dff..457d469b8d49 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -525,6 +525,46 @@ 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_CAP_P_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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 2/3] PCI: dwc: Add helper to Program T_POWER_ON
2026-04-28 8:37 [PATCH v5 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields Krishna Chaitanya Chundru
@ 2026-04-28 8:37 ` Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 3/3] PCI: qcom: " Krishna Chaitanya Chundru
2 siblings, 0 replies; 6+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-04-28 8:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Jingoo Han
Cc: linux-pci, linux-arm-msm, linux-kernel, mayank.rana,
quic_vbadigan, Krishna Chaitanya Chundru, Shawn Lin
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 c11cf61b8319..9e5fc9935a4e 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 = dw_pcie_readl_dbi(pci, 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);
+
+ dw_pcie_writel_dbi(pci, offset + PCI_L1SS_CAP, val);
+
+ 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 3e69ef60165b..6f741fd9d753 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -605,6 +605,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 3/3] PCI: qcom: Program T_POWER_ON
2026-04-28 8:37 [PATCH v5 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 2/3] PCI: dwc: Add helper to Program T_POWER_ON Krishna Chaitanya Chundru
@ 2026-04-28 8:37 ` Krishna Chaitanya Chundru
2 siblings, 0 replies; 6+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-04-28 8:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Jingoo Han
Cc: linux-pci, linux-arm-msm, linux-kernel, mayank.rana,
quic_vbadigan, 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 | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index af6bf5cce65b..4864e152625a 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;
};
@@ -1288,6 +1289,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);
@@ -1322,6 +1331,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) {
@@ -1764,6 +1775,9 @@ static int qcom_pcie_parse_port(struct qcom_pcie *pcie, struct device_node *node
if (ret)
return ret;
+ /* TODO: Need to move to DWC core once multi Root Port support is added. */
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields
2026-04-28 8:37 ` [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields Krishna Chaitanya Chundru
@ 2026-04-28 9:27 ` Ilpo Järvinen
2026-04-29 3:04 ` Krishna Chaitanya Chundru
0 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2026-04-28 9:27 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Jingoo Han,
linux-pci, linux-arm-msm, LKML, mayank.rana, quic_vbadigan,
Shawn Lin
[-- Attachment #1: Type: text/plain, Size: 4418 bytes --]
On Tue, 28 Apr 2026, 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.
>
> 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/pci.h | 6 ++++++
> drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4a14f88e543a..c379befe1ebe 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -1110,6 +1110,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) { }
> @@ -1118,6 +1119,11 @@ 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)
> +{
> + *scale = 0;
> + *value = 0;
> +}
> #endif
>
> #ifdef CONFIG_PCIE_ECRC
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 925373b98dff..457d469b8d49 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -525,6 +525,46 @@ 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)
Hi,
I don't know how the type for t_power_on_us was picked but if it was
arbitrary decision, I suggest you just go with 32-bit input.
That would also remove the u32 -> u16 truncate done in the other patches
of your series which would potentially corrupt the number (I assume
numbers that big would be invalid but they could alias to small u16
numbers).
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> +{
> + u8 maxv = FIELD_MAX(PCI_L1SS_CAP_P_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
>
>
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields
2026-04-28 9:27 ` Ilpo Järvinen
@ 2026-04-29 3:04 ` Krishna Chaitanya Chundru
0 siblings, 0 replies; 6+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-04-29 3:04 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Jingoo Han,
linux-pci, linux-arm-msm, LKML, mayank.rana, quic_vbadigan,
Shawn Lin
On 4/28/2026 2:57 PM, Ilpo Järvinen wrote:
> On Tue, 28 Apr 2026, 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.
>>
>> 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/pci.h | 6 ++++++
>> drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 46 insertions(+)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 4a14f88e543a..c379befe1ebe 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -1110,6 +1110,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) { }
>> @@ -1118,6 +1119,11 @@ 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)
>> +{
>> + *scale = 0;
>> + *value = 0;
>> +}
>> #endif
>>
>> #ifdef CONFIG_PCIE_ECRC
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index 925373b98dff..457d469b8d49 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -525,6 +525,46 @@ 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)
> Hi,
>
> I don't know how the type for t_power_on_us was picked but if it was
> arbitrary decision, I suggest you just go with 32-bit input.
The maximum value of the T power ON is 3100us, so we are using u16 here.
- Krishna Chaitanya.
> That would also remove the u32 -> u16 truncate done in the other patches
> of your series which would potentially corrupt the number (I assume
> numbers that big would be invalid but they could alias to small u16
> numbers).
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
>> +{
>> + u8 maxv = FIELD_MAX(PCI_L1SS_CAP_P_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
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-29 3:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 8:37 [PATCH v5 0/3] PCI: qcom: Program T_POWER_ON value for L1.2 exit timing Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 1/3] PCI/ASPM: Add helper to encode L1SS T_POWER_ON fields Krishna Chaitanya Chundru
2026-04-28 9:27 ` Ilpo Järvinen
2026-04-29 3:04 ` Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 2/3] PCI: dwc: Add helper to Program T_POWER_ON Krishna Chaitanya Chundru
2026-04-28 8:37 ` [PATCH v5 3/3] PCI: qcom: " Krishna Chaitanya Chundru
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox