[PATCH v3 0/3] OPP: Add support to find OPP for a set of keys

[PATCH v3 0/3] OPP: Add support to find OPP for a set of keys

[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 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 (3):
      OPP: Add support to find OPP for a set of keys
      arm64: dts: qcom: sm8450: 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 +++++++++++---
 drivers/opp/core.c                     | 100 +++++++++++++++++++++++++++++++++
 drivers/pci/controller/dwc/pcie-qcom.c |   6 +-
 include/linux/pm_opp.h                 |  23 ++++++++
 4 files changed, 160 insertions(+), 10 deletions(-)
---
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
change-id: 20250717-opp_pcie-793160b2b113

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

[PATCH v3 1/3] OPP: Add support to find OPP for a set of keys

[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     | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_opp.h |  23 ++++++++++++
 2 files changed, 123 insertions(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index edbd60501cf00dfd1957f7d19b228d1c61bbbdcc..ce359a3d444b0b7099cdd2421ab1019963d05d9f 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -461,6 +461,15 @@ int dev_pm_opp_get_opp_count(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count);
 
 /* Helpers to read keys */
+static unsigned long _read_opp_key(struct dev_pm_opp *opp, int index, struct dev_pm_opp_key *key)
+{
+	key->bandwidth = opp->bandwidth ? opp->bandwidth[index].peak : 0;
+	key->freq = opp->rates[index];
+	key->level = opp->level;
+
+	return true;
+}
+
 static unsigned long _read_freq(struct dev_pm_opp *opp, int index)
 {
 	return opp->rates[index];
@@ -488,6 +497,23 @@ static bool _compare_exact(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 = (opp_key.level == OPP_LEVEL_UNSET ||
+			    key.level == OPP_LEVEL_UNSET || opp_key.level == key.level);
+	bool bw_match = (opp_key.bandwidth == 0 ||
+			 key.bandwidth == 0 || opp_key.bandwidth == key.bandwidth);
+	bool freq_match = (key.freq == 0 || opp_key.freq == key.freq);
+
+	if (freq_match && level_match && bw_match) {
+		*opp = temp_opp;
+		return true;
+	}
+
+	return false;
+}
+
 static bool _compare_ceil(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
 			  unsigned long opp_key, unsigned long key)
 {
@@ -541,6 +567,40 @@ 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, int index, 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 && !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) {
+			read(temp_opp, index, &temp_key);
+			if (compare(&opp, temp_opp, temp_key, *key))
+				break;
+		}
+	}
+
+	/* Increment the reference count of OPP */
+	if (!IS_ERR(opp)) {
+		*key = temp_key;
+		dev_pm_opp_get(opp);
+	}
+
+	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 +692,46 @@ 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
+ *
+ * Return: Searches for an exact match the OPP key in the OPP table and returns
+ * pointer to the  matching opp if found, else returns ERR_PTR  in case of error
+ * and should  be handled 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, 0, 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..53e02098129d215970d0854b1f8ffaf4499f2bd4 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -98,6 +98,18 @@ struct dev_pm_opp_data {
 	unsigned long u_volt;
 };
 
+/**
+ * struct dev_pm_opp_key - Key used to identify OPP entries
+ * @freq:       Frequency in Hz
+ * @level:      Performance level associated with the OPP entry
+ * @bandwidth:  Bandwidth associated with the OPP entry
+ */
+struct dev_pm_opp_key {
+	unsigned long freq;
+	unsigned int level;
+	u32 bandwidth;
+};
+
 #if defined(CONFIG_PM_OPP)
 
 struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
@@ -131,6 +143,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 +305,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

[PATCH v3 2/3] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates

[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

[PATCH v3 3/3] PCI: qcom: Use frequency and level based OPP lookup

[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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 294babe1816e4d0c2b2343fe22d89af72afcd6cd..831c9138841ac089c6dd6b08a4a206751dfebc91 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,9 @@ 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;
+		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

Re: [PATCH v3 1/3] OPP: Add support to find OPP for a set of keys

[Viresh Kumar]

On 19-08-25, 11:04, Krishna Chaitanya Chundru wrote:
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index edbd60501cf00dfd1957f7d19b228d1c61bbbdcc..ce359a3d444b0b7099cdd2421ab1019963d05d9f 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -461,6 +461,15 @@ int dev_pm_opp_get_opp_count(struct device *dev)
>  EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count);
>  
>  /* Helpers to read keys */
> +static unsigned long _read_opp_key(struct dev_pm_opp *opp, int index, struct dev_pm_opp_key *key)

Move this to the end of the list, after _read_bw() i.e.

> +{
> +	key->bandwidth = opp->bandwidth ? opp->bandwidth[index].peak : 0;
> +	key->freq = opp->rates[index];
> +	key->level = opp->level;
> +
> +	return true;
> +}
> +
>  static unsigned long _read_freq(struct dev_pm_opp *opp, int index)
>  {
>  	return opp->rates[index];
> @@ -488,6 +497,23 @@ static bool _compare_exact(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)
> +{

And this after _compare_floor().

> +	bool level_match = (opp_key.level == OPP_LEVEL_UNSET ||

Why are we still checking this ? You removed this from freq check but
not level and bandwidth ?

> +			    key.level == OPP_LEVEL_UNSET || opp_key.level == key.level);
> +	bool bw_match = (opp_key.bandwidth == 0 ||
> +			 key.bandwidth == 0 || opp_key.bandwidth == key.bandwidth);
> +	bool freq_match = (key.freq == 0 || opp_key.freq == key.freq);
> +
> +	if (freq_match && level_match && bw_match) {
> +		*opp = temp_opp;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  static bool _compare_ceil(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
>  			  unsigned long opp_key, unsigned long key)
>  {
> @@ -541,6 +567,40 @@ 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, int index, 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 && !assert(opp_table, index))

Just drop the `assert` check, it isn't optional. Make the same change
in _opp_table_find_key() too in a separate patch if you can.

> +		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, index, &temp_key);
> +			if (compare(&opp, temp_opp, temp_key, *key))

Update *key and do dev_pm_opp_get() here itself. And same in
_opp_table_find_key().

> +				break;
> +		}
> +	}
> +
> +	/* Increment the reference count of OPP */
> +	if (!IS_ERR(opp)) {
> +		*key = temp_key;
> +		dev_pm_opp_get(opp);
> +	}
> +
> +	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 +692,46 @@ 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
> + *
> + * Return: Searches for an exact match the OPP key in the OPP table and returns

The `Return` section should only talk about the returned values. The
above line for example should be present as a standalone line before
the `Return` section.

> + * pointer to the  matching opp if found, else returns ERR_PTR  in case of error
> + * and should  be handled 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.

There are various minor issues in the text here, like double spaces,
not starting with a capital letter after a full stop, etc. Also put
arguments in `` block, like `available`.

> + */
> +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, 0, available, _read_opp_key,
> +				       _compare_opp_key_exact, assert_single_clk);

Since the only user doesn't use `index` for now, I wonder if that
should be added at all in all these functions.

> +}
> +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..53e02098129d215970d0854b1f8ffaf4499f2bd4 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -98,6 +98,18 @@ struct dev_pm_opp_data {
>  	unsigned long u_volt;
>  };
>  
> +/**
> + * struct dev_pm_opp_key - Key used to identify OPP entries
> + * @freq:       Frequency in Hz
> + * @level:      Performance level associated with the OPP entry
> + * @bandwidth:  Bandwidth associated with the OPP entry

Also mention the NOP value of all these keys, i.e. what the user must
fill them with if that key is not supposed to be matched.

> + */
> +struct dev_pm_opp_key {
> +	unsigned long freq;
> +	unsigned int level;
> +	u32 bandwidth;

Maybe use `bw` here like in other APIs.

> +};
> +
>  #if defined(CONFIG_PM_OPP)
>  
>  struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
> @@ -131,6 +143,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 +305,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

-- 
viresh

Re: [PATCH v3 1/3] OPP: Add support to find OPP for a set of keys

[Krishna Chaitanya Chundru]

On 8/19/2025 1:48 PM, Viresh Kumar wrote:
> On 19-08-25, 11:04, Krishna Chaitanya Chundru wrote:
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index edbd60501cf00dfd1957f7d19b228d1c61bbbdcc..ce359a3d444b0b7099cdd2421ab1019963d05d9f 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -461,6 +461,15 @@ int dev_pm_opp_get_opp_count(struct device *dev)
>>   EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count);
>>   
>>   /* Helpers to read keys */
>> +static unsigned long _read_opp_key(struct dev_pm_opp *opp, int index, struct dev_pm_opp_key *key)
> 
> Move this to the end of the list, after _read_bw() i.e.
> 
ack
>> +{
>> +	key->bandwidth = opp->bandwidth ? opp->bandwidth[index].peak : 0;
>> +	key->freq = opp->rates[index];
>> +	key->level = opp->level;
>> +
>> +	return true;
>> +}
>> +
>>   static unsigned long _read_freq(struct dev_pm_opp *opp, int index)
>>   {
>>   	return opp->rates[index];
>> @@ -488,6 +497,23 @@ static bool _compare_exact(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)
>> +{
> 
> And this after _compare_floor().
> 
ack
>> +	bool level_match = (opp_key.level == OPP_LEVEL_UNSET ||
> 
> Why are we still checking this ? You removed this from freq check but
> not level and bandwidth ?
> 
ok I will change for level and bw also similar to freq.
>> +			    key.level == OPP_LEVEL_UNSET || opp_key.level == key.level);
>> +	bool bw_match = (opp_key.bandwidth == 0 ||
>> +			 key.bandwidth == 0 || opp_key.bandwidth == key.bandwidth);
>> +	bool freq_match = (key.freq == 0 || opp_key.freq == key.freq);
>> +
>> +	if (freq_match && level_match && bw_match) {
>> +		*opp = temp_opp;
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>   static bool _compare_ceil(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp,
>>   			  unsigned long opp_key, unsigned long key)
>>   {
>> @@ -541,6 +567,40 @@ 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, int index, 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 && !assert(opp_table, index))
> 
> Just drop the `assert` check, it isn't optional. Make the same change
> in _opp_table_find_key() too in a separate patch if you can.
> 
ack
>> +		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, index, &temp_key);
>> +			if (compare(&opp, temp_opp, temp_key, *key))
> 
> Update *key and do dev_pm_opp_get() here itself. And same in
> _opp_table_find_key().
> 
ack
>> +				break;
>> +		}
>> +	}
>> +
>> +	/* Increment the reference count of OPP */
>> +	if (!IS_ERR(opp)) {
>> +		*key = temp_key;
>> +		dev_pm_opp_get(opp);
>> +	}
>> +
>> +	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 +692,46 @@ 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
>> + *
>> + * Return: Searches for an exact match the OPP key in the OPP table and returns
> 
> The `Return` section should only talk about the returned values. The
> above line for example should be present as a standalone line before
> the `Return` section.
> 
ack
>> + * pointer to the  matching opp if found, else returns ERR_PTR  in case of error
>> + * and should  be handled 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.
> 
> There are various minor issues in the text here, like double spaces,
> not starting with a capital letter after a full stop, etc. Also put
> arguments in `` block, like `available`.
> 
ack
>> + */
>> +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, 0, available, _read_opp_key,
>> +				       _compare_opp_key_exact, assert_single_clk);
> 
> Since the only user doesn't use `index` for now, I wonder if that
> should be added at all in all these functions.
> 
ok I will remove it.
>> +}
>> +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..53e02098129d215970d0854b1f8ffaf4499f2bd4 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -98,6 +98,18 @@ struct dev_pm_opp_data {
>>   	unsigned long u_volt;
>>   };
>>   
>> +/**
>> + * struct dev_pm_opp_key - Key used to identify OPP entries
>> + * @freq:       Frequency in Hz
>> + * @level:      Performance level associated with the OPP entry
>> + * @bandwidth:  Bandwidth associated with the OPP entry
> 
> Also mention the NOP value of all these keys, i.e. what the user must
> fill them with if that key is not supposed to be matched.
> 
ack
>> + */
>> +struct dev_pm_opp_key {
>> +	unsigned long freq;
>> +	unsigned int level;
>> +	u32 bandwidth;
> 
> Maybe use `bw` here like in other APIs.
> 
ack.

- Krishna Chaitanya.
>> +};
>> +
>>   #if defined(CONFIG_PM_OPP)
>>   
>>   struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
>> @@ -131,6 +143,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 +305,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
> 

Re: [PATCH v3 2/3] arm64: dts: qcom: sm8450: Add opp-level to indicate PCIe data rates

[Konrad Dybcio]

On 8/19/25 7:34 AM, 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>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Are there any other SoCs affected?

Konrad