All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
@ 2017-12-06 14:55 Andrey Zhizhikin
  2017-12-06 15:31 ` Greg KH
  2017-12-07 14:46 ` [PATCH v2 0/2] " Andrey Zhizhikin
  0 siblings, 2 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-06 14:55 UTC (permalink / raw)
  To: gregkh; +Cc: linux-kernel, Andrey Zhizhikin

Certain Kernel preemption models are using threaded interrupt handlers,
which is in general quite beneficial. However, threaded handlers
introducing additional scheduler overhead, when the bottom-half thread
should be woken up and scheduled for execution. This can result is
additional latency, which in certain cases is not desired.

UIO driver with Generic IRQ handler, that wraps a HW block might suffer
a small degradation when it's bottom half is executed, since it needs
its bottom half to be woken up by the scheduler every time INT is
delivered. For high rate INT signals, this also bring additional
undesired load on the scheduler itself.

Since the actual ACK is performed in the top-half, and bottom-half of
the UIO driver with Generic IRQ handler is relatively slick (only flag
is set based on the INT reception), it might be beneficial to move this
bottom-half to the irq_handler itself, rather than to have a separate
thread to service it.

This patch aims to address the task above, and in addition introduces
a new dt-binding which could be configured on a per-node basis. That
means developers utilizing the UIO driver could decide which UIO
instance is critical in terms of interrupt processing, and move their
corresponding bottom-halves to the irq_handler to fight additional
scheduling latency.

New DT binding:
- no-threaded-irq: when present, request_irq() is called with
  IRQF_NO_THREAD flag set, effectively skipping threaded interrupt
  handler and taking bottom-half into irq_handler

Signed-off-by: Andrey Zhizhikin <andrey.z@gmail.com>

diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index f598ecd..86427a4 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -108,6 +108,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 	struct uio_pdrv_genirq_platdata *priv;
 	struct uio_mem *uiomem;
 	int ret = -EINVAL;
