| Message ID | 1436277927-20159-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Simon Horman |
| Headers | show |
Hi Geert, Thanks for your patches!! On Tue, Jul 7, 2015 at 11:05 PM, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > Since commit 731bde02f622d694 ("ARM: shmobile: Select > HAVE_ARM_ARCH_TIMER from Kconfig"), CONFIG_ARM_ARCH_TIMER is always set > when building for R-Car Gen2 SoCs, as it is selected by > CONFIG_HAVE_ARM_ARCH_TIMER. Remove the obsolete #ifdef testing for it. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > arch/arm/mach-shmobile/setup-rcar-gen2.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c > index aa3339258d9c0232..570d6bd3784f7464 100644 > --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c > +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c > @@ -52,7 +52,6 @@ u32 rcar_gen2_read_mode_pins(void) > void __init rcar_gen2_timer_init(void) > { > u32 mode = rcar_gen2_read_mode_pins(); > -#ifdef CONFIG_ARM_ARCH_TIMER > void __iomem *base; > int extal_mhz = 0; > u32 freq; > @@ -125,7 +124,6 @@ void __init rcar_gen2_timer_init(void) > } > > iounmap(base); > -#endif /* CONFIG_ARM_ARCH_TIMER */ Uhm, these two variables at least used to work differently: CONFIG_HAVE_ARM_ARCH_TIMER - hardware may include arch timer CONFIG_ARM_ARCH_TIMER - user enabled arch timer device driver In arch/arm/Kconfig, isn't CONFIG_ARM_ARCH_TIMER user selectable? I like that users can select and deselect support for various timers, but if the user can't select then I think your patch is correct. =) Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus, On Tue, Jul 7, 2015 at 5:17 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > On Tue, Jul 7, 2015 at 11:05 PM, Geert Uytterhoeven > <geert+renesas@glider.be> wrote: >> Since commit 731bde02f622d694 ("ARM: shmobile: Select >> HAVE_ARM_ARCH_TIMER from Kconfig"), CONFIG_ARM_ARCH_TIMER is always set >> when building for R-Car Gen2 SoCs, as it is selected by >> CONFIG_HAVE_ARM_ARCH_TIMER. Remove the obsolete #ifdef testing for it. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> arch/arm/mach-shmobile/setup-rcar-gen2.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c >> index aa3339258d9c0232..570d6bd3784f7464 100644 >> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c >> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c >> @@ -52,7 +52,6 @@ u32 rcar_gen2_read_mode_pins(void) >> void __init rcar_gen2_timer_init(void) >> { >> u32 mode = rcar_gen2_read_mode_pins(); >> -#ifdef CONFIG_ARM_ARCH_TIMER >> void __iomem *base; >> int extal_mhz = 0; >> u32 freq; >> @@ -125,7 +124,6 @@ void __init rcar_gen2_timer_init(void) >> } >> >> iounmap(base); >> -#endif /* CONFIG_ARM_ARCH_TIMER */ > > Uhm, these two variables at least used to work differently: > CONFIG_HAVE_ARM_ARCH_TIMER - hardware may include arch timer > CONFIG_ARM_ARCH_TIMER - user enabled arch timer device driver > > In arch/arm/Kconfig, isn't CONFIG_ARM_ARCH_TIMER user selectable? > > I like that users can select and deselect support for various timers, > but if the user can't select then I think your patch is correct. =) HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that unconditional for R-Car Gen2 and APE6 ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Magnus, > > On Tue, Jul 7, 2015 at 5:17 PM, Magnus Damm <magnus.damm@gmail.com> wrote: >> On Tue, Jul 7, 2015 at 11:05 PM, Geert Uytterhoeven >> <geert+renesas@glider.be> wrote: >>> Since commit 731bde02f622d694 ("ARM: shmobile: Select >>> HAVE_ARM_ARCH_TIMER from Kconfig"), CONFIG_ARM_ARCH_TIMER is always set >>> when building for R-Car Gen2 SoCs, as it is selected by >>> CONFIG_HAVE_ARM_ARCH_TIMER. Remove the obsolete #ifdef testing for it. >>> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>> --- >>> arch/arm/mach-shmobile/setup-rcar-gen2.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c >>> index aa3339258d9c0232..570d6bd3784f7464 100644 >>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c >>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c >>> @@ -52,7 +52,6 @@ u32 rcar_gen2_read_mode_pins(void) >>> void __init rcar_gen2_timer_init(void) >>> { >>> u32 mode = rcar_gen2_read_mode_pins(); >>> -#ifdef CONFIG_ARM_ARCH_TIMER >>> void __iomem *base; >>> int extal_mhz = 0; >>> u32 freq; >>> @@ -125,7 +124,6 @@ void __init rcar_gen2_timer_init(void) >>> } >>> >>> iounmap(base); >>> -#endif /* CONFIG_ARM_ARCH_TIMER */ >> >> Uhm, these two variables at least used to work differently: >> CONFIG_HAVE_ARM_ARCH_TIMER - hardware may include arch timer >> CONFIG_ARM_ARCH_TIMER - user enabled arch timer device driver >> >> In arch/arm/Kconfig, isn't CONFIG_ARM_ARCH_TIMER user selectable? >> >> I like that users can select and deselect support for various timers, >> but if the user can't select then I think your patch is correct. =) > > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that > unconditional for R-Car Gen2 and APE6 ;-) Ouch - my bad! That's not at all what I intended to do. I only wanted to express that R-Car Gen2 and APE6 come with arch timer hardware, and in such case the user shall be able to select it. If R-Car Gen2 or APE6 isn't selected then there is no point in exposing such choice to the user. But obviously that wasn't what I ended up doing... What would be a sane way to approach this? I think always enabling it may not work for CA7-based SoCs from Renesas that at least used to have setup issues with arch timer... / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: > Hi Geert, > > On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: > > Hi Magnus, > > > > On Tue, Jul 7, 2015 at 5:17 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > >> On Tue, Jul 7, 2015 at 11:05 PM, Geert Uytterhoeven > >> <geert+renesas@glider.be> wrote: > >>> Since commit 731bde02f622d694 ("ARM: shmobile: Select > >>> HAVE_ARM_ARCH_TIMER from Kconfig"), CONFIG_ARM_ARCH_TIMER is always set > >>> when building for R-Car Gen2 SoCs, as it is selected by > >>> CONFIG_HAVE_ARM_ARCH_TIMER. Remove the obsolete #ifdef testing for it. > >>> > >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > >>> --- > >>> arch/arm/mach-shmobile/setup-rcar-gen2.c | 2 -- > >>> 1 file changed, 2 deletions(-) > >>> > >>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c > >>> index aa3339258d9c0232..570d6bd3784f7464 100644 > >>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c > >>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c > >>> @@ -52,7 +52,6 @@ u32 rcar_gen2_read_mode_pins(void) > >>> void __init rcar_gen2_timer_init(void) > >>> { > >>> u32 mode = rcar_gen2_read_mode_pins(); > >>> -#ifdef CONFIG_ARM_ARCH_TIMER > >>> void __iomem *base; > >>> int extal_mhz = 0; > >>> u32 freq; > >>> @@ -125,7 +124,6 @@ void __init rcar_gen2_timer_init(void) > >>> } > >>> > >>> iounmap(base); > >>> -#endif /* CONFIG_ARM_ARCH_TIMER */ > >> > >> Uhm, these two variables at least used to work differently: > >> CONFIG_HAVE_ARM_ARCH_TIMER - hardware may include arch timer > >> CONFIG_ARM_ARCH_TIMER - user enabled arch timer device driver > >> > >> In arch/arm/Kconfig, isn't CONFIG_ARM_ARCH_TIMER user selectable? > >> > >> I like that users can select and deselect support for various timers, > >> but if the user can't select then I think your patch is correct. =) > > > > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch > > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that > > unconditional for R-Car Gen2 and APE6 ;-) > > Ouch - my bad! That's not at all what I intended to do. > > I only wanted to express that R-Car Gen2 and APE6 come with arch timer > hardware, and in such case the user shall be able to select it. If > R-Car Gen2 or APE6 isn't selected then there is no point in exposing > such choice to the user. But obviously that wasn't what I ended up > doing... > > What would be a sane way to approach this? I think always enabling it > may not work for CA7-based SoCs from Renesas that at least used to > have setup issues with arch timer... Given the current arrangement in arch/arm/Kconfig: config HAVE_ARM_ARCH_TIMER bool "Architected timer support" depends on CPU_V7 select ARM_ARCH_TIMER select GENERIC_CLOCKEVENTS help This option enables support for the ARM architected timer It seems to me that a good solution would be to (go back to?) * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate Perhaps that can be achieved by dropping/reverting * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig At this stage I have not sent pull-requests for the above so I would be feasible for me to simply drop them if we agree on that. FWIW, I assumed that your patch would achieve its goal. The arrangement of HAVE_ARM_ARCH_TIMER and ARM_ARCH_TIMER seems entirely intuitive to me. But I'm reluctant to advise opening that Pandora's box. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Simon, On Wed, Jul 8, 2015 at 10:16 AM, Simon Horman <horms@verge.net.au> wrote: > On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: >> Hi Geert, >> >> On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven >> <geert@linux-m68k.org> wrote: >> > Hi Magnus, >> > >> > On Tue, Jul 7, 2015 at 5:17 PM, Magnus Damm <magnus.damm@gmail.com> wrote: >> >> On Tue, Jul 7, 2015 at 11:05 PM, Geert Uytterhoeven >> >> <geert+renesas@glider.be> wrote: >> >>> Since commit 731bde02f622d694 ("ARM: shmobile: Select >> >>> HAVE_ARM_ARCH_TIMER from Kconfig"), CONFIG_ARM_ARCH_TIMER is always set >> >>> when building for R-Car Gen2 SoCs, as it is selected by >> >>> CONFIG_HAVE_ARM_ARCH_TIMER. Remove the obsolete #ifdef testing for it. >> >>> >> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> >>> --- >> >>> arch/arm/mach-shmobile/setup-rcar-gen2.c | 2 -- >> >>> 1 file changed, 2 deletions(-) >> >>> >> >>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c >> >>> index aa3339258d9c0232..570d6bd3784f7464 100644 >> >>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c >> >>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c >> >>> @@ -52,7 +52,6 @@ u32 rcar_gen2_read_mode_pins(void) >> >>> void __init rcar_gen2_timer_init(void) >> >>> { >> >>> u32 mode = rcar_gen2_read_mode_pins(); >> >>> -#ifdef CONFIG_ARM_ARCH_TIMER >> >>> void __iomem *base; >> >>> int extal_mhz = 0; >> >>> u32 freq; >> >>> @@ -125,7 +124,6 @@ void __init rcar_gen2_timer_init(void) >> >>> } >> >>> >> >>> iounmap(base); >> >>> -#endif /* CONFIG_ARM_ARCH_TIMER */ >> >> >> >> Uhm, these two variables at least used to work differently: >> >> CONFIG_HAVE_ARM_ARCH_TIMER - hardware may include arch timer >> >> CONFIG_ARM_ARCH_TIMER - user enabled arch timer device driver >> >> >> >> In arch/arm/Kconfig, isn't CONFIG_ARM_ARCH_TIMER user selectable? >> >> >> >> I like that users can select and deselect support for various timers, >> >> but if the user can't select then I think your patch is correct. =) >> > >> > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch >> > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that >> > unconditional for R-Car Gen2 and APE6 ;-) >> >> Ouch - my bad! That's not at all what I intended to do. >> >> I only wanted to express that R-Car Gen2 and APE6 come with arch timer >> hardware, and in such case the user shall be able to select it. If >> R-Car Gen2 or APE6 isn't selected then there is no point in exposing >> such choice to the user. But obviously that wasn't what I ended up >> doing... >> >> What would be a sane way to approach this? I think always enabling it >> may not work for CA7-based SoCs from Renesas that at least used to >> have setup issues with arch timer... > > Given the current arrangement in arch/arm/Kconfig: > > config HAVE_ARM_ARCH_TIMER > bool "Architected timer support" > depends on CPU_V7 > select ARM_ARCH_TIMER > select GENERIC_CLOCKEVENTS > help > This option enables support for the ARM architected timer > > It seems to me that a good solution would be to (go back to?) > * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; > * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate That is one idea for sure. > Perhaps that can be achieved by dropping/reverting > * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig > * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig > > At this stage I have not sent pull-requests for the above so I would > be feasible for me to simply drop them if we agree on that. From my side simply dropping seems the easiest way forward. This reminds me of the good old "generating kernel configuration from DT", so in the future if we may be able to do so then that could perhaps that shmobile_defconfig bit for us. > FWIW, I assumed that your patch would achieve its goal. The arrangement of > HAVE_ARM_ARCH_TIMER and ARM_ARCH_TIMER seems entirely intuitive to me. But > I'm reluctant to advise opening that Pandora's box. Completely agree, but from my side you need to s/intuitive/counter intuitive/g. =) / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus, On Wed, Jul 08, 2015 at 10:40:18AM +0900, Magnus Damm wrote: > Hi Simon, > > On Wed, Jul 8, 2015 at 10:16 AM, Simon Horman <horms@verge.net.au> wrote: > > On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: > >> Hi Geert, > >> > >> On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven > >> <geert@linux-m68k.org> wrote: > >> > Hi Magnus, > >> > [snip] > >> > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch > >> > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that > >> > unconditional for R-Car Gen2 and APE6 ;-) > >> > >> Ouch - my bad! That's not at all what I intended to do. > >> > >> I only wanted to express that R-Car Gen2 and APE6 come with arch timer > >> hardware, and in such case the user shall be able to select it. If > >> R-Car Gen2 or APE6 isn't selected then there is no point in exposing > >> such choice to the user. But obviously that wasn't what I ended up > >> doing... > >> > >> What would be a sane way to approach this? I think always enabling it > >> may not work for CA7-based SoCs from Renesas that at least used to > >> have setup issues with arch timer... > > > > Given the current arrangement in arch/arm/Kconfig: > > > > config HAVE_ARM_ARCH_TIMER > > bool "Architected timer support" > > depends on CPU_V7 > > select ARM_ARCH_TIMER > > select GENERIC_CLOCKEVENTS > > help > > This option enables support for the ARM architected timer > > > > It seems to me that a good solution would be to (go back to?) > > * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; > > * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate > > That is one idea for sure. > > > Perhaps that can be achieved by dropping/reverting > > * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig > > * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig > > > > At this stage I have not sent pull-requests for the above so I would > > be feasible for me to simply drop them if we agree on that. > > >From my side simply dropping seems the easiest way forward. Me too. Lets sit on this for a day or so and see what Geert and others have to say. > This reminds me of the good old "generating kernel configuration from > DT", so in the future if we may be able to do so then that could > perhaps that shmobile_defconfig bit for us. :) > > FWIW, I assumed that your patch would achieve its goal. The arrangement of > > HAVE_ARM_ARCH_TIMER and ARM_ARCH_TIMER seems entirely intuitive to me. But > > I'm reluctant to advise opening that Pandora's box. > > Completely agree, but from my side you need to s/intuitive/counter > intuitive/g. =) Yes, sorry for that editing error. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Simon, Magnus, On Wed, Jul 8, 2015 at 4:05 AM, Simon Horman <horms@verge.net.au> wrote: > On Wed, Jul 08, 2015 at 10:40:18AM +0900, Magnus Damm wrote: >> On Wed, Jul 8, 2015 at 10:16 AM, Simon Horman <horms@verge.net.au> wrote: >> > On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: >> >> On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven >> >> <geert@linux-m68k.org> wrote: >> >> > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch >> >> > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that >> >> > unconditional for R-Car Gen2 and APE6 ;-) >> >> >> >> Ouch - my bad! That's not at all what I intended to do. >> >> >> >> I only wanted to express that R-Car Gen2 and APE6 come with arch timer >> >> hardware, and in such case the user shall be able to select it. If >> >> R-Car Gen2 or APE6 isn't selected then there is no point in exposing >> >> such choice to the user. But obviously that wasn't what I ended up >> >> doing... >> >> >> >> What would be a sane way to approach this? I think always enabling it >> >> may not work for CA7-based SoCs from Renesas that at least used to >> >> have setup issues with arch timer... >> > >> > Given the current arrangement in arch/arm/Kconfig: >> > >> > config HAVE_ARM_ARCH_TIMER >> > bool "Architected timer support" >> > depends on CPU_V7 >> > select ARM_ARCH_TIMER >> > select GENERIC_CLOCKEVENTS >> > help >> > This option enables support for the ARM architected timer >> > >> > It seems to me that a good solution would be to (go back to?) >> > * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; >> > * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate >> >> That is one idea for sure. >> >> > Perhaps that can be achieved by dropping/reverting >> > * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig >> > * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig >> > >> > At this stage I have not sent pull-requests for the above so I would >> > be feasible for me to simply drop them if we agree on that. >> >> >From my side simply dropping seems the easiest way forward. > > Me too. > Lets sit on this for a day or so and see what Geert and others have to say. Typically the "HAVE_*" config symbols are meant to be selected in Kconfig fragments, not to be configurable by the user, so Magnus' patches did make sense to me. ARM_ARCH_TIMER is the corresponding user-configurable config option, but it is auto-selected by HAVE_ARM_ARCH_TIMER these days. $ git kgrep HAVE_ARM_ARCH_TIMER arch/arm/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/Kconfig:config HAVE_ARM_ARCH_TIMER arch/arm/mach-alpine/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-axxia/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-exynos/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-hisi/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-imx/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-keystone/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-qcom/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-rockchip/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-sunxi/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER (`git kgrep' is aliased to `!git grep $* -- "*Kconf*"') Looking at e.g. arch/arm/mach-omap2/Kconfig, a mixed omap 3/4/5 multi-platform kernel would have this enabled, too, while it was selected for omap5 only. If having this enabled in Kconfig causes regressions (on CA7 --- I don't have a board with enabled CA7 support), the two patches should be reverted. However, given CONFIG_HAVE_ARM_ARCH_TIMER was enabled in shmobile_defconfig before, I believe such an issue on CA7 would have been present before, when using shmobile_defconfig? Or were all CA7 users rolling their own config files, not enabling it? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 08, 2015 at 08:56:20AM +0200, Geert Uytterhoeven wrote: > Hi Simon, Magnus, > > On Wed, Jul 8, 2015 at 4:05 AM, Simon Horman <horms@verge.net.au> wrote: > > On Wed, Jul 08, 2015 at 10:40:18AM +0900, Magnus Damm wrote: > >> On Wed, Jul 8, 2015 at 10:16 AM, Simon Horman <horms@verge.net.au> wrote: > >> > On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: > >> >> On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven > >> >> <geert@linux-m68k.org> wrote: > >> >> > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch > >> >> > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that > >> >> > unconditional for R-Car Gen2 and APE6 ;-) > >> >> > >> >> Ouch - my bad! That's not at all what I intended to do. > >> >> > >> >> I only wanted to express that R-Car Gen2 and APE6 come with arch timer > >> >> hardware, and in such case the user shall be able to select it. If > >> >> R-Car Gen2 or APE6 isn't selected then there is no point in exposing > >> >> such choice to the user. But obviously that wasn't what I ended up > >> >> doing... > >> >> > >> >> What would be a sane way to approach this? I think always enabling it > >> >> may not work for CA7-based SoCs from Renesas that at least used to > >> >> have setup issues with arch timer... > >> > > >> > Given the current arrangement in arch/arm/Kconfig: > >> > > >> > config HAVE_ARM_ARCH_TIMER > >> > bool "Architected timer support" > >> > depends on CPU_V7 > >> > select ARM_ARCH_TIMER > >> > select GENERIC_CLOCKEVENTS > >> > help > >> > This option enables support for the ARM architected timer > >> > > >> > It seems to me that a good solution would be to (go back to?) > >> > * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; > >> > * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate > >> > >> That is one idea for sure. > >> > >> > Perhaps that can be achieved by dropping/reverting > >> > * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig > >> > * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig > >> > > >> > At this stage I have not sent pull-requests for the above so I would > >> > be feasible for me to simply drop them if we agree on that. > >> > >> >From my side simply dropping seems the easiest way forward. > > > > Me too. > > Lets sit on this for a day or so and see what Geert and others have to say. > > Typically the "HAVE_*" config symbols are meant to be selected in > Kconfig fragments, not to be configurable by the user, so Magnus' patches > did make sense to me. > > ARM_ARCH_TIMER is the corresponding user-configurable config option, > but it is auto-selected by HAVE_ARM_ARCH_TIMER these days. > > $ git kgrep HAVE_ARM_ARCH_TIMER > arch/arm/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/Kconfig:config HAVE_ARM_ARCH_TIMER > arch/arm/mach-alpine/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-axxia/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-exynos/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-hisi/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-imx/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-keystone/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-qcom/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-rockchip/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-sunxi/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER > arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER > > (`git kgrep' is aliased to `!git grep $* -- "*Kconf*"') > > Looking at e.g. arch/arm/mach-omap2/Kconfig, a mixed omap 3/4/5 > multi-platform kernel would have this enabled, too, while it was selected > for omap5 only. > > If having this enabled in Kconfig causes regressions (on CA7 --- I don't have > a board with enabled CA7 support), the two patches should be reverted. > > However, given CONFIG_HAVE_ARM_ARCH_TIMER was enabled in > shmobile_defconfig before, I believe such an issue on CA7 would have been > present before, when using shmobile_defconfig? Or were all CA7 users rolling > their own config files, not enabling it? I think that the issue is that previously users could select arch-timer, although it was configured by default, but now they can't. It seem odd to me that HAVE_ARM_ARCH_TIMER selects ARM_ARCH_TIMER, but it does. And with that in mind, if we want to still allow users to select arch-timer, then we should drop the patches. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 10, 2015 at 01:26:44PM +0900, Simon Horman wrote: > On Wed, Jul 08, 2015 at 08:56:20AM +0200, Geert Uytterhoeven wrote: > > Hi Simon, Magnus, > > > > On Wed, Jul 8, 2015 at 4:05 AM, Simon Horman <horms@verge.net.au> wrote: > > > On Wed, Jul 08, 2015 at 10:40:18AM +0900, Magnus Damm wrote: > > >> On Wed, Jul 8, 2015 at 10:16 AM, Simon Horman <horms@verge.net.au> wrote: > > >> > On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: > > >> >> On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven > > >> >> <geert@linux-m68k.org> wrote: > > >> >> > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch > > >> >> > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that > > >> >> > unconditional for R-Car Gen2 and APE6 ;-) > > >> >> > > >> >> Ouch - my bad! That's not at all what I intended to do. > > >> >> > > >> >> I only wanted to express that R-Car Gen2 and APE6 come with arch timer > > >> >> hardware, and in such case the user shall be able to select it. If > > >> >> R-Car Gen2 or APE6 isn't selected then there is no point in exposing > > >> >> such choice to the user. But obviously that wasn't what I ended up > > >> >> doing... > > >> >> > > >> >> What would be a sane way to approach this? I think always enabling it > > >> >> may not work for CA7-based SoCs from Renesas that at least used to > > >> >> have setup issues with arch timer... > > >> > > > >> > Given the current arrangement in arch/arm/Kconfig: > > >> > > > >> > config HAVE_ARM_ARCH_TIMER > > >> > bool "Architected timer support" > > >> > depends on CPU_V7 > > >> > select ARM_ARCH_TIMER > > >> > select GENERIC_CLOCKEVENTS > > >> > help > > >> > This option enables support for the ARM architected timer > > >> > > > >> > It seems to me that a good solution would be to (go back to?) > > >> > * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; > > >> > * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate > > >> > > >> That is one idea for sure. > > >> > > >> > Perhaps that can be achieved by dropping/reverting > > >> > * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig > > >> > * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig > > >> > > > >> > At this stage I have not sent pull-requests for the above so I would > > >> > be feasible for me to simply drop them if we agree on that. > > >> > > >> >From my side simply dropping seems the easiest way forward. > > > > > > Me too. > > > Lets sit on this for a day or so and see what Geert and others have to say. > > > > Typically the "HAVE_*" config symbols are meant to be selected in > > Kconfig fragments, not to be configurable by the user, so Magnus' patches > > did make sense to me. > > > > ARM_ARCH_TIMER is the corresponding user-configurable config option, > > but it is auto-selected by HAVE_ARM_ARCH_TIMER these days. > > > > $ git kgrep HAVE_ARM_ARCH_TIMER > > arch/arm/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/Kconfig:config HAVE_ARM_ARCH_TIMER > > arch/arm/mach-alpine/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-axxia/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-exynos/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-hisi/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-imx/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-keystone/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-qcom/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-rockchip/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-sunxi/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER > > arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER > > > > (`git kgrep' is aliased to `!git grep $* -- "*Kconf*"') > > > > Looking at e.g. arch/arm/mach-omap2/Kconfig, a mixed omap 3/4/5 > > multi-platform kernel would have this enabled, too, while it was selected > > for omap5 only. > > > > If having this enabled in Kconfig causes regressions (on CA7 --- I don't have > > a board with enabled CA7 support), the two patches should be reverted. > > > > However, given CONFIG_HAVE_ARM_ARCH_TIMER was enabled in > > shmobile_defconfig before, I believe such an issue on CA7 would have been > > present before, when using shmobile_defconfig? Or were all CA7 users rolling > > their own config files, not enabling it? > > I think that the issue is that previously users could select arch-timer, > although it was configured by default, but now they can't. > > It seem odd to me that HAVE_ARM_ARCH_TIMER selects ARM_ARCH_TIMER, but > it does. And with that in mind, if we want to still allow users to > select arch-timer, then we should drop the patches. As they are blocking the progress of other, unrelated, patches into arm-soc I have dropped the above mentioned patches (731bde02f622, b46ddcdf484c) from next pending some consensus on if and how to move forwards. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 13, 2015 at 10:23 AM, Simon Horman <horms@verge.net.au> wrote: > On Fri, Jul 10, 2015 at 01:26:44PM +0900, Simon Horman wrote: >> On Wed, Jul 08, 2015 at 08:56:20AM +0200, Geert Uytterhoeven wrote: >> > Hi Simon, Magnus, >> > >> > On Wed, Jul 8, 2015 at 4:05 AM, Simon Horman <horms@verge.net.au> wrote: >> > > On Wed, Jul 08, 2015 at 10:40:18AM +0900, Magnus Damm wrote: >> > >> On Wed, Jul 8, 2015 at 10:16 AM, Simon Horman <horms@verge.net.au> wrote: >> > >> > On Wed, Jul 08, 2015 at 12:34:47AM +0900, Magnus Damm wrote: >> > >> >> On Wed, Jul 8, 2015 at 12:21 AM, Geert Uytterhoeven >> > >> >> <geert@linux-m68k.org> wrote: >> > >> >> > HAVE_ARM_ARCH_TIMER used to be user-selectable, but your patch >> > >> >> > "ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig" made that >> > >> >> > unconditional for R-Car Gen2 and APE6 ;-) >> > >> >> >> > >> >> Ouch - my bad! That's not at all what I intended to do. >> > >> >> >> > >> >> I only wanted to express that R-Car Gen2 and APE6 come with arch timer >> > >> >> hardware, and in such case the user shall be able to select it. If >> > >> >> R-Car Gen2 or APE6 isn't selected then there is no point in exposing >> > >> >> such choice to the user. But obviously that wasn't what I ended up >> > >> >> doing... >> > >> >> >> > >> >> What would be a sane way to approach this? I think always enabling it >> > >> >> may not work for CA7-based SoCs from Renesas that at least used to >> > >> >> have setup issues with arch timer... >> > >> > >> > >> > Given the current arrangement in arch/arm/Kconfig: >> > >> > >> > >> > config HAVE_ARM_ARCH_TIMER >> > >> > bool "Architected timer support" >> > >> > depends on CPU_V7 >> > >> > select ARM_ARCH_TIMER >> > >> > select GENERIC_CLOCKEVENTS >> > >> > help >> > >> > This option enables support for the ARM architected timer >> > >> > >> > >> > It seems to me that a good solution would be to (go back to?) >> > >> > * not selecting HAVE_ARM_ARCH_TIMER in arch/arm/mach-shmobile/Kconfig and; >> > >> > * enabling HAVE_ARM_ARCH_TIMER in defconfigs as appropriate >> > >> >> > >> That is one idea for sure. >> > >> >> > >> > Perhaps that can be achieved by dropping/reverting >> > >> > * 731bde02f622 ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig >> > >> > * b46ddcdf484c ARM: shmobile: Drop HAVE_ARM_ARCH_TIMER from defconfig >> > >> > >> > >> > At this stage I have not sent pull-requests for the above so I would >> > >> > be feasible for me to simply drop them if we agree on that. >> > >> >> > >> >From my side simply dropping seems the easiest way forward. >> > > >> > > Me too. >> > > Lets sit on this for a day or so and see what Geert and others have to say. >> > >> > Typically the "HAVE_*" config symbols are meant to be selected in >> > Kconfig fragments, not to be configurable by the user, so Magnus' patches >> > did make sense to me. >> > >> > ARM_ARCH_TIMER is the corresponding user-configurable config option, >> > but it is auto-selected by HAVE_ARM_ARCH_TIMER these days. >> > >> > $ git kgrep HAVE_ARM_ARCH_TIMER >> > arch/arm/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/Kconfig:config HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-alpine/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-axxia/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-bcm/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-exynos/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-hisi/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-imx/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-keystone/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-omap2/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-qcom/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-rockchip/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-shmobile/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-sunxi/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER >> > arch/arm/mach-tegra/Kconfig: select HAVE_ARM_ARCH_TIMER >> > >> > (`git kgrep' is aliased to `!git grep $* -- "*Kconf*"') >> > >> > Looking at e.g. arch/arm/mach-omap2/Kconfig, a mixed omap 3/4/5 >> > multi-platform kernel would have this enabled, too, while it was selected >> > for omap5 only. >> > >> > If having this enabled in Kconfig causes regressions (on CA7 --- I don't have >> > a board with enabled CA7 support), the two patches should be reverted. >> > >> > However, given CONFIG_HAVE_ARM_ARCH_TIMER was enabled in >> > shmobile_defconfig before, I believe such an issue on CA7 would have been >> > present before, when using shmobile_defconfig? Or were all CA7 users rolling >> > their own config files, not enabling it? >> >> I think that the issue is that previously users could select arch-timer, >> although it was configured by default, but now they can't. >> >> It seem odd to me that HAVE_ARM_ARCH_TIMER selects ARM_ARCH_TIMER, but >> it does. And with that in mind, if we want to still allow users to >> select arch-timer, then we should drop the patches. > > As they are blocking the progress of other, unrelated, patches into arm-soc > I have dropped the above mentioned patches (731bde02f622, b46ddcdf484c) > from next pending some consensus on if and how to move forwards. Sounds good, thank you! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Wed, Jul 8, 2015 at 3:56 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > However, given CONFIG_HAVE_ARM_ARCH_TIMER was enabled in > shmobile_defconfig before, I believe such an issue on CA7 would have been > present before, when using shmobile_defconfig? Or were all CA7 users rolling > their own config files, not enabling it? I think they were rolling heir own config files! Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c index aa3339258d9c0232..570d6bd3784f7464 100644 --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c @@ -52,7 +52,6 @@ u32 rcar_gen2_read_mode_pins(void) void __init rcar_gen2_timer_init(void) { u32 mode = rcar_gen2_read_mode_pins(); -#ifdef CONFIG_ARM_ARCH_TIMER void __iomem *base; int extal_mhz = 0; u32 freq; @@ -125,7 +124,6 @@ void __init rcar_gen2_timer_init(void) } iounmap(base); -#endif /* CONFIG_ARM_ARCH_TIMER */ rcar_gen2_clocks_init(mode); clocksource_of_init();
Since commit 731bde02f622d694 ("ARM: shmobile: Select HAVE_ARM_ARCH_TIMER from Kconfig"), CONFIG_ARM_ARCH_TIMER is always set when building for R-Car Gen2 SoCs, as it is selected by CONFIG_HAVE_ARM_ARCH_TIMER. Remove the obsolete #ifdef testing for it. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- arch/arm/mach-shmobile/setup-rcar-gen2.c | 2 -- 1 file changed, 2 deletions(-)