linux-kernel - Re: [PATCH] uio_pdrv: Unique IRQ Mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080610071032.GB22814@digi.com>
Date:	Tue, 10 Jun 2008 09:10:32 +0200
From:	Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
To:	Magnus Damm <magnus.damm@...il.com>
CC:	Paul Mundt <lethal@...ux-sh.org>,
	"Hans J. Koch" <hjk@...utronix.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"gregkh@...e.de" <gregkh@...e.de>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>
Subject: Re: [PATCH] uio_pdrv: Unique IRQ Mode

Magnus Damm wrote:
> Hi Uwe,
> 
> Thanks for your help.
> 
> On Mon, Jun 9, 2008 at 9:34 PM, Uwe Kleine-König
> <Uwe.Kleine-Koenig@...i.com> wrote:
> > Paul Mundt wrote:
> >>                                   [...] the constructive way forward
> >> seems to be to follow Uwe's suggestions and respin the patch.
> > Just to let you know: I'm willing to look on a new version.  And I like
> > "User IRQ Mode".
> 
> Sure, let's go with that name then.
> 
> But shall we really go down the path with separate exported functions?
> I think we discussed that earlier. Why I'm asking is that this adds
> extra complexity - function prototypes in header files,
> EXPORT_SYMBOL() stuff and more lines of code needed for each driver
> instance.
Well, at this point I agree with Hans that being explicit here is
preferable.

I think you can implement the handler and irqcontrol functions static
inline.  Then the need to statically compile uio goes away.  And if all
uio_pdrv devices are registered in the same .c file you don't even have
a (binary) size penalty.

> Isn't it enough to add a kconfig option for CONFIG_UIO_PDRV_USER_IRQ
> which depends on CONFIG_SUPERH (if we really want that) and wrap the
> code in CONFIG_UIO_PDRV_USER_IRQ?
No, I definitely don't like that.  Your last patch is completly
compatible with the current implementation.  It only uses a situation
(i.e. registering a uio_pdrv with no handler) a feature.  So
CONFIG_UIO_PDRV_USER_IRQ wouldn't break anything, only not having it.
So it should be enabled by default and then you don't need it.
(Note, this should not count as a +1 for implementing it without the
Kconfig symbol.)
 
> Let me know what you want and I'll respin the patch. Thanks!
... I started describing in detail my suggestion, but actually it's
easier to do the patch myself.  So here it comes.  Note it's not even
compile tested, but you should be able to get my intentions.

As a reply on this mail I will send a small cleanup patch that should
not conflict with any of the pending patches for uio.

Best regards
Uwe


From: Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
Date: Tue, 10 Jun 2008 09:06:24 +0200
Subject: [PATCH] UIO: wip

Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
---
 include/linux/uio_driver.h |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index cdf338d..671a7a8 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -63,6 +63,7 @@ struct uio_info {
 	long			irq;
 	unsigned long		irq_flags;
 	void			*priv;
+	unsigned int		flags;
 	irqreturn_t (*handler)(int irq, struct uio_info *dev_info);
 	int (*mmap)(struct uio_info *info, struct vm_area_struct *vma);
 	int (*open)(struct uio_info *info, struct inode *inode);
@@ -92,4 +93,36 @@ extern void uio_event_notify(struct uio_info *info);
 #define UIO_MEM_LOGICAL	2
 #define UIO_MEM_VIRTUAL 3
 
+/* defines for uio_info->flags */
+#define UIO_FLAGS_IRQDISABLED 0
+
+static inline irqreturn_t uio_userirq_handler(int irq,
+		struct uio_info *dev_info)
+{
+	int ret;
+
+	if (likely(dev_info->irqcontrol)) {
+		ret = dev_info->irqcontrol(dev_info, 0);
+		if (ret) {
+			pr_warning("%s: failed to disable irq\n", __func__);
+			return IRQ_NONE;
+		} else
+			return IRQ_HANDLED;
+	} else
+		return IRQ_NONE;
+}
+
+static inline int uio_userirq_irqcontrol(struct uio_info *info, s32 irq_on)
+{
+	if (irq_on) {
+		if (test_and_clear_bit(UIO_FLAGS_IRQDISABLED, &info->flags))
+			enable_irq(info->irq);
+	} else {
+		if (!test_and_set_bit(UIO_FLAGS_IRQDISABLED, &info->flags))
+			disable_irq(info->irq);
+	}
+
+	return 0;
+}
+
 #endif /* _LINUX_UIO_DRIVER_H_ */
-- 
1.5.5.3


-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/