[PATCH] usb: dwc3: core: fix clk_get_rate returning negative value

[Marek Vasut]

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 <eugen.hristev at collabora.com>
>>> ---
>>>   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);