| Message ID | 20221229224601.103851-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | [v2] clocksource/drivers/riscv: Get rid of clocksource_arch_init() callback | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 13 and now 13 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 2054 this patch: 2054 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 21 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On 12/29/22 16:46, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Having a clocksource_arch_init() callback always sets vdso_clock_mode to > VDSO_CLOCKMODE_ARCHTIMER if GENERIC_GETTIMEOFDAY is enabled, this is > required for the riscv-timer. > > This works for platforms where just riscv-timer clocksource is present. > On platforms where other clock sources are available we want them to > register with vdso_clock_mode set to VDSO_CLOCKMODE_NONE. > > On the Renesas RZ/Five SoC OSTM block can be used as clocksource [0], to > avoid multiple clock sources being registered as VDSO_CLOCKMODE_ARCHTIMER > move setting of vdso_clock_mode in the riscv-timer driver instead of doing > this in clocksource_arch_init() callback as done similarly for ARM/64 > architecture. > > [0] drivers/clocksource/renesas-ostm.c > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Tested-by: Samuel Holland <samuel@sholland.org> > --- > RFC -> v2 > * Dropped vdso_default static global variable > * Used IS_ENABLED() macro > * Included Tested-by from Samuel > > RFC: > https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221211215843.24024-1-prabhakar.mahadev-lad.rj@bp.renesas.com/ > --- > arch/riscv/Kconfig | 1 - > arch/riscv/kernel/time.c | 9 --------- > drivers/clocksource/timer-riscv.c | 5 +++++ > 3 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index e2b656043abf..9c687da7756d 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -12,7 +12,6 @@ config 32BIT > > config RISCV > def_bool y > - select ARCH_CLOCKSOURCE_INIT > select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION > select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2 > select ARCH_HAS_BINFMT_FLAT > diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c > index 8217b0f67c6c..42bee305e997 100644 > --- a/arch/riscv/kernel/time.c > +++ b/arch/riscv/kernel/time.c > @@ -30,12 +30,3 @@ void __init time_init(void) > of_clk_init(NULL); > timer_probe(); > } > - > -void clocksource_arch_init(struct clocksource *cs) > -{ > -#ifdef CONFIG_GENERIC_GETTIMEOFDAY > - cs->vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER; > -#else > - cs->vdso_clock_mode = VDSO_CLOCKMODE_NONE; > -#endif > -} > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 55dad7965f43..c416e5e934bf 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -77,6 +77,11 @@ static struct clocksource riscv_clocksource = { > .mask = CLOCKSOURCE_MASK(64), > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > .read = riscv_clocksource_rdtime, > +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY) > + .vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER, > +#else > + .vdso_clock_mode = VDSO_CLOCKMODE_NONE, The else case isn't strictly necessary because VDSO_CLOCKMODE_NONE is the default. Either way it looks fine to me. Reviewed-by: Samuel Holland <samuel@sholland.org> And I tested v2 as well. Regards, Samuel > +#endif > }; > > static int riscv_timer_starting_cpu(unsigned int cpu)
On Thu, Dec 29, 2022 at 10:50 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Having a clocksource_arch_init() callback always sets vdso_clock_mode to > VDSO_CLOCKMODE_ARCHTIMER if GENERIC_GETTIMEOFDAY is enabled, this is > required for the riscv-timer. > > This works for platforms where just riscv-timer clocksource is present. > On platforms where other clock sources are available we want them to > register with vdso_clock_mode set to VDSO_CLOCKMODE_NONE. > > On the Renesas RZ/Five SoC OSTM block can be used as clocksource [0], to > avoid multiple clock sources being registered as VDSO_CLOCKMODE_ARCHTIMER > move setting of vdso_clock_mode in the riscv-timer driver instead of doing > this in clocksource_arch_init() callback as done similarly for ARM/64 > architecture. > > [0] drivers/clocksource/renesas-ostm.c > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Tested-by: Samuel Holland <samuel@sholland.org> > --- > RFC -> v2 > * Dropped vdso_default static global variable > * Used IS_ENABLED() macro > * Included Tested-by from Samuel > > RFC: > https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221211215843.24024-1-prabhakar.mahadev-lad.rj@bp.renesas.com/ > --- > arch/riscv/Kconfig | 1 - > arch/riscv/kernel/time.c | 9 --------- > drivers/clocksource/timer-riscv.c | 5 +++++ > 3 files changed, 5 insertions(+), 10 deletions(-) > Gentle ping. Cheers, Prabhakar > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index e2b656043abf..9c687da7756d 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -12,7 +12,6 @@ config 32BIT > > config RISCV > def_bool y > - select ARCH_CLOCKSOURCE_INIT > select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION > select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2 > select ARCH_HAS_BINFMT_FLAT > diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c > index 8217b0f67c6c..42bee305e997 100644 > --- a/arch/riscv/kernel/time.c > +++ b/arch/riscv/kernel/time.c > @@ -30,12 +30,3 @@ void __init time_init(void) > of_clk_init(NULL); > timer_probe(); > } > - > -void clocksource_arch_init(struct clocksource *cs) > -{ > -#ifdef CONFIG_GENERIC_GETTIMEOFDAY > - cs->vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER; > -#else > - cs->vdso_clock_mode = VDSO_CLOCKMODE_NONE; > -#endif > -} > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 55dad7965f43..c416e5e934bf 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -77,6 +77,11 @@ static struct clocksource riscv_clocksource = { > .mask = CLOCKSOURCE_MASK(64), > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > .read = riscv_clocksource_rdtime, > +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY) > + .vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER, > +#else > + .vdso_clock_mode = VDSO_CLOCKMODE_NONE, > +#endif > }; > > static int riscv_timer_starting_cpu(unsigned int cpu) > -- > 2.25.1 >
On Thu, Dec 29, 2022 at 10:46:01PM +0000, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Having a clocksource_arch_init() callback always sets vdso_clock_mode to > VDSO_CLOCKMODE_ARCHTIMER if GENERIC_GETTIMEOFDAY is enabled, this is > required for the riscv-timer. > > This works for platforms where just riscv-timer clocksource is present. > On platforms where other clock sources are available we want them to > register with vdso_clock_mode set to VDSO_CLOCKMODE_NONE. > > On the Renesas RZ/Five SoC OSTM block can be used as clocksource [0], to > avoid multiple clock sources being registered as VDSO_CLOCKMODE_ARCHTIMER > move setting of vdso_clock_mode in the riscv-timer driver instead of doing > this in clocksource_arch_init() callback as done similarly for ARM/64 > architecture. > > [0] drivers/clocksource/renesas-ostm.c > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Tested-by: Samuel Holland <samuel@sholland.org> > --- > RFC -> v2 > * Dropped vdso_default static global variable > * Used IS_ENABLED() macro Not sure it really makes much difference here either way, but increased coverage is always good I suppose. rv32 is the user for the !GENERIC_GETTIMEOFDAY patch IIRC and nothing seemed obviously amiss there to me either. Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
On 29/12/2022 23:46, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Having a clocksource_arch_init() callback always sets vdso_clock_mode to > VDSO_CLOCKMODE_ARCHTIMER if GENERIC_GETTIMEOFDAY is enabled, this is > required for the riscv-timer. > > This works for platforms where just riscv-timer clocksource is present. > On platforms where other clock sources are available we want them to > register with vdso_clock_mode set to VDSO_CLOCKMODE_NONE. > > On the Renesas RZ/Five SoC OSTM block can be used as clocksource [0], to > avoid multiple clock sources being registered as VDSO_CLOCKMODE_ARCHTIMER > move setting of vdso_clock_mode in the riscv-timer driver instead of doing > this in clocksource_arch_init() callback as done similarly for ARM/64 > architecture. > > [0] drivers/clocksource/renesas-ostm.c > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Tested-by: Samuel Holland <samuel@sholland.org> > --- Applied, I had a trivial conflict which was fixed. Please have a look on the patch once it is push in the timers/next branch Thanks
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index e2b656043abf..9c687da7756d 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -12,7 +12,6 @@ config 32BIT config RISCV def_bool y - select ARCH_CLOCKSOURCE_INIT select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2 select ARCH_HAS_BINFMT_FLAT diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c index 8217b0f67c6c..42bee305e997 100644 --- a/arch/riscv/kernel/time.c +++ b/arch/riscv/kernel/time.c @@ -30,12 +30,3 @@ void __init time_init(void) of_clk_init(NULL); timer_probe(); } - -void clocksource_arch_init(struct clocksource *cs) -{ -#ifdef CONFIG_GENERIC_GETTIMEOFDAY - cs->vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER; -#else - cs->vdso_clock_mode = VDSO_CLOCKMODE_NONE; -#endif -} diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 55dad7965f43..c416e5e934bf 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -77,6 +77,11 @@ static struct clocksource riscv_clocksource = { .mask = CLOCKSOURCE_MASK(64), .flags = CLOCK_SOURCE_IS_CONTINUOUS, .read = riscv_clocksource_rdtime, +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY) + .vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER, +#else + .vdso_clock_mode = VDSO_CLOCKMODE_NONE, +#endif }; static int riscv_timer_starting_cpu(unsigned int cpu)