RE: [PATCH 1/2] plat-omap: dmtimer: Add support for interrupts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Tony,

I have investigated your proposal and written some code in order to
implement the chained interrupt as you mentioned. 
Unfortunatelly I had some troubles making it work and I would kindly ask for advice.
Basically what I found is that the chained handler interrupt is not called. The statistics timer->irq_stats.all
interrupt counter is not incremented even though from the client module I have enalbed the match and
overflow interrupt and set the timer in order to have interrupts every 10 seconds (match and then after another
10 seconds overflow). Note this worked very well with previous code whici required client module to register callback that
was called from timer interrupt.

Here is the code that implements the chained handler as you have suggested:

Signed-off-by: Andrei Varvara <andrei.varvara@xxxxxxxx>
---
 arch/arm/plat-omap/dmtimer.c              | 206 +++++++++++++++++++++++++++++-
 arch/arm/plat-omap/include/plat/dmtimer.h |  27 +++-
 2 files changed, 228 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c index 8ca94d3..f23ad9b 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -45,8 +45,11 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/dmtimer-omap.h>
-
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <plat/dmtimer.h>
+#include <linux/irqchip/chained_irq.h>
 
 static u32 omap_reserved_systimers;
 static LIST_HEAD(omap_timer_list);
@@ -359,9 +362,12 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
 
 int omap_dm_timer_get_irq(struct omap_dm_timer *timer)  {
-	if (timer)
-		return timer->irq;
-	return -EINVAL;
+	if (!timer) {
+		pr_err("%s: timer not available.\n", __func__);
+		return -EINVAL;
+	}
+
+	return timer->irq_ext;
 }
 EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
 
@@ -765,6 +771,27 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)  }  EXPORT_SYMBOL_GPL(omap_dm_timer_write_counter);
 
