Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1205493 > unrolled thread

[PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context Xander Huff <xander.huff@ni.com> - 2015-08-12 01:10 +0200
    Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq  context Lars-Peter Clausen <lars@metafoo.de> - 2015-08-12 17:20 +0200
      Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq  context Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2015-08-12 18:40 +0200
        Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq  context Jonathan Cameron <jic23@kernel.org> - 2015-08-15 22:00 +0200

#1205493 — [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

The driver currently registers a pair of irq handlers using
request_threaded_irq(), however the synchronization mechanism between the
hardirq and the threadedirq handler is a regular spinlock.

Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
and is thus not able to be acquired from a hardirq handler. This patch gets
rid of the threaded handler and pushes all interrupt handling into the
hardirq context, and uses request_irq().

To validate that this change has no impact on RT performance, here are
cyclictest values with no processes running:

$ sudo cyclictest -S -m -p 98
# /dev/cpu_dma_latency set to 0us
policy: fifo: loadavg: 0.00 0.01 0.05 1/174 2539
T: 0 ( 1405) P:98 I:1000 C:167010520 Min: 9 Act: 12 Avg: 12 Max: 75
T: 1 ( 1862) P:98 I:1500 C:111340339 Min: 9 Act: 12 Avg: 12 Max: 73

Then, all xadc raw handles were accessed in a continuous loop via
/sys/bus/iio/devices/iio:device0:

$ sudo cyclictest -S -m -p 98
# /dev/cpu_dma_latency set to 0us
policy: fifo: loadavg: 7.84 7.70 7.63 3/182 4260
T: 0 ( 2559) P:98 I:1000 C:241557018 Min: 11 Act: 18 Avg: 21 Max: 74
T: 1 ( 2560) P:98 I:1500 C:161038006 Min: 10 Act: 21 Avg: 20 Max: 73

Signed-off-by: Xander Huff <xander.huff@ni.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 37 ++++++++++---------------------------
 drivers/iio/adc/xilinx-xadc.h      |  2 --
 2 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index ce93bd8..0370624 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -273,33 +273,13 @@ static void xadc_zynq_unmask_worker(struct work_struct *work)
 		schedule_delayed_work(&xadc->zynq_unmask_work,
 				msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
 	}
-}
-
-static irqreturn_t xadc_zynq_threaded_interrupt_handler(int irq, void *devid)
-{
-	struct iio_dev *indio_dev = devid;
-	struct xadc *xadc = iio_priv(indio_dev);
-	unsigned int alarm;
-
-	spin_lock_irq(&xadc->lock);
-	alarm = xadc->zynq_alarm;
-	xadc->zynq_alarm = 0;
-	spin_unlock_irq(&xadc->lock);
-
-	xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(alarm));
 
