[v3] soc: qcom: icc-bwmon: Fix handling dev_pm_opp_find_bw_*() errors

[PATCH v3] soc: qcom: icc-bwmon: Fix handling dev_pm_opp_find_bw_*() errors

Posted by Krzysztof Kozlowski 1 month, 2 weeks ago

The ISR calls dev_pm_opp_find_bw_ceil(), which can return EINVAL, ERANGE
or ENODEV, and if that one fails with ERANGE, then it tries again with
floor dev_pm_opp_find_bw_floor().

In theory, following error paths are possible:
1. First dev_pm_opp_find_bw_ceil() failed with an error different than
   ERANGE,
2. Any error from second dev_pm_opp_find_bw_floor().

However in practice this would mean that there are no suitable OPPs at
all, which is already being checked in the drivers probe() function.
This is impossible condition.

Relying however in the interrupt handler bwmon_intr_thread() on
preconditions checked in probe() is not easy to follow from code
readability and is very difficult to handle in static analysis, thus
let's make the code just obvious to silence warning reported by Smatch:

  icc-bwmon.c:693 bwmon_intr_thread() error: 'target_opp' dereferencing possible ERR_PTR()

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/aJTNEQsRFjrFknG9@stanley.mountain/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes in v3:
1. Grammar commit msg fixes
2. Drop duplicated reported by smatch

Changes in v2:
1. Rephrase commit msg (Konrad)
2. Drop Fixes and cc-stable as this is impossible to trigger
---
 drivers/soc/qcom/icc-bwmon.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
index 3dfa448bf8cf..597f9025e422 100644
--- a/drivers/soc/qcom/icc-bwmon.c
+++ b/drivers/soc/qcom/icc-bwmon.c
@@ -656,6 +656,9 @@ static irqreturn_t bwmon_intr_thread(int irq, void *dev_id)
 	if (IS_ERR(target_opp) && PTR_ERR(target_opp) == -ERANGE)
 		target_opp = dev_pm_opp_find_bw_floor(bwmon->dev, &bw_kbps, 0);
 
+	if (IS_ERR(target_opp))
+		return IRQ_HANDLED;
+
 	bwmon->target_kbps = bw_kbps;
 
 	bw_kbps--;
-- 
2.48.1

Re: [PATCH v3] soc: qcom: icc-bwmon: Fix handling dev_pm_opp_find_bw_*() errors

Posted by Bjorn Andersson 1 month, 1 week ago

On Mon, 18 Aug 2025 14:38:52 +0200, Krzysztof Kozlowski wrote:
> The ISR calls dev_pm_opp_find_bw_ceil(), which can return EINVAL, ERANGE
> or ENODEV, and if that one fails with ERANGE, then it tries again with
> floor dev_pm_opp_find_bw_floor().
> 
> In theory, following error paths are possible:
> 1. First dev_pm_opp_find_bw_ceil() failed with an error different than
>    ERANGE,
> 2. Any error from second dev_pm_opp_find_bw_floor().
> 
> [...]

Applied, thanks!

[1/1] soc: qcom: icc-bwmon: Fix handling dev_pm_opp_find_bw_*() errors
      commit: 13650cd32cfc640fbd49f0424640f64d57335d4c

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH v3] soc: qcom: icc-bwmon: Fix handling dev_pm_opp_find_bw_*() errors

Posted by Markus Elfring 1 month, 2 weeks ago

…
> let's make the code just obvious to silence warning reported by Smatch:
> 
>   icc-bwmon.c:693 bwmon_intr_thread() error: 'target_opp' dereferencing possible ERR_PTR()
…

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?


…
> +++ b/drivers/soc/qcom/icc-bwmon.c
> @@ -656,6 +656,9 @@ static irqreturn_t bwmon_intr_thread(int irq, void *dev_id)
>  	if (IS_ERR(target_opp) && PTR_ERR(target_opp) == -ERANGE)
>  		target_opp = dev_pm_opp_find_bw_floor(bwmon->dev, &bw_kbps, 0);
>  
> +	if (IS_ERR(target_opp))
> +		return IRQ_HANDLED;
> +
>  	bwmon->target_kbps = bw_kbps;
…

Would you like to avoid a duplicate condition check here?

	if (IS_ERR(target_opp) {
		if (PTR_ERR(target_opp) == -ERANGE)
			target_opp = dev_pm_opp_find_bw_floor(bwmon->dev, &bw_kbps, 0);

		return IRQ_HANDLED;
	}


Regards,
Markus