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:
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 (6):
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
drivers/clocksource/timer-tegra.c | 59 +++++++++++++++++++------------
1 file changed, 37 insertions(+), 22 deletions(-)
--
2.21.0
Convert all 1MHz literals to a verbose constant for better readability.
Suggested-by: Daniel Lezcano <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
index a7fa12488066..2a428fdf702f 100644
--- a/drivers/clocksource/timer-tegra.c
+++ b/drivers/clocksource/timer-tegra.c
@@ -44,6 +44,8 @@
#define TIMER1_IRQ_IDX 0
#define TIMER10_IRQ_IDX 10
+#define TIMER_1MHz 1000000
+
static u32 usec_config;
static void __iomem *timer_reg_base;
@@ -158,7 +160,7 @@ static unsigned long tegra_delay_timer_read_counter_long(void)
static struct delay_timer tegra_delay_timer = {
.read_current_timer = tegra_delay_timer_read_counter_long,
- .freq = 1000000,
+ .freq = TIMER_1MHz,
};
#endif
@@ -224,7 +226,7 @@ static inline unsigned long tegra_rate_for_timer(struct timer_of *to,
* parent clock.
*/
if (tegra20)
- return 1000000;
+ return TIMER_1MHz;
return timer_of_rate(to);
}
@@ -313,11 +315,11 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
}
}
- sched_clock_register(tegra_read_sched_clock, 32, 1000000);
+ sched_clock_register(tegra_read_sched_clock, 32, TIMER_1MHz);
ret = clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
- "timer_us", 1000000,
- 300, 32, clocksource_mmio_readl_up);
+ "timer_us", TIMER_1MHz, 300, 32,
+ clocksource_mmio_readl_up);
if (ret)
pr_err("failed to register clocksource: %d\n", ret);
--
2.21.0
The clocksource rate is initialized only for the first per-CPU clocksource
and then that rate shall be replicated for the rest of clocksource's
because they are initialized manually in the code.
Fixes: 3be2a85a0b61 ("Support per-CPU timers on all Tegra's")
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 9406855781ff..830c66e2d927 100644
--- a/drivers/clocksource/timer-tegra.c
+++ b/drivers/clocksource/timer-tegra.c
@@ -277,6 +277,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
*/
if (tegra20)
cpu_to->of_clk.rate = 1000000;
+ else
+ cpu_to->of_clk.rate = timer_of_rate(to);
cpu_to = per_cpu_ptr(&tegra_to, cpu);
cpu_to->of_base.base = timer_reg_base + base;
--
2.21.0
On 10/06/2019 17:43, Dmitry Osipenko wrote:
> The clocksource rate is initialized only for the first per-CPU clocksource
> and then that rate shall be replicated for the rest of clocksource's
> because they are initialized manually in the code.
>
> Fixes: 3be2a85a0b61 ("Support per-CPU timers on all Tegra's")
> 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 9406855781ff..830c66e2d927 100644
> --- a/drivers/clocksource/timer-tegra.c
> +++ b/drivers/clocksource/timer-tegra.c
> @@ -277,6 +277,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
> */
> if (tegra20)
> cpu_to->of_clk.rate = 1000000;
> + else
> + cpu_to->of_clk.rate = timer_of_rate(to);
>
> cpu_to = per_cpu_ptr(&tegra_to, cpu);
> cpu_to->of_base.base = timer_reg_base + base;
Thanks. This fixes a boot regression we are seeing on -next with
Tegra210 (introduced by the commit referenced above). So ...
Acked-by: Jon Hunter <[email protected]>
Tested-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
12.06.2019 11:30, Jon Hunter пишет:
>
> On 10/06/2019 17:43, Dmitry Osipenko wrote:
>> The clocksource rate is initialized only for the first per-CPU clocksource
>> and then that rate shall be replicated for the rest of clocksource's
>> because they are initialized manually in the code.
>>
>> Fixes: 3be2a85a0b61 ("Support per-CPU timers on all Tegra's")
>> 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 9406855781ff..830c66e2d927 100644
>> --- a/drivers/clocksource/timer-tegra.c
>> +++ b/drivers/clocksource/timer-tegra.c
>> @@ -277,6 +277,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
>> */
>> if (tegra20)
>> cpu_to->of_clk.rate = 1000000;
>> + else
>> + cpu_to->of_clk.rate = timer_of_rate(to);
>>
>> cpu_to = per_cpu_ptr(&tegra_to, cpu);
>> cpu_to->of_base.base = timer_reg_base + base;
>
> Thanks. This fixes a boot regression we are seeing on -next with
> Tegra210 (introduced by the commit referenced above). So ...
>
> Acked-by: Jon Hunter <[email protected]>
> Tested-by: Jon Hunter <[email protected]>
Thanks for the testing :)
On 10/06/2019 17:43, Dmitry Osipenko wrote:
> Convert all 1MHz literals to a verbose constant for better readability.
>
> Suggested-by: Daniel Lezcano <[email protected]>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/clocksource/timer-tegra.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clocksource/timer-tegra.c b/drivers/clocksource/timer-tegra.c
> index a7fa12488066..2a428fdf702f 100644
> --- a/drivers/clocksource/timer-tegra.c
> +++ b/drivers/clocksource/timer-tegra.c
> @@ -44,6 +44,8 @@
> #define TIMER1_IRQ_IDX 0
> #define TIMER10_IRQ_IDX 10
>
> +#define TIMER_1MHz 1000000
> +
> static u32 usec_config;
> static void __iomem *timer_reg_base;
>
> @@ -158,7 +160,7 @@ static unsigned long tegra_delay_timer_read_counter_long(void)
>
> static struct delay_timer tegra_delay_timer = {
> .read_current_timer = tegra_delay_timer_read_counter_long,
> - .freq = 1000000,
> + .freq = TIMER_1MHz,
> };
> #endif
>
> @@ -224,7 +226,7 @@ static inline unsigned long tegra_rate_for_timer(struct timer_of *to,
> * parent clock.
> */
> if (tegra20)
> - return 1000000;
> + return TIMER_1MHz;
>
> return timer_of_rate(to);
> }
> @@ -313,11 +315,11 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
> }
> }
>
> - sched_clock_register(tegra_read_sched_clock, 32, 1000000);
> + sched_clock_register(tegra_read_sched_clock, 32, TIMER_1MHz);
>
> ret = clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
> - "timer_us", 1000000,
> - 300, 32, clocksource_mmio_readl_up);
> + "timer_us", TIMER_1MHz, 300, 32,
> + clocksource_mmio_readl_up);
> if (ret)
> pr_err("failed to register clocksource: %d\n", ret);
>
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic