Hi Anson,


On 06/11/2018 06:15, Anson Huang wrote:
> Convert the driver to use the timer_of helpers.
> This allows to handle timer base, clock and irq using
> common timer_of driver and it simplifies the code.

Can you do an extra move by passing the timer_of structure around for:
 - tpm_timer_disable
 - tpm_timer_enable
 - tpm_irq_acknowledge

and use to_timer_of(clkevt) to retrieve the timer_base value for other
clockevent callbacks ?

The clockevent code will be self-encapsulated.



> Signed-off-by: Anson Huang <anson.hu...@nxp.com>
> ---
>  drivers/clocksource/timer-imx-tpm.c | 136 
> +++++++++++++++---------------------
>  1 file changed, 55 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-imx-tpm.c 
> b/drivers/clocksource/timer-imx-tpm.c
> index b7aa2b8..c3dd4d2 100644
> --- a/drivers/clocksource/timer-imx-tpm.c
> +++ b/drivers/clocksource/timer-imx-tpm.c
> @@ -12,6 +12,8 @@
>  #include <linux/of_irq.h>
>  #include <linux/sched_clock.h>
>  
> +#include "timer-of.h"
> +
>  #define TPM_PARAM                    0x4
>  #define TPM_PARAM_WIDTH_SHIFT                16
>  #define TPM_PARAM_WIDTH_MASK         (0xff << 16)
> @@ -33,9 +35,7 @@
>  #define TPM_C0V                              0x24
>  
>  static int counter_width;
> -static int rating;
>  static void __iomem *timer_base;
> -static struct clock_event_device clockevent_tpm;
>  
>  static inline void tpm_timer_disable(void)
>  {
> @@ -80,19 +80,6 @@ static u64 notrace tpm_read_sched_clock(void)
>       return tpm_read_counter();
>  }
>  
> -static int __init tpm_clocksource_init(unsigned long rate)
> -{
> -     tpm_delay_timer.read_current_timer = &tpm_read_current_timer;
> -     tpm_delay_timer.freq = rate;
> -     register_current_timer_delay(&tpm_delay_timer);
> -
> -     sched_clock_register(tpm_read_sched_clock, counter_width, rate);
> -
> -     return clocksource_mmio_init(timer_base + TPM_CNT, "imx-tpm",
> -                                  rate, rating, counter_width,
> -                                  clocksource_mmio_readl_up);
> -}
> -
>  static int tpm_set_next_event(unsigned long delta,
>                               struct clock_event_device *evt)
>  {
> @@ -137,74 +124,77 @@ static irqreturn_t tpm_timer_interrupt(int irq, void 
> *dev_id)
>       return IRQ_HANDLED;
>  }
>  
> -static struct clock_event_device clockevent_tpm = {
> -     .name                   = "i.MX7ULP TPM Timer",
> -     .features               = CLOCK_EVT_FEAT_ONESHOT,
> -     .set_state_oneshot      = tpm_set_state_oneshot,
> -     .set_next_event         = tpm_set_next_event,
> -     .set_state_shutdown     = tpm_set_state_shutdown,
> +static struct timer_of to_tpm = {
> +     .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> +     .clkevt = {
> +             .name                   = "i.MX7ULP TPM Timer",
> +             .rating                 = 200,
> +             .features               = CLOCK_EVT_FEAT_ONESHOT,
> +             .set_state_shutdown     = tpm_set_state_shutdown,
> +             .set_state_oneshot      = tpm_set_state_oneshot,
> +             .set_next_event         = tpm_set_next_event,
> +             .cpumask                = cpu_possible_mask,
> +     },
> +     .of_irq = {
> +             .handler                = tpm_timer_interrupt,
> +             .flags                  = IRQF_TIMER | IRQF_IRQPOLL,
> +     },
>  };
>  
> -static int __init tpm_clockevent_init(unsigned long rate, int irq)
> +static int __init tpm_clocksource_init(void)
>  {
> -     int ret;
> +     tpm_delay_timer.read_current_timer = &tpm_read_current_timer;
> +     tpm_delay_timer.freq = timer_of_rate(&to_tpm) >> 3;
> +     register_current_timer_delay(&tpm_delay_timer);
>  
> -     ret = request_irq(irq, tpm_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
> -                       "i.MX7ULP TPM Timer", &clockevent_tpm);
> +     sched_clock_register(tpm_read_sched_clock, counter_width,
> +                          timer_of_rate(&to_tpm) >> 3);
>  
> -     clockevent_tpm.rating = rating;
> -     clockevent_tpm.cpumask = cpumask_of(0);
> -     clockevent_tpm.irq = irq;
> -     clockevents_config_and_register(&clockevent_tpm, rate, 300,
> -                                     GENMASK(counter_width - 1, 1));
> +     return clocksource_mmio_init(timer_base + TPM_CNT,
> +                                  "imx-tpm",
> +                                  timer_of_rate(&to_tpm) >> 3,
> +                                  to_tpm.clkevt.rating,
> +                                  counter_width,
> +                                  clocksource_mmio_readl_up);
> +}
>  
> -     return ret;
> +static void __init tpm_clockevent_init(void)
> +{
> +     clockevents_config_and_register(&to_tpm.clkevt,
> +                                     timer_of_rate(&to_tpm) >> 3,
> +                                     300,
> +                                     GENMASK(counter_width - 1,
> +                                     1));
>  }
>  
>  static int __init tpm_timer_init(struct device_node *np)
>  {
> -     struct clk *ipg, *per;
> -     int irq, ret;
> -     u32 rate;
> -
> -     timer_base = of_iomap(np, 0);
> -     if (!timer_base) {
> -             pr_err("tpm: failed to get base address\n");
> -             return -ENXIO;
> -     }
> -
> -     irq = irq_of_parse_and_map(np, 0);
> -     if (!irq) {
> -             pr_err("tpm: failed to get irq\n");
> -             ret = -ENOENT;
> -             goto err_iomap;
> -     }
> +     struct clk *ipg;
> +     int ret;
>  
>       ipg = of_clk_get_by_name(np, "ipg");
> -     per = of_clk_get_by_name(np, "per");
> -     if (IS_ERR(ipg) || IS_ERR(per)) {
> -             pr_err("tpm: failed to get ipg or per clk\n");
> -             ret = -ENODEV;
> -             goto err_clk_get;
> +     if (IS_ERR(ipg)) {
> +             pr_err("tpm: failed to get ipg clk\n");
> +             return -ENODEV;
>       }
> -
>       /* enable clk before accessing registers */
>       ret = clk_prepare_enable(ipg);
>       if (ret) {
>               pr_err("tpm: ipg clock enable failed (%d)\n", ret);
> -             goto err_clk_get;
> +             clk_put(ipg);
> +             return ret;
>       }
>  
> -     ret = clk_prepare_enable(per);
> -     if (ret) {
> -             pr_err("tpm: per clock enable failed (%d)\n", ret);
> -             goto err_per_clk_enable;
> -     }
> +     ret = timer_of_init(np, &to_tpm);
> +     if (ret)
> +             return ret;
> +
> +     timer_base = timer_of_base(&to_tpm);
>  
> -     counter_width = (readl(timer_base + TPM_PARAM) & TPM_PARAM_WIDTH_MASK)
> -             >> TPM_PARAM_WIDTH_SHIFT;
> +     counter_width = (readl(timer_base + TPM_PARAM)
> +             & TPM_PARAM_WIDTH_MASK) >> TPM_PARAM_WIDTH_SHIFT;
>       /* use rating 200 for 32-bit counter and 150 for 16-bit counter */
> -     rating = counter_width == 0x20 ? 200 : 150;
> +     to_tpm.clkevt.rating = counter_width == 0x20 ? 200 : 150;
>  
>       /*
>        * Initialize tpm module to a known state
> @@ -229,29 +219,13 @@ static int __init tpm_timer_init(struct device_node *np)
>       writel(TPM_SC_CMOD_INC_PER_CNT |
>               (counter_width == 0x20 ?
>               TPM_SC_CMOD_DIV_DEFAULT : TPM_SC_CMOD_DIV_MAX),
> -                  timer_base + TPM_SC);
> +             timer_base + TPM_SC);
>  
>       /* set MOD register to maximum for free running mode */
>       writel(GENMASK(counter_width - 1, 0), timer_base + TPM_MOD);
>  
> -     rate = clk_get_rate(per) >> 3;
> -     ret = tpm_clocksource_init(rate);
> -     if (ret)
> -             goto err_per_clk_enable;
> -
> -     ret = tpm_clockevent_init(rate, irq);
> -     if (ret)
> -             goto err_per_clk_enable;
> -
> -     return 0;
> +     tpm_clockevent_init();
>  
> -err_per_clk_enable:
> -     clk_disable_unprepare(ipg);
> -err_clk_get:
> -     clk_put(per);
> -     clk_put(ipg);
> -err_iomap:
> -     iounmap(timer_base);
> -     return ret;
> +     return tpm_clocksource_init();
>  }
>  TIMER_OF_DECLARE(imx7ulp, "fsl,imx7ulp-tpm", tpm_timer_init);
> 


-- 
 <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

Reply via email to