+	int no_threaded_irq = 0;
 	int i;
 
 	if (pdev->dev.of_node) {
@@ -121,6 +122,14 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 		uioinfo->name = pdev->dev.of_node->name;
 		uioinfo->version = "devicetree";
 		/* Multiple IRQs are not supported */
+
+		/* read additional property (if exists) and decide whether
+		 * to have IRQ bottom half to be executed in a separate
+		 * thread, or to have it executed in the irq_handler
+		 * context
+		 */
+		if (of_property_read_bool(pdev->dev.of_node, "no-threaded-irq"))
+			no_threaded_irq = 1;
 	}
 
 	if (!uioinfo || !uioinfo->name || !uioinfo->version) {
@@ -134,6 +143,12 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* execute BH in irq_handler if property set in FDT */
+	if ((no_threaded_irq > 0) && !(uioinfo->irq_flags & IRQF_NO_THREAD)) {
+		dev_info(&pdev->dev, "promoting INT with IRQF_NO_THREAD\n");
+		uioinfo->irq_flags |= IRQF_NO_THREAD;
+	}
+
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
 		dev_err(&pdev->dev, "unable to kmalloc\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
  2017-12-06 14:55 [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding Andrey Zhizhikin
@ 2017-12-06 15:31 ` Greg KH
  2017-12-06 16:59   ` Andrey Zhizhikin
  2017-12-07 14:46 ` [PATCH v2 0/2] " Andrey Zhizhikin
  1 sibling, 1 reply; 13+ messages in thread
From: Greg KH @ 2017-12-06 15:31 UTC (permalink / raw)
  To: Andrey Zhizhikin; +Cc: linux-kernel

On Wed, Dec 06, 2017 at 03:55:40PM +0100, Andrey Zhizhikin wrote:
> Certain Kernel preemption models are using threaded interrupt handlers,
> which is in general quite beneficial. However, threaded handlers
> introducing additional scheduler overhead, when the bottom-half thread
> should be woken up and scheduled for execution. This can result is
> additional latency, which in certain cases is not desired.
> 
> UIO driver with Generic IRQ handler, that wraps a HW block might suffer
> a small degradation when it's bottom half is executed, since it needs
> its bottom half to be woken up by the scheduler every time INT is
> delivered. For high rate INT signals, this also bring additional
> undesired load on the scheduler itself.
> 
> Since the actual ACK is performed in the top-half, and bottom-half of
> the UIO driver with Generic IRQ handler is relatively slick (only flag
> is set based on the INT reception), it might be beneficial to move this
> bottom-half to the irq_handler itself, rather than to have a separate
> thread to service it.
> 
> This patch aims to address the task above, and in addition introduces
> a new dt-binding which could be configured on a per-node basis. That
> means developers utilizing the UIO driver could decide which UIO
> instance is critical in terms of interrupt processing, and move their
> corresponding bottom-halves to the irq_handler to fight additional
> scheduling latency.
> 
> New DT binding:
> - no-threaded-irq: when present, request_irq() is called with
>   IRQF_NO_THREAD flag set, effectively skipping threaded interrupt
>   handler and taking bottom-half into irq_handler
> 
> Signed-off-by: Andrey Zhizhikin <andrey.z@gmail.com>

For new DT bindings, don't you have to add them to the in-kernel
documentation and get an ack from the DT maintainers?  Please do that
here.

ALso, how much does this really save in latency/delay by not allowing a
threaded irq?  What about systems that run all irqs in threaded mode?
Will that break something here?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
  2017-12-06 15:31 ` Greg KH
@ 2017-12-06 16:59   ` Andrey Zhizhikin
  2017-12-07  9:16     ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-06 16:59 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

Hello Greg,

Thanks a lot for your prompt reply!

First of, this is my first patch submission to the Kernel, so thanks a
lot for your additional guidelines here regarding missing pieces.
Please don't judge me hard here. :)

I would add new DT bindings to Documentation and contact DT
maintainers to have a new binding discussed. However, I was not able
to find any dt-binding documentation for uio drivers in the kernel,
presumably I would have to create a new entry there...


As for the win against latency and running the patch against the
system which has all IRQ in threaded mode:

I've actually originated this patch based on the PREEMPT_RT kernel
configuration, where all IRQs are threaded. I have ARM-based system
running around 20 genirq UIO instances, and was demoting 2 of those
from threaded to non-threaded IRQ handlers without any issues recorded
to all the IRQ handlers. This patch actually is aimed exactly with the
logic that if new property is not found - then system behavior is not
amended, and all IRQs stays threaded. If needed, then a developer can
enable this property for it's node, but then he should be well-aware
of what this property implications are.

In average, using ftrace and kernelshark to analyze I observed the
gain of 20-30 usec per chain: irq_handler_entry -> irq/XX-uio -> <User
space IRQ part> so I would say the gain is not very significant for
average user-space task. However IMHO, there are several hidden
benefits here with having this modification, namely:
- It eliminates few re-scheduling operations, making INT ACK code more
robust and relieves the pressure from the scheduler when HW interrupt
for this IRQ is signaled at a high-enough frequency;
- It makes top and bottom half to be executed back-to-back with IRQ
OFF, making operation pseudo-atomic;
- Above gain might be significant when average latency times for the
systems are comparable.

I do have a worst-case latency measured with cyclictest here at 50
usec, so as a developer I would consider to have above gain in my
system. :)

Please let me know what you think on those points, if they all make
sense to you - otherwise I can drop this patch out.

-- 
Regards,
Andrey.

On Wed, Dec 6, 2017 at 4:31 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Dec 06, 2017 at 03:55:40PM +0100, Andrey Zhizhikin wrote:
>> Certain Kernel preemption models are using threaded interrupt handlers,
>> which is in general quite beneficial. However, threaded handlers
>> introducing additional scheduler overhead, when the bottom-half thread
>> should be woken up and scheduled for execution. This can result is
>> additional latency, which in certain cases is not desired.
>>
>> UIO driver with Generic IRQ handler, that wraps a HW block might suffer
>> a small degradation when it's bottom half is executed, since it needs
>> its bottom half to be woken up by the scheduler every time INT is
>> delivered. For high rate INT signals, this also bring additional
>> undesired load on the scheduler itself.
>>
>> Since the actual ACK is performed in the top-half, and bottom-half of
>> the UIO driver with Generic IRQ handler is relatively slick (only flag
>> is set based on the INT reception), it might be beneficial to move this
>> bottom-half to the irq_handler itself, rather than to have a separate
>> thread to service it.
>>
>> This patch aims to address the task above, and in addition introduces
>> a new dt-binding which could be configured on a per-node basis. That
>> means developers utilizing the UIO driver could decide which UIO
>> instance is critical in terms of interrupt processing, and move their
>> corresponding bottom-halves to the irq_handler to fight additional
>> scheduling latency.
>>
>> New DT binding:
>> - no-threaded-irq: when present, request_irq() is called with
>>   IRQF_NO_THREAD flag set, effectively skipping threaded interrupt
>>   handler and taking bottom-half into irq_handler
>>
>> Signed-off-by: Andrey Zhizhikin <andrey.z@gmail.com>
>
> For new DT bindings, don't you have to add them to the in-kernel
> documentation and get an ack from the DT maintainers?  Please do that
> here.
>
> ALso, how much does this really save in latency/delay by not allowing a
> threaded irq?  What about systems that run all irqs in threaded mode?
> Will that break something here?
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
  2017-12-06 16:59   ` Andrey Zhizhikin
@ 2017-12-07  9:16     ` Greg KH
  2017-12-07 11:17       ` Andrey Zhizhikin
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2017-12-07  9:16 UTC (permalink / raw)
  To: Andrey Zhizhikin; +Cc: linux-kernel


A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Wed, Dec 06, 2017 at 05:59:01PM +0100, Andrey Zhizhikin wrote:
> Hello Greg,
> 
> Thanks a lot for your prompt reply!
> 
> First of, this is my first patch submission to the Kernel, so thanks a
> lot for your additional guidelines here regarding missing pieces.
> Please don't judge me hard here. :)

No worries, everyone has to start somewhere :)

> I would add new DT bindings to Documentation and contact DT
> maintainers to have a new binding discussed. However, I was not able
> to find any dt-binding documentation for uio drivers in the kernel,
> presumably I would have to create a new entry there...

The uio-phys driver does use DT bindings, so perhaps look at how those
are defined.

> As for the win against latency and running the patch against the
> system which has all IRQ in threaded mode:
> 
> I've actually originated this patch based on the PREEMPT_RT kernel
> configuration, where all IRQs are threaded. I have ARM-based system
> running around 20 genirq UIO instances, and was demoting 2 of those
> from threaded to non-threaded IRQ handlers without any issues recorded
> to all the IRQ handlers. This patch actually is aimed exactly with the
> logic that if new property is not found - then system behavior is not
> amended, and all IRQs stays threaded. If needed, then a developer can
> enable this property for it's node, but then he should be well-aware
> of what this property implications are.
> 
> In average, using ftrace and kernelshark to analyze I observed the
> gain of 20-30 usec per chain: irq_handler_entry -> irq/XX-uio -> <User
> space IRQ part> so I would say the gain is not very significant for
> average user-space task. However IMHO, there are several hidden
> benefits here with having this modification, namely:
> - It eliminates few re-scheduling operations, making INT ACK code more
> robust and relieves the pressure from the scheduler when HW interrupt
> for this IRQ is signaled at a high-enough frequency;
> - It makes top and bottom half to be executed back-to-back with IRQ
> OFF, making operation pseudo-atomic;
> - Above gain might be significant when average latency times for the
> systems are comparable.

Ok, that all seems like a good thing to have the ability to do here, you
should mention it in the changelog text when you redo this patch.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
  2017-12-07  9:16     ` Greg KH
@ 2017-12-07 11:17       ` Andrey Zhizhikin
  0 siblings, 0 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-07 11:17 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

On Thu, Dec 7, 2017 at 10:16 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top

Understood, rules are golden! :)

>
> The uio-phys driver does use DT bindings, so perhaps look at how those
> are defined.

UIO in general uses standard bindings (like "compatible", "reg",
"interrupt"), but the documentation part is completely missing. What I
was able to find in Kernel tree is a HOW-TO from Hans-Juergen Koch
dated 2006 and kobject reference on Embedding kobjects with UIO as an
example. I've created a new binding document under
Documentation/devicetree/bindings/uio/ and would submit it together
with updated patch in series.

If anyone happen to know which place I missed in the Kernel to look at
for uio dt-binding Documentation part - please point me out.

>
> Ok, that all seems like a good thing to have the ability to do here, you
> should mention it in the changelog text when you redo this patch.

Would be done!

I am planning to have all above corrected and re-submit the patch as
series with Documentation part included.

Cheers,
Andrey.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 0/2] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
  2017-12-06 14:55 [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding Andrey Zhizhikin
  2017-12-06 15:31 ` Greg KH
@ 2017-12-07 14:46 ` Andrey Zhizhikin
  2017-12-07 14:46     ` Andrey Zhizhikin
  2017-12-07 14:46   ` [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation Andrey Zhizhikin
  1 sibling, 2 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-07 14:46 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland; +Cc: devicetree, linux-kernel, Andrey Zhizhikin

This patch series aimed to introduce additional feature for UIO driver with
generic interrupt handler to allow IRQ bottom half to be executed in
irq_handler context rather than as threaded IRQ.

Andrey Zhizhikin (2):
  uio: Allow to take irq bottom-half into irq_handler with additional
    dt-binding
  uio: Introduce UIO driver dt-binding documentation

 .../devicetree/bindings/uio/uio-pdrv-genirq.txt    | 46 ++++++++++++++++++++++
 drivers/uio/uio_pdrv_genirq.c                      | 15 +++++++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt

-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/2] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
@ 2017-12-07 14:46     ` Andrey Zhizhikin
  0 siblings, 0 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-07 14:46 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland; +Cc: devicetree, linux-kernel, Andrey Zhizhikin

Certain Kernel preemption models are using threaded interrupt handlers,
which is in general quite beneficial. However, threaded handlers
introducing additional scheduler overhead, when the bottom-half thread
should be woken up and scheduled for execution. This can result is
additional latency, which in certain cases is not desired.

UIO driver with Generic IRQ handler, that wraps a HW block might suffer
a small degradation when it's bottom half is executed, since it needs
its bottom half to be woken up by the scheduler every time INT is
delivered. For high rate INT signals, this also bring additional
undesired load on the scheduler itself.

Since the actual ACK is performed in the top-half, and bottom-half of
the UIO driver with Generic IRQ handler is relatively slick (only flag
is set based on the INT reception), it might be beneficial to move this
bottom-half to the irq_handler itself, rather than to have a separate
thread to service it.

This patch aims to address the task above by supplying IRQF_NO_THREAD to
request_irq(), based on dt-binding which could be configured on a per-node
basis. That means developers utilizing the UIO driver could decide which
UIO instance is critical in terms of interrupt processing, and move their
corresponding bottom-halves to the irq_handler to fight additional
scheduling latency. When additional property is not found in corresponding
dt-node, then instance behavior is not amended and overall system stays
with default configuration for threaded IRQ (depending on how they are
configured by Preemption model).

Patch was originated on the ARM-based system with Kernel configuration
CONFIG_PREEMPT_RT_FULL set, which effectively promotes all bottom-halves
to threaded interrupt handlers. Once this patch has been enabled on 2
individual uio device nodes (out of around 20 registered in the system),
no additional negative impact has been noted on the system overall.

Having this patch enabled for individual UIO node allowed to have a
latency reduction of around 20-30 usec from INT trigger to the user space
IRQ handler. Those results can vary based on the platform and CPU
architecture, but could be quite beneficial if above gain in comparable
to the worst-case latency figures.

This modification also brings several additional benefits:
- It eliminates few re-scheduling operations, making INT ACK code more
robust and relieves the pressure from the scheduler when HW interrupt
for this IRQ is signaled at a high-enough frequency;
- It makes top and bottom half to be executed back-to-back with IRQ
OFF, making operation pseudo-atomic;
- Above gain might be significant when average latency times for the
systems are comparable

Signed-off-by: Andrey Zhizhikin <andrey.z@gmail.com>

diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index f598ecd..86427a4 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -108,6 +108,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 	struct uio_pdrv_genirq_platdata *priv;
 	struct uio_mem *uiomem;
 	int ret = -EINVAL;
+	int no_threaded_irq = 0;
 	int i;
 
 	if (pdev->dev.of_node) {
@@ -121,6 +122,14 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 		uioinfo->name = pdev->dev.of_node->name;
 		uioinfo->version = "devicetree";
 		/* Multiple IRQs are not supported */
+
+		/* read additional property (if exists) and decide whether
+		 * to have IRQ bottom half to be executed in a separate
+		 * thread, or to have it executed in the irq_handler
+		 * context
+		 */
+		if (of_property_read_bool(pdev->dev.of_node, "no-threaded-irq"))
+			no_threaded_irq = 1;
 	}
 
 	if (!uioinfo || !uioinfo->name || !uioinfo->version) {
@@ -134,6 +143,12 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* execute BH in irq_handler if property set in FDT */
+	if ((no_threaded_irq > 0) && !(uioinfo->irq_flags & IRQF_NO_THREAD)) {
+		dev_info(&pdev->dev, "promoting INT with IRQF_NO_THREAD\n");
+		uioinfo->irq_flags |= IRQF_NO_THREAD;
+	}
+
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
 		dev_err(&pdev->dev, "unable to kmalloc\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 1/2] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding
@ 2017-12-07 14:46     ` Andrey Zhizhikin
  0 siblings, 0 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-07 14:46 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrey Zhizhikin

Certain Kernel preemption models are using threaded interrupt handlers,
which is in general quite beneficial. However, threaded handlers
introducing additional scheduler overhead, when the bottom-half thread
should be woken up and scheduled for execution. This can result is
additional latency, which in certain cases is not desired.

UIO driver with Generic IRQ handler, that wraps a HW block might suffer
a small degradation when it's bottom half is executed, since it needs
its bottom half to be woken up by the scheduler every time INT is
delivered. For high rate INT signals, this also bring additional
undesired load on the scheduler itself.

Since the actual ACK is performed in the top-half, and bottom-half of
the UIO driver with Generic IRQ handler is relatively slick (only flag
is set based on the INT reception), it might be beneficial to move this
bottom-half to the irq_handler itself, rather than to have a separate
thread to service it.

This patch aims to address the task above by supplying IRQF_NO_THREAD to
request_irq(), based on dt-binding which could be configured on a per-node
basis. That means developers utilizing the UIO driver could decide which
UIO instance is critical in terms of interrupt processing, and move their
corresponding bottom-halves to the irq_handler to fight additional
scheduling latency. When additional property is not found in corresponding
dt-node, then instance behavior is not amended and overall system stays
with default configuration for threaded IRQ (depending on how they are
configured by Preemption model).

Patch was originated on the ARM-based system with Kernel configuration
CONFIG_PREEMPT_RT_FULL set, which effectively promotes all bottom-halves
to threaded interrupt handlers. Once this patch has been enabled on 2
individual uio device nodes (out of around 20 registered in the system),
no additional negative impact has been noted on the system overall.

Having this patch enabled for individual UIO node allowed to have a
latency reduction of around 20-30 usec from INT trigger to the user space
IRQ handler. Those results can vary based on the platform and CPU
architecture, but could be quite beneficial if above gain in comparable
to the worst-case latency figures.

This modification also brings several additional benefits:
- It eliminates few re-scheduling operations, making INT ACK code more
robust and relieves the pressure from the scheduler when HW interrupt
for this IRQ is signaled at a high-enough frequency;
- It makes top and bottom half to be executed back-to-back with IRQ
OFF, making operation pseudo-atomic;
- Above gain might be significant when average latency times for the
systems are comparable

Signed-off-by: Andrey Zhizhikin <andrey.z-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index f598ecd..86427a4 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -108,6 +108,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 	struct uio_pdrv_genirq_platdata *priv;
 	struct uio_mem *uiomem;
 	int ret = -EINVAL;
+	int no_threaded_irq = 0;
 	int i;
 
 	if (pdev->dev.of_node) {
@@ -121,6 +122,14 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 		uioinfo->name = pdev->dev.of_node->name;
 		uioinfo->version = "devicetree";
 		/* Multiple IRQs are not supported */
+
+		/* read additional property (if exists) and decide whether
+		 * to have IRQ bottom half to be executed in a separate
+		 * thread, or to have it executed in the irq_handler
+		 * context
+		 */
+		if (of_property_read_bool(pdev->dev.of_node, "no-threaded-irq"))
+			no_threaded_irq = 1;
 	}
 
 	if (!uioinfo || !uioinfo->name || !uioinfo->version) {
@@ -134,6 +143,12 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* execute BH in irq_handler if property set in FDT */
+	if ((no_threaded_irq > 0) && !(uioinfo->irq_flags & IRQF_NO_THREAD)) {
+		dev_info(&pdev->dev, "promoting INT with IRQF_NO_THREAD\n");
+		uioinfo->irq_flags |= IRQF_NO_THREAD;
+	}
+
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
 		dev_err(&pdev->dev, "unable to kmalloc\n");
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation
  2017-12-07 14:46 ` [PATCH v2 0/2] " Andrey Zhizhikin
  2017-12-07 14:46     ` Andrey Zhizhikin
@ 2017-12-07 14:46   ` Andrey Zhizhikin
  2017-12-07 23:42       ` Rob Herring
  1 sibling, 1 reply; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-07 14:46 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland; +Cc: devicetree, linux-kernel, Andrey Zhizhikin

Create Documentation portion of UIO driver with Generic Interrupt Handler.

This patch creates a dt-binding documentation portion of the UIO Driver.

In addition to definition of standard required properties, new optional
property defined:
- no-threaded-irq: when present, request_irq() is called with
  IRQF_NO_THREAD flag set, effectively skipping threaded interrupt
  handler and taking bottom-half into irq_handler

Signed-off-by: Andrey Zhizhikin <andrey.z@gmail.com>

diff --git a/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt b/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt
new file mode 100644
index 0000000..dfcc362
--- /dev/null
+++ b/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt
@@ -0,0 +1,46 @@
+* Userspace I/O platform driver with generic IRQ handling code.
+
+Platform driver for User-space IO handling with generic IRQ code.
+This driver is very similar to the regular UIO platform driver, but is
+only suitable for devices that are connected to the interrupt
+controller using unique interrupt lines.
+
+Required properties:
+- compatible: Driver compatible string. For UIO device with generic IRQ
+  handling code this property is defined in device tree to any desired
+  name, and then set via module parameters passed to Kernel command line
+  in a form:
+  uio_pdrv_genirq.of_id=<your name>
+
+- reg: Physical base address and size for memory mapped region to be accessed
+  via UIO driver.
+
+- interrupts: Platform IRQ standard definition. For details on defining this
+  property, see interrupt-controller/interrupts.txt
+
+- interrupt-parent: Parent interrupt controller node.
+  For details, see interrupt-controller/interrupts.txt
+
+Optional properties:
+- no-threaded-irq: when present, request_irq() is called with IRQF_NO_THREAD
+  flag set, effectively skipping threaded interrupt handler and taking
+  bottom-half into irq_handler
+
+
+Examples:
+
+base-uio {
+	compatible = "platform-uio";
+	reg = < 0xC0000000 0x00020000 >;
+	interrupts = < 0 10 1 >;
+	interrupt-parent = <&intc>;
+};
+
+nothreaded-uio {
+	compatible = "platform-uio";
+	reg = < 0xC0040000 0x00020000 >;
+	interrupts = < 0 27 1 >;
+	interrupt-parent = <&intc>;
+	no-threaded-irq;
+};
+
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation
@ 2017-12-07 23:42       ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2017-12-07 23:42 UTC (permalink / raw)
  To: Andrey Zhizhikin; +Cc: gregkh, mark.rutland, devicetree, linux-kernel

On Thu, Dec 07, 2017 at 03:46:59PM +0100, Andrey Zhizhikin wrote:
> Create Documentation portion of UIO driver with Generic Interrupt Handler.
> 
> This patch creates a dt-binding documentation portion of the UIO Driver.
> 
> In addition to definition of standard required properties, new optional
> property defined:
> - no-threaded-irq: when present, request_irq() is called with
>   IRQF_NO_THREAD flag set, effectively skipping threaded interrupt
>   handler and taking bottom-half into irq_handler
> 
> Signed-off-by: Andrey Zhizhikin <andrey.z@gmail.com>
> 
> diff --git a/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt b/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt
> new file mode 100644
> index 0000000..dfcc362
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt
> @@ -0,0 +1,46 @@
> +* Userspace I/O platform driver with generic IRQ handling code.

No. See prior discussions:

https://lkml.org/lkml/2016/5/18/457
https://lkml.org/lkml/2017/10/8/238

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation
@ 2017-12-07 23:42       ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2017-12-07 23:42 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, Dec 07, 2017 at 03:46:59PM +0100, Andrey Zhizhikin wrote:
> Create Documentation portion of UIO driver with Generic Interrupt Handler.
> 
> This patch creates a dt-binding documentation portion of the UIO Driver.
> 
> In addition to definition of standard required properties, new optional
> property defined:
> - no-threaded-irq: when present, request_irq() is called with
>   IRQF_NO_THREAD flag set, effectively skipping threaded interrupt
>   handler and taking bottom-half into irq_handler
> 
> Signed-off-by: Andrey Zhizhikin <andrey.z-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> diff --git a/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt b/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt
> new file mode 100644
> index 0000000..dfcc362
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/uio/uio-pdrv-genirq.txt
> @@ -0,0 +1,46 @@
> +* Userspace I/O platform driver with generic IRQ handling code.

No. See prior discussions:

https://lkml.org/lkml/2016/5/18/457
https://lkml.org/lkml/2017/10/8/238
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation
  2017-12-07 23:42       ` Rob Herring
@ 2017-12-08 12:52         ` Andrey Zhizhikin
  -1 siblings, 0 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-08 12:52 UTC (permalink / raw)
  To: Rob Herring; +Cc: Greg KH, mark.rutland, devicetree, linux-kernel

Hello Rob,

Thanks for your reply here!

>
> No. See prior discussions:
>
> https://lkml.org/lkml/2016/5/18/457
> https://lkml.org/lkml/2017/10/8/238

Just for my clarify; to understand how to move on with the patch:
Since UIO is not considered as a real HW and rather a aggregate, which
could be used for to wrap virtually any HW block - does that mean no
new dt-bindings would be accepted to it?

I agree it is really hard (and practically impossible) to create
compatible strings for all HW units potentially using UIO as a
container, therefore I can see clearly arguments here.

But if considered from a driver perspective: there is already DT
awareness in the UIO driver (specifically the pdrv-genirq), and we
have a "HW-like" functionality (irq handler, iomem and ioreg regions)
which could be covered by the DT bindings. What if those "generic"
properties that covers only those pieces of the UIO driver code could
be assigned corresponding dt-bindings? I believe it is not the best
and rather contradictory approach to not have any "compatible" string,
but as long as those driver parameters are covered by DT properties,
isn't it OK to have them?

I can assume a lot of people are using UIO in exactly this way:
defining a node in their DTS, assigning "compatible" via kernel
command line and having devnode instantiated. Why can't a possibility
be provided to them to have a generic description of how they can
configure, tweak and use their UIO drivers in the correct and
efficient way? This is especially true for people that are using FPGAs
and enveloping their Soft-IPs with UIO to have a generic
status/control capabilities. I cannot imagine how this tremendous
amount of variations could be easily accommodated inside UIO
compatible names...

Or am I completely missing a point here?

-- 
Regards,
Andrey.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation
@ 2017-12-08 12:52         ` Andrey Zhizhikin
  0 siblings, 0 replies; 13+ messages in thread
From: Andrey Zhizhikin @ 2017-12-08 12:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg KH, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hello Rob,

Thanks for your reply here!

>
> No. See prior discussions:
>
> https://lkml.org/lkml/2016/5/18/457
> https://lkml.org/lkml/2017/10/8/238

Just for my clarify; to understand how to move on with the patch:
Since UIO is not considered as a real HW and rather a aggregate, which
could be used for to wrap virtually any HW block - does that mean no
new dt-bindings would be accepted to it?

I agree it is really hard (and practically impossible) to create
compatible strings for all HW units potentially using UIO as a
container, therefore I can see clearly arguments here.

But if considered from a driver perspective: there is already DT
awareness in the UIO driver (specifically the pdrv-genirq), and we
have a "HW-like" functionality (irq handler, iomem and ioreg regions)
which could be covered by the DT bindings. What if those "generic"
properties that covers only those pieces of the UIO driver code could
be assigned corresponding dt-bindings? I believe it is not the best
and rather contradictory approach to not have any "compatible" string,
but as long as those driver parameters are covered by DT properties,
isn't it OK to have them?

I can assume a lot of people are using UIO in exactly this way:
defining a node in their DTS, assigning "compatible" via kernel
command line and having devnode instantiated. Why can't a possibility
be provided to them to have a generic description of how they can
configure, tweak and use their UIO drivers in the correct and
efficient way? This is especially true for people that are using FPGAs
and enveloping their Soft-IPs with UIO to have a generic
status/control capabilities. I cannot imagine how this tremendous
amount of variations could be easily accommodated inside UIO
compatible names...

Or am I completely missing a point here?

-- 
Regards,
Andrey.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-12-08 12:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-06 14:55 [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding Andrey Zhizhikin
2017-12-06 15:31 ` Greg KH
2017-12-06 16:59   ` Andrey Zhizhikin
2017-12-07  9:16     ` Greg KH
2017-12-07 11:17       ` Andrey Zhizhikin
2017-12-07 14:46 ` [PATCH v2 0/2] " Andrey Zhizhikin
2017-12-07 14:46   ` [PATCH v2 1/2] " Andrey Zhizhikin
2017-12-07 14:46     ` Andrey Zhizhikin
2017-12-07 14:46   ` [PATCH v2 2/2] uio: Introduce UIO driver dt-binding documentation Andrey Zhizhikin
2017-12-07 23:42     ` Rob Herring
2017-12-07 23:42       ` Rob Herring
2017-12-08 12:52       ` Andrey Zhizhikin
2017-12-08 12:52         ` Andrey Zhizhikin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.