On 6/16/23 09:31, Eugen Hristev wrote:
On 6/15/23 23:59, Marek Vasut wrote:
On 6/15/23 16:59, Eugen Hristev wrote:
Unlike it's Linux counterpart, clk_get_rate can return a negative
value, -ve.
The driver does not take that into account, stores the rate into
an unsigned long, and if clk_get_rate fails, it will take into
consideration
the actual value and wrongly program the hardware.
E.g. on error -2 (no such entry), the rate will be 18446744073709551614
and this will be subsequently used by the driver to program the DWC3
To fix this, exit the function if the value is negative.
Fixes: 6bae0eb5b8bd ("usb: dwc3: Calculate REFCLKPER based on
reference clock")
Signed-off-by: Eugen Hristev <[email protected]>
---
drivers/usb/dwc3/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 49f6a1900b01..5a8c29424578 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -138,7 +138,7 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
if (dwc->ref_clk) {
rate = clk_get_rate(dwc->ref_clk);
This ^ assigns to unsigned long .
That's right, because clk_get_rate returns an unsigned long
Please just change the data type of the $rate to 'long' (drop the
unsigned).
clk_get_rate returns unsigned long, so it's not normal to assign it to a
signed long.
It is not normal that clk_get_rate returns negative values in an
unsigned, but oh well, that's how the clock API is in U-boot
Ah, I see. Then why not do this diff, and fix any clock driver which
returns negative from clk_get_rate() instead ?
diff --git a/include/clk.h b/include/clk.h
index d91285235f7..4e5c1fc90f6 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -453,8 +453,7 @@ void clk_free(struct clk *clk);
* @clk: A clock struct that was previously successfully
requested by
* clk_request/get_by_*().
*
- * Return: clock rate in Hz on success, 0 for invalid clock, or -ve
error code
- * for other errors.
+ * Return: clock rate in Hz on success, 0 for invalid clock.
*/
ulong clk_get_rate(struct clk *clk);
