From: Pei Xiao xiaopei01@kylinos.cn
[ Upstream commit 2dc2ca9000eea2eb749f658196204cb84d4306f7 ]
./drivers/clk/tegra/clk-periph.c:59:5-9: WARNING: Unsigned expression compared with zero: rate < 0
The unsigned long 'rate' variable caused: - Incorrect handling of negative errors - Compile warning: "Unsigned expression compared with zero"
Fix by changing to long type and adding req->rate cast.
Signed-off-by: Pei Xiao xiaopei01@kylinos.cn Link: https://lore.kernel.org/r/79c7f01e29876c612e90d6d0157fb1572ca8b3fb.175204627... Acked-by: Thierry Reding treding@nvidia.com Signed-off-by: Stephen Boyd sboyd@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
The return type is indeed `long`, which can represent negative error codes. Now let me verify the proposed fix addresses this issue correctly by looking at the patch again and understanding the cast:
The fix changes: 1. `unsigned long rate` to `long rate` - This allows the variable to hold negative values 2. Adds a cast when assigning to `req->rate`: `req->rate = (unsigned long)rate` - This is safe because we only do this assignment after checking that rate is not negative
**Backport Status: YES**
This commit should be backported to stable kernel trees for the following reasons:
1. **It fixes a real bug** that was introduced in v6.10 (commit 4d78bd80cef7): The original code declares `rate` as `unsigned long` while `div_ops->round_rate()` returns a `long` that can be negative to indicate errors. The comparison `if (rate < 0)` on line 59 will never be true for an unsigned variable, causing error conditions from the underlying clock driver to be ignored and potentially passed as valid (very large) rate values.
2. **The fix is minimal and contained**: The change only modifies two lines - changing the variable type from `unsigned long` to `long` and adding an explicit cast when assigning to `req->rate`. This is a surgical fix that doesn't alter the logic or introduce new features.
3. **Low risk of regression**: The fix properly handles the error case that was being missed before. The cast to `unsigned long` when assigning to `req->rate` is safe because it only happens after verifying the rate is not negative. This maintains compatibility with the existing API while fixing the error handling.
4. **Affects a critical subsystem**: Clock drivers are fundamental to system operation. Incorrect error handling in clock rate calculations could lead to system instability, incorrect clock frequencies being set, or error conditions being silently ignored.
5. **The bug has clear symptoms**: The commit message mentions both a compile-time warning ("Unsigned expression compared with zero") and incorrect runtime behavior (error values not being properly handled). This makes it easy to verify the fix addresses real issues.
6. **Applies to currently supported stable kernels**: The bug was introduced in v6.10, which is still within the stable kernel maintenance window. The fix should be backported to v6.10.y stable series to ensure proper error handling in Tegra peripheral clock operations.
The commit perfectly aligns with stable kernel criteria: it fixes a clear bug without adding features or making architectural changes, and the risk of introducing new issues is minimal given the straightforward nature of the fix.
drivers/clk/tegra/clk-periph.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/tegra/clk-periph.c b/drivers/clk/tegra/clk-periph.c index 0626650a7011..c9fc52a36fce 100644 --- a/drivers/clk/tegra/clk-periph.c +++ b/drivers/clk/tegra/clk-periph.c @@ -51,7 +51,7 @@ static int clk_periph_determine_rate(struct clk_hw *hw, struct tegra_clk_periph *periph = to_clk_periph(hw); const struct clk_ops *div_ops = periph->div_ops; struct clk_hw *div_hw = &periph->divider.hw; - unsigned long rate; + long rate;
__clk_hw_set_clk(div_hw, hw);
@@ -59,7 +59,7 @@ static int clk_periph_determine_rate(struct clk_hw *hw, if (rate < 0) return rate;
- req->rate = rate; + req->rate = (unsigned long)rate; return 0; }