+int omap_dm_timer_read_counter_captures(struct omap_dm_timer *timer, u32 *tcar1,
+					u32 *tcar2)
+{
+	unsigned long flags;
+
+	if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
+		pr_err("%s: timer not available or enabled.\n", __func__);
+		return -EINVAL;
+	}
+
+	raw_spin_lock_irqsave(&timer->raw_lock, flags);
+
+	*tcar1 = timer->context.tcar1;
+	*tcar2 = timer->context.tcar2;
+
+	raw_spin_unlock_irqrestore(&timer->raw_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(omap_dm_timer_read_counter_captures);
+
 int omap_dm_timers_active(void)
 {
 	struct omap_dm_timer *timer;
@@ -782,8 +809,119 @@ int omap_dm_timers_active(void)  }  EXPORT_SYMBOL_GPL(omap_dm_timers_active);
 
+int print_timer_irq_statistics(struct omap_dm_timer *timer) {
+	u32 capture, overflow, match;
+	u64 all;
+	unsigned long flags = 0;
+
+	if (!timer) {
+		pr_err("Invalid timer handle.\n");
+		return -EINVAL;
+	}
+
+	raw_spin_lock_irqsave(&timer->raw_lock, flags);
+
+	capture = timer->irq_stats.capture;
+	overflow = timer->irq_stats.overflow;
+	match = timer->irq_stats.match;
+	all = timer->irq_stats.all;
+
+	raw_spin_unlock_irqrestore(&timer->raw_lock, flags);
+
+	pr_info("dmtimer: %s irq statistics:\n", timer->pdev->name);
+	pr_info("capture %u\n", capture);
+	pr_info("overflow_irqs %u\n", overflow);
+	pr_info("match %u\n", match);
+	pr_info("all %llu\n", all);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(print_timer_irq_statistics);
+
 static const struct of_device_id omap_timer_match[];
 
+static void timer_irq_mask(struct irq_data *data) {
+	struct omap_dm_timer *timer = irq_data_get_irq_chip_data(data);
+	int ret;
+
+	ret = omap_dm_timer_set_int_disable(timer, OMAP_TIMER_INT_MATCH |
+			OMAP_TIMER_INT_OVERFLOW | OMAP_TIMER_INT_CAPTURE); }
+
+static void timer_irq_unmask(struct irq_data *data) {
+	struct omap_dm_timer *timer = irq_data_get_irq_chip_data(data);
+	int ret;
+
+	ret = omap_dm_timer_set_int_enable(timer, OMAP_TIMER_INT_MATCH |
+			OMAP_TIMER_INT_OVERFLOW | OMAP_TIMER_INT_CAPTURE); }
+
+static struct irq_chip timer_irqchip = {
+         .name = "dmtimer-irq",
+         .irq_mask = timer_irq_mask, // omap_dm_timer_set_int_disable
+         .irq_unmask = timer_irq_unmask };
+
+static int timer_irq_map(struct irq_domain *d, unsigned int virq,
+			 irq_hw_number_t hw)
+{
+	irq_set_chip_and_handler(virq, &timer_irqchip, handle_level_irq);
+	irq_set_chip_data(virq, d->host_data);
+
+	return 0;
+}
+
+static void timer_irq_unmap(struct irq_domain *d, unsigned int virq) {
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops timer_irqdomain_ops = {
+	.map    = timer_irq_map,
+	.unmap  = timer_irq_unmap
+};
+
+static void timer_irq_chained_handler(struct irq_desc *desc) {
+	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
+	struct omap_dm_timer *timer = irq_desc_get_handler_data(desc);
+	unsigned int status;
+
+	timer->irq_stats.all += 1000;
+
+	chained_irq_enter(irq_chip, desc);
+
+	raw_spin_lock(&timer->raw_lock);
+	status = readl_relaxed(timer->irq_stat);
+	if (status & OMAP_TIMER_INT_CAPTURE) {
+		timer->context.tcar1 = __omap_dm_timer_read(timer,
+					OMAP_TIMER_CAPTURE_REG, timer->posted);
+		timer->context.tcar2 = __omap_dm_timer_read(timer,
+					OMAP_TIMER_CAPTURE2_REG, timer->posted);
+		timer->irq_stats.capture++;
+	}
+	if (status & OMAP_TIMER_INT_OVERFLOW)
+		timer->irq_stats.overflow++;
+	if (status & OMAP_TIMER_INT_MATCH)
+		timer->irq_stats.match++;
+	timer->irq_stats.all++;
+	raw_spin_unlock(&timer->raw_lock);
+
+	generic_handle_irq(irq_find_mapping(timer->irq_domain, 0));
+
+	chained_irq_exit(irq_chip, desc);
+
+	/*
+	 * Instruct timer to deassert the interrupt, also it will trigger
+	 * another capture process.
+	 */
+	writel_relaxed(status, timer->irq_stat); }
+
 /**
  * omap_dm_timer_probe - probe function called for every registered device
  * @pdev:	pointer to current timer platform device
@@ -854,6 +992,12 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
 	timer->irq = irq->start;
 	timer->pdev = pdev;
 
+	/*
+	 * Initialize the non preemptible spinlock that protects the captured
+	 * timer values.
+	 */
+	raw_spin_lock_init(&timer->raw_lock);
+
 	/* Skip pm_runtime_enable for OMAP1 */
 	if (!(timer->capability & OMAP_TIMER_NEEDS_RESET)) {
 		pm_runtime_enable(dev);
@@ -871,6 +1015,60 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
 		pm_runtime_put(dev);
 	}
 
+	/* Set timer */
+	ret = omap_dm_timer_set_load(timer, true, 1);
+	if (ret) {
+		devm_kfree(dev, timer);
+		return ret;
+	}
+
+	ret = omap_dm_timer_set_prescaler(timer, 0);
+	if (ret) {
+		printk("Failed to set prescaler for %s.\n", dev_name(dev));
+		devm_kfree(dev, timer);
+		return ret;
+	}
+
+	omap_dm_timer_enable(timer);
+	ret = omap_dm_timer_write_counter(timer, 10);
+	if (ret) {
+		printk("Failed to set initial counter value for %s.\n", dev_name(dev));
+		devm_kfree(dev, timer);
+		return ret;
+	}
+	omap_dm_timer_disable(timer);
+
+	ret = irq_set_chip(timer->irq, &timer_irqchip);
+	if (ret) {
+		dev_err(dev, "%s: set irq_chip failed!\n", __func__);
+		devm_kfree(dev, timer);
+		return ret;
+	}
+
+	ret = irq_set_chip_data(timer->irq, timer);
+	if (ret) {
+		dev_err(dev, "%s: set irq_set_chip_data failed!\n", __func__);
+		devm_kfree(dev, timer);
+		return ret;
+	}
+
+	timer->irq_domain = irq_domain_add_linear(dev->of_node, 1,
+						  &timer_irqdomain_ops, timer);
+	if (!timer->irq_domain) {
+		dev_err(dev, "%s: create irq domain failed!\n", __func__);
+		devm_kfree(dev, timer);
+		return ret;
+	}
+
+	timer->irq_ext = irq_create_mapping(timer->irq_domain, 0);
+
+	/* set up irq for timer */
+	pr_info("dmtimer: %s set chained irq by no: %d\n",
+		dev_name(dev), timer->irq);
+
+	irq_set_chained_handler_and_data(timer->irq, timer_irq_chained_handler,
+					 timer);
+
 	/* add the timer element to the list */
 	spin_lock_irqsave(&dm_timer_lock, flags);
 	list_add_tail(&timer->node, &omap_timer_list); diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index dd79f30..31feb39 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -99,9 +99,25 @@ struct timer_regs {
 	u32 towr;
 };
 
+struct timer_irq_stats {
+	u32 capture;
+	u32 overflow;
+	u32 match;
+	u64 all;
+};
+
+enum timer_irq_event {
+	MATCH = 1,
+	OVERFLOW,
+	CAPTURE
+};
+
+struct omap_dm_timer;
+
 struct omap_dm_timer {
 	int id;
 	int irq;
+	int irq_ext;			/* returned to client module */
 	struct clk *fclk;
 
 	void __iomem	*io_base;
@@ -115,6 +131,9 @@ struct omap_dm_timer {
 	unsigned reserved:1;
 	unsigned posted:1;
 	struct timer_regs context;
+	struct timer_irq_stats irq_stats;
+	struct irq_domain *irq_domain;
+
 	int (*get_context_loss_count)(struct device *);
 	int ctx_loss_count;
 	int revision;
@@ -122,6 +141,9 @@ struct omap_dm_timer {
 	u32 errata;
 	struct platform_device *pdev;
 	struct list_head node;
+
+	/* protects tcar1, tcar2 and timer_irq_stats from concurent access */
+	raw_spinlock_t raw_lock;
 };
 
 int omap_dm_timer_reserve_systimer(int id); @@ -151,14 +173,17 @@ int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler);
 
 int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer, unsigned int value);  int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask);
-
 unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer);  int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value);  unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer);  int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value);
