linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Clément Péron" <peron.clem@gmail.com>
To: stefan@agner.ch
Cc: "Colin Didier" <colin.didier@devialet.com>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, "Rob Herring" <robh@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Daniel Lezcano" <daniel.lezcano@linaro.org>,
	"Clément Peron" <clement.peron@devialet.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Fabio Estevam" <fabio.estevam@nxp.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Vladimir Zapolskiy" <vladimir_zapolskiy@mentor.com>
Subject: Re: [PATCH v6 4/5] clocksource: add driver for i.MX EPIT timer
Date: Mon, 11 Jun 2018 14:42:04 +0200	[thread overview]
Message-ID: <CAJiuCcdHKpfCkLhV=rZcLHx2k9BAh7bnDQMWDkquXJOYiQoTeA@mail.gmail.com> (raw)
In-Reply-To: <5df13d8f6422d9099c1692dd2091d9ca@agner.ch>

Hi Stefan,


> > +
> > +#define EPITCR                               0x00
> > +#define EPITSR                               0x04
> > +#define EPITLR                               0x08
> > +#define EPITCMPR                     0x0c
> > +#define EPITCNR                              0x10
> > +
> > +#define EPITCR_EN                    BIT(0)
> > +#define EPITCR_ENMOD                 BIT(1)
> > +#define EPITCR_OCIEN                 BIT(2)
> > +#define EPITCR_RLD                   BIT(3)
> > +#define EPITCR_PRESC(x)                      (((x) & 0xfff) << 4)
> > +#define EPITCR_SWR                   BIT(16)
> > +#define EPITCR_IOVW                  BIT(17)
> > +#define EPITCR_DBGEN                 BIT(18)
> > +#define EPITCR_WAITEN                        BIT(19)
> > +#define EPITCR_RES                   BIT(20)
> > +#define EPITCR_STOPEN                        BIT(21)
> > +#define EPITCR_OM_DISCON             (0 << 22)
> > +#define EPITCR_OM_TOGGLE             (1 << 22)
> > +#define EPITCR_OM_CLEAR                      (2 << 22)
> > +#define EPITCR_OM_SET                        (3 << 22)
> > +#define EPITCR_CLKSRC_OFF            (0 << 24)
> > +#define EPITCR_CLKSRC_PERIPHERAL     (1 << 24)
> > +#define EPITCR_CLKSRC_REF_HIGH               (2 << 24)
> > +#define EPITCR_CLKSRC_REF_LOW                (3 << 24)
> > +
> > +#define EPITSR_OCIF                  BIT(0)
> > +
> > +struct epit_timer {
> > +     void __iomem *base;
> > +     int irq;
> > +     struct clk *clk;
> > +     struct clock_event_device ced;
> > +     struct irqaction act;
> > +};
> > +
> > +static void __iomem *sched_clock_reg;
> > +
> > +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced)
> > +{
> > +     return container_of(ced, struct epit_timer, ced);
> > +}
> > +
> > +static inline void epit_irq_disable(struct epit_timer *epittm)
> > +{
> > +     u32 val;
> > +
> > +     val = readl_relaxed(epittm->base + EPITCR);
> > +     writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR);
> > +}
> > +
> > +static inline void epit_irq_enable(struct epit_timer *epittm)
> > +{
> > +     u32 val;
> > +
> > +     val = readl_relaxed(epittm->base + EPITCR);
> > +     writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR);
> > +}
> > +
> > +static void epit_irq_acknowledge(struct epit_timer *epittm)
> > +{
> > +     writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR);
> > +}
> > +
> > +static u64 notrace epit_read_sched_clock(void)
> > +{
> > +     return ~readl_relaxed(sched_clock_reg);
> > +}
> > +
> > +static int epit_set_next_event(unsigned long cycles,
> > +                            struct clock_event_device *ced)
> > +{
> > +     struct epit_timer *epittm = to_epit_timer(ced);
> > +     unsigned long tcmp;
> > +
> > +     tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles;
> > +     writel_relaxed(tcmp, epittm->base + EPITCMPR);
> > +
> > +     return 0;
> > +}
> > +
> > +/* Left event sources disabled, no more interrupts appear */
> > +static int epit_shutdown(struct clock_event_device *ced)
> > +{
> > +     struct epit_timer *epittm = to_epit_timer(ced);
> > +     unsigned long flags;
> > +
> > +     /*
> > +      * The timer interrupt generation is disabled at least
> > +      * for enough time to call epit_set_next_event()
> > +      */
> > +     local_irq_save(flags);
> > +
> > +     /* Disable interrupt in EPIT module */
> > +     epit_irq_disable(epittm);
> > +
> > +     /* Clear pending interrupt */
> > +     epit_irq_acknowledge(epittm);
> > +
> > +     local_irq_restore(flags);
> > +
> > +     return 0;
> > +}
> > +
> > +static int epit_set_oneshot(struct clock_event_device *ced)
> > +{
> > +     struct epit_timer *epittm = to_epit_timer(ced);
> > +     unsigned long flags;
> > +
> > +     /*
> > +      * The timer interrupt generation is disabled at least
> > +      * for enough time to call epit_set_next_event()
> > +      */
> > +     local_irq_save(flags);
> > +
> > +     /* Disable interrupt in EPIT module */
> > +     epit_irq_disable(epittm);
> > +
> > +     /* Clear pending interrupt, only while switching mode */
> > +     if (!clockevent_state_oneshot(ced))
> > +             epit_irq_acknowledge(epittm);
> > +
> > +     /*
> > +      * Do not put overhead of interrupt enable/disable into
> > +      * epit_set_next_event(), the core has about 4 minutes
> > +      * to call epit_set_next_event() or shutdown clock after
> > +      * mode switching
> > +      */
> > +     epit_irq_enable(epittm);
> > +     local_irq_restore(flags);
> > +
> > +     return 0;
> > +}
> > +
> > +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id)
> > +{
> > +     struct clock_event_device *ced = dev_id;
> > +     struct epit_timer *epittm = to_epit_timer(ced);
> > +
> > +     epit_irq_acknowledge(epittm);
> > +
> > +     ced->event_handler(ced);
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static int __init epit_clocksource_init(struct epit_timer *epittm)
> > +{
> > +     unsigned int c = clk_get_rate(epittm->clk);
> > +
> > +     sched_clock_reg = epittm->base + EPITCNR;
> > +     sched_clock_register(epit_read_sched_clock, 32, c);
> > +
> > +     return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32,
> > +                                  clocksource_mmio_readl_down);
> > +}
> > +
> > +static int __init epit_clockevent_init(struct epit_timer *epittm)
> > +{
> > +     struct clock_event_device *ced = &epittm->ced;
> > +     struct irqaction *act = &epittm->act;
> > +
> > +     ced->name = "epit";
> > +     ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
> > +     ced->set_state_shutdown = epit_shutdown;
> > +     ced->tick_resume = epit_shutdown;
> > +     ced->set_state_oneshot = epit_set_oneshot;
> > +     ced->set_next_event = epit_set_next_event;
> > +     ced->rating = 200;
> > +     ced->cpumask = cpumask_of(0);
> > +     ced->irq = epittm->irq;
> > +     clockevents_config_and_register(ced, clk_get_rate(epittm->clk),
> > +                                     0xff, 0xfffffffe);
> > +
> > +     act->name = "i.MX EPIT Timer Tick",
> > +     act->flags = IRQF_TIMER | IRQF_IRQPOLL;
> > +     act->handler = epit_timer_interrupt;
> > +     act->dev_id = ced;
> > +
> > +     /* Make irqs happen */
> > +     return setup_irq(epittm->irq, act);
> > +}
> > +
> > +static int __init epit_timer_init(struct device_node *np)
> > +{
> > +     struct epit_timer *epittm;
> > +     int ret;
> > +
> > +     epittm = kzalloc(sizeof(*epittm), GFP_KERNEL);
> > +     if (!epittm)
> > +             return -ENOMEM;
> > +
> > +     epittm->base = of_iomap(np, 0);
> > +     if (!epittm->base) {
> > +             ret = -ENXIO;
> > +             goto out_kfree;
> > +     }
> > +
> > +     epittm->irq = irq_of_parse_and_map(np, 0);
> > +     if (!epittm->irq) {
> > +             ret = -EINVAL;
> > +             goto out_iounmap;
> > +     }
> > +
> > +        /* Get EPIT clock */
> > +        epittm->clk = of_clk_get(np, 0);
> > +        if (IS_ERR(epittm->clk)) {
> > +             pr_err("i.MX EPIT: unable to get clk\n");
> > +             ret = PTR_ERR(epittm->clk);
> > +             goto out_iounmap;
> > +        }
>
> There is something off with indent here.
Thanks for pointing out that, I will fix it.

>
> There is a helper library in drivers/clocksource/timer-of.c which might
> be useful for this driver.
Indeed, but will require a bit of rewrite.

Clement

>
> --
> Stefan
>
> > +
> > +     ret = clk_prepare_enable(epittm->clk);
> > +     if (ret) {
> > +             pr_err("i.MX EPIT: unable to prepare+enable clk\n");
> > +             goto out_iounmap;
> > +     }
> > +
> > +     /* Initialise to a known state (all timers off, and timing reset) */
> > +     writel_relaxed(0x0, epittm->base + EPITCR);
> > +     writel_relaxed(0xffffffff, epittm->base + EPITLR);
> > +     writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN,
> > +                    epittm->base + EPITCR);
> > +
> > +     ret = epit_clocksource_init(epittm);
> > +     if (ret) {
> > +             pr_err("i.MX EPIT: failed to init clocksource\n");
> > +             goto out_clk_disable;
> > +     }
> > +
> > +     ret = epit_clockevent_init(epittm);
> > +     if (ret) {
> > +             pr_err("i.MX EPIT: failed to init clockevent\n");
> > +             goto out_clk_disable;
> > +     }
> > +
> > +     return 0;
> > +
> > +out_clk_disable:
> > +     clk_disable_unprepare(epittm->clk);
> > +out_iounmap:
> > +     iounmap(epittm->base);
> > +out_kfree:
> > +     kfree(epittm);
> > +
> > +     return ret;
> > +}
> > +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init);

  reply	other threads:[~2018-06-11 12:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-07 14:05 [PATCH v6 0/5] Reintroduce i.MX EPIT Timer Clément Péron
2018-06-07 14:05 ` [PATCH v6 1/5] clk: imx6: add EPIT clock support Clément Péron
2018-06-07 14:05 ` [PATCH v6 2/5] ARM: imx: remove inexistant EPIT timer init Clément Péron
2018-06-11 14:39   ` Vladimir Zapolskiy
2018-06-07 14:05 ` [PATCH v6 3/5] dt-bindings: timer: add i.MX EPIT timer binding Clément Péron
2018-06-11 14:39   ` Vladimir Zapolskiy
2018-06-11 18:10   ` Rob Herring
2018-06-12  8:06     ` Clément Péron
2018-06-07 14:05 ` [PATCH v6 4/5] clocksource: add driver for i.MX EPIT timer Clément Péron
2018-06-11 12:31   ` Stefan Agner
2018-06-11 12:42     ` Clément Péron [this message]
2018-06-11 12:46       ` Stefan Agner
2018-06-11 14:40   ` Vladimir Zapolskiy
2018-06-07 14:05 ` [PATCH v6 5/5] ARM: dts: imx: add missing compatible and clock properties for EPIT Clément Péron
2018-06-11 14:40   ` Vladimir Zapolskiy
2018-06-11 14:39 ` [PATCH v6 0/5] Reintroduce i.MX EPIT Timer Vladimir Zapolskiy
2018-06-11 14:46   ` Clément Péron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJiuCcdHKpfCkLhV=rZcLHx2k9BAh7bnDQMWDkquXJOYiQoTeA@mail.gmail.com' \
    --to=peron.clem@gmail.com \
    --cc=clement.peron@devialet.com \
    --cc=colin.didier@devialet.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=stefan@agner.ch \
    --cc=tglx@linutronix.de \
    --cc=vladimir_zapolskiy@mentor.com \
    --subject='Re: [PATCH v6 4/5] clocksource: add driver for i.MX EPIT timer' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).