Few more cleanups for tegra-timer
Subsystems (2)
cover Few more cleanups for tegra-timer
Hello,
I took a look at tegra-timer once again and spotted few more things that could be improved in addition to [0] which is already in linux-next.
Please apply this small series in addition to [0], thanks in advance!
[0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=111529
Dmitry Osipenko (3):
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
drivers/clocksource/timer-tegra.c | 47 +++++++++++++++++++------------ 1 file changed, 29 insertions(+), 18 deletions(-)
Signature · 1 line
2.21.0
PATCH 1/3 clocksource/drivers/tegra: Remove duplicated use of per_cpu_ptr
It was left unnoticed by accident, which means that the code could be cleaned up a tad more.
--- drivers/clocksource/timer-tegra.c | 40 +++++++++++++++++++------------ 1 file changed, 25 insertions(+), 15 deletions(-)
drivers/clocksource/timer-tegra.c +25 -15
@@ -216,6 +216,19 @@ static inline unsigned int tegra_irq_idx_for_cpu(int cpu, bool tegra20)return TIMER10_IRQ_IDX + cpu; } +static inline unsigned long tegra_rate_for_timer (struct timer_of *to, +bool tegra20) +{ +/* +* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the +* parent clock. +*/ +if (tegra20) +return 1000000 ; + +return to->of_clk.rate; +} +static int __init tegra_init_timer (struct device_node *np,bool tegra20,int rating) {
@@ -268,30 +281,27 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,for_each_possible_cpu (cpu) {struct timer_of *cpu_to =per_cpu_ptr (&tegra_to, cpu); +unsigned long flags =IRQF_TIMER |IRQF_NOBALANCING ; +unsigned long rate =tegra_rate_for_timer (&tegra_to, tegra20);unsigned int base =tegra_base_for_cpu (cpu, tegra20);unsigned int idx =tegra_irq_idx_for_cpu (cpu, tegra20); +unsigned int irq =irq_of_parse_and_map (np, idx); -/* -* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the -* parent clock. -*/ -if (tegra20) - cpu_to->of_clk.rate =1000000 ; - - cpu_to =per_cpu_ptr (&tegra_to, cpu); - cpu_to->of_base.base = timer_reg_base + base; - cpu_to->clkevt.rating = rating; - cpu_to->clkevt.cpumask =cpumask_of (cpu); - cpu_to->clkevt.irq =irq_of_parse_and_map (np, idx); -if (!cpu_to->clkevt.irq) { +if (!irq) {pr_err ("failed to map irq for cpu%d\n" , cpu); ret = -EINVAL ; goto out_irq; } + cpu_to->clkevt.irq = irq; + 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.rate = rate; +irq_set_status_flags (cpu_to->clkevt.irq,IRQ_NOAUTOEN ); - ret =request_irq (cpu_to->clkevt.irq, tegra_timer_isr, -IRQF_TIMER |IRQF_NOBALANCING , + + ret =request_irq (cpu_to->clkevt.irq, tegra_timer_isr, flags, cpu_to->clkevt.name, &cpu_to->clkevt);if (ret) { pr_err("failed to set up irq for cpu%d: %d\n",
Signature · 1 line
2.21.0
Re: [PATCH v1 1/3] clocksource/drivers/tegra: Remove duplicated use of per_cpu_ptr
Hi Dmitry,
Quoted 15 lines · Dmitry Osipenko
It was left unnoticed by accident, which means that the code could be cleaned up a tad more.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/clocksource/timer-tegra.c | 40 +++++++++++++++++++------------ 1 file changed, 25 insertions(+), 15 deletions(-)drivers/clocksource/timer-tegra.c +0
-0@@ -216,6 +216,19 @@ static inline unsigned int tegra_irq_idx_for_cpu(int cpu, bool tegra20)return TIMER10_IRQ_IDX + cpu; }
drivers/clocksource/timer-tegra.c static inline unsigned int tegra_irq_idx_for_cpu(int cpu, bool tegra20) · line 227
Quoted 10 lines
drivers/clocksource/timer-tegra.c +9
-0+static inline unsigned long tegra_rate_for_timer (struct timer_of *to, +bool tegra20) +{ +/* +* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the +* parent clock. +*/ +if (tegra20) +return 1000000 ;
Mind to take the opportunity to convert the literal value to a constant?
Quoted 51 lines
drivers/clocksource/timer-tegra.c +16
-15+ +return to->of_clk.rate; +} +static int __init tegra_init_timer (struct device_node *np,bool tegra20,int rating) {@@ -268,30 +281,27 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,for_each_possible_cpu (cpu) {struct timer_of *cpu_to =per_cpu_ptr (&tegra_to, cpu); +unsigned long flags =IRQF_TIMER |IRQF_NOBALANCING ; +unsigned long rate =tegra_rate_for_timer (&tegra_to, tegra20);unsigned int base =tegra_base_for_cpu (cpu, tegra20);unsigned int idx =tegra_irq_idx_for_cpu (cpu, tegra20); +unsigned int irq =irq_of_parse_and_map (np, idx); -/* -* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the -* parent clock. -*/ -if (tegra20) - cpu_to->of_clk.rate =1000000 ; - - cpu_to =per_cpu_ptr (&tegra_to, cpu); - cpu_to->of_base.base = timer_reg_base + base; - cpu_to->clkevt.rating = rating; - cpu_to->clkevt.cpumask =cpumask_of (cpu); - cpu_to->clkevt.irq =irq_of_parse_and_map (np, idx); -if (!cpu_to->clkevt.irq) { +if (!irq) {pr_err ("failed to map irq for cpu%d\n" , cpu); ret = -EINVAL ; goto out_irq; } + cpu_to->clkevt.irq = irq; + 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.rate = rate; +irq_set_status_flags (cpu_to->clkevt.irq,IRQ_NOAUTOEN ); - ret =request_irq (cpu_to->clkevt.irq, tegra_timer_isr, -IRQF_TIMER |IRQF_NOBALANCING , + + ret =request_irq (cpu_to->clkevt.irq, tegra_timer_isr, flags, cpu_to->clkevt.name, &cpu_to->clkevt);if (ret) { pr_err("failed to set up irq for cpu%d: %d\n",
Signature · 5 lines
<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
Re: [PATCH v1 1/3] clocksource/drivers/tegra: Remove duplicated use of per_cpu_ptr
10.06.2019 11:11, Daniel Lezcano пишет:
Quoted 21 lines
Hi Dmitry,
Quoted 16 lines · Dmitry Osipenko
It was left unnoticed by accident, which means that the code could be cleaned up a tad more.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/clocksource/timer-tegra.c | 40 +++++++++++++++++++------------ 1 file changed, 25 insertions(+), 15 deletions(-)drivers/clocksource/timer-tegra.c +0
-0@@ -216,6 +216,19 @@ static inline unsigned int tegra_irq_idx_for_cpu(int cpu, bool tegra20)return TIMER10_IRQ_IDX + cpu; }
Quoted 11 lines
drivers/clocksource/timer-tegra.c static inline unsigned int tegra_irq_idx_for_cpu(int cpu, bool tegra20) · line 227
Quoted 9 lines
drivers/clocksource/timer-tegra.c +9
-0+static inline unsigned long tegra_rate_for_timer (struct timer_of *to, +bool tegra20) +{ +/* +* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the +* parent clock. +*/ +if (tegra20) +return 1000000 ;Mind to take the opportunity to convert the literal value to a constant?
Sure!
Quoted 9 lines
Quoted 9 lines
drivers/clocksource/timer-tegra.c +4
-0+ +return to->of_clk.rate; +} +static int __init tegra_init_timer (struct device_node *np,bool tegra20,int rating) {@@ -268,30 +281,27 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20,
Quoted 4 lines
drivers/clocksource/timer-tegra.c static int __init tegra_init_timer(struct device_node *np, bool tegra20, · line 285
Quoted 4 lines
drivers/clocksource/timer-tegra.c +2
-0for_each_possible_cpu (cpu) {struct timer_of *cpu_to =per_cpu_ptr (&tegra_to, cpu); +unsigned long flags =IRQF_TIMER |IRQF_NOBALANCING ; +unsigned long rate =tegra_rate_for_timer (&tegra_to, tegra20);
Oh, this actually shall be (to, tegra20). Will correct this in v2!
Quoted 10 lines
drivers/clocksource/timer-tegra.c static int __init tegra_init_timer(struct device_node *np, bool tegra20, · line 279
Quoted 10 lines
drivers/clocksource/timer-tegra.c +1
-6unsigned int base =tegra_base_for_cpu (cpu, tegra20);unsigned int idx =tegra_irq_idx_for_cpu (cpu, tegra20); +unsigned int irq =irq_of_parse_and_map (np, idx); -/* -* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the -* parent clock. -*/ -if (tegra20) - cpu_to->of_clk.rate =1000000 ;
I also spotted that there is a bug here that I introduced in a previous patch. The of_clk.rate is initialized only for the first per-cpu clocksource and then we need to replicate it for the rest of CPU's in a case of T210. I'll add an explicit fixup for this.
PATCH 2/3 clocksource/drivers/tegra: Set and use timer's period
In reply to Dmitry OsipenkoThe 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.
--- drivers/clocksource/timer-tegra.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/clocksource/timer-tegra.c +3 -2
@@ -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 =DIV_ROUND_UP (rate,HZ ); cpu_to->of_clk.rate = rate;irq_set_status_flags (cpu_to->clkevt.irq,IRQ_NOAUTOEN );
Signature · 1 line
2.21.0
PATCH 3/3 clocksource/drivers/tegra: Drop unneeded typecasting in one place
In reply to Dmitry OsipenkoThere is no need to cast void because kernel allows to do that without a warning message from a compiler.
--- drivers/clocksource/timer-tegra.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clocksource/timer-tegra.c +1 -1
@@ -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 );
Signature · 1 line
2.21.0