* [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
@ 2015-07-07 14:05 Geert Uytterhoeven
2015-07-07 15:17 ` Magnus Damm
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2015-07-07 14:05 UTC (permalink / raw)
To: linux-sh
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 */
rcar_gen2_clocks_init(mode);
clocksource_of_init();
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
@ 2015-07-07 15:17 ` Magnus Damm
2015-07-07 15:21 ` Geert Uytterhoeven
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Magnus Damm @ 2015-07-07 15:17 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
2015-07-07 15:17 ` Magnus Damm
@ 2015-07-07 15:21 ` Geert Uytterhoeven
2015-07-07 15:34 ` Magnus Damm
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2015-07-07 15:21 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
2015-07-07 15:17 ` Magnus Damm
2015-07-07 15:21 ` Geert Uytterhoeven
@ 2015-07-07 15:34 ` Magnus Damm
2015-07-08 1:16 ` Simon Horman
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Magnus Damm @ 2015-07-07 15:34 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (2 preceding siblings ...)
2015-07-07 15:34 ` Magnus Damm
@ 2015-07-08 1:16 ` Simon Horman
2015-07-08 1:40 ` Magnus Damm
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2015-07-08 1:16 UTC (permalink / raw)
To: linux-sh
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (3 preceding siblings ...)
2015-07-08 1:16 ` Simon Horman
@ 2015-07-08 1:40 ` Magnus Damm
2015-07-08 2:05 ` Simon Horman
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Magnus Damm @ 2015-07-08 1:40 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (4 preceding siblings ...)
2015-07-08 1:40 ` Magnus Damm
@ 2015-07-08 2:05 ` Simon Horman
2015-07-08 6:56 ` Geert Uytterhoeven
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2015-07-08 2:05 UTC (permalink / raw)
To: linux-sh
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (5 preceding siblings ...)
2015-07-08 2:05 ` Simon Horman
@ 2015-07-08 6:56 ` Geert Uytterhoeven
2015-07-10 4:26 ` Simon Horman
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2015-07-08 6:56 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (6 preceding siblings ...)
2015-07-08 6:56 ` Geert Uytterhoeven
@ 2015-07-10 4:26 ` Simon Horman
2015-07-13 1:23 ` Simon Horman
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2015-07-10 4:26 UTC (permalink / raw)
To: linux-sh
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (7 preceding siblings ...)
2015-07-10 4:26 ` Simon Horman
@ 2015-07-13 1:23 ` Simon Horman
2015-07-13 5:20 ` Magnus Damm
2015-07-14 10:28 ` Magnus Damm
10 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2015-07-13 1:23 UTC (permalink / raw)
To: linux-sh
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (8 preceding siblings ...)
2015-07-13 1:23 ` Simon Horman
@ 2015-07-13 5:20 ` Magnus Damm
2015-07-14 10:28 ` Magnus Damm
10 siblings, 0 replies; 12+ messages in thread
From: Magnus Damm @ 2015-07-13 5:20 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
` (9 preceding siblings ...)
2015-07-13 5:20 ` Magnus Damm
@ 2015-07-14 10:28 ` Magnus Damm
10 siblings, 0 replies; 12+ messages in thread
From: Magnus Damm @ 2015-07-14 10:28 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-14 10:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-07 14:05 [PATCH] ARM: shmobile: rcar-gen2: CONFIG_ARM_ARCH_TIMER is always set Geert Uytterhoeven
2015-07-07 15:17 ` Magnus Damm
2015-07-07 15:21 ` Geert Uytterhoeven
2015-07-07 15:34 ` Magnus Damm
2015-07-08 1:16 ` Simon Horman
2015-07-08 1:40 ` Magnus Damm
2015-07-08 2:05 ` Simon Horman
2015-07-08 6:56 ` Geert Uytterhoeven
2015-07-10 4:26 ` Simon Horman
2015-07-13 1:23 ` Simon Horman
2015-07-13 5:20 ` Magnus Damm
2015-07-14 10:28 ` Magnus Damm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox