[PATCH v2 1/1] timer: imx-gpt: Add timer support for i.MX SoCs family

[Giulio Benetti]

On 2/11/21 7:54 PM, Jesse T wrote:
[SNIP]
>      >
>      > The IPG_CLK_24M needs a different prescaler and a second enable bit.
>      > This will completely bypass all other clock sources making the
>     check for
>      > the clock rate useless.
> 
>     Yes, in the operative way yes, you could also avoid passing clock
>     source
>     through dts, but this way we check that the right clock source is
>     passed
>     to dts, and that is the correct way to work I think.
> 
>      > It will also mean that even if we don't have a
>      > correct clock source it will work at the correct timing.
> 
>     Yes if they provide 24Mhz.
> 
>     I wanted it to be like that at the moment, because a lot of imx SoCs
>     setup GPT like that. Take a look here:
>     https://gitlab.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-imx/timer.c#L80-99
> 
>     This way the imx6* and imx7 could add their .compatible to this driver
>     in the future. If someone will need some specific tweaking then they
>     would send a patch adding such new DT property, like using 32K source.
>     But that comes next IMHO, otherwise we should describe entirely GPT
>     peripheral but what we need at the moment is getting 1ms tick like lot
>     of other imx SoCs already do.
> 
>     The other chance would be to treat all the clock sources possibilities,
>     but, at least for me, to begin this is sufficient and can be
>     improved later.
> 
>      > I changed it to use only the 24MHz clock and ignore all others,
> 
>     Ok
> 
>      > at some
>      > point it would be nice to have it only as a backup clock if the
>     ccm was
>      > not configured.
> 
>     I don't understand what you mean with backup clock can you elaborate
>     more?
> 
> If we have a clock source that is 0, we can still use the 24MHz clock as 
> that is an always known source, and isn't controlled by the ccm. 
> Therefore if we have a dummy clock the soc will still have delays and 
> timeouts at the right time. But this would make it so that we never 
> return an error from clk_get_rate(&clk);

I'm not sure it is that safe. I'd prefer something that doesn't work at
all rather than something that works by a default specified inside a 
driver. Since we're trying to imitate linux driver I would avoid this 
solution. Often code is exchanged between u-boot, linux etc. so I'd 
prefer to keep it like linux does.

Best regards
-- 
Giulio Benetti
Benetti Engineering sas

> 
>     Thank you
> 
>     Best regards
>     -- 
>     Giulio Benetti
>     Benetti Engineering sas
>