Re: [PATCH v6 2/2] clocksource: Ingenic: Add support for the Ingenic X1000 OST.

[Zhou Yanjie]

Hi Daniel,

å 2020/7/17 äå12:20, Daniel Lezcano åé:
> On 10/07/2020 19:02, åçæ (Zhou Yanjie) wrote:
> > X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate
> > OST, it no longer belongs to TCU. This driver will register both a
> > clocksource and a sched_clock to the system.
> >
> > Tested-by: åæ (Zhou Zheng) <sernia.zhou@xxxxxxxxxxx>
> > Co-developed-by: æéæ (Qi Pengzhen) <aric.pzqi@xxxxxxxxxxx>
> > Signed-off-by: æéæ (Qi Pengzhen) <aric.pzqi@xxxxxxxxxxx>
> > Signed-off-by: åçæ (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
> > Reviewed-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> > ---
> >
> > Notes:
> >      v1->v2:
> >      Fix compile warnings.
> >      Reported-by: kernel test robot <lkp@xxxxxxxxx>
> >      
> >      v2->v3:
> >      No change.
> >      
> >      v3->v4:
> >      1.Rename "ost" to "sysost"
> >      1.Remove unrelated changes.
> >      2.Remove ost_clock_parent enum.
> >      3.Remove ost->percpu_timer_channel/ost->global_timer_channel.
> >      4.Set up independent .recalc_rate/.set_rate for percpu/global timer.
> >      5.No longer call functions in variable declarations.
> >      
> >      v4->v5:
> >      Use "of_io_request_and_map()" instead "of_iomap()".
> >      Suggested-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> >      
> >      v5->v6:
> >      No change.
> >
> >   drivers/clocksource/Kconfig          |  11 +
> >   drivers/clocksource/Makefile         |   1 +
> >   drivers/clocksource/ingenic-sysost.c | 539 +++++++++++++++++++++++++++++++++++
> >   3 files changed, 551 insertions(+)
> >   create mode 100644 drivers/clocksource/ingenic-sysost.c
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 91418381fcd4..1bca8b8fb30f 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -696,6 +696,17 @@ config INGENIC_TIMER
> >   	help
> >   	  Support for the timer/counter unit of the Ingenic JZ SoCs.
> >   
> > +config INGENIC_SYSOST
> > +	bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"
> We usually use silent options and let the platform's Kconfig enable it.
> We show up the option only when COMPILE_TEST is enabled.
>
> Is there a reason to do it differently?


Do you mean

bool "Clocksource/timer using the SYSOST in Ingenic X SoCs"

or

default MACH_INGENIC ?

This driver has some origins from "INGENIC_TIMER" driver and "INGENIC_OST" driver.
Early Ingenic processors used TCU (timer/counter unit, has 6 or 8 generic timer channels) to provide clocksource and clockevent (both with only 16bit precision). This part of the processor can only use "INGENIC_TIMER" driver.

Later processors provide an independent 32bit or 64bit timer channel (still under TCU, known as ost channel, this channel can not generate interrupt) to provid higher precision clocksource. The "INGENIC_OST" driver is for this channel. These processors can use "INGENIC_TIMER" driver, but using "INGENIC_OST" driver to provide higher precision clocksource would be a better choice (clockevent still needs to be provided by generic timer channel of TCU, and still 16bit precision).

And the recent processors provide a SYSOST components, it is independent from TCU, including a 64bit timer channel for clocksource and a 32bit timer channel for clockevent. Although these processors can also use "INGENIC_TIMER" driver, but the better choice is completely independent use of "INGENIC_SYSOST" driver to provide higher precision clocksource and clockevent.

You may have already noticed that this independent SYSOST component is like an upgraded and streamlined TCU, which only retains one generic timer channel that can generate interrupts, upgrade it from 16bit to 32bit, and then retain the 64bit ost channel. so the driver code and Kconfig code of this patch is largely referenced
"INGENIC_TIMER" driver and "INGENIC_OST" driver.

Thanks and best regards!

> > +	default MACH_INGENIC
> > +	depends on MIPS || COMPILE_TEST
> > +	depends on COMMON_CLK
> > +	select MFD_SYSCON
> > +	select TIMER_OF
> > +	select IRQ_DOMAIN
> > +	help
> > +	  Support for the SYSOST of the Ingenic X Series SoCs.
> > +
> [ ... ]