-	/* unmask the required interrupts in timer. */
-	schedule_delayed_work(&xadc->zynq_unmask_work,
-			msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
-
-	return IRQ_HANDLED;
 }
 
 static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
 {
 	struct iio_dev *indio_dev = devid;
 	struct xadc *xadc = iio_priv(indio_dev);
-	irqreturn_t ret = IRQ_HANDLED;
 	uint32_t status;
 
 	xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
@@ -321,18 +301,23 @@ static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
 
 	status &= XADC_ZYNQ_INT_ALARM_MASK;
 	if (status) {
-		xadc->zynq_alarm |= status;
 		xadc->zynq_masked_alarm |= status;
 		/*
 		 * mask the current event interrupt,
 		 * unmask it when the interrupt is no more active.
 		 */
 		xadc_zynq_update_intmsk(xadc, 0, 0);
-		ret = IRQ_WAKE_THREAD;
+
+		xadc_handle_events(indio_dev,
+				xadc_zynq_transform_alarm(status));
+
+		/* unmask the required interrupts in timer. */
+		schedule_delayed_work(&xadc->zynq_unmask_work,
+				msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
 	}
 	spin_unlock(&xadc->lock);
 
-	return ret;
+	return IRQ_HANDLED;
 }
 
 #define XADC_ZYNQ_TCK_RATE_MAX 50000000
@@ -437,7 +422,6 @@ static const struct xadc_ops xadc_zynq_ops = {
 	.setup = xadc_zynq_setup,
 	.get_dclk_rate = xadc_zynq_get_dclk_rate,
 	.interrupt_handler = xadc_zynq_interrupt_handler,
-	.threaded_interrupt_handler = xadc_zynq_threaded_interrupt_handler,
 	.update_alarm = xadc_zynq_update_alarm,
 };
 
@@ -1225,9 +1209,8 @@ static int xadc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_samplerate_trigger;
 
-	ret = request_threaded_irq(irq, xadc->ops->interrupt_handler,
-				xadc->ops->threaded_interrupt_handler,
-				0, dev_name(&pdev->dev), indio_dev);
+	ret = request_irq(irq, xadc->ops->interrupt_handler, 0,
+			dev_name(&pdev->dev), indio_dev);
 	if (ret)
 		goto err_clk_disable_unprepare;
 
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index 54adc50..f6f0819 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -60,7 +60,6 @@ struct xadc {
 
 	enum xadc_external_mux_mode external_mux_mode;
 
-	unsigned int zynq_alarm;
 	unsigned int zynq_masked_alarm;
 	unsigned int zynq_intmask;
 	struct delayed_work zynq_unmask_work;
@@ -79,7 +78,6 @@ struct xadc_ops {
 	void (*update_alarm)(struct xadc *, unsigned int);
 	unsigned long (*get_dclk_rate)(struct xadc *);
 	irqreturn_t (*interrupt_handler)(int, void *);
-	irqreturn_t (*threaded_interrupt_handler)(int, void *);
 
 	unsigned int flags;
 };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1206202 — Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

On 08/12/2015 01:00 AM, Xander Huff wrote:
> The driver currently registers a pair of irq handlers using
> request_threaded_irq(), however the synchronization mechanism between the
> hardirq and the threadedirq handler is a regular spinlock.
> 
> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
> and is thus not able to be acquired from a hardirq handler. This patch gets
> rid of the threaded handler and pushes all interrupt handling into the
> hardirq context, and uses request_irq().
> 
> To validate that this change has no impact on RT performance, here are
> cyclictest values with no processes running:
> 
> $ sudo cyclictest -S -m -p 98
> # /dev/cpu_dma_latency set to 0us
> policy: fifo: loadavg: 0.00 0.01 0.05 1/174 2539
> T: 0 ( 1405) P:98 I:1000 C:167010520 Min: 9 Act: 12 Avg: 12 Max: 75
> T: 1 ( 1862) P:98 I:1500 C:111340339 Min: 9 Act: 12 Avg: 12 Max: 73
> 
> Then, all xadc raw handles were accessed in a continuous loop via
> /sys/bus/iio/devices/iio:device0:
> 
> $ sudo cyclictest -S -m -p 98
> # /dev/cpu_dma_latency set to 0us
> policy: fifo: loadavg: 7.84 7.70 7.63 3/182 4260
> T: 0 ( 2559) P:98 I:1000 C:241557018 Min: 11 Act: 18 Avg: 21 Max: 74
> T: 1 ( 2560) P:98 I:1500 C:161038006 Min: 10 Act: 21 Avg: 20 Max: 73
> 
> Signed-off-by: Xander Huff <xander.huff@ni.com>

Looks good, thanks.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1206256 — Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

On 08/12/2015 05:17 PM, Lars-Peter Clausen wrote:
> On 08/12/2015 01:00 AM, Xander Huff wrote:
>> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
>> and is thus not able to be acquired from a hardirq handler. This patch gets
>> rid of the threaded handler and pushes all interrupt handling into the
>> hardirq context, and uses request_irq().
>>
>> To validate that this change has no impact on RT performance, here are
>> cyclictest values with no processes running:
> 
> Looks good, thanks.
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Yes, I'm fine with the rework, too.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1208138 — Re: [PATCH v4] iio: adc: xilinx-xadc: Push interrupts into hardirq context

On 12/08/15 17:33, Sebastian Andrzej Siewior wrote:
> On 08/12/2015 05:17 PM, Lars-Peter Clausen wrote:
>> On 08/12/2015 01:00 AM, Xander Huff wrote:
>>> Unfortunately, this breaks PREEMPT_RT builds, where a spinlock can sleep,
>>> and is thus not able to be acquired from a hardirq handler. This patch gets
>>> rid of the threaded handler and pushes all interrupt handling into the
>>> hardirq context, and uses request_irq().
>>>
>>> To validate that this change has no impact on RT performance, here are
>>> cyclictest values with no processes running:
>>
>> Looks good, thanks.
>>
>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Yes, I'm fine with the rework, too.
Formal Acked-by would be good but I'll take this on the basis of Lars' one
and that the code clearly should work from a read through.

Applied to the togreg branch of iio.git.  Thanks for following through
the various twists of this one. Will initially be pushed out as testing.
Note this has almost certainly (unless Linus announces a delay) missed the
upcoming merge window so will be lined up for the 4.4 one.

Jonathan
> 
> Sebastian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web