Hi Jesse, Sean,
On 2/10/21 9:13 PM, Giulio Benetti wrote:
[SNIP]
>
>
>> + 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".
>
>> + dev_err(dev, "Could not get clock rate...\n");
>> + return -EINVAL;
>> + }
>> + /* Only support 24MHz clock */
>
> Extend to /* Only support 24MHz crystal clock */ otherwise it seems that
> every 24Mhz clock is accepted and it's not that way.
>
> I don't know a sure way to be bounded to crystal clock, suggestions are
> welcome.
Why is it necessary? What are the constraints which require only using
the 24MHz reference?
To tell the truth there are no constraints, the reason is that I'm
trying to keep this driver compatible and reusable by imx6* and imx7*
since they setup the timer this way:
https://gitlab.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-imx/timer.c#L80-99
But if this timer needs to support any kind of clock source then let's
modify it, but it gets more complicated and I think it could be done if
needed.
This is the Linux way to make it work even if clock source is not 24Mhz:
https://github.com/torvalds/linux/blob/master/drivers/clocksource/timer-imx-gpt.c#L314-L329
It checks if clock source is 24Mhz and set CLKSRC(named V2_TPRER_PRE24M)
to IPG_CLK_24M(named MXC_TPRER). Otherwise it uses peripheral clock and
that's all.
At this point it's easier than I thought.
Jesse, can you please add that handling imitating Linux driver?
Best regards
--
Giulio Benetti
Benetti Engineering sas
