Hello,
This a followup to [0] that includes some more fixes and further
prettifies the driver's code.
[0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=111529
Changelog:
v3: Addressed request from Jon Hunter that was made in a review comment
to v2 by dropping the timer's period rounding-up in the "Set and use
timer' period" patch.
Appended two new patches to this series that were already sent out
and reviewed after the v2 of this series:
clocksource/drivers/tegra: Cycles can't be 0
clocksource/drivers/tegra: Set up maximum-ticks limit properly
In this two new patches I addressed review comments that were made by
Thierry Reding by adding clarifying comments to the code and extending
the commit messages a tad.
Corrected the "Fixes" tag in a "Restore timer rate on Tegra210"
patch such that linux-next checker won't complain about the shortened
commit's subject.
v2: Fixed a bug that was introduced by [0] in a newly added patch:
"Restore timer rate on Tegra210".
Fixed potential problem in regards to error handling in another new
patch: "Restore base address before cleanup".
Added new patch "Add verbose definition for 1MHz constant" as per
Daniel's Lezcano recommendation.
Fixed a code typo that was made in "Remove duplicated use of per_cpu_ptr"
of v1.
Dmitry Osipenko (8):
clocksource/drivers/tegra: Restore timer rate on Tegra210
clocksource/drivers/tegra: Remove duplicated use of per_cpu_ptr
clocksource/drivers/tegra: Set and use timer's period
clocksource/drivers/tegra: Drop unneeded typecasting in one place
clocksource/drivers/tegra: Add verbose definition for 1MHz constant
clocksource/drivers/tegra: Restore base address before cleanup
clocksource/drivers/tegra: Cycles can't be 0
clocksource/drivers/tegra: Set up maximum-ticks limit properly
drivers/clocksource/timer-tegra.c | 82 +++++++++++++++++++++----------
1 file changed, 56 insertions(+), 26 deletions(-)
--
2.22.0
The of_clk structure has a period field that is set up initially by
timer_of_clk_init(), that period value need to be adjusted for a case of
TIMER1-9 that are running at a fixed rate that doesn't match the clock's
rate. Note that the period value is currently used only by some of the
clocksource drivers internally and hence this is just a minor cleanup
change that doesn't fix anything.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
index ff5a4ccb5d52..e6221e070499 100644
--- a/drivers/clocksource/timer-tegra.c
+++ b/drivers/clocksource/timer-tegra.c
@@ -71,9 +71,9 @@ static int tegra_timer_shutdown(struct clock_event_device *evt)
static int tegra_timer_set_periodic(struct clock_event_device *evt)
{
void __iomem *reg_base = timer_of_base(to_timer_of(evt));
+ unsigned long period = timer_of_period(to_timer_of(evt));
- writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER |
- ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
+ writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER | (period - 1),
reg_base + TIMER_PTV);
return 0;
@@ -297,6 +297,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
cpu_to->clkevt.rating = rating;
cpu_to->clkevt.cpumask = cpumask_of(cpu);
cpu_to->of_base.base = timer_reg_base + base;
+ cpu_to->of_clk.period = rate / HZ;
cpu_to->of_clk.rate = rate;
irq_set_status_flags(cpu_to->clkevt.irq, IRQ_NOAUTOEN);
--
2.22.0
We're adjusting the timer's base for each per-CPU timer to point to the
actual start of the timer since device-tree defines a compound registers
range that includes all of the timers. In this case the original base
need to be restore before calling iounmap to unmap the proper address.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
index ddf5531c48a9..2673b6e0caa8 100644
--- a/drivers/clocksource/timer-tegra.c
+++ b/drivers/clocksource/timer-tegra.c
@@ -345,6 +345,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
irq_dispose_mapping(cpu_to->clkevt.irq);
}
}
+
+ to->of_base.base = timer_reg_base;
out:
timer_of_cleanup(to);
--
2.22.0
There is no need to cast void because kernel allows to do that without
a warning message from a compiler.
Acked-by: Jon Hunter <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
index e6221e070499..3afa66c6730b 100644
--- a/drivers/clocksource/timer-tegra.c
+++ b/drivers/clocksource/timer-tegra.c
@@ -81,7 +81,7 @@ static int tegra_timer_set_periodic(struct clock_event_device *evt)
static irqreturn_t tegra_timer_isr(int irq, void *dev_id)
{
- struct clock_event_device *evt = (struct clock_event_device *)dev_id;
+ struct clock_event_device *evt = dev_id;
void __iomem *reg_base = timer_of_base(to_timer_of(evt));
writel_relaxed(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
--
2.22.0
On 18/06/2019 15:03, Dmitry Osipenko wrote:
> The of_clk structure has a period field that is set up initially by
> timer_of_clk_init(), that period value need to be adjusted for a case of
> TIMER1-9 that are running at a fixed rate that doesn't match the clock's
> rate. Note that the period value is currently used only by some of the
> clocksource drivers internally and hence this is just a minor cleanup
> change that doesn't fix anything.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/clocksource/timer-tegra.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
> index ff5a4ccb5d52..e6221e070499 100644
> --- a/drivers/clocksource/timer-tegra.c
> +++ b/drivers/clocksource/timer-tegra.c
> @@ -71,9 +71,9 @@ static int tegra_timer_shutdown(struct clock_event_device *evt)
> static int tegra_timer_set_periodic(struct clock_event_device *evt)
> {
> void __iomem *reg_base = timer_of_base(to_timer_of(evt));
> + unsigned long period = timer_of_period(to_timer_of(evt));
>
> - writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER |
> - ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
> + writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER | (period - 1),
> reg_base + TIMER_PTV);
>
> return 0;
> @@ -297,6 +297,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
> cpu_to->clkevt.rating = rating;
> cpu_to->clkevt.cpumask = cpumask_of(cpu);
> cpu_to->of_base.base = timer_reg_base + base;
> + cpu_to->of_clk.period = rate / HZ;
> cpu_to->of_clk.rate = rate;
>
> irq_set_status_flags(cpu_to->clkevt.irq, IRQ_NOAUTOEN);
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
On 18/06/2019 15:03, Dmitry Osipenko wrote:
> We're adjusting the timer's base for each per-CPU timer to point to the
> actual start of the timer since device-tree defines a compound registers
> range that includes all of the timers. In this case the original base
> need to be restore before calling iounmap to unmap the proper address.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/clocksource/timer-tegra.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
> index ddf5531c48a9..2673b6e0caa8 100644
> --- a/drivers/clocksource/timer-tegra.c
> +++ b/drivers/clocksource/timer-tegra.c
> @@ -345,6 +345,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
> irq_dispose_mapping(cpu_to->clkevt.irq);
> }
> }
> +
> + to->of_base.base = timer_reg_base;
> out:
> timer_of_cleanup(to);
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
18.06.2019 19:32, Jon Hunter пишет:
>
> On 18/06/2019 15:03, Dmitry Osipenko wrote:
>> The of_clk structure has a period field that is set up initially by
>> timer_of_clk_init(), that period value need to be adjusted for a case of
>> TIMER1-9 that are running at a fixed rate that doesn't match the clock's
>> rate. Note that the period value is currently used only by some of the
>> clocksource drivers internally and hence this is just a minor cleanup
>> change that doesn't fix anything.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/clocksource/timer-tegra.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
>> index ff5a4ccb5d52..e6221e070499 100644
>> --- a/drivers/clocksource/timer-tegra.c
>> +++ b/drivers/clocksource/timer-tegra.c
>> @@ -71,9 +71,9 @@ static int tegra_timer_shutdown(struct clock_event_device *evt)
>> static int tegra_timer_set_periodic(struct clock_event_device *evt)
>> {
>> void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>> + unsigned long period = timer_of_period(to_timer_of(evt));
>>
>> - writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER |
>> - ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
>> + writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER | (period - 1),
>> reg_base + TIMER_PTV);
>>
>> return 0;
>> @@ -297,6 +297,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
>> cpu_to->clkevt.rating = rating;
>> cpu_to->clkevt.cpumask = cpumask_of(cpu);
>> cpu_to->of_base.base = timer_reg_base + base;
>> + cpu_to->of_clk.period = rate / HZ;
>> cpu_to->of_clk.rate = rate;
>>
>> irq_set_status_flags(cpu_to->clkevt.irq, IRQ_NOAUTOEN);
>
> Acked-by: Jon Hunter <[email protected]>
Thanks!
On Tue, Jun 18, 2019 at 05:03:53PM +0300, Dmitry Osipenko wrote:
> The of_clk structure has a period field that is set up initially by
> timer_of_clk_init(), that period value need to be adjusted for a case of
> TIMER1-9 that are running at a fixed rate that doesn't match the clock's
> rate. Note that the period value is currently used only by some of the
> clocksource drivers internally and hence this is just a minor cleanup
> change that doesn't fix anything.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/clocksource/timer-tegra.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Acked-by: Thierry Reding <[email protected]>
On Tue, Jun 18, 2019 at 05:03:54PM +0300, Dmitry Osipenko wrote:
> There is no need to cast void because kernel allows to do that without
> a warning message from a compiler.
>
> Acked-by: Jon Hunter <[email protected]>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/clocksource/timer-tegra.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Thierry Reding <[email protected]>
On Tue, Jun 18, 2019 at 05:03:56PM +0300, Dmitry Osipenko wrote:
> We're adjusting the timer's base for each per-CPU timer to point to the
> actual start of the timer since device-tree defines a compound registers
> range that includes all of the timers. In this case the original base
> need to be restore before calling iounmap to unmap the proper address.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/clocksource/timer-tegra.c | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Thierry Reding <[email protected]>