* [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys
@ 2025-08-20 8:28 Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 1/7] " Krishna Chaitanya Chundru
` (7 more replies)
0 siblings, 8 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
The existing OPP table in the device tree for PCIe is shared across
different link configurations such as data rates 8GT/s x2 and 16GT/s x1.
These configurations often operate at the same frequency, allowing them
to reuse the same OPP entries. However, 8GT/s and 16 GT/s may have
different characteristics beyond frequency—such as RPMh votes in QCOM
case, which cannot be represented accurately when sharing a single OPP.
In such cases, frequency alone is not sufficient to uniquely identify
an OPP. To support these scenarios, introduce a new API
dev_pm_opp_find_key_exact() that allows OPP lookup for set of keys like
frequency, level & bandwidth.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v4:
- Included dtsi changes for all platforms.
- Made the changes as requested by Viresh like adding comments, some
coding styles etc.
- Link to v3: https://lore.kernel.org/r/20250819-opp_pcie-v3-0-f8bd7e05ce41@oss.qualcomm.com
Changes in v3:
- Always check for frequency match unless user doesn't pass it (Viresh).
- Make dev_pm_opp_key public and let user pass the key (Viresh).
- Include bandwidth as part of dev_pm_opp_key (Viresh).
- Link to v2: https://lore.kernel.org/r/20250818-opp_pcie-v2-0-071524d98967@oss.qualcomm.com
Changes in v2:
- Use opp-level to indentify data rate and use both frequency and level
to identify the OPP. (Viresh)
- Link to v1: https://lore.kernel.org/r/20250717-opp_pcie-v1-0-dde6f452571b@oss.qualcomm.com
---
Krishna Chaitanya Chundru (7):
OPP: Add support to find OPP for a set of keys
OPP: Move refcount and key update for readability in _opp_table_find_key()
arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates
arm64: dts: qcom: sm8550: Add opp-level to indicate PCIe data rates
arm64: dts: qcom: sm8650: Add opp-level to indicate PCIe data rates
arm64: dts: qcom: x1e80100: Add opp-level to indicate PCIe data rates
PCI: qcom: Use frequency and level based OPP lookup
arch/arm64/boot/dts/qcom/sm8450.dtsi | 41 ++++++++++---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 63 ++++++++++++++-----
arch/arm64/boot/dts/qcom/sm8650.dtsi | 63 ++++++++++++++-----
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 90 ++++++++++++++++++++++-----
drivers/opp/core.c | 107 +++++++++++++++++++++++++++++++--
drivers/pci/controller/dwc/pcie-qcom.c | 7 ++-
include/linux/pm_opp.h | 30 +++++++++
7 files changed, 341 insertions(+), 60 deletions(-)
---
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
change-id: 20250717-opp_pcie-793160b2b113
Best regards,
--
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 1/7] OPP: Add support to find OPP for a set of keys
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-22 6:50 ` Viresh Kumar
2025-08-20 8:28 ` [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key() Krishna Chaitanya Chundru
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
Some clients, such as PCIe, may operate at the same clock frequency
across different data rates by varying link width. In such cases,
frequency alone is not sufficient to uniquely identify an OPP.
To support these scenarios, introduce a new API
dev_pm_opp_find_key_exact() that allows OPP lookup with different
set of keys like freq, level & bandwidth.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/opp/core.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_opp.h | 30 ++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index edbd60501cf00dfd1957f7d19b228d1c61bbbdcc..a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -476,6 +476,15 @@ static unsigned long _read_bw(struct dev_pm_opp *opp, int index)
return opp->bandwidth[index].peak;
}
+static unsigned long _read_opp_key(struct dev_pm_opp *opp, int index, struct dev_pm_opp_key *key)
+{
+ key->bw = opp->bandwidth ? opp->bandwidth[index].peak : 0;
+ key->freq = opp->rates[index];
+ key->level = opp->level;
+
+ return true;
+}
+
/* Generic comparison helpers */
static bool _compare_exact(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
unsigned long opp_key, unsigned long key)
@@ -509,6 +518,21 @@ static bool _compare_floor(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
return false;
}
+static bool _compare_opp_key_exact(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
+ struct dev_pm_opp_key opp_key, struct dev_pm_opp_key key)
+{
+ bool level_match = (key.level == OPP_LEVEL_UNSET || opp_key.level == key.level);
+ bool freq_match = (key.freq == 0 || opp_key.freq == key.freq);
+ bool bw_match = (key.bw == 0 || opp_key.bw == key.bw);
+
+ if (freq_match && level_match && bw_match) {
+ *opp = temp_opp;
+ return true;
+ }
+
+ return false;
+}
+
/* Generic key finding helpers */
static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
unsigned long *key, int index, bool available,
@@ -541,6 +565,38 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
return opp;
}
+static struct dev_pm_opp *_opp_table_find_opp_key(struct opp_table *opp_table,
+ struct dev_pm_opp_key *key, bool available,
+ unsigned long (*read)(struct dev_pm_opp *opp, int index,
+ struct dev_pm_opp_key *key),
+ bool (*compare)(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
+ struct dev_pm_opp_key opp_key, struct dev_pm_opp_key key),
+ bool (*assert)(struct opp_table *opp_table, unsigned int index))
+{
+ struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
+ struct dev_pm_opp_key temp_key;
+
+ /* Assert that the requirement is met */
+ if (!assert(opp_table, 0))
+ return ERR_PTR(-EINVAL);
+
+ guard(mutex)(&opp_table->lock);
+
+ list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
+ if (temp_opp->available == available) {
+ read(temp_opp, 0, &temp_key);
+ if (compare(&opp, temp_opp, temp_key, *key)) {
+ /* Increment the reference count of OPP */
+ *key = temp_key;
+ dev_pm_opp_get(opp);
+ break;
+ }
+ }
+ }
+
+ return opp;
+}
+
static struct dev_pm_opp *
_find_key(struct device *dev, unsigned long *key, int index, bool available,
unsigned long (*read)(struct dev_pm_opp *opp, int index),
@@ -632,6 +688,47 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
}
EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact);
+/**
+ * dev_pm_opp_find_key_exact() - Search for an exact OPP key
+ * @dev: Device for which the OPP is being searched
+ * @key: OPP key to match
+ * @available: true/false - match for available OPP
+ *
+ * Search for an exact match the OPP key in the OPP table.
+ *
+ * Return: matching *opp, else returns ERR_PTR in case of error and should
+ * be using IS_ERR. Error return values can be:
+ * EINVAL: for bad pointer
+ * ERANGE: no match found for search
+ * ENODEV: if device not found in list of registered devices
+ *
+ * Note: 'available' is a modifier for the search. If 'available'=true,
+ * then the match is for exact matching key and is available in the stored
+ * OPP table. If false, the match is for exact key which is not available.
+ *
+ * This provides a mechanism to enable an OPP which is not available currently
+ * or the opposite as well.
+ *
+ * The callers are required to call dev_pm_opp_put() for the returned OPP after
+ * use.
+ */
+struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
+ struct dev_pm_opp_key key,
+ bool available)
+{
+ struct opp_table *opp_table __free(put_opp_table) = _find_opp_table(dev);
+
+ if (IS_ERR(opp_table)) {
+ dev_err(dev, "%s: OPP table not found (%ld)\n", __func__,
+ PTR_ERR(opp_table));
+ return ERR_CAST(opp_table);
+ }
+
+ return _opp_table_find_opp_key(opp_table, &key, available, _read_opp_key,
+ _compare_opp_key_exact, assert_single_clk);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_find_key_exact);
+
/**
* dev_pm_opp_find_freq_exact_indexed() - Search for an exact freq for the
* clock corresponding to the index
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index cf477beae4bbede88223566df5f43d85adc5a816..5d244bf974891d55b5e21ca9459c147c7223b9b0 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -98,6 +98,25 @@ struct dev_pm_opp_data {
unsigned long u_volt;
};
+/**
+ * struct dev_pm_opp_key - Key used to identify OPP entries
+ * @freq: Frequency in Hz. Use 0 if frequency is not to be matched.
+ * @level: Performance level associated with the OPP entry.
+ * Use OPP_LEVEL_UNSET if level is not to be matched.
+ * @bw: Bandwidth associated with the OPP entry.
+ * Use 0 if bandwidth is not to be matched.
+ *
+ * This structure is used to uniquely identify an OPP entry based on
+ * frequency, performance level, and bandwidth. Each field can be
+ * selectively ignored during matching by setting it to its respective
+ * NOP value.
+ */
+struct dev_pm_opp_key {
+ unsigned long freq;
+ unsigned int level;
+ u32 bw;
+};
+
#if defined(CONFIG_PM_OPP)
struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
@@ -131,6 +150,10 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
unsigned long freq,
bool available);
+struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
+ struct dev_pm_opp_key key,
+ bool available);
+
struct dev_pm_opp *
dev_pm_opp_find_freq_exact_indexed(struct device *dev, unsigned long freq,
u32 index, bool available);
@@ -289,6 +312,13 @@ static inline struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
return ERR_PTR(-EOPNOTSUPP);
}
+static inline struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
+ struct dev_pm_opp_key key,
+ bool available)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
static inline struct dev_pm_opp *
dev_pm_opp_find_freq_exact_indexed(struct device *dev, unsigned long freq,
u32 index, bool available)
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 1/7] " Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-22 6:51 ` Viresh Kumar
[not found] ` <CGME20250825135939eucas1p206b6e2b5ba115f51618c773a1f37939c@eucas1p2.samsung.com>
2025-08-20 8:28 ` [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates Krishna Chaitanya Chundru
` (5 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
Refactor _opp_table_find_key() to improve readability by moving the
reference count increment and key update inside the match condition block.
Also make the 'assert' check mandatory instead of treating it as optional.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/opp/core.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153..bf49709b8c39271431772924daf0c003b45eec7f 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -544,24 +544,22 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
/* Assert that the requirement is met */
- if (assert && !assert(opp_table, index))
+ if (!assert(opp_table, index))
return ERR_PTR(-EINVAL);
guard(mutex)(&opp_table->lock);
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
- if (compare(&opp, temp_opp, read(temp_opp, index), *key))
+ if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
+ /* Increment the reference count of OPP */
+ *key = read(opp, index);
+ dev_pm_opp_get(opp);
break;
+ }
}
}
- /* Increment the reference count of OPP */
- if (!IS_ERR(opp)) {
- *key = read(opp, index);
- dev_pm_opp_get(opp);
- }
-
return opp;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 1/7] " Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key() Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-26 5:57 ` Manivannan Sadhasivam
2025-08-26 6:08 ` Manivannan Sadhasivam
2025-08-20 8:28 ` [PATCH v4 4/7] arm64: dts: qcom: sm8550: " Krishna Chaitanya Chundru
` (4 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
Add opp-level to indicate PCIe data rates and also define OPP enteries
for each link width and data rate. Append the opp level to name of the
opp node to indicate both frequency and level.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 41 +++++++++++++++++++++++++++++-------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 33574ad706b915136546c7f92c7cd0b8a0d62b7e..d7f8706ca4949e253a4102474c92b393a345262f 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -2052,6 +2052,7 @@ opp-2500000 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
};
/* GEN 2 x1 */
@@ -2059,6 +2060,7 @@ opp-5000000 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
@@ -2066,6 +2068,7 @@ opp-8000000 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
};
};
@@ -2210,45 +2213,67 @@ pcie1_opp_table: opp-table {
compatible = "operating-points-v2";
/* GEN 1 x1 */
- opp-2500000 {
+ opp-2500000-1 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x2 and GEN 2 x1 */
- opp-5000000 {
+ /* GEN 1 x2 */
+ opp-5000000-1 {
+ opp-hz = /bits/ 64 <5000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <500000 1>;
+ opp-level = <1>;
+ };
+
+ /* GEN 2 x1 */
+ opp-5000000-2 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
};
/* GEN 2 x2 */
- opp-10000000 {
+ opp-10000000-2 {
opp-hz = /bits/ 64 <10000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <1000000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
- opp-8000000 {
+ opp-8000000-3 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
+ };
+
+ /* GEN 3 x2 */
+ opp-16000000-3 {
+ opp-hz = /bits/ 64 <16000000>;
+ required-opps = <&rpmhpd_opp_nom>;
+ opp-peak-kBps = <1969000 1>;
+ opp-level = <3>;
};
- /* GEN 3 x2 and GEN 4 x1 */
- opp-16000000 {
+ /* GEN 4 x1 */
+ opp-16000000-4 {
opp-hz = /bits/ 64 <16000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <1969000 1>;
+ opp-level = <4>;
};
/* GEN 4 x2 */
- opp-32000000 {
+ opp-32000000-4 {
opp-hz = /bits/ 64 <32000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <3938000 1>;
+ opp-level = <4>;
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 4/7] arm64: dts: qcom: sm8550: Add opp-level to indicate PCIe data rates
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
` (2 preceding siblings ...)
2025-08-20 8:28 ` [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 5/7] arm64: dts: qcom: sm8650: " Krishna Chaitanya Chundru
` (3 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
Add opp-level to indicate PCIe data rates and also define OPP enteries
for each link width and data rate. Append the opp level to name of the
opp node to indicate both frequency and level.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 63 ++++++++++++++++++++++++++++--------
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 45713d46f3c52487d2638b7ab194c111f58679ce..c51489b5cd8a5e73797f9a4381bc7588259a8bf5 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2028,38 +2028,51 @@ pcie0_opp_table: opp-table {
compatible = "operating-points-v2";
/* GEN 1 x1 */
- opp-2500000 {
+ opp-2500000-1 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x2 and GEN 2 x1 */
- opp-5000000 {
+ /* GEN 1 x2 */
+ opp-5000000-1 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <1>;
+ };
+
+ /* GEN 2 x1 */
+ opp-5000000-2 {
+ opp-hz = /bits/ 64 <5000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
};
/* GEN 2 x2 */
- opp-10000000 {
+ opp-10000000-2 {
opp-hz = /bits/ 64 <10000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <1000000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
- opp-8000000 {
+ opp-8000000-3 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
};
/* GEN 3 x2 */
- opp-16000000 {
+ opp-16000000-3 {
opp-hz = /bits/ 64 <16000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <1969000 1>;
+ opp-level = <3>;
};
};
@@ -2195,45 +2208,67 @@ pcie1_opp_table: opp-table {
compatible = "operating-points-v2";
/* GEN 1 x1 */
- opp-2500000 {
+ opp-2500000-1 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
+ };
+
+ /* GEN 1 x2 */
+ opp-5000000-1 {
+ opp-hz = /bits/ 64 <5000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <500000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x2 and GEN 2 x1 */
- opp-5000000 {
+ /* GEN 2 x1 */
+ opp-5000000-2 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
};
/* GEN 2 x2 */
- opp-10000000 {
+ opp-10000000-2 {
opp-hz = /bits/ 64 <10000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <1000000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
- opp-8000000 {
+ opp-8000000-3 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
+ };
+
+ /* GEN 3 x2 */
+ opp-16000000-3 {
+ opp-hz = /bits/ 64 <16000000>;
+ required-opps = <&rpmhpd_opp_nom>;
+ opp-peak-kBps = <1969000 1>;
+ opp-level = <3>;
};
- /* GEN 3 x2 and GEN 4 x1 */
- opp-16000000 {
+ /* GEN 4 x1 */
+ opp-16000000-4 {
opp-hz = /bits/ 64 <16000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <1969000 1>;
+ opp-level = <4>;
};
/* GEN 4 x2 */
- opp-32000000 {
+ opp-32000000-4 {
opp-hz = /bits/ 64 <32000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <3938000 1>;
+ opp-level = <4>;
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 5/7] arm64: dts: qcom: sm8650: Add opp-level to indicate PCIe data rates
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
` (3 preceding siblings ...)
2025-08-20 8:28 ` [PATCH v4 4/7] arm64: dts: qcom: sm8550: " Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 6/7] arm64: dts: qcom: x1e80100: " Krishna Chaitanya Chundru
` (2 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
Add opp-level to indicate PCIe data rates and also define OPP enteries
for each link width and data rate. Append the opp level to name of the
opp node to indicate both frequency and level.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8650.dtsi | 63 ++++++++++++++++++++++++++++--------
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index e14d3d778b71bbbd0c8fcc851eebc9df9ac09c31..fc05f5eee870ca67cbafab5d989e5f861d96add7 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -3660,38 +3660,51 @@ pcie0_opp_table: opp-table {
compatible = "operating-points-v2";
/* GEN 1 x1 */
- opp-2500000 {
+ opp-2500000-1 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x2 and GEN 2 x1 */
- opp-5000000 {
+ /* GEN 1 x2 */
+ opp-5000000-1 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <1>;
+ };
+
+ /* GEN 2 x1 */
+ opp-5000000-2 {
+ opp-hz = /bits/ 64 <5000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
};
/* GEN 2 x2 */
- opp-10000000 {
+ opp-10000000-2 {
opp-hz = /bits/ 64 <10000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <1000000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
- opp-8000000 {
+ opp-8000000-3 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
};
/* GEN 3 x2 */
- opp-16000000 {
+ opp-16000000-3 {
opp-hz = /bits/ 64 <16000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <1969000 1>;
+ opp-level = <3>;
};
};
@@ -3840,45 +3853,67 @@ pcie1_opp_table: opp-table {
compatible = "operating-points-v2";
/* GEN 1 x1 */
- opp-2500000 {
+ opp-2500000-1 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
+ };
+
+ /* GEN 1 x2 */
+ opp-5000000-1 {
+ opp-hz = /bits/ 64 <5000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <500000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x2 and GEN 2 x1 */
- opp-5000000 {
+ /* GEN 2 x1 */
+ opp-5000000-2 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
};
/* GEN 2 x2 */
- opp-10000000 {
+ opp-10000000-2 {
opp-hz = /bits/ 64 <10000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <1000000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
- opp-8000000 {
+ opp-8000000-3 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
+ };
+
+ /* GEN 3 x2 */
+ opp-16000000-3 {
+ opp-hz = /bits/ 64 <16000000>;
+ required-opps = <&rpmhpd_opp_nom>;
+ opp-peak-kBps = <1969000 1>;
+ opp-level = <3>;
};
- /* GEN 3 x2 and GEN 4 x1 */
- opp-16000000 {
+ /* GEN 4 x1 */
+ opp-16000000-4 {
opp-hz = /bits/ 64 <16000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <1969000 1>;
+ opp-level = <4>;
};
/* GEN 4 x2 */
- opp-32000000 {
+ opp-32000000-4 {
opp-hz = /bits/ 64 <32000000>;
required-opps = <&rpmhpd_opp_nom>;
opp-peak-kBps = <3938000 1>;
+ opp-level = <4>;
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 6/7] arm64: dts: qcom: x1e80100: Add opp-level to indicate PCIe data rates
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
` (4 preceding siblings ...)
2025-08-20 8:28 ` [PATCH v4 5/7] arm64: dts: qcom: sm8650: " Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup Krishna Chaitanya Chundru
2025-08-25 16:44 ` [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Wasim Nazir
7 siblings, 0 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
Add opp-level to indicate PCIe data rates and also define OPP enteries
for each link width and data rate. Append the opp level to name of the
opp node to indicate both frequency and level.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 90 ++++++++++++++++++++++++++++------
1 file changed, 74 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index a9a7bb676c6f8ac48a2e443d28efdc8c9b5e52c0..6644017132bdd7677dcb4fccf90b1e5b36326647 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -3237,73 +3237,131 @@ pcie3_opp_table: opp-table {
compatible = "operating-points-v2";
/* GEN 1 x1 */
- opp-2500000 {
+ opp-2500000-1 {
opp-hz = /bits/ 64 <2500000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <250000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x2 and GEN 2 x1 */
- opp-5000000 {
+ /* GEN 1 x2 */
+ opp-5000000-1 {
opp-hz = /bits/ 64 <5000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <500000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x4 and GEN 2 x2 */
- opp-10000000 {
+ /* GEN 1 x4 */
+ opp-10000000-1 {
opp-hz = /bits/ 64 <10000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <1000000 1>;
+ opp-level = <1>;
};
- /* GEN 1 x8 and GEN 2 x4 */
- opp-20000000 {
+ /* GEN 1 x8 */
+ opp-20000000-1 {
opp-hz = /bits/ 64 <20000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <2000000 1>;
+ opp-level = <1>;
+ };
+
+ /* GEN 2 x1 */
+ opp-5000000-2 {
+ opp-hz = /bits/ 64 <5000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <500000 1>;
+ opp-level = <2>;
+ };
+
+ /* GEN 2 x2 */
+ opp-10000000-2 {
+ opp-hz = /bits/ 64 <10000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <1000000 1>;
+ opp-level = <2>;
+ };
+
+ /* GEN 2 x4 */
+ opp-20000000-2 {
+ opp-hz = /bits/ 64 <20000000>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ opp-peak-kBps = <2000000 1>;
+ opp-level = <2>;
};
/* GEN 2 x8 */
- opp-40000000 {
+ opp-40000000-2 {
opp-hz = /bits/ 64 <40000000>;
required-opps = <&rpmhpd_opp_low_svs>;
opp-peak-kBps = <4000000 1>;
+ opp-level = <2>;
};
/* GEN 3 x1 */
- opp-8000000 {
+ opp-8000000-3 {
opp-hz = /bits/ 64 <8000000>;
required-opps = <&rpmhpd_opp_svs>;
opp-peak-kBps = <984500 1>;
+ opp-level = <3>;
+ };
+
+ /* GEN 3 x2 */
+ opp-16000000-3 {
+ opp-hz = /bits/ 64 <16000000>;
+ required-opps = <&rpmhpd_opp_svs>;
+ opp-peak-kBps = <1969000 1>;
+ opp-level = <3>;
+ };
+
+ /* GEN 3 x4 */
+ opp-32000000-3 {
+ opp-hz = /bits/ 64 <32000000>;
+ required-opps = <&rpmhpd_opp_svs>;
+ opp-peak-kBps = <3938000 1>;
+ opp-level = <3>;
+ };
+
+ /* GEN 3 x8 */
+ opp-64000000-3 {
+ opp-hz = /bits/ 64 <64000000>;
+ required-opps = <&rpmhpd_opp_svs>;
+ opp-peak-kBps = <7876000 1>;
+ opp-level = <3>;
};
- /* GEN 3 x2 and GEN 4 x1 */
- opp-16000000 {
+ /* GEN 4 x1 */
+ opp-16000000-4 {
opp-hz = /bits/ 64 <16000000>;
required-opps = <&rpmhpd_opp_svs>;
opp-peak-kBps = <1969000 1>;
+ opp-level = <4>;
};
- /* GEN 3 x4 and GEN 4 x2 */
- opp-32000000 {
+ /* GEN 4 x2 */
+ opp-32000000-4 {
opp-hz = /bits/ 64 <32000000>;
required-opps = <&rpmhpd_opp_svs>;
opp-peak-kBps = <3938000 1>;
+ opp-level = <4>;
};
- /* GEN 3 x8 and GEN 4 x4 */
- opp-64000000 {
+ /* GEN 4 x4 */
+ opp-64000000-4 {
opp-hz = /bits/ 64 <64000000>;
required-opps = <&rpmhpd_opp_svs>;
opp-peak-kBps = <7876000 1>;
+ opp-level = <4>;
};
/* GEN 4 x8 */
- opp-128000000 {
+ opp-128000000-4 {
opp-hz = /bits/ 64 <128000000>;
required-opps = <&rpmhpd_opp_svs>;
opp-peak-kBps = <15753000 1>;
+ opp-level = <4>;
};
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
` (5 preceding siblings ...)
2025-08-20 8:28 ` [PATCH v4 6/7] arm64: dts: qcom: x1e80100: " Krishna Chaitanya Chundru
@ 2025-08-20 8:28 ` Krishna Chaitanya Chundru
2025-08-20 8:55 ` Neil Armstrong
2025-08-26 5:54 ` Manivannan Sadhasivam
2025-08-25 16:44 ` [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Wasim Nazir
7 siblings, 2 replies; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-20 8:28 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree,
Krishna Chaitanya Chundru
PCIe supports multiple data rates that may operate at the same clock
frequency by varying the link width. In such cases, frequency alone
is insufficient to identify the correct OPP. Use the newly introduced
dev_pm_opp_find_key_exact() API to match both frequency and
level when selecting an OPP, here level indicates PCIe data rate.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..4f40fc7b828483419b87057c53e2f754811bdda0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1555,6 +1555,7 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
{
u32 offset, status, width, speed;
struct dw_pcie *pci = pcie->pci;
+ struct dev_pm_opp_key key;
unsigned long freq_kbps;
struct dev_pm_opp *opp;
int ret, freq_mbps;
@@ -1582,8 +1583,10 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
return;
freq_kbps = freq_mbps * KILO;
- opp = dev_pm_opp_find_freq_exact(pci->dev, freq_kbps * width,
- true);
+ key.freq = freq_kbps * width;
+ key.level = speed;
+ key.bw = 0;
+ opp = dev_pm_opp_find_key_exact(pci->dev, key, true);
if (!IS_ERR(opp)) {
ret = dev_pm_opp_set_opp(pci->dev, opp);
if (ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup
2025-08-20 8:28 ` [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup Krishna Chaitanya Chundru
@ 2025-08-20 8:55 ` Neil Armstrong
2025-08-26 5:54 ` Manivannan Sadhasivam
1 sibling, 0 replies; 25+ messages in thread
From: Neil Armstrong @ 2025-08-20 8:55 UTC (permalink / raw)
To: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree
On 20/08/2025 10:28, Krishna Chaitanya Chundru wrote:
> PCIe supports multiple data rates that may operate at the same clock
> frequency by varying the link width. In such cases, frequency alone
> is insufficient to identify the correct OPP. Use the newly introduced
> dev_pm_opp_find_key_exact() API to match both frequency and
> level when selecting an OPP, here level indicates PCIe data rate.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..4f40fc7b828483419b87057c53e2f754811bdda0 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1555,6 +1555,7 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
> {
> u32 offset, status, width, speed;
> struct dw_pcie *pci = pcie->pci;
> + struct dev_pm_opp_key key;
> unsigned long freq_kbps;
> struct dev_pm_opp *opp;
> int ret, freq_mbps;
> @@ -1582,8 +1583,10 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
> return;
>
> freq_kbps = freq_mbps * KILO;
> - opp = dev_pm_opp_find_freq_exact(pci->dev, freq_kbps * width,
> - true);
> + key.freq = freq_kbps * width;
> + key.level = speed;
> + key.bw = 0;
> + opp = dev_pm_opp_find_key_exact(pci->dev, key, true);
> if (!IS_ERR(opp)) {
> ret = dev_pm_opp_set_opp(pci->dev, opp);
> if (ret)
>
Fine but you should still support DTs without the opp-level property as fallback,
since stable kernels has the opp tables without level property (v6.12+ for 8450/x1e, v6.16 for 8550/8650)
Neil
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 1/7] OPP: Add support to find OPP for a set of keys
2025-08-20 8:28 ` [PATCH v4 1/7] " Krishna Chaitanya Chundru
@ 2025-08-22 6:50 ` Viresh Kumar
0 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2025-08-22 6:50 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley,
linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree
On 20-08-25, 13:58, Krishna Chaitanya Chundru wrote:
> Some clients, such as PCIe, may operate at the same clock frequency
> across different data rates by varying link width. In such cases,
> frequency alone is not sufficient to uniquely identify an OPP.
> To support these scenarios, introduce a new API
> dev_pm_opp_find_key_exact() that allows OPP lookup with different
> set of keys like freq, level & bandwidth.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/opp/core.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pm_opp.h | 30 ++++++++++++++++
> 2 files changed, 127 insertions(+)
Applied with this diff:
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index a36c3daac39c..bba4f7daff8c 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -476,7 +476,8 @@ static unsigned long _read_bw(struct dev_pm_opp *opp, int index)
return opp->bandwidth[index].peak;
}
-static unsigned long _read_opp_key(struct dev_pm_opp *opp, int index, struct dev_pm_opp_key *key)
+static unsigned long _read_opp_key(struct dev_pm_opp *opp, int index,
+ struct dev_pm_opp_key *key)
{
key->bw = opp->bandwidth ? opp->bandwidth[index].peak : 0;
key->freq = opp->rates[index];
@@ -518,12 +519,13 @@ static bool _compare_floor(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
return false;
}
-static bool _compare_opp_key_exact(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
- struct dev_pm_opp_key opp_key, struct dev_pm_opp_key key)
+static bool _compare_opp_key_exact(struct dev_pm_opp **opp,
+ struct dev_pm_opp *temp_opp, struct dev_pm_opp_key *opp_key,
+ struct dev_pm_opp_key *key)
{
- bool level_match = (key.level == OPP_LEVEL_UNSET || opp_key.level == key.level);
- bool freq_match = (key.freq == 0 || opp_key.freq == key.freq);
- bool bw_match = (key.bw == 0 || opp_key.bw == key.bw);
+ bool level_match = (key->level == OPP_LEVEL_UNSET || opp_key->level == key->level);
+ bool freq_match = (key->freq == 0 || opp_key->freq == key->freq);
+ bool bw_match = (key->bw == 0 || opp_key->bw == key->bw);
if (freq_match && level_match && bw_match) {
*opp = temp_opp;
@@ -570,7 +572,7 @@ static struct dev_pm_opp *_opp_table_find_opp_key(struct opp_table *opp_table,
unsigned long (*read)(struct dev_pm_opp *opp, int index,
struct dev_pm_opp_key *key),
bool (*compare)(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
- struct dev_pm_opp_key opp_key, struct dev_pm_opp_key key),
+ struct dev_pm_opp_key *opp_key, struct dev_pm_opp_key *key),
bool (*assert)(struct opp_table *opp_table, unsigned int index))
{
struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
@@ -585,9 +587,8 @@ static struct dev_pm_opp *_opp_table_find_opp_key(struct opp_table *opp_table,
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
read(temp_opp, 0, &temp_key);
- if (compare(&opp, temp_opp, temp_key, *key)) {
+ if (compare(&opp, temp_opp, &temp_key, key)) {
/* Increment the reference count of OPP */
- *key = temp_key;
dev_pm_opp_get(opp);
break;
}
@@ -689,20 +690,20 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact);
/**
- * dev_pm_opp_find_key_exact() - Search for an exact OPP key
- * @dev: Device for which the OPP is being searched
- * @key: OPP key to match
- * @available: true/false - match for available OPP
+ * dev_pm_opp_find_key_exact() - Search for an OPP with exact key set
+ * @dev: Device for which the OPP is being searched
+ * @key: OPP key set to match
+ * @available: true/false - match for available OPP
*
- * Search for an exact match the OPP key in the OPP table.
+ * Search for an exact match of the key set in the OPP table.
*
- * Return: matching *opp, else returns ERR_PTR in case of error and should
- * be using IS_ERR. Error return values can be:
- * EINVAL: for bad pointer
- * ERANGE: no match found for search
- * ENODEV: if device not found in list of registered devices
+ * Return: A matching opp on success, else ERR_PTR in case of error.
+ * Possible error values:
+ * EINVAL: for bad pointers
+ * ERANGE: no match found for search
+ * ENODEV: if device not found in list of registered devices
*
- * Note: 'available' is a modifier for the search. If 'available'=true,
+ * Note: 'available' is a modifier for the search. If 'available' == true,
* then the match is for exact matching key and is available in the stored
* OPP table. If false, the match is for exact key which is not available.
*
@@ -713,7 +714,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact);
* use.
*/
struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
- struct dev_pm_opp_key key,
+ struct dev_pm_opp_key *key,
bool available)
{
struct opp_table *opp_table __free(put_opp_table) = _find_opp_table(dev);
@@ -724,8 +725,9 @@ struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
return ERR_CAST(opp_table);
}
- return _opp_table_find_opp_key(opp_table, &key, available, _read_opp_key,
- _compare_opp_key_exact, assert_single_clk);
+ return _opp_table_find_opp_key(opp_table, key, available,
+ _read_opp_key, _compare_opp_key_exact,
+ assert_single_clk);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_find_key_exact);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5d244bf97489..789406d95e69 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -151,7 +151,7 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
bool available);
struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
- struct dev_pm_opp_key key,
+ struct dev_pm_opp_key *key,
bool available);
struct dev_pm_opp *
@@ -313,7 +313,7 @@ static inline struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
}
static inline struct dev_pm_opp *dev_pm_opp_find_key_exact(struct device *dev,
- struct dev_pm_opp_key key,
+ struct dev_pm_opp_key *key,
bool available)
{
return ERR_PTR(-EOPNOTSUPP);
--
viresh
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-20 8:28 ` [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key() Krishna Chaitanya Chundru
@ 2025-08-22 6:51 ` Viresh Kumar
[not found] ` <CGME20250825135939eucas1p206b6e2b5ba115f51618c773a1f37939c@eucas1p2.samsung.com>
1 sibling, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2025-08-22 6:51 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley,
linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree
On 20-08-25, 13:58, Krishna Chaitanya Chundru wrote:
> Refactor _opp_table_find_key() to improve readability by moving the
> reference count increment and key update inside the match condition block.
>
> Also make the 'assert' check mandatory instead of treating it as optional.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/opp/core.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
Applied with:
@@ -554,8 +554,9 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
- /* Increment the reference count of OPP */
*key = read(opp, index);
+
+ /* Increment the reference count of OPP */
dev_pm_opp_get(opp);
break;
}
--
viresh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
[not found] ` <CGME20250825135939eucas1p206b6e2b5ba115f51618c773a1f37939c@eucas1p2.samsung.com>
@ 2025-08-25 13:59 ` Marek Szyprowski
2025-08-25 15:56 ` Krishna Chaitanya Chundru
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Marek Szyprowski @ 2025-08-25 13:59 UTC (permalink / raw)
To: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree
On 20.08.2025 10:28, Krishna Chaitanya Chundru wrote:
> Refactor _opp_table_find_key() to improve readability by moving the
> reference count increment and key update inside the match condition block.
>
> Also make the 'assert' check mandatory instead of treating it as optional.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
This patch landed in today's linux-next (20250825) as commit
b5323835f050 ("OPP: Reorganize _opp_table_find_key()"). In my tests I
found that it causes regressions on my test boards. Reverting this
change on top of linux-next fixes booting of all the affected boards.
Here are kernel logs with lockdep enabled:
1. Exynos4412-based Odroid-U3 board (ARM 32bit):
============================================
WARNING: possible recursive locking detected
6.17.0-rc3-next-20250825 #10901 Not tainted
--------------------------------------------
kworker/u16:0/12 is trying to acquire lock:
cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_notifier_call+0x30/0x124
but task is already holding lock:
cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&devfreq->lock);
lock(&devfreq->lock);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by kworker/u16:0/12:
#0: c289d0b4 ((wq_completion)devfreq_wq){+.+.}-{0:0}, at:
process_one_work+0x1b0/0x70c
#1: f0899f18
((work_completion)(&(&devfreq->work)->work)#2){+.+.}-{0:0}, at:
process_one_work+0x1dc/0x70c
#2: cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
#3: c2e78c4c (&(&opp_table->head)->rwsem){++++}-{3:3}, at:
blocking_notifier_call_chain+0x28/0x60
stack backtrace:
CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted
6.17.0-rc3-next-20250825 #10901 PREEMPT
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: devfreq_wq devfreq_monitor
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x68/0x88
dump_stack_lvl from print_deadlock_bug+0x370/0x380
print_deadlock_bug from __lock_acquire+0x1428/0x29ec
__lock_acquire from lock_acquire+0x134/0x388
lock_acquire from __mutex_lock+0xac/0x10c0
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from devfreq_notifier_call+0x30/0x124
devfreq_notifier_call from notifier_call_chain+0x84/0x1d4
notifier_call_chain from blocking_notifier_call_chain+0x44/0x60
blocking_notifier_call_chain from _opp_kref_release+0x3c/0x5c
_opp_kref_release from exynos_bus_target+0x24/0x70
exynos_bus_target from devfreq_set_target+0x8c/0x2e8
devfreq_set_target from devfreq_update_target+0x9c/0xf8
devfreq_update_target from devfreq_monitor+0x28/0x1a4
devfreq_monitor from process_one_work+0x24c/0x70c
process_one_work from worker_thread+0x1b8/0x3bc
worker_thread from kthread+0x13c/0x264
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0899fb0 to 0xf0899ff8)
...
2. Exynos5422-based Odroid-XU3 board (ARM 32bit):
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address
00000000 when read
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 7 UID: 0 PID: 68 Comm: kworker/u34:1 Not tainted
6.17.0-rc3-next-20250825 #10901 PREEMPT
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: devfreq_wq devfreq_monitor
PC is at _opp_compare_key+0x30/0xb4
LR is at 0xfffffffc
pc : [<c09831c4>] lr : [<fffffffc>] psr: 20000013
sp : f0a89de0 ip : cfb0e94c fp : c1574880
r10: c14095a4 r9 : f0a89e44 r8 : c2a9c010
r7 : cfb0ea80 r6 : 00000001 r5 : cfb0e900 r4 : 00000001
r3 : 00000000 r2 : cfb0e900 r1 : cfb0ea80 r0 : cfaf5800
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 4000406a DAC: 00000051
Register r0 information: slab kmalloc-1k start cfaf5800 pointer offset 0
size 1024
Register r1 information: slab kmalloc-128 start cfb0ea80 pointer offset
0 size 128
Register r2 information: slab kmalloc-128 start cfb0e900 pointer offset
0 size 128
Register r3 information: NULL pointer
Register r4 information: non-paged memory
Register r5 information: slab kmalloc-128 start cfb0e900 pointer offset
0 size 128
Register r6 information: non-paged memory
Register r7 information: slab kmalloc-128 start cfb0ea80 pointer offset
0 size 128
Register r8 information: slab kmalloc-1k start c2a9c000 pointer offset
16 size 1024
Register r9 information: 2-page vmalloc region starting at 0xf0a88000
allocated at kernel_clone+0x58/0x3c4
Register r10 information: non-slab/vmalloc memory
Register r11 information: non-slab/vmalloc memory
Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
76 size 128
Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
Stack: (0xf0a89de0 to 0xf0a8a000)
..
Call trace:
_opp_compare_key from _set_opp+0x78/0x50c
_set_opp from dev_pm_opp_set_rate+0x15c/0x21c
dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
devfreq_set_target from devfreq_update_target+0x9c/0xf8
devfreq_update_target from devfreq_monitor+0x28/0x1a4
devfreq_monitor from process_one_work+0x24c/0x70c
process_one_work from worker_thread+0x1b8/0x3bc
worker_thread from kthread+0x13c/0x264
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0a89fb0 to 0xf0a89ff8)
...
---[ end trace 0000000000000000 ]---
3. Qualcomm Technologies, Inc. Robotics RB5(ARM 64bit):
ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable to find
vdd-hba-supply regulator, assuming enabled
ufshcd-qcom 1d84000.ufshc: Failed to find OPP for MIN frequency
ufshcd-qcom 1d84000.ufshc: ufshcd_pltfrm_init: OPP parse failed -34
ufshcd-qcom 1d84000.ufshc: error -ERANGE: ufshcd_pltfrm_init() failed
ufshcd-qcom 1d84000.ufshc: probe with driver ufshcd-qcom failed with
error -34
> ---
> drivers/opp/core.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153..bf49709b8c39271431772924daf0c003b45eec7f 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -544,24 +544,22 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
> struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>
> /* Assert that the requirement is met */
> - if (assert && !assert(opp_table, index))
> + if (!assert(opp_table, index))
> return ERR_PTR(-EINVAL);
>
> guard(mutex)(&opp_table->lock);
>
> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
> if (temp_opp->available == available) {
> - if (compare(&opp, temp_opp, read(temp_opp, index), *key))
> + if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
> + /* Increment the reference count of OPP */
> + *key = read(opp, index);
> + dev_pm_opp_get(opp);
> break;
> + }
> }
> }
>
> - /* Increment the reference count of OPP */
> - if (!IS_ERR(opp)) {
> - *key = read(opp, index);
> - dev_pm_opp_get(opp);
> - }
> -
> return opp;
> }
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-25 13:59 ` Marek Szyprowski
@ 2025-08-25 15:56 ` Krishna Chaitanya Chundru
2025-08-26 6:10 ` Viresh Kumar
2025-08-26 6:06 ` Viresh Kumar
2025-08-26 11:25 ` Krzysztof Kozlowski
2 siblings, 1 reply; 25+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-08-25 15:56 UTC (permalink / raw)
To: Marek Szyprowski, Viresh Kumar
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm,
Krzysztof Wilczyński, Conor Dooley, devicetree,
Lorenzo Pieralisi, Nishanth Menon, Stephen Boyd,
Rafael J. Wysocki, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Krzysztof Kozlowski,
Konrad Dybcio
On 8/25/2025 7:29 PM, Marek Szyprowski wrote:
> On 20.08.2025 10:28, Krishna Chaitanya Chundru wrote:
>> Refactor _opp_table_find_key() to improve readability by moving the
>> reference count increment and key update inside the match condition block.
>>
>> Also make the 'assert' check mandatory instead of treating it as optional.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>
> This patch landed in today's linux-next (20250825) as commit
> b5323835f050 ("OPP: Reorganize _opp_table_find_key()"). In my tests I
> found that it causes regressions on my test boards. Reverting this
> change on top of linux-next fixes booting of all the affected boards.
>
> Here are kernel logs with lockdep enabled:
>
> 1. Exynos4412-based Odroid-U3 board (ARM 32bit):
>
> ============================================
> WARNING: possible recursive locking detected
> 6.17.0-rc3-next-20250825 #10901 Not tainted
> --------------------------------------------
> kworker/u16:0/12 is trying to acquire lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_notifier_call+0x30/0x124
>
> but task is already holding lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&devfreq->lock);
> lock(&devfreq->lock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 4 locks held by kworker/u16:0/12:
> #0: c289d0b4 ((wq_completion)devfreq_wq){+.+.}-{0:0}, at:
> process_one_work+0x1b0/0x70c
> #1: f0899f18
> ((work_completion)(&(&devfreq->work)->work)#2){+.+.}-{0:0}, at:
> process_one_work+0x1dc/0x70c
> #2: cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
> #3: c2e78c4c (&(&opp_table->head)->rwsem){++++}-{3:3}, at:
> blocking_notifier_call_chain+0x28/0x60
>
> stack backtrace:
> CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x68/0x88
> dump_stack_lvl from print_deadlock_bug+0x370/0x380
> print_deadlock_bug from __lock_acquire+0x1428/0x29ec
> __lock_acquire from lock_acquire+0x134/0x388
> lock_acquire from __mutex_lock+0xac/0x10c0
> __mutex_lock from mutex_lock_nested+0x1c/0x24
> mutex_lock_nested from devfreq_notifier_call+0x30/0x124
> devfreq_notifier_call from notifier_call_chain+0x84/0x1d4
> notifier_call_chain from blocking_notifier_call_chain+0x44/0x60
> blocking_notifier_call_chain from _opp_kref_release+0x3c/0x5c
> _opp_kref_release from exynos_bus_target+0x24/0x70
> exynos_bus_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0899fb0 to 0xf0899ff8)
>
> ...
>
>
> 2. Exynos5422-based Odroid-XU3 board (ARM 32bit):
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 when read
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 7 UID: 0 PID: 68 Comm: kworker/u34:1 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> PC is at _opp_compare_key+0x30/0xb4
> LR is at 0xfffffffc
> pc : [<c09831c4>] lr : [<fffffffc>] psr: 20000013
> sp : f0a89de0 ip : cfb0e94c fp : c1574880
> r10: c14095a4 r9 : f0a89e44 r8 : c2a9c010
> r7 : cfb0ea80 r6 : 00000001 r5 : cfb0e900 r4 : 00000001
> r3 : 00000000 r2 : cfb0e900 r1 : cfb0ea80 r0 : cfaf5800
> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 4000406a DAC: 00000051
> Register r0 information: slab kmalloc-1k start cfaf5800 pointer offset 0
> size 1024
> Register r1 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r2 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r3 information: NULL pointer
> Register r4 information: non-paged memory
> Register r5 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r6 information: non-paged memory
> Register r7 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r8 information: slab kmalloc-1k start c2a9c000 pointer offset
> 16 size 1024
> Register r9 information: 2-page vmalloc region starting at 0xf0a88000
> allocated at kernel_clone+0x58/0x3c4
> Register r10 information: non-slab/vmalloc memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
> 76 size 128
> Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
> Stack: (0xf0a89de0 to 0xf0a8a000)
> ..
> Call trace:
> _opp_compare_key from _set_opp+0x78/0x50c
> _set_opp from dev_pm_opp_set_rate+0x15c/0x21c
> dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
> panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0a89fb0 to 0xf0a89ff8)
> ...
> ---[ end trace 0000000000000000 ]---
>
>
> 3. Qualcomm Technologies, Inc. Robotics RB5(ARM 64bit):
>
> ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
> ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable to find
> vdd-hba-supply regulator, assuming enabled
> ufshcd-qcom 1d84000.ufshc: Failed to find OPP for MIN frequency
> ufshcd-qcom 1d84000.ufshc: ufshcd_pltfrm_init: OPP parse failed -34
> ufshcd-qcom 1d84000.ufshc: error -ERANGE: ufshcd_pltfrm_init() failed
> ufshcd-qcom 1d84000.ufshc: probe with driver ufshcd-qcom failed with
> error -34
>
>
>
>> ---
>> drivers/opp/core.c | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index a36c3daac39cd0bdd2a1f7e9bad5b92f0c756153..bf49709b8c39271431772924daf0c003b45eec7f 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -544,24 +544,22 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
>> struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
>>
>> /* Assert that the requirement is met */
>> - if (assert && !assert(opp_table, index))
>> + if (!assert(opp_table, index))
>> return ERR_PTR(-EINVAL);
>>
>> guard(mutex)(&opp_table->lock);
>>
>> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
>> if (temp_opp->available == available) {
>> - if (compare(&opp, temp_opp, read(temp_opp, index), *key))
>> + if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
>> + /* Increment the reference count of OPP */
>> + *key = read(opp, index);
>> + dev_pm_opp_get(opp);
>> break;
>> + }
>> }
>> }
>>
>> - /* Increment the reference count of OPP */
>> - if (!IS_ERR(opp)) {
>> - *key = read(opp, index);
>> - dev_pm_opp_get(opp);
>> - }
>> -
>> return opp;
>> }
>>
>>
> Best regards
Thanks Marek for reporting.
Viresh,
looks like for compare_floor we need to iterate to the OPP table till
the OPP key is greater than the target key and return previous OPP.
In that case the updation of the key and dev_pm_opp_get() should be
outside as before. We need to remove this part of the patch.
Thanks & Regards,
Krishna Chaitanya.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
` (6 preceding siblings ...)
2025-08-20 8:28 ` [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup Krishna Chaitanya Chundru
@ 2025-08-25 16:44 ` Wasim Nazir
2025-08-26 5:20 ` Viresh Kumar
7 siblings, 1 reply; 25+ messages in thread
From: Wasim Nazir @ 2025-08-25 16:44 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley,
linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree
On Wed, Aug 20, 2025 at 01:58:46PM +0530, Krishna Chaitanya Chundru wrote:
> The existing OPP table in the device tree for PCIe is shared across
> different link configurations such as data rates 8GT/s x2 and 16GT/s x1.
> These configurations often operate at the same frequency, allowing them
> to reuse the same OPP entries. However, 8GT/s and 16 GT/s may have
> different characteristics beyond frequency—such as RPMh votes in QCOM
> case, which cannot be represented accurately when sharing a single OPP.
>
> In such cases, frequency alone is not sufficient to uniquely identify
> an OPP. To support these scenarios, introduce a new API
> dev_pm_opp_find_key_exact() that allows OPP lookup for set of keys like
> frequency, level & bandwidth.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> Changes in v4:
> - Included dtsi changes for all platforms.
> - Made the changes as requested by Viresh like adding comments, some
> coding styles etc.
> - Link to v3: https://lore.kernel.org/r/20250819-opp_pcie-v3-0-f8bd7e05ce41@oss.qualcomm.com
>
> Changes in v3:
> - Always check for frequency match unless user doesn't pass it (Viresh).
> - Make dev_pm_opp_key public and let user pass the key (Viresh).
> - Include bandwidth as part of dev_pm_opp_key (Viresh).
> - Link to v2: https://lore.kernel.org/r/20250818-opp_pcie-v2-0-071524d98967@oss.qualcomm.com
>
> Changes in v2:
> - Use opp-level to indentify data rate and use both frequency and level
> to identify the OPP. (Viresh)
> - Link to v1: https://lore.kernel.org/r/20250717-opp_pcie-v1-0-dde6f452571b@oss.qualcomm.com
>
> ---
> Krishna Chaitanya Chundru (7):
> OPP: Add support to find OPP for a set of keys
> OPP: Move refcount and key update for readability in _opp_table_find_key()
Hi Krishna,
Patch 2/7 is applied in linux-next (20250825) as commit
b5323835f050 (OPP: Reorganize _opp_table_find_key()) which is causing
regression on my board (lemans-evk (arm64)).
Reverting the change is resolving the issue.
Kernel log:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000016
...
Call trace:
_read_bw+0x0/0x10 (P)
_find_key+0xb8/0x194
dev_pm_opp_find_bw_floor+0x54/0x8c
bwmon_intr_thread+0x84/0x284 [icc_bwmon]
irq_thread_fn+0x2c/0xa8
irq_thread+0x174/0x334
kthread+0x134/0x208
ret_from_fork+0x10/0x20
> arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates
> arm64: dts: qcom: sm8550: Add opp-level to indicate PCIe data rates
> arm64: dts: qcom: sm8650: Add opp-level to indicate PCIe data rates
> arm64: dts: qcom: x1e80100: Add opp-level to indicate PCIe data rates
> PCI: qcom: Use frequency and level based OPP lookup
>
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 41 ++++++++++---
> arch/arm64/boot/dts/qcom/sm8550.dtsi | 63 ++++++++++++++-----
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 63 ++++++++++++++-----
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 90 ++++++++++++++++++++++-----
> drivers/opp/core.c | 107 +++++++++++++++++++++++++++++++--
> drivers/pci/controller/dwc/pcie-qcom.c | 7 ++-
> include/linux/pm_opp.h | 30 +++++++++
> 7 files changed, 341 insertions(+), 60 deletions(-)
> ---
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> change-id: 20250717-opp_pcie-793160b2b113
>
> Best regards,
> --
> Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>
--
Regards,
Wasim
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys
2025-08-25 16:44 ` [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Wasim Nazir
@ 2025-08-26 5:20 ` Viresh Kumar
2025-08-26 5:36 ` Viresh Kumar
0 siblings, 1 reply; 25+ messages in thread
From: Viresh Kumar @ 2025-08-26 5:20 UTC (permalink / raw)
To: Wasim Nazir
Cc: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On 25-08-25, 22:14, Wasim Nazir wrote:
> Patch 2/7 is applied in linux-next (20250825) as commit
> b5323835f050 (OPP: Reorganize _opp_table_find_key()) which is causing
> regression on my board (lemans-evk (arm64)).
> Reverting the change is resolving the issue.
>
> Kernel log:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000016
> ...
> Call trace:
> _read_bw+0x0/0x10 (P)
> _find_key+0xb8/0x194
> dev_pm_opp_find_bw_floor+0x54/0x8c
> bwmon_intr_thread+0x84/0x284 [icc_bwmon]
> irq_thread_fn+0x2c/0xa8
> irq_thread+0x174/0x334
> kthread+0x134/0x208
> ret_from_fork+0x10/0x20
Hmm, this happened because it is possible for the `opp` to be invalid
(error) even if `_compare_floor()` returned true, if the target key is
lower than the lowest freq of the table.
Dropped the patch for now anyway.
--
viresh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys
2025-08-26 5:20 ` Viresh Kumar
@ 2025-08-26 5:36 ` Viresh Kumar
2025-08-26 8:27 ` Viresh Kumar
0 siblings, 1 reply; 25+ messages in thread
From: Viresh Kumar @ 2025-08-26 5:36 UTC (permalink / raw)
To: Wasim Nazir
Cc: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On 26-08-25, 10:50, Viresh Kumar wrote:
> On 25-08-25, 22:14, Wasim Nazir wrote:
> > Kernel log:
> > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000016
> > ...
> > Call trace:
> > _read_bw+0x0/0x10 (P)
> > _find_key+0xb8/0x194
> > dev_pm_opp_find_bw_floor+0x54/0x8c
> > bwmon_intr_thread+0x84/0x284 [icc_bwmon]
> > irq_thread_fn+0x2c/0xa8
> > irq_thread+0x174/0x334
> > kthread+0x134/0x208
> > ret_from_fork+0x10/0x20
>
> Hmm, this happened because it is possible for the `opp` to be invalid
> (error) even if `_compare_floor()` returned true, if the target key is
> lower than the lowest freq of the table.
>
> Dropped the patch for now anyway.
Can you help me testing this over your failing branch please ?
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 81fb7dd7f323..5b24255733b5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -554,10 +554,10 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
- *key = read(opp, index);
-
- /* Increment the reference count of OPP */
- dev_pm_opp_get(opp);
+ if (!IS_ERR(opp)) {
+ *key = read(opp, index);
+ dev_pm_opp_get(opp);
+ }
break;
}
}
--
viresh
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup
2025-08-20 8:28 ` [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup Krishna Chaitanya Chundru
2025-08-20 8:55 ` Neil Armstrong
@ 2025-08-26 5:54 ` Manivannan Sadhasivam
1 sibling, 0 replies; 25+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-26 5:54 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On Wed, Aug 20, 2025 at 01:58:53PM GMT, Krishna Chaitanya Chundru wrote:
> PCIe supports multiple data rates that may operate at the same clock
> frequency by varying the link width. In such cases, frequency alone
> is insufficient to identify the correct OPP.
You need to reword the description. It mostly sounds like you want to select OPP
based on freq and link width instead of freq and data rate due to that fact that
you used 'link width' as the differentiating factor in the first sentence.
>Use the newly introduced
> dev_pm_opp_find_key_exact() API to match both frequency and
> level when selecting an OPP, here level indicates PCIe data rate.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..4f40fc7b828483419b87057c53e2f754811bdda0 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1555,6 +1555,7 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
> {
> u32 offset, status, width, speed;
> struct dw_pcie *pci = pcie->pci;
> + struct dev_pm_opp_key key;
> unsigned long freq_kbps;
> struct dev_pm_opp *opp;
> int ret, freq_mbps;
> @@ -1582,8 +1583,10 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
> return;
>
> freq_kbps = freq_mbps * KILO;
> - opp = dev_pm_opp_find_freq_exact(pci->dev, freq_kbps * width,
> - true);
> + key.freq = freq_kbps * width;
> + key.level = speed;
> + key.bw = 0;
> + opp = dev_pm_opp_find_key_exact(pci->dev, key, true);
As Neil said, this needs to work with older DTs too where there were no 'level'
properties.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates
2025-08-20 8:28 ` [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates Krishna Chaitanya Chundru
@ 2025-08-26 5:57 ` Manivannan Sadhasivam
2025-08-26 6:08 ` Manivannan Sadhasivam
1 sibling, 0 replies; 25+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-26 5:57 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On Wed, Aug 20, 2025 at 01:58:49PM GMT, Krishna Chaitanya Chundru wrote:
> Add opp-level to indicate PCIe data rates and also define OPP enteries
> for each link width and data rate. Append the opp level to name of the
> opp node to indicate both frequency and level.
>
First define the problem statement of why this change is needed. You've
mentioned it in the cover letter, but that won't be preserved in git history.
Then you need to justify the change and make it clear that *this* platform
doesn't suffer the issue but you are doing it for the unification.
This needs to be done for all DTS patches.
- Mani
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 41 +++++++++++++++++++++++++++++-------
> 1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 33574ad706b915136546c7f92c7cd0b8a0d62b7e..d7f8706ca4949e253a4102474c92b393a345262f 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2052,6 +2052,7 @@ opp-2500000 {
> opp-hz = /bits/ 64 <2500000>;
> required-opps = <&rpmhpd_opp_low_svs>;
> opp-peak-kBps = <250000 1>;
> + opp-level = <1>;
> };
>
> /* GEN 2 x1 */
> @@ -2059,6 +2060,7 @@ opp-5000000 {
> opp-hz = /bits/ 64 <5000000>;
> required-opps = <&rpmhpd_opp_low_svs>;
> opp-peak-kBps = <500000 1>;
> + opp-level = <2>;
> };
>
> /* GEN 3 x1 */
> @@ -2066,6 +2068,7 @@ opp-8000000 {
> opp-hz = /bits/ 64 <8000000>;
> required-opps = <&rpmhpd_opp_nom>;
> opp-peak-kBps = <984500 1>;
> + opp-level = <3>;
> };
> };
>
> @@ -2210,45 +2213,67 @@ pcie1_opp_table: opp-table {
> compatible = "operating-points-v2";
>
> /* GEN 1 x1 */
> - opp-2500000 {
> + opp-2500000-1 {
> opp-hz = /bits/ 64 <2500000>;
> required-opps = <&rpmhpd_opp_low_svs>;
> opp-peak-kBps = <250000 1>;
> + opp-level = <1>;
> };
>
> - /* GEN 1 x2 and GEN 2 x1 */
> - opp-5000000 {
> + /* GEN 1 x2 */
> + opp-5000000-1 {
> + opp-hz = /bits/ 64 <5000000>;
> + required-opps = <&rpmhpd_opp_low_svs>;
> + opp-peak-kBps = <500000 1>;
> + opp-level = <1>;
> + };
> +
> + /* GEN 2 x1 */
> + opp-5000000-2 {
> opp-hz = /bits/ 64 <5000000>;
> required-opps = <&rpmhpd_opp_low_svs>;
> opp-peak-kBps = <500000 1>;
> + opp-level = <2>;
> };
>
> /* GEN 2 x2 */
> - opp-10000000 {
> + opp-10000000-2 {
> opp-hz = /bits/ 64 <10000000>;
> required-opps = <&rpmhpd_opp_low_svs>;
> opp-peak-kBps = <1000000 1>;
> + opp-level = <2>;
> };
>
> /* GEN 3 x1 */
> - opp-8000000 {
> + opp-8000000-3 {
> opp-hz = /bits/ 64 <8000000>;
> required-opps = <&rpmhpd_opp_nom>;
> opp-peak-kBps = <984500 1>;
> + opp-level = <3>;
> + };
> +
> + /* GEN 3 x2 */
> + opp-16000000-3 {
> + opp-hz = /bits/ 64 <16000000>;
> + required-opps = <&rpmhpd_opp_nom>;
> + opp-peak-kBps = <1969000 1>;
> + opp-level = <3>;
> };
>
> - /* GEN 3 x2 and GEN 4 x1 */
> - opp-16000000 {
> + /* GEN 4 x1 */
> + opp-16000000-4 {
> opp-hz = /bits/ 64 <16000000>;
> required-opps = <&rpmhpd_opp_nom>;
> opp-peak-kBps = <1969000 1>;
> + opp-level = <4>;
> };
>
> /* GEN 4 x2 */
> - opp-32000000 {
> + opp-32000000-4 {
> opp-hz = /bits/ 64 <32000000>;
> required-opps = <&rpmhpd_opp_nom>;
> opp-peak-kBps = <3938000 1>;
> + opp-level = <4>;
> };
> };
>
>
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-25 13:59 ` Marek Szyprowski
2025-08-25 15:56 ` Krishna Chaitanya Chundru
@ 2025-08-26 6:06 ` Viresh Kumar
2025-08-26 7:26 ` Marek Szyprowski
2025-08-26 11:25 ` Krzysztof Kozlowski
2 siblings, 1 reply; 25+ messages in thread
From: Viresh Kumar @ 2025-08-26 6:06 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
Marek,
Thanks for the detailed logs. I would need a little more help from
you.
Can you give this a try over your failing branch (I have already
dropped the patch from my tree for now):
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 81fb7dd7f323..5b24255733b5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -554,10 +554,10 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
if (temp_opp->available == available) {
if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
- *key = read(opp, index);
-
- /* Increment the reference count of OPP */
- dev_pm_opp_get(opp);
+ if (!IS_ERR(opp)) {
+ *key = read(opp, index);
+ dev_pm_opp_get(opp);
+ }
break;
}
}
On 25-08-25, 15:59, Marek Szyprowski wrote:
> 1. Exynos4412-based Odroid-U3 board (ARM 32bit):
>
> ============================================
> WARNING: possible recursive locking detected
> 6.17.0-rc3-next-20250825 #10901 Not tainted
> --------------------------------------------
> kworker/u16:0/12 is trying to acquire lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_notifier_call+0x30/0x124
>
> but task is already holding lock:
> cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&devfreq->lock);
> lock(&devfreq->lock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 4 locks held by kworker/u16:0/12:
> #0: c289d0b4 ((wq_completion)devfreq_wq){+.+.}-{0:0}, at:
> process_one_work+0x1b0/0x70c
> #1: f0899f18
> ((work_completion)(&(&devfreq->work)->work)#2){+.+.}-{0:0}, at:
> process_one_work+0x1dc/0x70c
> #2: cf896040 (&devfreq->lock){+.+.}-{3:3}, at: devfreq_monitor+0x1c/0x1a4
> #3: c2e78c4c (&(&opp_table->head)->rwsem){++++}-{3:3}, at:
> blocking_notifier_call_chain+0x28/0x60
>
> stack backtrace:
> CPU: 2 UID: 0 PID: 12 Comm: kworker/u16:0 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x68/0x88
> dump_stack_lvl from print_deadlock_bug+0x370/0x380
> print_deadlock_bug from __lock_acquire+0x1428/0x29ec
> __lock_acquire from lock_acquire+0x134/0x388
> lock_acquire from __mutex_lock+0xac/0x10c0
> __mutex_lock from mutex_lock_nested+0x1c/0x24
> mutex_lock_nested from devfreq_notifier_call+0x30/0x124
> devfreq_notifier_call from notifier_call_chain+0x84/0x1d4
> notifier_call_chain from blocking_notifier_call_chain+0x44/0x60
> blocking_notifier_call_chain from _opp_kref_release+0x3c/0x5c
> _opp_kref_release from exynos_bus_target+0x24/0x70
> exynos_bus_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0899fb0 to 0xf0899ff8)
I guess there is some issue with devfreq here which showed up because
we tried to do a dev_pm_opp_get() for an invalid opp (which very well
could have been valid here anyway). This was always done with the OPP
table's lock anyway, nothing changed after the commit AFAICT.
> 2. Exynos5422-based Odroid-XU3 board (ARM 32bit):
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 when read
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 7 UID: 0 PID: 68 Comm: kworker/u34:1 Not tainted
> 6.17.0-rc3-next-20250825 #10901 PREEMPT
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: devfreq_wq devfreq_monitor
> PC is at _opp_compare_key+0x30/0xb4
> LR is at 0xfffffffc
> pc : [<c09831c4>] lr : [<fffffffc>] psr: 20000013
> sp : f0a89de0 ip : cfb0e94c fp : c1574880
> r10: c14095a4 r9 : f0a89e44 r8 : c2a9c010
> r7 : cfb0ea80 r6 : 00000001 r5 : cfb0e900 r4 : 00000001
> r3 : 00000000 r2 : cfb0e900 r1 : cfb0ea80 r0 : cfaf5800
> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 4000406a DAC: 00000051
> Register r0 information: slab kmalloc-1k start cfaf5800 pointer offset 0
> size 1024
> Register r1 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r2 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r3 information: NULL pointer
> Register r4 information: non-paged memory
> Register r5 information: slab kmalloc-128 start cfb0e900 pointer offset
> 0 size 128
> Register r6 information: non-paged memory
> Register r7 information: slab kmalloc-128 start cfb0ea80 pointer offset
> 0 size 128
> Register r8 information: slab kmalloc-1k start c2a9c000 pointer offset
> 16 size 1024
> Register r9 information: 2-page vmalloc region starting at 0xf0a88000
> allocated at kernel_clone+0x58/0x3c4
> Register r10 information: non-slab/vmalloc memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
> 76 size 128
> Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
> Stack: (0xf0a89de0 to 0xf0a8a000)
> ..
> Call trace:
> _opp_compare_key from _set_opp+0x78/0x50c
> _set_opp from dev_pm_opp_set_rate+0x15c/0x21c
> dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
> panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0a89fb0 to 0xf0a89ff8)
I don't fully understand why this happened yet.
> ---[ end trace 0000000000000000 ]---
>
>
> 3. Qualcomm Technologies, Inc. Robotics RB5(ARM 64bit):
>
> ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
> ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable to find
> vdd-hba-supply regulator, assuming enabled
> ufshcd-qcom 1d84000.ufshc: Failed to find OPP for MIN frequency
> ufshcd-qcom 1d84000.ufshc: ufshcd_pltfrm_init: OPP parse failed -34
> ufshcd-qcom 1d84000.ufshc: error -ERANGE: ufshcd_pltfrm_init() failed
> ufshcd-qcom 1d84000.ufshc: probe with driver ufshcd-qcom failed with
> error -34
This too.
--
viresh
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates
2025-08-20 8:28 ` [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates Krishna Chaitanya Chundru
2025-08-26 5:57 ` Manivannan Sadhasivam
@ 2025-08-26 6:08 ` Manivannan Sadhasivam
1 sibling, 0 replies; 25+ messages in thread
From: Manivannan Sadhasivam @ 2025-08-26 6:08 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On Wed, Aug 20, 2025 at 01:58:49PM GMT, Krishna Chaitanya Chundru wrote:
> Add opp-level to indicate PCIe data rates and also define OPP enteries
> for each link width and data rate. Append the opp level to name of the
> opp node to indicate both frequency and level.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 41 +++++++++++++++++++++++++++++-------
> 1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 33574ad706b915136546c7f92c7cd0b8a0d62b7e..d7f8706ca4949e253a4102474c92b393a345262f 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2052,6 +2052,7 @@ opp-2500000 {
> opp-hz = /bits/ 64 <2500000>;
> required-opps = <&rpmhpd_opp_low_svs>;
> opp-peak-kBps = <250000 1>;
> + opp-level = <1>;
> };
>
> /* GEN 2 x1 */
While you are doing the change, please use data rate instead of 'GEN' to align
with the spec. Like,
/* 5 GT/s x1 */
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-25 15:56 ` Krishna Chaitanya Chundru
@ 2025-08-26 6:10 ` Viresh Kumar
0 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2025-08-26 6:10 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Marek Szyprowski, Viresh Kumar, linux-pm, linux-kernel, linux-pci,
linux-arm-msm, Krzysztof Wilczyński, Conor Dooley,
devicetree, Lorenzo Pieralisi, Nishanth Menon, Stephen Boyd,
Rafael J. Wysocki, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Krzysztof Kozlowski,
Konrad Dybcio
On 25-08-25, 21:26, Krishna Chaitanya Chundru wrote:
> looks like for compare_floor we need to iterate to the OPP table till
> the OPP key is greater than the target key and return previous OPP.
It depends on what kind of comparison we need to do. And this this is
how it works for the _floor variants. But we don't always return the
previous OPP. `compare` directly updates the `opp` and so it can be
currently iterated one too.
> In that case the updation of the key and dev_pm_opp_get() should be
> outside as before. We need to remove this part of the patch.
I think all we need to do here is check if an error is there or not,
maybe I am misreading it.
--
viresh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-26 6:06 ` Viresh Kumar
@ 2025-08-26 7:26 ` Marek Szyprowski
2025-08-26 8:24 ` Viresh Kumar
0 siblings, 1 reply; 25+ messages in thread
From: Marek Szyprowski @ 2025-08-26 7:26 UTC (permalink / raw)
To: Viresh Kumar
Cc: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On 26.08.2025 08:06, Viresh Kumar wrote:
> Marek,
>
> Thanks for the detailed logs. I would need a little more help from
> you.
>
> Can you give this a try over your failing branch (I have already
> dropped the patch from my tree for now):
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 81fb7dd7f323..5b24255733b5 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -554,10 +554,10 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
> if (temp_opp->available == available) {
> if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
> - *key = read(opp, index);
> -
> - /* Increment the reference count of OPP */
> - dev_pm_opp_get(opp);
> + if (!IS_ERR(opp)) {
> + *key = read(opp, index);
> + dev_pm_opp_get(opp);
> + }
> break;
> }
> }
This doesn't help. I've stared a bit at that code and did some tests
and it looks that the issue is caused by _opp_table_find_key() returning
the last opp from opp_list without updating the *key and calling
opp_get() for it (happens when compare() returns false).
> ...
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-26 7:26 ` Marek Szyprowski
@ 2025-08-26 8:24 ` Viresh Kumar
0 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2025-08-26 8:24 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On 26-08-25, 09:26, Marek Szyprowski wrote:
> This doesn't help. I've stared a bit at that code and did some tests
> and it looks that the issue is caused by _opp_table_find_key() returning
> the last opp from opp_list without updating the *key and calling
> opp_get() for it (happens when compare() returns false).
Ahh, right. So `compare()` may never return `true` and in that case we
returned the last OPP of the table earlier.
Thanks.
--
viresh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys
2025-08-26 5:36 ` Viresh Kumar
@ 2025-08-26 8:27 ` Viresh Kumar
0 siblings, 0 replies; 25+ messages in thread
From: Viresh Kumar @ 2025-08-26 8:27 UTC (permalink / raw)
To: Wasim Nazir
Cc: Krishna Chaitanya Chundru, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Bjorn Andersson, Konrad Dybcio,
Krzysztof Kozlowski, Conor Dooley, linux-pm, linux-kernel,
linux-pci, linux-arm-msm, devicetree
On 26-08-25, 11:06, Viresh Kumar wrote:
> Can you help me testing this over your failing branch please ?
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 81fb7dd7f323..5b24255733b5 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -554,10 +554,10 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table,
> list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
> if (temp_opp->available == available) {
> if (compare(&opp, temp_opp, read(temp_opp, index), *key)) {
> - *key = read(opp, index);
> -
> - /* Increment the reference count of OPP */
> - dev_pm_opp_get(opp);
> + if (!IS_ERR(opp)) {
> + *key = read(opp, index);
> + dev_pm_opp_get(opp);
> + }
There are other issues too, dropping the patch. Thanks.
--
viresh
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key()
2025-08-25 13:59 ` Marek Szyprowski
2025-08-25 15:56 ` Krishna Chaitanya Chundru
2025-08-26 6:06 ` Viresh Kumar
@ 2025-08-26 11:25 ` Krzysztof Kozlowski
2 siblings, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-26 11:25 UTC (permalink / raw)
To: Marek Szyprowski, Krishna Chaitanya Chundru, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, linux-kernel, linux-pci, linux-arm-msm, devicetree
On 25/08/2025 15:59, Marek Szyprowski wrote:
> Register r10 information: non-slab/vmalloc memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: slab kmalloc-128 start cfb0e900 pointer offset
> 76 size 128
> Process kworker/u34:1 (pid: 68, stack limit = 0x050eb3d7)
> Stack: (0xf0a89de0 to 0xf0a8a000)
> ..
> Call trace:
> _opp_compare_key from _set_opp+0x78/0x50c
> _set_opp from dev_pm_opp_set_rate+0x15c/0x21c
> dev_pm_opp_set_rate from panfrost_devfreq_target+0x2c/0x3c
> panfrost_devfreq_target from devfreq_set_target+0x8c/0x2e8
> devfreq_set_target from devfreq_update_target+0x9c/0xf8
> devfreq_update_target from devfreq_monitor+0x28/0x1a4
> devfreq_monitor from process_one_work+0x24c/0x70c
> process_one_work from worker_thread+0x1b8/0x3bc
> worker_thread from kthread+0x13c/0x264
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0a89fb0 to 0xf0a89ff8)
I also saw this on today's next:
https://krzk.eu/#/builders/21/builds/6690/steps/13/logs/serial0
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2025-08-26 11:25 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 8:28 [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 1/7] " Krishna Chaitanya Chundru
2025-08-22 6:50 ` Viresh Kumar
2025-08-20 8:28 ` [PATCH v4 2/7] OPP: Move refcount and key update for readability in _opp_table_find_key() Krishna Chaitanya Chundru
2025-08-22 6:51 ` Viresh Kumar
[not found] ` <CGME20250825135939eucas1p206b6e2b5ba115f51618c773a1f37939c@eucas1p2.samsung.com>
2025-08-25 13:59 ` Marek Szyprowski
2025-08-25 15:56 ` Krishna Chaitanya Chundru
2025-08-26 6:10 ` Viresh Kumar
2025-08-26 6:06 ` Viresh Kumar
2025-08-26 7:26 ` Marek Szyprowski
2025-08-26 8:24 ` Viresh Kumar
2025-08-26 11:25 ` Krzysztof Kozlowski
2025-08-20 8:28 ` [PATCH v4 3/7] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates Krishna Chaitanya Chundru
2025-08-26 5:57 ` Manivannan Sadhasivam
2025-08-26 6:08 ` Manivannan Sadhasivam
2025-08-20 8:28 ` [PATCH v4 4/7] arm64: dts: qcom: sm8550: " Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 5/7] arm64: dts: qcom: sm8650: " Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 6/7] arm64: dts: qcom: x1e80100: " Krishna Chaitanya Chundru
2025-08-20 8:28 ` [PATCH v4 7/7] PCI: qcom: Use frequency and level based OPP lookup Krishna Chaitanya Chundru
2025-08-20 8:55 ` Neil Armstrong
2025-08-26 5:54 ` Manivannan Sadhasivam
2025-08-25 16:44 ` [PATCH v4 0/7] OPP: Add support to find OPP for a set of keys Wasim Nazir
2025-08-26 5:20 ` Viresh Kumar
2025-08-26 5:36 ` Viresh Kumar
2025-08-26 8:27 ` Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).