Hi Jesse, On 2/13/21 4:10 AM, Jesse T wrote:
sry for top posting I'm on my phone just b4 bed. any way the comment in the header file says it returns an int.
No, it says ulong
I don't remember what it actually returns but it should have more clarity.
I've sent a new patch to clarify it and you're in Cc. as for moving all the
bit manipulation to a separate init function , I would essentially have to remake the poll function without the clock driver stuff. I'm assuming u did this so that it could be made more portable to other soc's. What I'm going to do is declare the function with the parameters being the timers udevice struct. and define it based on the soc family. similar to how the Linux kernel does it
As as I can understand it's ok, I've given you an example below, so try following that one. Waiting for new patch. Best regards -- Giulio Benetti Benetti Engineering sas
On Fri, Feb 12, 2021, 8:22 PM Giulio Benetti <giulio.bene...@benettiengineering.com <mailto:giulio.bene...@benettiengineering.com>> wrote: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 BenettiBenetti Engineering sas
