| Message ID | 1524131457-19234-14-git-send-email-ley.foon.tan@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Add Intel Stratix 10 SoC support | expand |
On 04/19/2018 11:50 AM, Ley Foon Tan wrote: > Add timer support for Stratix SoC Is this really custom timer or is that some armv8 thing you're adding here ? Don't we already have a generic implementation for that ? If not, that's what we should do here. > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > --- > arch/arm/mach-socfpga/timer.c | 17 ++++++++++++++++- > 1 files changed, 16 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-socfpga/timer.c b/arch/arm/mach-socfpga/timer.c > index 253cde3..4c90b57 100644 > --- a/arch/arm/mach-socfpga/timer.c > +++ b/arch/arm/mach-socfpga/timer.c > @@ -1,5 +1,6 @@ > /* > - * Copyright (C) 2012 Altera Corporation <www.altera.com> > + * Copyright (C) 2016-2018 Intel Corporation <www.intel.com> > + * Copyright (C) 2012-2016 Altera Corporation <www.altera.com> > * > * SPDX-License-Identifier: GPL-2.0+ > */ > @@ -10,15 +11,29 @@ > > #define TIMER_LOAD_VAL 0xFFFFFFFF > > +#if !defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > static const struct socfpga_timer *timer_base = (void *)CONFIG_SYS_TIMERBASE; > +#endif > > /* > * Timer initialization > */ > int timer_init(void) > { > +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > + int enable = 0x3; /* timer enable + output signal masked */ > + int loadval = ~0; > + > + /* enable system counter */ > + writel(enable, SOCFPGA_GTIMER_SEC_ADDRESS); > + /* enable processor pysical counter */ > + asm volatile("msr cntp_ctl_el0, %0" : : "r" (enable)); > + asm volatile("msr cntp_tval_el0, %0" : : "r" (loadval)); > + > +#else > writel(TIMER_LOAD_VAL, &timer_base->load_val); > writel(TIMER_LOAD_VAL, &timer_base->curr_val); > writel(readl(&timer_base->ctrl) | 0x3, &timer_base->ctrl); > +#endif > return 0; > } >
On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: > On 04/19/2018 11:50 AM, Ley Foon Tan wrote: > > > > Add timer support for Stratix SoC > Is this really custom timer or is that some armv8 thing you're adding > here ? Don't we already have a generic implementation for that ? If > not, > that's what we should do here. Yes but not the init function. It's left with platform specific code to init it. Thanks Chin Liang
On 04/19/2018 07:26 AM, See, Chin Liang wrote: > On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: >> On 04/19/2018 11:50 AM, Ley Foon Tan wrote: >>> >>> Add timer support for Stratix SoC >> Is this really custom timer or is that some armv8 thing you're adding >> here ? Don't we already have a generic implementation for that ? If >> not, >> that's what we should do here. > > Yes but not the init function. It's left with platform specific code to > init it. Where is the common part ?
On Thu, Apr 19, 2018 at 4:21 PM, Marek Vasut <marex@denx.de> wrote: > On 04/19/2018 07:26 AM, See, Chin Liang wrote: >> On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: >>> On 04/19/2018 11:50 AM, Ley Foon Tan wrote: >>>> >>>> Add timer support for Stratix SoC >>> Is this really custom timer or is that some armv8 thing you're adding >>> here ? Don't we already have a generic implementation for that ? If >>> not, >>> that's what we should do here. >> >> Yes but not the init function. It's left with platform specific code to >> init it. > > Where is the common part ? > > -- > Best regards, > Marek Vasut timer_init weak function is in lib/time.c and common code is in this file as well. int __weak timer_init(void) { return 0; }
On 04/23/2018 03:54 AM, Ley Foon Tan wrote: > On Thu, Apr 19, 2018 at 4:21 PM, Marek Vasut <marex@denx.de> wrote: >> On 04/19/2018 07:26 AM, See, Chin Liang wrote: >>> On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: >>>> On 04/19/2018 11:50 AM, Ley Foon Tan wrote: >>>>> >>>>> Add timer support for Stratix SoC >>>> Is this really custom timer or is that some armv8 thing you're adding >>>> here ? Don't we already have a generic implementation for that ? If >>>> not, >>>> that's what we should do here. >>> >>> Yes but not the init function. It's left with platform specific code to >>> init it. >> >> Where is the common part ? >> >> -- >> Best regards, >> Marek Vasut > > timer_init weak function is in lib/time.c and common code is in this > file as well. > > int __weak timer_init(void) > { > return 0; > } Oh, that's what you use, I see. I suspect having a timer_gen5. and timer_gen10.c would be a bit cleaner than the ifdef.
On Mon, Apr 23, 2018 at 11:40 AM, Marek Vasut <marex@denx.de> wrote: > On 04/23/2018 03:54 AM, Ley Foon Tan wrote: >> On Thu, Apr 19, 2018 at 4:21 PM, Marek Vasut <marex@denx.de> wrote: >>> On 04/19/2018 07:26 AM, See, Chin Liang wrote: >>>> On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: >>>>> On 04/19/2018 11:50 AM, Ley Foon Tan wrote: >>>>>> >>>>>> Add timer support for Stratix SoC >>>>> Is this really custom timer or is that some armv8 thing you're adding >>>>> here ? Don't we already have a generic implementation for that ? If >>>>> not, >>>>> that's what we should do here. >>>> >>>> Yes but not the init function. It's left with platform specific code to >>>> init it. >>> >>> Where is the common part ? >>> >>> -- >>> Best regards, >>> Marek Vasut >> >> timer_init weak function is in lib/time.c and common code is in this >> file as well. >> >> int __weak timer_init(void) >> { >> return 0; >> } > > Oh, that's what you use, I see. > > I suspect having a timer_gen5. and timer_gen10.c would be a bit cleaner > than the ifdef. > > -- Okay, will split them into different files. Thanks. Regards Ley Foon
On 04/23/2018 08:00 AM, Ley Foon Tan wrote: > On Mon, Apr 23, 2018 at 11:40 AM, Marek Vasut <marex@denx.de> wrote: >> On 04/23/2018 03:54 AM, Ley Foon Tan wrote: >>> On Thu, Apr 19, 2018 at 4:21 PM, Marek Vasut <marex@denx.de> wrote: >>>> On 04/19/2018 07:26 AM, See, Chin Liang wrote: >>>>> On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: >>>>>> On 04/19/2018 11:50 AM, Ley Foon Tan wrote: >>>>>>> >>>>>>> Add timer support for Stratix SoC >>>>>> Is this really custom timer or is that some armv8 thing you're adding >>>>>> here ? Don't we already have a generic implementation for that ? If >>>>>> not, >>>>>> that's what we should do here. >>>>> >>>>> Yes but not the init function. It's left with platform specific code to >>>>> init it. >>>> >>>> Where is the common part ? >>>> >>>> -- >>>> Best regards, >>>> Marek Vasut >>> >>> timer_init weak function is in lib/time.c and common code is in this >>> file as well. >>> >>> int __weak timer_init(void) >>> { >>> return 0; >>> } >> >> Oh, that's what you use, I see. >> >> I suspect having a timer_gen5. and timer_gen10.c would be a bit cleaner >> than the ifdef. >> >> -- > Okay, will split them into different files. Thanks Also please rebase the patchset on u-boot/master , so you won't get collisions.
On Mon, Apr 23, 2018 at 7:59 PM, Marek Vasut <marex@denx.de> wrote: > On 04/23/2018 08:00 AM, Ley Foon Tan wrote: >> On Mon, Apr 23, 2018 at 11:40 AM, Marek Vasut <marex@denx.de> wrote: >>> On 04/23/2018 03:54 AM, Ley Foon Tan wrote: >>>> On Thu, Apr 19, 2018 at 4:21 PM, Marek Vasut <marex@denx.de> wrote: >>>>> On 04/19/2018 07:26 AM, See, Chin Liang wrote: >>>>>> On Thu, 2018-04-19 at 04:59 +0200, Marek Vasut wrote: >>>>>>> On 04/19/2018 11:50 AM, Ley Foon Tan wrote: >>>>>>>> >>>>>>>> Add timer support for Stratix SoC >>>>>>> Is this really custom timer or is that some armv8 thing you're adding >>>>>>> here ? Don't we already have a generic implementation for that ? If >>>>>>> not, >>>>>>> that's what we should do here. >>>>>> >>>>>> Yes but not the init function. It's left with platform specific code to >>>>>> init it. >>>>> >>>>> Where is the common part ? >>>>> >>>>> -- >>>>> Best regards, >>>>> Marek Vasut >>>> >>>> timer_init weak function is in lib/time.c and common code is in this >>>> file as well. >>>> >>>> int __weak timer_init(void) >>>> { >>>> return 0; >>>> } >>> >>> Oh, that's what you use, I see. >>> >>> I suspect having a timer_gen5. and timer_gen10.c would be a bit cleaner >>> than the ifdef. >>> >>> -- >> Okay, will split them into different files. > > Thanks > > Also please rebase the patchset on u-boot/master , so you won't get > collisions. > Noted, will do that. Thanks. Regards Ley Foon
diff --git a/arch/arm/mach-socfpga/timer.c b/arch/arm/mach-socfpga/timer.c index 253cde3..4c90b57 100644 --- a/arch/arm/mach-socfpga/timer.c +++ b/arch/arm/mach-socfpga/timer.c @@ -1,5 +1,6 @@ /* - * Copyright (C) 2012 Altera Corporation <www.altera.com> + * Copyright (C) 2016-2018 Intel Corporation <www.intel.com> + * Copyright (C) 2012-2016 Altera Corporation <www.altera.com> * * SPDX-License-Identifier: GPL-2.0+ */ @@ -10,15 +11,29 @@ #define TIMER_LOAD_VAL 0xFFFFFFFF +#if !defined(CONFIG_TARGET_SOCFPGA_STRATIX10) static const struct socfpga_timer *timer_base = (void *)CONFIG_SYS_TIMERBASE; +#endif /* * Timer initialization */ int timer_init(void) { +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10) + int enable = 0x3; /* timer enable + output signal masked */ + int loadval = ~0; + + /* enable system counter */ + writel(enable, SOCFPGA_GTIMER_SEC_ADDRESS); + /* enable processor pysical counter */ + asm volatile("msr cntp_ctl_el0, %0" : : "r" (enable)); + asm volatile("msr cntp_tval_el0, %0" : : "r" (loadval)); + +#else writel(TIMER_LOAD_VAL, &timer_base->load_val); writel(TIMER_LOAD_VAL, &timer_base->curr_val); writel(readl(&timer_base->ctrl) | 0x3, &timer_base->ctrl); +#endif return 0; }