Hi Jesse,
On 2/11/21 7:54 PM, Jesse T wrote:
[SNIP]
>>> +static int imx_gpt_timer_probe(struct udevice *dev)
>>> +{
>>> + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>>> + struct imx_gpt_timer_priv *priv = dev_get_priv(dev);
>>> + struct imx_gpt_timer_regs *regs;
>>> + struct clk clk;
>>> + fdt_addr_t addr;
>>> + u32 prescaler;
>>> + u32 rate;
>>> + int ret;
>>> +
>>> + addr = dev_read_addr(dev);
>>> + if (addr == FDT_ADDR_T_NONE)
>>> + return -EINVAL;
>>> +
>>> + priv->base = (struct imx_gpt_timer_regs *)addr;
>>> +
>>> + ret = clk_get_by_index(dev, 0, &clk);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = clk_enable(&clk);
>>> + if (ret) {
>>> + dev_err(dev, "Failed to enable clock\n");
>>> + return ret;
>>> + }
>>> +
>>> + regs = priv->base;
>>> +
>>> + /* Reset the timer */
>>> + setbits_le32(®s->cr, GPT_CR_SWR);
>>> +
>>> + /* Wait for timer to finish reset */
>>> + while (readl(®s->cr) & GPT_CR_SWR)
>>> + ;
>>> +
>>> + /* Get timer clock rate */
>>> + rate = clk_get_rate(&clk);
>>
>> clk_get_rate() has a wrong description in include/clk.h saying:
>> "@return clock rate in Hz, or -ve error code."
>>
>> This ^^^ is wrong.
>> If you dig into drivers/clk/clk-uclass.c and look for
clk_get_rate(),
>> you will see that it returns 0 on error and > 0 if succesfull.
>>
>> I've just sent a patch for this:
>>
https://patchwork.ozlabs.org/project/uboot/patch/20210210173722.4823-1-giulio.bene...@benettiengineering.com/
>>
>>
>>> + if ((int)rate <= 0) {
>>
>> This ^^^^ is a cast trying to solve the problem above but
it's
>> not correct. clk_get_rate() returns ulong, not int, so modify "int
>> rate" into "ulong rate".
> Ah this makes sense now.
Here it's my bad, clk_get_rate() really returns ulong but can return a
negative number too, so my patch has been dropped. You can verify it in
clk-uclass.c where function is implemented, in get_rate() function
pointer is null the return -ENOSYS. Then please declare rate as ulong
and check it in if statement with IS_ERR_VALUE(). That way you're sure
it's not negative.
Thank you
Best regards
--
Giulio Benetti
Benetti Engineering sas
