| Message ID | 20190111033628.18677-1-andrew@aj.id.au |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | [RFC,linux,dev-4.19] clocksource: fttmr010: Detect back-pressure for Aspeed controllers | expand |
On 1/11/19 4:36 AM, Andrew Jeffery wrote: > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> This patch should not have any impact on a real system where l == cycles. It might be difficult to merge though. C. > --- > > See the cover letter on the corresponding qemu RFC series, to be sent after > this patch. > > arch/arm/mach-aspeed/Kconfig | 1 + > drivers/clocksource/timer-fttmr010.c | 7 +++++++ > 2 files changed, 8 insertions(+) > > diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig > index 2d5570e6e186..6b82d5416cf0 100644 > --- a/arch/arm/mach-aspeed/Kconfig > +++ b/arch/arm/mach-aspeed/Kconfig > @@ -7,6 +7,7 @@ menuconfig ARCH_ASPEED > select FTTMR010_TIMER > select MFD_SYSCON > select PINCTRL > + select GENERIC_CLOCKEVENTS_MIN_ADJUST > help > Say Y here if you want to run your kernel on an ASpeed BMC SoC. > > diff --git a/drivers/clocksource/timer-fttmr010.c b/drivers/clocksource/timer-fttmr010.c > index cf93f6419b51..f15386b98289 100644 > --- a/drivers/clocksource/timer-fttmr010.c > +++ b/drivers/clocksource/timer-fttmr010.c > @@ -124,6 +124,7 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, > { > struct fttmr010 *fttmr010 = to_fttmr010(evt); > u32 cr; > + u32 l; > > /* Stop */ > cr = readl(fttmr010->base + TIMER_CR); > @@ -136,6 +137,12 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, > * into TIMER1_COUNT register when the timer is re-enabled. > */ > writel(cycles, fttmr010->base + TIMER1_LOAD); > +#if CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST> + /* QEMU model may provide back-pressure on requested period */ > + l = readl(fttmr010->base + TIMER1_LOAD); > + if (l > cycles) > + return -EINVAL; > +#endif > } else { > /* Setup the match register forward in time */ > cr = readl(fttmr010->base + TIMER1_COUNT); >
On Fri, 11 Jan 2019, at 19:36, Cédric Le Goater wrote: > On 1/11/19 4:36 AM, Andrew Jeffery wrote: > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > This patch should not have any impact on a real system where l == cycles. > It might be difficult to merge though. I agree. Note however that the qemu patches resolve the issue regardless of whether this patch to Linux is applied, just if it's not Linux will not be aware that the timer isn't behaving how Linux requested. Maybe we live with that? Andrew > > C. > > > --- > > > > See the cover letter on the corresponding qemu RFC series, to be sent after > > this patch. > > > > arch/arm/mach-aspeed/Kconfig | 1 + > > drivers/clocksource/timer-fttmr010.c | 7 +++++++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig > > index 2d5570e6e186..6b82d5416cf0 100644 > > --- a/arch/arm/mach-aspeed/Kconfig > > +++ b/arch/arm/mach-aspeed/Kconfig > > @@ -7,6 +7,7 @@ menuconfig ARCH_ASPEED > > select FTTMR010_TIMER > > select MFD_SYSCON > > select PINCTRL > > + select GENERIC_CLOCKEVENTS_MIN_ADJUST > > help > > Say Y here if you want to run your kernel on an ASpeed BMC SoC. > > > > diff --git a/drivers/clocksource/timer-fttmr010.c b/drivers/clocksource/timer-fttmr010.c > > index cf93f6419b51..f15386b98289 100644 > > --- a/drivers/clocksource/timer-fttmr010.c > > +++ b/drivers/clocksource/timer-fttmr010.c > > @@ -124,6 +124,7 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, > > { > > struct fttmr010 *fttmr010 = to_fttmr010(evt); > > u32 cr; > > + u32 l; > > > > /* Stop */ > > cr = readl(fttmr010->base + TIMER_CR); > > @@ -136,6 +137,12 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, > > * into TIMER1_COUNT register when the timer is re-enabled. > > */ > > writel(cycles, fttmr010->base + TIMER1_LOAD); > > +#if CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST> + /* QEMU model may provide back-pressure on requested period */ > > + l = readl(fttmr010->base + TIMER1_LOAD); > > + if (l > cycles) > > + return -EINVAL; > > +#endif > > } else { > > /* Setup the match register forward in time */ > > cr = readl(fttmr010->base + TIMER1_COUNT); > > >
On 1/14/19 12:04 AM, Andrew Jeffery wrote: > On Fri, 11 Jan 2019, at 19:36, Cédric Le Goater wrote: >> On 1/11/19 4:36 AM, Andrew Jeffery wrote: >>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> >> >> This patch should not have any impact on a real system where l == cycles. >> It might be difficult to merge though. > > I agree. > > Note however that the qemu patches resolve the issue regardless of whether this > patch to Linux is applied, just if it's not Linux will not be aware that the timer isn't > behaving how Linux requested. Indeed. I checked on a ppc64el. > Maybe we live with that? yes ! Could please send your QEMU patchset ? Thanks, C. > Andrew > >> >> C. >> >>> --- >>> >>> See the cover letter on the corresponding qemu RFC series, to be sent after >>> this patch. >>> >>> arch/arm/mach-aspeed/Kconfig | 1 + >>> drivers/clocksource/timer-fttmr010.c | 7 +++++++ >>> 2 files changed, 8 insertions(+) >>> >>> diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig >>> index 2d5570e6e186..6b82d5416cf0 100644 >>> --- a/arch/arm/mach-aspeed/Kconfig >>> +++ b/arch/arm/mach-aspeed/Kconfig >>> @@ -7,6 +7,7 @@ menuconfig ARCH_ASPEED >>> select FTTMR010_TIMER >>> select MFD_SYSCON >>> select PINCTRL >>> + select GENERIC_CLOCKEVENTS_MIN_ADJUST >>> help >>> Say Y here if you want to run your kernel on an ASpeed BMC SoC. >>> >>> diff --git a/drivers/clocksource/timer-fttmr010.c b/drivers/clocksource/timer-fttmr010.c >>> index cf93f6419b51..f15386b98289 100644 >>> --- a/drivers/clocksource/timer-fttmr010.c >>> +++ b/drivers/clocksource/timer-fttmr010.c >>> @@ -124,6 +124,7 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, >>> { >>> struct fttmr010 *fttmr010 = to_fttmr010(evt); >>> u32 cr; >>> + u32 l; >>> >>> /* Stop */ >>> cr = readl(fttmr010->base + TIMER_CR); >>> @@ -136,6 +137,12 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, >>> * into TIMER1_COUNT register when the timer is re-enabled. >>> */ >>> writel(cycles, fttmr010->base + TIMER1_LOAD); >>> +#if CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST> + /* QEMU model may provide back-pressure on requested period */ >>> + l = readl(fttmr010->base + TIMER1_LOAD); >>> + if (l > cycles) >>> + return -EINVAL; >>> +#endif >>> } else { >>> /* Setup the match register forward in time */ >>> cr = readl(fttmr010->base + TIMER1_COUNT); >>> >>
diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig index 2d5570e6e186..6b82d5416cf0 100644 --- a/arch/arm/mach-aspeed/Kconfig +++ b/arch/arm/mach-aspeed/Kconfig @@ -7,6 +7,7 @@ menuconfig ARCH_ASPEED select FTTMR010_TIMER select MFD_SYSCON select PINCTRL + select GENERIC_CLOCKEVENTS_MIN_ADJUST help Say Y here if you want to run your kernel on an ASpeed BMC SoC. diff --git a/drivers/clocksource/timer-fttmr010.c b/drivers/clocksource/timer-fttmr010.c index cf93f6419b51..f15386b98289 100644 --- a/drivers/clocksource/timer-fttmr010.c +++ b/drivers/clocksource/timer-fttmr010.c @@ -124,6 +124,7 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, { struct fttmr010 *fttmr010 = to_fttmr010(evt); u32 cr; + u32 l; /* Stop */ cr = readl(fttmr010->base + TIMER_CR); @@ -136,6 +137,12 @@ static int fttmr010_timer_set_next_event(unsigned long cycles, * into TIMER1_COUNT register when the timer is re-enabled. */ writel(cycles, fttmr010->base + TIMER1_LOAD); +#if CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST + /* QEMU model may provide back-pressure on requested period */ + l = readl(fttmr010->base + TIMER1_LOAD); + if (l > cycles) + return -EINVAL; +#endif } else { /* Setup the match register forward in time */ cr = readl(fttmr010->base + TIMER1_COUNT);
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- See the cover letter on the corresponding qemu RFC series, to be sent after this patch. arch/arm/mach-aspeed/Kconfig | 1 + drivers/clocksource/timer-fttmr010.c | 7 +++++++ 2 files changed, 8 insertions(+)