* [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 3:02 ` Joseph Lo 0 siblings, 0 replies; 12+ messages in thread From: Joseph Lo @ 2019-04-02 3:02 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Daniel Lezcano, Thomas Gleixner Cc: linux-tegra, linux-arm-kernel, linux-kernel, Joseph Lo Since the clocksource framework has the support for suspend time compensation. Re-work the driver to use that, so we can reduce the duplicate code. Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> Signed-off-by: Joseph Lo <josephl@nvidia.com> --- drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c index fdb3d795a409..919b3568c495 100644 --- a/drivers/clocksource/timer-tegra20.c +++ b/drivers/clocksource/timer-tegra20.c @@ -60,9 +60,6 @@ static u32 usec_config; static void __iomem *timer_reg_base; #ifdef CONFIG_ARM -static void __iomem *rtc_base; -static struct timespec64 persistent_ts; -static u64 persistent_ms, last_persistent_ms; static struct delay_timer tegra_delay_timer; #endif @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) return readl(timer_reg_base + TIMERUS_CNTR_1US); } +static struct timer_of suspend_rtc_to = { + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, +}; + /* * tegra_rtc_read - Reads the Tegra RTC registers * Care must be taken that this funciton is not called while the * tegra_rtc driver could be executing to avoid race conditions * on the RTC shadow register */ -static u64 tegra_rtc_read_ms(void) +static u64 tegra_rtc_read_ms(struct clocksource *cs) { - u32 ms = readl(rtc_base + RTC_MILLISECONDS); - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); return (u64)s * MSEC_PER_SEC + ms; } -/* - * tegra_read_persistent_clock64 - Return time from a persistent clock. - * - * Reads the time from a source which isn't disabled during PM, the - * 32k sync timer. Convert the cycles elapsed since last read into - * nsecs and adds to a monotonically increasing timespec64. - * Care must be taken that this funciton is not called while the - * tegra_rtc driver could be executing to avoid race conditions - * on the RTC shadow register - */ -static void tegra_read_persistent_clock64(struct timespec64 *ts) -{ - u64 delta; - - last_persistent_ms = persistent_ms; - persistent_ms = tegra_rtc_read_ms(); - delta = persistent_ms - last_persistent_ms; - - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); - *ts = persistent_ts; -} +static struct clocksource suspend_rtc_clocksource = { + .name = "tegra_suspend_timer", + .rating = 200, + .read = tegra_rtc_read_ms, + .mask = CLOCKSOURCE_MASK(32), + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, +}; #endif static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) static int __init tegra20_init_rtc(struct device_node *np) { - struct clk *clk; + int ret; - rtc_base = of_iomap(np, 0); - if (!rtc_base) { - pr_err("Can't map RTC registers\n"); - return -ENXIO; - } + ret = timer_of_init(np, &suspend_rtc_to); + if (ret) + return ret; - /* - * rtc registers are used by read_persistent_clock, keep the rtc clock - * enabled - */ - clk = of_clk_get(np, 0); - if (IS_ERR(clk)) - pr_warn("Unable to get rtc-tegra clock\n"); - else - clk_prepare_enable(clk); + clocksource_register_hz(&suspend_rtc_clocksource, 1000); - return register_persistent_clock(tegra_read_persistent_clock64); + return 0; } TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 3:02 ` Joseph Lo 0 siblings, 0 replies; 12+ messages in thread From: Joseph Lo @ 2019-04-02 3:02 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Daniel Lezcano, Thomas Gleixner Cc: linux-tegra, linux-arm-kernel, linux-kernel, Joseph Lo Since the clocksource framework has the support for suspend time compensation. Re-work the driver to use that, so we can reduce the duplicate code. Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> Signed-off-by: Joseph Lo <josephl@nvidia.com> --- drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c index fdb3d795a409..919b3568c495 100644 --- a/drivers/clocksource/timer-tegra20.c +++ b/drivers/clocksource/timer-tegra20.c @@ -60,9 +60,6 @@ static u32 usec_config; static void __iomem *timer_reg_base; #ifdef CONFIG_ARM -static void __iomem *rtc_base; -static struct timespec64 persistent_ts; -static u64 persistent_ms, last_persistent_ms; static struct delay_timer tegra_delay_timer; #endif @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) return readl(timer_reg_base + TIMERUS_CNTR_1US); } +static struct timer_of suspend_rtc_to = { + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, +}; + /* * tegra_rtc_read - Reads the Tegra RTC registers * Care must be taken that this funciton is not called while the * tegra_rtc driver could be executing to avoid race conditions * on the RTC shadow register */ -static u64 tegra_rtc_read_ms(void) +static u64 tegra_rtc_read_ms(struct clocksource *cs) { - u32 ms = readl(rtc_base + RTC_MILLISECONDS); - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); return (u64)s * MSEC_PER_SEC + ms; } -/* - * tegra_read_persistent_clock64 - Return time from a persistent clock. - * - * Reads the time from a source which isn't disabled during PM, the - * 32k sync timer. Convert the cycles elapsed since last read into - * nsecs and adds to a monotonically increasing timespec64. - * Care must be taken that this funciton is not called while the - * tegra_rtc driver could be executing to avoid race conditions - * on the RTC shadow register - */ -static void tegra_read_persistent_clock64(struct timespec64 *ts) -{ - u64 delta; - - last_persistent_ms = persistent_ms; - persistent_ms = tegra_rtc_read_ms(); - delta = persistent_ms - last_persistent_ms; - - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); - *ts = persistent_ts; -} +static struct clocksource suspend_rtc_clocksource = { + .name = "tegra_suspend_timer", + .rating = 200, + .read = tegra_rtc_read_ms, + .mask = CLOCKSOURCE_MASK(32), + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, +}; #endif static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) static int __init tegra20_init_rtc(struct device_node *np) { - struct clk *clk; + int ret; - rtc_base = of_iomap(np, 0); - if (!rtc_base) { - pr_err("Can't map RTC registers\n"); - return -ENXIO; - } + ret = timer_of_init(np, &suspend_rtc_to); + if (ret) + return ret; - /* - * rtc registers are used by read_persistent_clock, keep the rtc clock - * enabled - */ - clk = of_clk_get(np, 0); - if (IS_ERR(clk)) - pr_warn("Unable to get rtc-tegra clock\n"); - else - clk_prepare_enable(clk); + clocksource_register_hz(&suspend_rtc_clocksource, 1000); - return register_persistent_clock(tegra_read_persistent_clock64); + return 0; } TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 3:02 ` Joseph Lo 0 siblings, 0 replies; 12+ messages in thread From: Joseph Lo @ 2019-04-02 3:02 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Daniel Lezcano, Thomas Gleixner Cc: linux-tegra, linux-kernel, linux-arm-kernel, Joseph Lo Since the clocksource framework has the support for suspend time compensation. Re-work the driver to use that, so we can reduce the duplicate code. Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> Signed-off-by: Joseph Lo <josephl@nvidia.com> --- drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c index fdb3d795a409..919b3568c495 100644 --- a/drivers/clocksource/timer-tegra20.c +++ b/drivers/clocksource/timer-tegra20.c @@ -60,9 +60,6 @@ static u32 usec_config; static void __iomem *timer_reg_base; #ifdef CONFIG_ARM -static void __iomem *rtc_base; -static struct timespec64 persistent_ts; -static u64 persistent_ms, last_persistent_ms; static struct delay_timer tegra_delay_timer; #endif @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) return readl(timer_reg_base + TIMERUS_CNTR_1US); } +static struct timer_of suspend_rtc_to = { + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, +}; + /* * tegra_rtc_read - Reads the Tegra RTC registers * Care must be taken that this funciton is not called while the * tegra_rtc driver could be executing to avoid race conditions * on the RTC shadow register */ -static u64 tegra_rtc_read_ms(void) +static u64 tegra_rtc_read_ms(struct clocksource *cs) { - u32 ms = readl(rtc_base + RTC_MILLISECONDS); - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); return (u64)s * MSEC_PER_SEC + ms; } -/* - * tegra_read_persistent_clock64 - Return time from a persistent clock. - * - * Reads the time from a source which isn't disabled during PM, the - * 32k sync timer. Convert the cycles elapsed since last read into - * nsecs and adds to a monotonically increasing timespec64. - * Care must be taken that this funciton is not called while the - * tegra_rtc driver could be executing to avoid race conditions - * on the RTC shadow register - */ -static void tegra_read_persistent_clock64(struct timespec64 *ts) -{ - u64 delta; - - last_persistent_ms = persistent_ms; - persistent_ms = tegra_rtc_read_ms(); - delta = persistent_ms - last_persistent_ms; - - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); - *ts = persistent_ts; -} +static struct clocksource suspend_rtc_clocksource = { + .name = "tegra_suspend_timer", + .rating = 200, + .read = tegra_rtc_read_ms, + .mask = CLOCKSOURCE_MASK(32), + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, +}; #endif static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) static int __init tegra20_init_rtc(struct device_node *np) { - struct clk *clk; + int ret; - rtc_base = of_iomap(np, 0); - if (!rtc_base) { - pr_err("Can't map RTC registers\n"); - return -ENXIO; - } + ret = timer_of_init(np, &suspend_rtc_to); + if (ret) + return ret; - /* - * rtc registers are used by read_persistent_clock, keep the rtc clock - * enabled - */ - clk = of_clk_get(np, 0); - if (IS_ERR(clk)) - pr_warn("Unable to get rtc-tegra clock\n"); - else - clk_prepare_enable(clk); + clocksource_register_hz(&suspend_rtc_clocksource, 1000); - return register_persistent_clock(tegra_read_persistent_clock64); + return 0; } TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); #endif -- 2.21.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time 2019-04-02 3:02 ` Joseph Lo @ 2019-04-02 14:46 ` Thierry Reding -1 siblings, 0 replies; 12+ messages in thread From: Thierry Reding @ 2019-04-02 14:46 UTC (permalink / raw) To: Joseph Lo Cc: Jonathan Hunter, Daniel Lezcano, Thomas Gleixner, linux-tegra, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 4268 bytes --] On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: > Since the clocksource framework has the support for suspend time > compensation. Re-work the driver to use that, so we can reduce the > duplicate code. > > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- > drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- > 1 file changed, 20 insertions(+), 43 deletions(-) Nice! > > diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c > index fdb3d795a409..919b3568c495 100644 > --- a/drivers/clocksource/timer-tegra20.c > +++ b/drivers/clocksource/timer-tegra20.c > @@ -60,9 +60,6 @@ > static u32 usec_config; > static void __iomem *timer_reg_base; > #ifdef CONFIG_ARM > -static void __iomem *rtc_base; > -static struct timespec64 persistent_ts; > -static u64 persistent_ms, last_persistent_ms; > static struct delay_timer tegra_delay_timer; > #endif > > @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) > return readl(timer_reg_base + TIMERUS_CNTR_1US); > } > > +static struct timer_of suspend_rtc_to = { > + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, > +}; > + > /* > * tegra_rtc_read - Reads the Tegra RTC registers > * Care must be taken that this funciton is not called while the > * tegra_rtc driver could be executing to avoid race conditions > * on the RTC shadow register > */ > -static u64 tegra_rtc_read_ms(void) > +static u64 tegra_rtc_read_ms(struct clocksource *cs) > { > - u32 ms = readl(rtc_base + RTC_MILLISECONDS); > - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); > + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); > + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); > return (u64)s * MSEC_PER_SEC + ms; > } > > -/* > - * tegra_read_persistent_clock64 - Return time from a persistent clock. > - * > - * Reads the time from a source which isn't disabled during PM, the > - * 32k sync timer. Convert the cycles elapsed since last read into > - * nsecs and adds to a monotonically increasing timespec64. > - * Care must be taken that this funciton is not called while the > - * tegra_rtc driver could be executing to avoid race conditions > - * on the RTC shadow register > - */ > -static void tegra_read_persistent_clock64(struct timespec64 *ts) > -{ > - u64 delta; > - > - last_persistent_ms = persistent_ms; > - persistent_ms = tegra_rtc_read_ms(); > - delta = persistent_ms - last_persistent_ms; > - > - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); > - *ts = persistent_ts; > -} > +static struct clocksource suspend_rtc_clocksource = { > + .name = "tegra_suspend_timer", > + .rating = 200, > + .read = tegra_rtc_read_ms, > + .mask = CLOCKSOURCE_MASK(32), > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, > +}; > #endif > > static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) > @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) > > static int __init tegra20_init_rtc(struct device_node *np) > { > - struct clk *clk; > + int ret; > > - rtc_base = of_iomap(np, 0); > - if (!rtc_base) { > - pr_err("Can't map RTC registers\n"); > - return -ENXIO; > - } > + ret = timer_of_init(np, &suspend_rtc_to); > + if (ret) > + return ret; > > - /* > - * rtc registers are used by read_persistent_clock, keep the rtc clock > - * enabled > - */ > - clk = of_clk_get(np, 0); > - if (IS_ERR(clk)) > - pr_warn("Unable to get rtc-tegra clock\n"); > - else > - clk_prepare_enable(clk); > + clocksource_register_hz(&suspend_rtc_clocksource, 1000); > > - return register_persistent_clock(tegra_read_persistent_clock64); > + return 0; > } > TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); > #endif I wonder if there's any reason left for the #ifdefs now. My recollection is that these were only needed because register_persistent_clock() was not available on 64-bit ARM. The new APIs seem to be available regardless of architecture, so do we still need to differentiate? Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 14:46 ` Thierry Reding 0 siblings, 0 replies; 12+ messages in thread From: Thierry Reding @ 2019-04-02 14:46 UTC (permalink / raw) To: Joseph Lo Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra, Thomas Gleixner, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 4268 bytes --] On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: > Since the clocksource framework has the support for suspend time > compensation. Re-work the driver to use that, so we can reduce the > duplicate code. > > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- > drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- > 1 file changed, 20 insertions(+), 43 deletions(-) Nice! > > diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c > index fdb3d795a409..919b3568c495 100644 > --- a/drivers/clocksource/timer-tegra20.c > +++ b/drivers/clocksource/timer-tegra20.c > @@ -60,9 +60,6 @@ > static u32 usec_config; > static void __iomem *timer_reg_base; > #ifdef CONFIG_ARM > -static void __iomem *rtc_base; > -static struct timespec64 persistent_ts; > -static u64 persistent_ms, last_persistent_ms; > static struct delay_timer tegra_delay_timer; > #endif > > @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) > return readl(timer_reg_base + TIMERUS_CNTR_1US); > } > > +static struct timer_of suspend_rtc_to = { > + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, > +}; > + > /* > * tegra_rtc_read - Reads the Tegra RTC registers > * Care must be taken that this funciton is not called while the > * tegra_rtc driver could be executing to avoid race conditions > * on the RTC shadow register > */ > -static u64 tegra_rtc_read_ms(void) > +static u64 tegra_rtc_read_ms(struct clocksource *cs) > { > - u32 ms = readl(rtc_base + RTC_MILLISECONDS); > - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); > + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); > + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); > return (u64)s * MSEC_PER_SEC + ms; > } > > -/* > - * tegra_read_persistent_clock64 - Return time from a persistent clock. > - * > - * Reads the time from a source which isn't disabled during PM, the > - * 32k sync timer. Convert the cycles elapsed since last read into > - * nsecs and adds to a monotonically increasing timespec64. > - * Care must be taken that this funciton is not called while the > - * tegra_rtc driver could be executing to avoid race conditions > - * on the RTC shadow register > - */ > -static void tegra_read_persistent_clock64(struct timespec64 *ts) > -{ > - u64 delta; > - > - last_persistent_ms = persistent_ms; > - persistent_ms = tegra_rtc_read_ms(); > - delta = persistent_ms - last_persistent_ms; > - > - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); > - *ts = persistent_ts; > -} > +static struct clocksource suspend_rtc_clocksource = { > + .name = "tegra_suspend_timer", > + .rating = 200, > + .read = tegra_rtc_read_ms, > + .mask = CLOCKSOURCE_MASK(32), > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, > +}; > #endif > > static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) > @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) > > static int __init tegra20_init_rtc(struct device_node *np) > { > - struct clk *clk; > + int ret; > > - rtc_base = of_iomap(np, 0); > - if (!rtc_base) { > - pr_err("Can't map RTC registers\n"); > - return -ENXIO; > - } > + ret = timer_of_init(np, &suspend_rtc_to); > + if (ret) > + return ret; > > - /* > - * rtc registers are used by read_persistent_clock, keep the rtc clock > - * enabled > - */ > - clk = of_clk_get(np, 0); > - if (IS_ERR(clk)) > - pr_warn("Unable to get rtc-tegra clock\n"); > - else > - clk_prepare_enable(clk); > + clocksource_register_hz(&suspend_rtc_clocksource, 1000); > > - return register_persistent_clock(tegra_read_persistent_clock64); > + return 0; > } > TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); > #endif I wonder if there's any reason left for the #ifdefs now. My recollection is that these were only needed because register_persistent_clock() was not available on 64-bit ARM. The new APIs seem to be available regardless of architecture, so do we still need to differentiate? Thierry [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time 2019-04-02 14:46 ` Thierry Reding (?) @ 2019-04-02 15:50 ` Joseph Lo -1 siblings, 0 replies; 12+ messages in thread From: Joseph Lo @ 2019-04-02 15:50 UTC (permalink / raw) To: Thierry Reding Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra, Thomas Gleixner, linux-arm-kernel On 4/2/19 10:46 PM, Thierry Reding wrote: > On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: >> Since the clocksource framework has the support for suspend time >> compensation. Re-work the driver to use that, so we can reduce the >> duplicate code. >> >> Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> Signed-off-by: Joseph Lo <josephl@nvidia.com> >> --- >> drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- >> 1 file changed, 20 insertions(+), 43 deletions(-) > > Nice! > >> >> diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c >> index fdb3d795a409..919b3568c495 100644 >> --- a/drivers/clocksource/timer-tegra20.c >> +++ b/drivers/clocksource/timer-tegra20.c >> @@ -60,9 +60,6 @@ >> static u32 usec_config; >> static void __iomem *timer_reg_base; >> #ifdef CONFIG_ARM >> -static void __iomem *rtc_base; >> -static struct timespec64 persistent_ts; >> -static u64 persistent_ms, last_persistent_ms; >> static struct delay_timer tegra_delay_timer; >> #endif >> >> @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) >> return readl(timer_reg_base + TIMERUS_CNTR_1US); >> } >> >> +static struct timer_of suspend_rtc_to = { >> + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, >> +}; >> + >> /* >> * tegra_rtc_read - Reads the Tegra RTC registers >> * Care must be taken that this funciton is not called while the >> * tegra_rtc driver could be executing to avoid race conditions >> * on the RTC shadow register >> */ >> -static u64 tegra_rtc_read_ms(void) >> +static u64 tegra_rtc_read_ms(struct clocksource *cs) >> { >> - u32 ms = readl(rtc_base + RTC_MILLISECONDS); >> - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); >> + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); >> + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); >> return (u64)s * MSEC_PER_SEC + ms; >> } >> >> -/* >> - * tegra_read_persistent_clock64 - Return time from a persistent clock. >> - * >> - * Reads the time from a source which isn't disabled during PM, the >> - * 32k sync timer. Convert the cycles elapsed since last read into >> - * nsecs and adds to a monotonically increasing timespec64. >> - * Care must be taken that this funciton is not called while the >> - * tegra_rtc driver could be executing to avoid race conditions >> - * on the RTC shadow register >> - */ >> -static void tegra_read_persistent_clock64(struct timespec64 *ts) >> -{ >> - u64 delta; >> - >> - last_persistent_ms = persistent_ms; >> - persistent_ms = tegra_rtc_read_ms(); >> - delta = persistent_ms - last_persistent_ms; >> - >> - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); >> - *ts = persistent_ts; >> -} >> +static struct clocksource suspend_rtc_clocksource = { >> + .name = "tegra_suspend_timer", >> + .rating = 200, >> + .read = tegra_rtc_read_ms, >> + .mask = CLOCKSOURCE_MASK(32), >> + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, >> +}; >> #endif >> >> static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) >> @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) >> >> static int __init tegra20_init_rtc(struct device_node *np) >> { >> - struct clk *clk; >> + int ret; >> >> - rtc_base = of_iomap(np, 0); >> - if (!rtc_base) { >> - pr_err("Can't map RTC registers\n"); >> - return -ENXIO; >> - } >> + ret = timer_of_init(np, &suspend_rtc_to); >> + if (ret) >> + return ret; >> >> - /* >> - * rtc registers are used by read_persistent_clock, keep the rtc clock >> - * enabled >> - */ >> - clk = of_clk_get(np, 0); >> - if (IS_ERR(clk)) >> - pr_warn("Unable to get rtc-tegra clock\n"); >> - else >> - clk_prepare_enable(clk); >> + clocksource_register_hz(&suspend_rtc_clocksource, 1000); >> >> - return register_persistent_clock(tegra_read_persistent_clock64); >> + return 0; >> } >> TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); >> #endif > > I wonder if there's any reason left for the #ifdefs now. My recollection > is that these were only needed because register_persistent_clock() was > not available on 64-bit ARM. The new APIs seem to be available > regardless of architecture, so do we still need to differentiate? > Actually, only Tegra20/30 that doesn't have ARM arch timer support need this. The latter Tegra chips which have ARM arch timer support use TSC ( time stamp counter or timer system counter depends on the chip it has different name) as the timer source in the PMC. And it uses OSC during runtime and switches to 32KHz always-on clock source to keep counting when the chip is in the SC7 or LP0 state. So I didn't change that for this reason. Thanks, Joseph ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 15:50 ` Joseph Lo 0 siblings, 0 replies; 12+ messages in thread From: Joseph Lo @ 2019-04-02 15:50 UTC (permalink / raw) To: Thierry Reding Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra, Thomas Gleixner, linux-arm-kernel On 4/2/19 10:46 PM, Thierry Reding wrote: > On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: >> Since the clocksource framework has the support for suspend time >> compensation. Re-work the driver to use that, so we can reduce the >> duplicate code. >> >> Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> Signed-off-by: Joseph Lo <josephl@nvidia.com> >> --- >> drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- >> 1 file changed, 20 insertions(+), 43 deletions(-) > > Nice! > >> >> diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c >> index fdb3d795a409..919b3568c495 100644 >> --- a/drivers/clocksource/timer-tegra20.c >> +++ b/drivers/clocksource/timer-tegra20.c >> @@ -60,9 +60,6 @@ >> static u32 usec_config; >> static void __iomem *timer_reg_base; >> #ifdef CONFIG_ARM >> -static void __iomem *rtc_base; >> -static struct timespec64 persistent_ts; >> -static u64 persistent_ms, last_persistent_ms; >> static struct delay_timer tegra_delay_timer; >> #endif >> >> @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) >> return readl(timer_reg_base + TIMERUS_CNTR_1US); >> } >> >> +static struct timer_of suspend_rtc_to = { >> + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, >> +}; >> + >> /* >> * tegra_rtc_read - Reads the Tegra RTC registers >> * Care must be taken that this funciton is not called while the >> * tegra_rtc driver could be executing to avoid race conditions >> * on the RTC shadow register >> */ >> -static u64 tegra_rtc_read_ms(void) >> +static u64 tegra_rtc_read_ms(struct clocksource *cs) >> { >> - u32 ms = readl(rtc_base + RTC_MILLISECONDS); >> - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); >> + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); >> + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); >> return (u64)s * MSEC_PER_SEC + ms; >> } >> >> -/* >> - * tegra_read_persistent_clock64 - Return time from a persistent clock. >> - * >> - * Reads the time from a source which isn't disabled during PM, the >> - * 32k sync timer. Convert the cycles elapsed since last read into >> - * nsecs and adds to a monotonically increasing timespec64. >> - * Care must be taken that this funciton is not called while the >> - * tegra_rtc driver could be executing to avoid race conditions >> - * on the RTC shadow register >> - */ >> -static void tegra_read_persistent_clock64(struct timespec64 *ts) >> -{ >> - u64 delta; >> - >> - last_persistent_ms = persistent_ms; >> - persistent_ms = tegra_rtc_read_ms(); >> - delta = persistent_ms - last_persistent_ms; >> - >> - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); >> - *ts = persistent_ts; >> -} >> +static struct clocksource suspend_rtc_clocksource = { >> + .name = "tegra_suspend_timer", >> + .rating = 200, >> + .read = tegra_rtc_read_ms, >> + .mask = CLOCKSOURCE_MASK(32), >> + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, >> +}; >> #endif >> >> static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) >> @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) >> >> static int __init tegra20_init_rtc(struct device_node *np) >> { >> - struct clk *clk; >> + int ret; >> >> - rtc_base = of_iomap(np, 0); >> - if (!rtc_base) { >> - pr_err("Can't map RTC registers\n"); >> - return -ENXIO; >> - } >> + ret = timer_of_init(np, &suspend_rtc_to); >> + if (ret) >> + return ret; >> >> - /* >> - * rtc registers are used by read_persistent_clock, keep the rtc clock >> - * enabled >> - */ >> - clk = of_clk_get(np, 0); >> - if (IS_ERR(clk)) >> - pr_warn("Unable to get rtc-tegra clock\n"); >> - else >> - clk_prepare_enable(clk); >> + clocksource_register_hz(&suspend_rtc_clocksource, 1000); >> >> - return register_persistent_clock(tegra_read_persistent_clock64); >> + return 0; >> } >> TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); >> #endif > > I wonder if there's any reason left for the #ifdefs now. My recollection > is that these were only needed because register_persistent_clock() was > not available on 64-bit ARM. The new APIs seem to be available > regardless of architecture, so do we still need to differentiate? > Actually, only Tegra20/30 that doesn't have ARM arch timer support need this. The latter Tegra chips which have ARM arch timer support use TSC ( time stamp counter or timer system counter depends on the chip it has different name) as the timer source in the PMC. And it uses OSC during runtime and switches to 32KHz always-on clock source to keep counting when the chip is in the SC7 or LP0 state. So I didn't change that for this reason. Thanks, Joseph ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 15:50 ` Joseph Lo 0 siblings, 0 replies; 12+ messages in thread From: Joseph Lo @ 2019-04-02 15:50 UTC (permalink / raw) To: Thierry Reding Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra, Thomas Gleixner, linux-arm-kernel On 4/2/19 10:46 PM, Thierry Reding wrote: > On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: >> Since the clocksource framework has the support for suspend time >> compensation. Re-work the driver to use that, so we can reduce the >> duplicate code. >> >> Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> Signed-off-by: Joseph Lo <josephl@nvidia.com> >> --- >> drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- >> 1 file changed, 20 insertions(+), 43 deletions(-) > > Nice! > >> >> diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c >> index fdb3d795a409..919b3568c495 100644 >> --- a/drivers/clocksource/timer-tegra20.c >> +++ b/drivers/clocksource/timer-tegra20.c >> @@ -60,9 +60,6 @@ >> static u32 usec_config; >> static void __iomem *timer_reg_base; >> #ifdef CONFIG_ARM >> -static void __iomem *rtc_base; >> -static struct timespec64 persistent_ts; >> -static u64 persistent_ms, last_persistent_ms; >> static struct delay_timer tegra_delay_timer; >> #endif >> >> @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) >> return readl(timer_reg_base + TIMERUS_CNTR_1US); >> } >> >> +static struct timer_of suspend_rtc_to = { >> + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, >> +}; >> + >> /* >> * tegra_rtc_read - Reads the Tegra RTC registers >> * Care must be taken that this funciton is not called while the >> * tegra_rtc driver could be executing to avoid race conditions >> * on the RTC shadow register >> */ >> -static u64 tegra_rtc_read_ms(void) >> +static u64 tegra_rtc_read_ms(struct clocksource *cs) >> { >> - u32 ms = readl(rtc_base + RTC_MILLISECONDS); >> - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); >> + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); >> + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); >> return (u64)s * MSEC_PER_SEC + ms; >> } >> >> -/* >> - * tegra_read_persistent_clock64 - Return time from a persistent clock. >> - * >> - * Reads the time from a source which isn't disabled during PM, the >> - * 32k sync timer. Convert the cycles elapsed since last read into >> - * nsecs and adds to a monotonically increasing timespec64. >> - * Care must be taken that this funciton is not called while the >> - * tegra_rtc driver could be executing to avoid race conditions >> - * on the RTC shadow register >> - */ >> -static void tegra_read_persistent_clock64(struct timespec64 *ts) >> -{ >> - u64 delta; >> - >> - last_persistent_ms = persistent_ms; >> - persistent_ms = tegra_rtc_read_ms(); >> - delta = persistent_ms - last_persistent_ms; >> - >> - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); >> - *ts = persistent_ts; >> -} >> +static struct clocksource suspend_rtc_clocksource = { >> + .name = "tegra_suspend_timer", >> + .rating = 200, >> + .read = tegra_rtc_read_ms, >> + .mask = CLOCKSOURCE_MASK(32), >> + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, >> +}; >> #endif >> >> static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) >> @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) >> >> static int __init tegra20_init_rtc(struct device_node *np) >> { >> - struct clk *clk; >> + int ret; >> >> - rtc_base = of_iomap(np, 0); >> - if (!rtc_base) { >> - pr_err("Can't map RTC registers\n"); >> - return -ENXIO; >> - } >> + ret = timer_of_init(np, &suspend_rtc_to); >> + if (ret) >> + return ret; >> >> - /* >> - * rtc registers are used by read_persistent_clock, keep the rtc clock >> - * enabled >> - */ >> - clk = of_clk_get(np, 0); >> - if (IS_ERR(clk)) >> - pr_warn("Unable to get rtc-tegra clock\n"); >> - else >> - clk_prepare_enable(clk); >> + clocksource_register_hz(&suspend_rtc_clocksource, 1000); >> >> - return register_persistent_clock(tegra_read_persistent_clock64); >> + return 0; >> } >> TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); >> #endif > > I wonder if there's any reason left for the #ifdefs now. My recollection > is that these were only needed because register_persistent_clock() was > not available on 64-bit ARM. The new APIs seem to be available > regardless of architecture, so do we still need to differentiate? > Actually, only Tegra20/30 that doesn't have ARM arch timer support need this. The latter Tegra chips which have ARM arch timer support use TSC ( time stamp counter or timer system counter depends on the chip it has different name) as the timer source in the PMC. And it uses OSC during runtime and switches to 32KHz always-on clock source to keep counting when the chip is in the SC7 or LP0 state. So I didn't change that for this reason. Thanks, Joseph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time 2019-04-02 15:50 ` Joseph Lo @ 2019-04-02 16:49 ` Thierry Reding -1 siblings, 0 replies; 12+ messages in thread From: Thierry Reding @ 2019-04-02 16:49 UTC (permalink / raw) To: Joseph Lo Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra, Thomas Gleixner, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 5344 bytes --] On Tue, Apr 02, 2019 at 11:50:20PM +0800, Joseph Lo wrote: > On 4/2/19 10:46 PM, Thierry Reding wrote: > > On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: > > > Since the clocksource framework has the support for suspend time > > > compensation. Re-work the driver to use that, so we can reduce the > > > duplicate code. > > > > > > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > > Signed-off-by: Joseph Lo <josephl@nvidia.com> > > > --- > > > drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- > > > 1 file changed, 20 insertions(+), 43 deletions(-) > > > > Nice! > > > > > > > > diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c > > > index fdb3d795a409..919b3568c495 100644 > > > --- a/drivers/clocksource/timer-tegra20.c > > > +++ b/drivers/clocksource/timer-tegra20.c > > > @@ -60,9 +60,6 @@ > > > static u32 usec_config; > > > static void __iomem *timer_reg_base; > > > #ifdef CONFIG_ARM > > > -static void __iomem *rtc_base; > > > -static struct timespec64 persistent_ts; > > > -static u64 persistent_ms, last_persistent_ms; > > > static struct delay_timer tegra_delay_timer; > > > #endif > > > @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) > > > return readl(timer_reg_base + TIMERUS_CNTR_1US); > > > } > > > +static struct timer_of suspend_rtc_to = { > > > + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, > > > +}; > > > + > > > /* > > > * tegra_rtc_read - Reads the Tegra RTC registers > > > * Care must be taken that this funciton is not called while the > > > * tegra_rtc driver could be executing to avoid race conditions > > > * on the RTC shadow register > > > */ > > > -static u64 tegra_rtc_read_ms(void) > > > +static u64 tegra_rtc_read_ms(struct clocksource *cs) > > > { > > > - u32 ms = readl(rtc_base + RTC_MILLISECONDS); > > > - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); > > > + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); > > > + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); > > > return (u64)s * MSEC_PER_SEC + ms; > > > } > > > -/* > > > - * tegra_read_persistent_clock64 - Return time from a persistent clock. > > > - * > > > - * Reads the time from a source which isn't disabled during PM, the > > > - * 32k sync timer. Convert the cycles elapsed since last read into > > > - * nsecs and adds to a monotonically increasing timespec64. > > > - * Care must be taken that this funciton is not called while the > > > - * tegra_rtc driver could be executing to avoid race conditions > > > - * on the RTC shadow register > > > - */ > > > -static void tegra_read_persistent_clock64(struct timespec64 *ts) > > > -{ > > > - u64 delta; > > > - > > > - last_persistent_ms = persistent_ms; > > > - persistent_ms = tegra_rtc_read_ms(); > > > - delta = persistent_ms - last_persistent_ms; > > > - > > > - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); > > > - *ts = persistent_ts; > > > -} > > > +static struct clocksource suspend_rtc_clocksource = { > > > + .name = "tegra_suspend_timer", > > > + .rating = 200, > > > + .read = tegra_rtc_read_ms, > > > + .mask = CLOCKSOURCE_MASK(32), > > > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, > > > +}; > > > #endif > > > static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) > > > @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) > > > static int __init tegra20_init_rtc(struct device_node *np) > > > { > > > - struct clk *clk; > > > + int ret; > > > - rtc_base = of_iomap(np, 0); > > > - if (!rtc_base) { > > > - pr_err("Can't map RTC registers\n"); > > > - return -ENXIO; > > > - } > > > + ret = timer_of_init(np, &suspend_rtc_to); > > > + if (ret) > > > + return ret; > > > - /* > > > - * rtc registers are used by read_persistent_clock, keep the rtc clock > > > - * enabled > > > - */ > > > - clk = of_clk_get(np, 0); > > > - if (IS_ERR(clk)) > > > - pr_warn("Unable to get rtc-tegra clock\n"); > > > - else > > > - clk_prepare_enable(clk); > > > + clocksource_register_hz(&suspend_rtc_clocksource, 1000); > > > - return register_persistent_clock(tegra_read_persistent_clock64); > > > + return 0; > > > } > > > TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); > > > #endif > > > > I wonder if there's any reason left for the #ifdefs now. My recollection > > is that these were only needed because register_persistent_clock() was > > not available on 64-bit ARM. The new APIs seem to be available > > regardless of architecture, so do we still need to differentiate? > > > > Actually, only Tegra20/30 that doesn't have ARM arch timer support need > this. The latter Tegra chips which have ARM arch timer support use TSC ( > time stamp counter or timer system counter depends on the chip it has > different name) as the timer source in the PMC. And it uses OSC during > runtime and switches to 32KHz always-on clock source to keep counting when > the chip is in the SC7 or LP0 state. > > So I didn't change that for this reason. Okay, looks good then. Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-02 16:49 ` Thierry Reding 0 siblings, 0 replies; 12+ messages in thread From: Thierry Reding @ 2019-04-02 16:49 UTC (permalink / raw) To: Joseph Lo Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra, Thomas Gleixner, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 5344 bytes --] On Tue, Apr 02, 2019 at 11:50:20PM +0800, Joseph Lo wrote: > On 4/2/19 10:46 PM, Thierry Reding wrote: > > On Tue, Apr 02, 2019 at 11:02:34AM +0800, Joseph Lo wrote: > > > Since the clocksource framework has the support for suspend time > > > compensation. Re-work the driver to use that, so we can reduce the > > > duplicate code. > > > > > > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > > Signed-off-by: Joseph Lo <josephl@nvidia.com> > > > --- > > > drivers/clocksource/timer-tegra20.c | 63 +++++++++-------------------- > > > 1 file changed, 20 insertions(+), 43 deletions(-) > > > > Nice! > > > > > > > > diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c > > > index fdb3d795a409..919b3568c495 100644 > > > --- a/drivers/clocksource/timer-tegra20.c > > > +++ b/drivers/clocksource/timer-tegra20.c > > > @@ -60,9 +60,6 @@ > > > static u32 usec_config; > > > static void __iomem *timer_reg_base; > > > #ifdef CONFIG_ARM > > > -static void __iomem *rtc_base; > > > -static struct timespec64 persistent_ts; > > > -static u64 persistent_ms, last_persistent_ms; > > > static struct delay_timer tegra_delay_timer; > > > #endif > > > @@ -199,40 +196,30 @@ static unsigned long tegra_delay_timer_read_counter_long(void) > > > return readl(timer_reg_base + TIMERUS_CNTR_1US); > > > } > > > +static struct timer_of suspend_rtc_to = { > > > + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK, > > > +}; > > > + > > > /* > > > * tegra_rtc_read - Reads the Tegra RTC registers > > > * Care must be taken that this funciton is not called while the > > > * tegra_rtc driver could be executing to avoid race conditions > > > * on the RTC shadow register > > > */ > > > -static u64 tegra_rtc_read_ms(void) > > > +static u64 tegra_rtc_read_ms(struct clocksource *cs) > > > { > > > - u32 ms = readl(rtc_base + RTC_MILLISECONDS); > > > - u32 s = readl(rtc_base + RTC_SHADOW_SECONDS); > > > + u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS); > > > + u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS); > > > return (u64)s * MSEC_PER_SEC + ms; > > > } > > > -/* > > > - * tegra_read_persistent_clock64 - Return time from a persistent clock. > > > - * > > > - * Reads the time from a source which isn't disabled during PM, the > > > - * 32k sync timer. Convert the cycles elapsed since last read into > > > - * nsecs and adds to a monotonically increasing timespec64. > > > - * Care must be taken that this funciton is not called while the > > > - * tegra_rtc driver could be executing to avoid race conditions > > > - * on the RTC shadow register > > > - */ > > > -static void tegra_read_persistent_clock64(struct timespec64 *ts) > > > -{ > > > - u64 delta; > > > - > > > - last_persistent_ms = persistent_ms; > > > - persistent_ms = tegra_rtc_read_ms(); > > > - delta = persistent_ms - last_persistent_ms; > > > - > > > - timespec64_add_ns(&persistent_ts, delta * NSEC_PER_MSEC); > > > - *ts = persistent_ts; > > > -} > > > +static struct clocksource suspend_rtc_clocksource = { > > > + .name = "tegra_suspend_timer", > > > + .rating = 200, > > > + .read = tegra_rtc_read_ms, > > > + .mask = CLOCKSOURCE_MASK(32), > > > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP, > > > +}; > > > #endif > > > static int tegra_timer_common_init(struct device_node *np, struct timer_of *to) > > > @@ -385,25 +372,15 @@ static int __init tegra_init_timer(struct device_node *np) > > > static int __init tegra20_init_rtc(struct device_node *np) > > > { > > > - struct clk *clk; > > > + int ret; > > > - rtc_base = of_iomap(np, 0); > > > - if (!rtc_base) { > > > - pr_err("Can't map RTC registers\n"); > > > - return -ENXIO; > > > - } > > > + ret = timer_of_init(np, &suspend_rtc_to); > > > + if (ret) > > > + return ret; > > > - /* > > > - * rtc registers are used by read_persistent_clock, keep the rtc clock > > > - * enabled > > > - */ > > > - clk = of_clk_get(np, 0); > > > - if (IS_ERR(clk)) > > > - pr_warn("Unable to get rtc-tegra clock\n"); > > > - else > > > - clk_prepare_enable(clk); > > > + clocksource_register_hz(&suspend_rtc_clocksource, 1000); > > > - return register_persistent_clock(tegra_read_persistent_clock64); > > > + return 0; > > > } > > > TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); > > > #endif > > > > I wonder if there's any reason left for the #ifdefs now. My recollection > > is that these were only needed because register_persistent_clock() was > > not available on 64-bit ARM. The new APIs seem to be available > > regardless of architecture, so do we still need to differentiate? > > > > Actually, only Tegra20/30 that doesn't have ARM arch timer support need > this. The latter Tegra chips which have ARM arch timer support use TSC ( > time stamp counter or timer system counter depends on the chip it has > different name) as the timer source in the PMC. And it uses OSC during > runtime and switches to 32KHz always-on clock source to keep counting when > the chip is in the SC7 or LP0 state. > > So I didn't change that for this reason. Okay, looks good then. Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time 2019-04-02 3:02 ` Joseph Lo @ 2019-04-11 13:21 ` Daniel Lezcano -1 siblings, 0 replies; 12+ messages in thread From: Daniel Lezcano @ 2019-04-11 13:21 UTC (permalink / raw) To: Joseph Lo, Thierry Reding, Jonathan Hunter, Thomas Gleixner Cc: linux-tegra, linux-arm-kernel, linux-kernel On 02/04/2019 05:02, Joseph Lo wrote: > Since the clocksource framework has the support for suspend time > compensation. Re-work the driver to use that, so we can reduce the > duplicate code. > > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- Awesome, thanks for this patch. Applied for 5.2 -- Daniel -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time @ 2019-04-11 13:21 ` Daniel Lezcano 0 siblings, 0 replies; 12+ messages in thread From: Daniel Lezcano @ 2019-04-11 13:21 UTC (permalink / raw) To: Joseph Lo, Thierry Reding, Jonathan Hunter, Thomas Gleixner Cc: linux-tegra, linux-kernel, linux-arm-kernel On 02/04/2019 05:02, Joseph Lo wrote: > Since the clocksource framework has the support for suspend time > compensation. Re-work the driver to use that, so we can reduce the > duplicate code. > > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- Awesome, thanks for this patch. Applied for 5.2 -- Daniel -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-11 13:21 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-02 3:02 [PATCH] clocksource/drivers/tegra: rework for compensation of suspend time Joseph Lo 2019-04-02 3:02 ` Joseph Lo 2019-04-02 3:02 ` Joseph Lo 2019-04-02 14:46 ` Thierry Reding 2019-04-02 14:46 ` Thierry Reding 2019-04-02 15:50 ` Joseph Lo 2019-04-02 15:50 ` Joseph Lo 2019-04-02 15:50 ` Joseph Lo 2019-04-02 16:49 ` Thierry Reding 2019-04-02 16:49 ` Thierry Reding 2019-04-11 13:21 ` Daniel Lezcano 2019-04-11 13:21 ` Daniel Lezcano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.