+int omap_dm_timer_read_counter_captures(struct omap_dm_timer *timer, u32 *tcar1,
+					u32 *tcar2);
 
 int omap_dm_timers_active(void);
 
+int print_timer_irq_statistics(struct omap_dm_timer *timer);
+
 /*
  * Do not use the defines below, they are not needed. They should be only
  * used by dmtimer.c and sys_timer related code.
--
1.9.1

Please have a look on this and if you spot something that is wrong reveal it to me.

Many thanks and best wishes,
Andrei Varvara

> -----Original Message-----
> From: Tony Lindgren [mailto:tony@xxxxxxxxxxx]
> Sent: Thursday, March 24, 2016 6:23 PM
> To: Andrei Varvara <Andrei.Varvara@xxxxxxxx>
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] plat-omap: dmtimer: Add support for interrupts
> 
> * Andrei Varvara <Andrei.Varvara@xxxxxxxx> [160324 09:03]:
> > Hello Tony,
> >
> > The way you are proposing means that I should not touch dmtimer.c? I got
> it wrong?
> > I'm not sure what are you suggesting with "let's implement it using a
> > Linux generic Framework instead, request_irq". I used request_irq in
> > the dmtimer.c code to set a ISR for every timer that is probed. Inside
> > ISR it is called the user callback set through
> omap_dm_timer_set_isr_callback.
> 
> Sorry if I was a bit unclear. Linux has a framework that automates all that for
> you with no custom callbacks needed. All you need to do is implement a
> chained handler in dmtimer.c. See the header linux/irqchip/chained_irq.h.
> 
> For some exmaples, see gpiochip_set_chained_irqchip() and
> pcs_irq_init_chained_handler(). There's probably even a simpler example
> somewhere, maybe grep under drivers for chained_irq.h.
> 
> > I found in the kernel source code places where it was implemented
> > identical or very similar to what I did. Please have a look in the following.
> > arch/mips/bcm63xx/timer.c --- client provides the callback which is
> > called from timer ISR routine powerpc/sysdev/mpic_timer.c - client
> > provides the actual ISR callback
> 
> Yeah request_irq does that for you too after you implement the chained
> handler in dmtimer.c :)
> 
> > Please explain a little bit what you have though should be done in
> > dmtimer.c regarding the interrupt support.
> 
> Looks like you've already done all the code needed, you just need to wire it to
> the chainged handler. That way we can specify the timer to use from the .dts
> files with no custom APIs or new custom DT bindings.
> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux