| Message ID | 20200710170259.29028-1-zhouyanjie@wanyeetech.com |
|---|---|
| Headers | show |
| Series | Add support for the OST in Ingenic X1000. | expand |
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@foxmail.com> > Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> > Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> > Reviewed-by: Paul Cercueil <paul@crapouillou.net> > --- > > Notes: > v1->v2: > Fix compile warnings. > Reported-by: kernel test robot <lkp@intel.com> > > 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@crapouillou.net> > > 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? > + 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. > + [ ... ]
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@foxmail.com> >> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >> --- >> >> Notes: >> v1->v2: >> Fix compile warnings. >> Reported-by: kernel test robot <lkp@intel.com> >> >> 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@crapouillou.net> >> >> 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. >> + > [ ... ] > >
On 17/07/2020 08:13, Zhou Yanjie wrote: > 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@foxmail.com> >>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>> --- >>> >>> Notes: >>> v1->v2: >>> Fix compile warnings. >>> Reported-by: kernel test robot <lkp@intel.com> >>> 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@crapouillou.net> >>> 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 ? Both, no default here. eg. bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if COMPILE_TEST and in arch/mips/Kconfig in the config MACH_INGENIC section : ... select INGENIC_SYSOST ... > 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. Ok, the rating should do the job then. Thanks for the explanation. > 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. >>> + >> [ ... ] >> >>
Hi Daniel, 在 2020/7/17 下午4:02, Daniel Lezcano 写道: > On 17/07/2020 08:13, Zhou Yanjie wrote: >> 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@foxmail.com> >>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>> --- >>>> >>>> Notes: >>>> v1->v2: >>>> Fix compile warnings. >>>> Reported-by: kernel test robot <lkp@intel.com> >>>> 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@crapouillou.net> >>>> 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 ? > Both, no default here. > > eg. > > bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if COMPILE_TEST > > and > > in arch/mips/Kconfig in the config MACH_INGENIC section : > > ... > select INGENIC_SYSOST > ... Sure, I will do it in the next version. Thanks and best regards! >> 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. > Ok, the rating should do the job then. > > Thanks for the explanation. > >> 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. >>>> + >>> [ ... ] >>> >>> >
Hi Daniel, Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano <daniel.lezcano@linaro.org> a écrit : > On 17/07/2020 08:13, Zhou Yanjie wrote: >> 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@foxmail.com> >>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>> --- >>>> >>>> Notes: >>>> v1->v2: >>>> Fix compile warnings. >>>> Reported-by: kernel test robot <lkp@intel.com> >>>> 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@crapouillou.net> >>>> 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 ? > > Both, no default here. > > eg. > > bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if > COMPILE_TEST > > and > > in arch/mips/Kconfig in the config MACH_INGENIC section : > > ... > select INGENIC_SYSOST > ... Disagreed. That's not how we do things on MIPS. Selecting MACH_INGENIC means "this kernel will support Ingenic SoCs", but not that it will only support these. Hence the depends on MIPS / default MACH_INGENIC. As for the select INGENIC_SYSOST, this driver only applies to a few SoCs, I certainly don't want it to be force-enabled. I don't even wait it to be force-enabled on X1000, since it is optional there too. Cheers, -Paul > >> 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. > > Ok, the rating should do the job then. > > Thanks for the explanation. > >> 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. >>>> + >>> [ ... ] >>> >>> > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM > SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog
Hello Paul and Daniel, 在 2020/7/18 下午9:12, Paul Cercueil 写道: > Hi Daniel, > > Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano > <daniel.lezcano@linaro.org> a écrit : >> On 17/07/2020 08:13, Zhou Yanjie wrote: >>> 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@foxmail.com> >>>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >>>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>>> --- >>>>> >>>>> Notes: >>>>> v1->v2: >>>>> Fix compile warnings. >>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>> 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@crapouillou.net> >>>>> 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 ? >> >> Both, no default here. >> >> eg. >> >> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if >> COMPILE_TEST >> >> and >> >> in arch/mips/Kconfig in the config MACH_INGENIC section : >> >> ... >> select INGENIC_SYSOST >> ... > > Disagreed. That's not how we do things on MIPS. Selecting MACH_INGENIC > means "this kernel will support Ingenic SoCs", but not that it will > only support these. Hence the depends on MIPS / default MACH_INGENIC. > > As for the select INGENIC_SYSOST, this driver only applies to a few > SoCs, I certainly don't want it to be force-enabled. I don't even wait > it to be force-enabled on X1000, since it is optional there too. > > Cheers, > -Paul If we still need to keep the "default MACH_INGENIC", then Daniel can directly apply the v6 version. If we need to use the silent options, maybe we can enable them separately according to MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830. In fact, I think X1000 and X1830 need to enable this driver in most cases, because the current test has found that use TCU to provide clocksource and clockevent will cause data loss/error when transmitting data through spi or ethernet. And these errors no longer appear after using OST. Thanks and best regards! > >> >>> 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. >> >> Ok, the rating should do the job then. >> >> Thanks for the explanation. >> >>> 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. >>>>> + >>>> [ ... ] >>>> >>>> >> >> >> -- >> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs >> >> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | >> <http://twitter.com/#!/linaroorg> Twitter | >> <http://www.linaro.org/linaro-blog/> Blog >
Hi Zhou, Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie <zhouyanjie@wanyeetech.com> a écrit : > Hello Paul and Daniel, > > 在 2020/7/18 下午9:12, Paul Cercueil 写道: >> Hi Daniel, >> >> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano >> <daniel.lezcano@linaro.org> a écrit : >>> On 17/07/2020 08:13, Zhou Yanjie wrote: >>>> 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@foxmail.com> >>>>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>> Signed-off-by: 周琰杰 (Zhou Yanjie) >>>>>> <zhouyanjie@wanyeetech.com> >>>>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>>>> --- >>>>>> >>>>>> Notes: >>>>>> v1->v2: >>>>>> Fix compile warnings. >>>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>>> 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@crapouillou.net> >>>>>> 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 ? >>> >>> Both, no default here. >>> >>> eg. >>> >>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if >>> COMPILE_TEST >>> >>> and >>> >>> in arch/mips/Kconfig in the config MACH_INGENIC section : >>> >>> ... >>> select INGENIC_SYSOST >>> ... >> >> Disagreed. That's not how we do things on MIPS. Selecting >> MACH_INGENIC means "this kernel will support Ingenic SoCs", but not >> that it will only support these. Hence the depends on MIPS / >> default MACH_INGENIC. >> >> As for the select INGENIC_SYSOST, this driver only applies to a few >> SoCs, I certainly don't want it to be force-enabled. I don't even >> wait it to be force-enabled on X1000, since it is optional there >> too. >> >> Cheers, >> -Paul > > > If we still need to keep the "default MACH_INGENIC", then Daniel can > directly apply the v6 version. > > If we need to use the silent options, maybe we can enable them > separately according to > MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830. > > In fact, I think X1000 and X1830 need to enable this driver in most > cases, because the current test has found that use TCU to provide > clocksource and clockevent will cause data loss/error when > transmitting data through spi or ethernet. And these errors no longer > appear after using OST. This is likely because the clock is too fast, try to reduce it by a factor 2 or 4, it should behave better. If it turns out OST is really needed, then it should be selected from MACH_X1000/X1830, not MACH_INGENIC. Cheers, -Paul > Thanks and best regards! > > >> >>> >>>> 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. >>> >>> Ok, the rating should do the job then. >>> >>> Thanks for the explanation. >>> >>>> 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. >>>>>> + >>>>> [ ... ] >>>>> >>>>> >>> >>> >>> -- >>> <http://www.linaro.org/> Linaro.org │ Open source software for >>> ARM SoCs >>> >>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | >>> <http://twitter.com/#!/linaroorg> Twitter | >>> <http://www.linaro.org/linaro-blog/> Blog >>
Hi Paul, 在 2020/7/18 下午11:44, Paul Cercueil 写道: > Hi Zhou, > > Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie <zhouyanjie@wanyeetech.com> > a écrit : >> Hello Paul and Daniel, >> >> 在 2020/7/18 下午9:12, Paul Cercueil 写道: >>> Hi Daniel, >>> >>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano >>> <daniel.lezcano@linaro.org> a écrit : >>>> On 17/07/2020 08:13, Zhou Yanjie wrote: >>>>> 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@foxmail.com> >>>>>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >>>>>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>>>>> --- >>>>>>> >>>>>>> Notes: >>>>>>> v1->v2: >>>>>>> Fix compile warnings. >>>>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>>>> 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@crapouillou.net> >>>>>>> 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 ? >>>> >>>> Both, no default here. >>>> >>>> eg. >>>> >>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if >>>> COMPILE_TEST >>>> >>>> and >>>> >>>> in arch/mips/Kconfig in the config MACH_INGENIC section : >>>> >>>> ... >>>> select INGENIC_SYSOST >>>> ... >>> >>> Disagreed. That's not how we do things on MIPS. Selecting >>> MACH_INGENIC means "this kernel will support Ingenic SoCs", but not >>> that it will only support these. Hence the depends on MIPS / >>> default MACH_INGENIC. >>> >>> As for the select INGENIC_SYSOST, this driver only applies to a few >>> SoCs, I certainly don't want it to be force-enabled. I don't even >>> wait it to be force-enabled on X1000, since it is optional there too. >>> >>> Cheers, >>> -Paul >> >> >> If we still need to keep the "default MACH_INGENIC", then Daniel can >> directly apply the v6 version. >> >> If we need to use the silent options, maybe we can enable them >> separately according to >> MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830. >> >> In fact, I think X1000 and X1830 need to enable this driver in most >> cases, because the current test has found that use TCU to provide >> clocksource and clockevent will cause data loss/error when >> transmitting data through spi or ethernet. And these errors no longer >> appear after using OST. > > This is likely because the clock is too fast, try to reduce it by a > factor 2 or 4, it should behave better. > Yes, it is indeed because the clock is too fast, especially the TCU only has 16-bit, which makes this problem more serious. I even reduced the timing clock to the lowest possible 23kHz (divided by 1024). The problem has been alleviated, but it cannot completely solved, and finally there is no problem after opening the OST. > If it turns out OST is really needed, then it should be selected from > MACH_X1000/X1830, not MACH_INGENIC. > Sure, I will do it in v8. Thanks and best regards! > Cheers, > -Paul > >> Thanks and best regards! >> >> >>> >>>> >>>>> 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. >>>> >>>> Ok, the rating should do the job then. >>>> >>>> Thanks for the explanation. >>>> >>>>> 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. >>>>>>> + >>>>>> [ ... ] >>>>>> >>>>>> >>>> >>>> >>>> -- >>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM >>>> SoCs >>>> >>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | >>>> <http://twitter.com/#!/linaroorg> Twitter | >>>> <http://www.linaro.org/linaro-blog/> Blog >>> >
Le sam. 18 juil. 2020 à 23:55, Zhou Yanjie <zhouyanjie@wanyeetech.com> a écrit : > Hi Paul, > > 在 2020/7/18 下午11:44, Paul Cercueil 写道: >> Hi Zhou, >> >> Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie >> <zhouyanjie@wanyeetech.com> a écrit : >>> Hello Paul and Daniel, >>> >>> 在 2020/7/18 下午9:12, Paul Cercueil 写道: >>>> Hi Daniel, >>>> >>>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano >>>> <daniel.lezcano@linaro.org> a écrit : >>>>> On 17/07/2020 08:13, Zhou Yanjie wrote: >>>>>> 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@foxmail.com> >>>>>>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) >>>>>>>> <aric.pzqi@ingenic.com> >>>>>>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>>>> Signed-off-by: 周琰杰 (Zhou Yanjie) >>>>>>>> <zhouyanjie@wanyeetech.com> >>>>>>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>>>>>> --- >>>>>>>> >>>>>>>> Notes: >>>>>>>> v1->v2: >>>>>>>> Fix compile warnings. >>>>>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>>>>> 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@crapouillou.net> >>>>>>>> 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 ? >>>>> >>>>> Both, no default here. >>>>> >>>>> eg. >>>>> >>>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if >>>>> COMPILE_TEST >>>>> >>>>> and >>>>> >>>>> in arch/mips/Kconfig in the config MACH_INGENIC section : >>>>> >>>>> ... >>>>> select INGENIC_SYSOST >>>>> ... >>>> >>>> Disagreed. That's not how we do things on MIPS. Selecting >>>> MACH_INGENIC means "this kernel will support Ingenic SoCs", >>>> but not that it will only support these. Hence the depends on >>>> MIPS / default MACH_INGENIC. >>>> >>>> As for the select INGENIC_SYSOST, this driver only applies to a >>>> few SoCs, I certainly don't want it to be force-enabled. I >>>> don't even wait it to be force-enabled on X1000, since it is >>>> optional there too. >>>> >>>> Cheers, >>>> -Paul >>> >>> >>> If we still need to keep the "default MACH_INGENIC", then Daniel >>> can directly apply the v6 version. >>> >>> If we need to use the silent options, maybe we can enable them >>> separately according to >>> MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830. >>> >>> In fact, I think X1000 and X1830 need to enable this driver in most >>> cases, because the current test has found that use TCU to provide >>> clocksource and clockevent will cause data loss/error when >>> transmitting data through spi or ethernet. And these errors no >>> longer appear after using OST. >> >> This is likely because the clock is too fast, try to reduce it by a >> factor 2 or 4, it should behave better. >> > > Yes, it is indeed because the clock is too fast, especially the TCU > only has 16-bit, which makes this problem more serious. I even > reduced the timing clock to the lowest possible 23kHz (divided by > 1024). The problem has been alleviated, but it cannot completely > solved, and finally there is no problem after opening the OST. > > >> If it turns out OST is really needed, then it should be selected >> from MACH_X1000/X1830, not MACH_INGENIC. >> > > Sure, I will do it in v8. > > Thanks and best regards! Please do it in a separate patch then (but in the same patchset). Cheers, -Paul > >> Cheers, >> -Paul >> >>> Thanks and best regards! >>> >>> >>>> >>>>> >>>>>> 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. >>>>> >>>>> Ok, the rating should do the job then. >>>>> >>>>> Thanks for the explanation. >>>>> >>>>>> 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. >>>>>>>> + >>>>>>> [ ... ] >>>>>>> >>>>>>> >>>>> >>>>> >>>>> -- >>>>> <http://www.linaro.org/> Linaro.org │ Open source software for >>>>> ARM SoCs >>>>> >>>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | >>>>> <http://twitter.com/#!/linaroorg> Twitter | >>>>> <http://www.linaro.org/linaro-blog/> Blog >>>> >>
在 2020/7/19 上午12:11, Paul Cercueil 写道: > > > Le sam. 18 juil. 2020 à 23:55, Zhou Yanjie <zhouyanjie@wanyeetech.com> > a écrit : >> Hi Paul, >> >> 在 2020/7/18 下午11:44, Paul Cercueil 写道: >>> Hi Zhou, >>> >>> Le sam. 18 juil. 2020 à 21:42, Zhou Yanjie >>> <zhouyanjie@wanyeetech.com> a écrit : >>>> Hello Paul and Daniel, >>>> >>>> 在 2020/7/18 下午9:12, Paul Cercueil 写道: >>>>> Hi Daniel, >>>>> >>>>> Le ven. 17 juil. 2020 à 10:02, Daniel Lezcano >>>>> <daniel.lezcano@linaro.org> a écrit : >>>>>> On 17/07/2020 08:13, Zhou Yanjie wrote: >>>>>>> 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@foxmail.com> >>>>>>>>> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>>>>> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com> >>>>>>>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >>>>>>>>> Reviewed-by: Paul Cercueil <paul@crapouillou.net> >>>>>>>>> --- >>>>>>>>> >>>>>>>>> Notes: >>>>>>>>> v1->v2: >>>>>>>>> Fix compile warnings. >>>>>>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>>>>>> 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@crapouillou.net> >>>>>>>>> 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 ? >>>>>> >>>>>> Both, no default here. >>>>>> >>>>>> eg. >>>>>> >>>>>> bool "Clocksource/timer using the SYSOST in Ingenic X SoCs" if >>>>>> COMPILE_TEST >>>>>> >>>>>> and >>>>>> >>>>>> in arch/mips/Kconfig in the config MACH_INGENIC section : >>>>>> >>>>>> ... >>>>>> select INGENIC_SYSOST >>>>>> ... >>>>> >>>>> Disagreed. That's not how we do things on MIPS. Selecting >>>>> MACH_INGENIC means "this kernel will support Ingenic SoCs", >>>>> but not that it will only support these. Hence the depends on >>>>> MIPS / default MACH_INGENIC. >>>>> >>>>> As for the select INGENIC_SYSOST, this driver only applies to a >>>>> few SoCs, I certainly don't want it to be force-enabled. I >>>>> don't even wait it to be force-enabled on X1000, since it is >>>>> optional there too. >>>>> >>>>> Cheers, >>>>> -Paul >>>> >>>> >>>> If we still need to keep the "default MACH_INGENIC", then Daniel >>>> can directly apply the v6 version. >>>> >>>> If we need to use the silent options, maybe we can enable them >>>> separately according to >>>> MACH_JZ4740/MACH_JZ4770/MACH_JZ4780/MACH_X1000/MACH_X1830. >>>> >>>> In fact, I think X1000 and X1830 need to enable this driver in most >>>> cases, because the current test has found that use TCU to provide >>>> clocksource and clockevent will cause data loss/error when >>>> transmitting data through spi or ethernet. And these errors no >>>> longer appear after using OST. >>> >>> This is likely because the clock is too fast, try to reduce it by a >>> factor 2 or 4, it should behave better. >>> >> >> Yes, it is indeed because the clock is too fast, especially the TCU >> only has 16-bit, which makes this problem more serious. I even >> reduced the timing clock to the lowest possible 23kHz (divided by >> 1024). The problem has been alleviated, but it cannot completely >> solved, and finally there is no problem after opening the OST. >> >> >>> If it turns out OST is really needed, then it should be selected >>> from MACH_X1000/X1830, not MACH_INGENIC. >>> >> >> Sure, I will do it in v8. >> >> Thanks and best regards! > > Please do it in a separate patch then (but in the same patchset). > Okay, I will remove "default MACH_INGENIC" first, and then enable OST later. > Cheers, > -Paul > >> >>> Cheers, >>> -Paul >>> >>>> Thanks and best regards! >>>> >>>> >>>>> >>>>>> >>>>>>> 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. >>>>>> >>>>>> Ok, the rating should do the job then. >>>>>> >>>>>> Thanks for the explanation. >>>>>> >>>>>>> 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. >>>>>>>>> + >>>>>>>> [ ... ] >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> <http://www.linaro.org/> Linaro.org │ Open source software for >>>>>> ARM SoCs >>>>>> >>>>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | >>>>>> <http://twitter.com/#!/linaroorg> Twitter | >>>>>> <http://www.linaro.org/linaro-blog/> Blog >>>>> >>> >