Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
Christophe Leroy writes: > Le 26/08/2019 à 13:41, Michael Ellerman a écrit : >> Christophe Leroy writes: >>> sched_clock(), used by printk(), calls __USE_RTC() to know >>> whether to use realtime clock or timebase. >>> >>> __USE_RTC() uses cpu_has_feature() which is initialised by >>> machine_init(). Before machine_init(), __USE_RTC() returns true, >>> leading to a program check exception on CPUs not having realtime >>> clock. >>> >>> In order to be able to use printk() earlier, use feature fixup. >>> Feature fixups are applies in early_init(), enabling the use of >>> printk() earlier. >>> >>> Signed-off-by: Christophe Leroy >>> --- >>> arch/powerpc/include/asm/time.h | 9 - >>> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> The other option would be just to make this a compile time decision, eg. >> add CONFIG_PPC_601 and use that to gate whether we use RTC. > > Euh ... yes OK, why not. And that would help simplify many places in the > code. I can propose something in that direction, but it will be a bigger > change. Thanks. >> Given how many 601 users there are, maybe 1?, I think that would be a >> simpler option and avoids complicating the code / binary for everyone >> else. > > However this patch doesn't complicate things more than it was with > cpu_has_feature() which does exactly the same but using static keys, > does it ? It's more complicated in that it's not using cpu_has_feature() it's doing some custom thing that is not used anywhere else. But yeah I guess it's not much extra complication. cheers
Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
Benjamin Herrenschmidt writes: > On Mon, 2019-08-26 at 21:41 +1000, Michael Ellerman wrote: >> Christophe Leroy writes: >> > sched_clock(), used by printk(), calls __USE_RTC() to know >> > whether to use realtime clock or timebase. >> > >> > __USE_RTC() uses cpu_has_feature() which is initialised by >> > machine_init(). Before machine_init(), __USE_RTC() returns true, >> > leading to a program check exception on CPUs not having realtime >> > clock. >> > >> > In order to be able to use printk() earlier, use feature fixup. >> > Feature fixups are applies in early_init(), enabling the use of >> > printk() earlier. >> > >> > Signed-off-by: Christophe Leroy >> > --- >> > arch/powerpc/include/asm/time.h | 9 - >> > 1 file changed, 8 insertions(+), 1 deletion(-) >> >> The other option would be just to make this a compile time decision, eg. >> add CONFIG_PPC_601 and use that to gate whether we use RTC. >> >> Given how many 601 users there are, maybe 1?, I think that would be a >> simpler option and avoids complicating the code / binary for everyone >> else. > > Didn't we ditch 601 support years ago anyway ? We had workaround we > threw out I think... Paul said his still booted recently. cheers
Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
Le 26/08/2019 à 15:25, Benjamin Herrenschmidt a écrit : On Mon, 2019-08-26 at 21:41 +1000, Michael Ellerman wrote: Christophe Leroy writes: sched_clock(), used by printk(), calls __USE_RTC() to know whether to use realtime clock or timebase. __USE_RTC() uses cpu_has_feature() which is initialised by machine_init(). Before machine_init(), __USE_RTC() returns true, leading to a program check exception on CPUs not having realtime clock. In order to be able to use printk() earlier, use feature fixup. Feature fixups are applies in early_init(), enabling the use of printk() earlier. Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/time.h | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) The other option would be just to make this a compile time decision, eg. add CONFIG_PPC_601 and use that to gate whether we use RTC. Given how many 601 users there are, maybe 1?, I think that would be a simpler option and avoids complicating the code / binary for everyone else. Didn't we ditch 601 support years ago anyway ? We had workaround we threw out I think... There are still workarounds for 601 in the kernel, for exemple (in arch/powerpc/include/asm/ppc_asm.h) /* various errata or part fixups */ #ifdef CONFIG_PPC601_SYNC_FIX #define SYNC\ BEGIN_FTR_SECTION \ sync; \ isync; \ END_FTR_SECTION_IFSET(CPU_FTR_601) #define SYNC_601\ BEGIN_FTR_SECTION \ sync; \ END_FTR_SECTION_IFSET(CPU_FTR_601) #define ISYNC_601 \ BEGIN_FTR_SECTION \ isync; \ END_FTR_SECTION_IFSET(CPU_FTR_601) #else #define SYNC #define SYNC_601 #define ISYNC_601 #endif But if you think we can get rid of 601 completely, I'm happy with that. Christophe Cheers, Ben. cheers diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h index 54f4ec1f9fab..3455cb54c333 100644 --- a/arch/powerpc/include/asm/time.h +++ b/arch/powerpc/include/asm/time.h @@ -42,7 +42,14 @@ struct div_result { /* Accessor functions for the timebase (RTC on 601) registers. */ /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */ #ifdef CONFIG_PPC_BOOK3S_32 -#define __USE_RTC()(cpu_has_feature(CPU_FTR_USE_RTC)) +static inline bool __USE_RTC(void) +{ + asm_volatile_goto(ASM_FTR_IFCLR("nop;", "b %1;", %0) :: + "i" (CPU_FTR_USE_RTC) :: l_use_rtc); + return false; +l_use_rtc: + return true; +} #else #define __USE_RTC() 0 #endif -- 2.13.3
Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
On Mon, 2019-08-26 at 21:41 +1000, Michael Ellerman wrote: > Christophe Leroy writes: > > sched_clock(), used by printk(), calls __USE_RTC() to know > > whether to use realtime clock or timebase. > > > > __USE_RTC() uses cpu_has_feature() which is initialised by > > machine_init(). Before machine_init(), __USE_RTC() returns true, > > leading to a program check exception on CPUs not having realtime > > clock. > > > > In order to be able to use printk() earlier, use feature fixup. > > Feature fixups are applies in early_init(), enabling the use of > > printk() earlier. > > > > Signed-off-by: Christophe Leroy > > --- > > arch/powerpc/include/asm/time.h | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > The other option would be just to make this a compile time decision, eg. > add CONFIG_PPC_601 and use that to gate whether we use RTC. > > Given how many 601 users there are, maybe 1?, I think that would be a > simpler option and avoids complicating the code / binary for everyone > else. Didn't we ditch 601 support years ago anyway ? We had workaround we threw out I think... Cheers, Ben. > cheers > > > diff --git a/arch/powerpc/include/asm/time.h > > b/arch/powerpc/include/asm/time.h > > index 54f4ec1f9fab..3455cb54c333 100644 > > --- a/arch/powerpc/include/asm/time.h > > +++ b/arch/powerpc/include/asm/time.h > > @@ -42,7 +42,14 @@ struct div_result { > > /* Accessor functions for the timebase (RTC on 601) registers. */ > > /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */ > > #ifdef CONFIG_PPC_BOOK3S_32 > > -#define __USE_RTC()(cpu_has_feature(CPU_FTR_USE_RTC)) > > +static inline bool __USE_RTC(void) > > +{ > > + asm_volatile_goto(ASM_FTR_IFCLR("nop;", "b %1;", %0) :: > > + "i" (CPU_FTR_USE_RTC) :: l_use_rtc); > > + return false; > > +l_use_rtc: > > + return true; > > +} > > #else > > #define __USE_RTC()0 > > #endif > > -- > > 2.13.3
Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
On Mon, Aug 26, 2019 at 09:41:39PM +1000, Michael Ellerman wrote: > Given how many 601 users there are, maybe 1?, I think that would be a > simpler option and avoids complicating the code / binary for everyone > else. Or you could remove 601 support altogether? Segher
Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
Le 26/08/2019 à 13:41, Michael Ellerman a écrit : Christophe Leroy writes: sched_clock(), used by printk(), calls __USE_RTC() to know whether to use realtime clock or timebase. __USE_RTC() uses cpu_has_feature() which is initialised by machine_init(). Before machine_init(), __USE_RTC() returns true, leading to a program check exception on CPUs not having realtime clock. In order to be able to use printk() earlier, use feature fixup. Feature fixups are applies in early_init(), enabling the use of printk() earlier. Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/time.h | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) The other option would be just to make this a compile time decision, eg. add CONFIG_PPC_601 and use that to gate whether we use RTC. Euh ... yes OK, why not. And that would help simplify many places in the code. I can propose something in that direction, but it will be a bigger change. Given how many 601 users there are, maybe 1?, I think that would be a simpler option and avoids complicating the code / binary for everyone else. However this patch doesn't complicate things more than it was with cpu_has_feature() which does exactly the same but using static keys, does it ? Christophe cheers diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h index 54f4ec1f9fab..3455cb54c333 100644 --- a/arch/powerpc/include/asm/time.h +++ b/arch/powerpc/include/asm/time.h @@ -42,7 +42,14 @@ struct div_result { /* Accessor functions for the timebase (RTC on 601) registers. */ /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */ #ifdef CONFIG_PPC_BOOK3S_32 -#define __USE_RTC()(cpu_has_feature(CPU_FTR_USE_RTC)) +static inline bool __USE_RTC(void) +{ + asm_volatile_goto(ASM_FTR_IFCLR("nop;", "b %1;", %0) :: + "i" (CPU_FTR_USE_RTC) :: l_use_rtc); + return false; +l_use_rtc: + return true; +} #else #define __USE_RTC() 0 #endif -- 2.13.3
Re: [PATCH] powerpc/time: use feature fixup in __USE_RTC() instead of cpu feature.
Christophe Leroy writes: > sched_clock(), used by printk(), calls __USE_RTC() to know > whether to use realtime clock or timebase. > > __USE_RTC() uses cpu_has_feature() which is initialised by > machine_init(). Before machine_init(), __USE_RTC() returns true, > leading to a program check exception on CPUs not having realtime > clock. > > In order to be able to use printk() earlier, use feature fixup. > Feature fixups are applies in early_init(), enabling the use of > printk() earlier. > > Signed-off-by: Christophe Leroy > --- > arch/powerpc/include/asm/time.h | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) The other option would be just to make this a compile time decision, eg. add CONFIG_PPC_601 and use that to gate whether we use RTC. Given how many 601 users there are, maybe 1?, I think that would be a simpler option and avoids complicating the code / binary for everyone else. cheers > diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h > index 54f4ec1f9fab..3455cb54c333 100644 > --- a/arch/powerpc/include/asm/time.h > +++ b/arch/powerpc/include/asm/time.h > @@ -42,7 +42,14 @@ struct div_result { > /* Accessor functions for the timebase (RTC on 601) registers. */ > /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */ > #ifdef CONFIG_PPC_BOOK3S_32 > -#define __USE_RTC() (cpu_has_feature(CPU_FTR_USE_RTC)) > +static inline bool __USE_RTC(void) > +{ > + asm_volatile_goto(ASM_FTR_IFCLR("nop;", "b %1;", %0) :: > + "i" (CPU_FTR_USE_RTC) :: l_use_rtc); > + return false; > +l_use_rtc: > + return true; > +} > #else > #define __USE_RTC() 0 > #endif > -- > 2.13.3