Skip to contentSkip to conversation

tty: the briq panel isn't a tty, make it use its own locking

PATCH In review Alan Cox 2 participants 2 messages Feb 23, 2012 Char and misc drivers

lore
Share & tools
lore

Alan Cox

author
#0 lore raw

PATCH 1/1 tty: the briq panel isn't a tty, make it use its own locking

From: Alan Cox <[email protected]>

The driver appears to be terminally broken, if nobody is maintaining it then perhaps it should go into staging and the out-tray.

Signed-off-by Alan Cox <[email protected]>

---

 drivers/char/briq_panel.c |   25 +++++++------------------
 1 files changed, 7 insertions(+), 18 deletions(-)
drivers/char/briq_panel.c +7 -18
index 8abfd26..1c323f4 100644
@@ -20,6 +20,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 
+#include <asm/bitops.h>
 #include <asm/uaccess.h>
 #include <asm/io.h>
 #include <asm/prom.h>

@@ -30,7 +31,7 @@
 #define		BRIQ_PANEL_VER		"1.1 (04/20/2002)"
 #define		BRIQ_PANEL_MSG0		"Loading Linux"
 
-static int		vfd_is_open;
+static unsigned long	vfd_is_open;
 static unsigned char	vfd[40];
 static int		vfd_cursor;
 static unsigned char	ledpb, led;

@@ -66,37 +67,28 @@ static void set_led(char state)
 
 static int briq_panel_open(struct inode *ino, struct file *filep)
 {
-	tty_lock();
-	/* enforce single access, vfd_is_open is protected by BKL */
-	if (vfd_is_open) {
+	/* enforce single open */
+	if (test_and_set_bit(0, &vfd_is_open)) {
 		tty_unlock();
 		return -EBUSY;
 	}
-	vfd_is_open = 1;
-
-	tty_unlock();
 	return 0;
 }
 
 static int briq_panel_release(struct inode *ino, struct file *filep)
 {
-	if (!vfd_is_open)
-		return -ENODEV;
-
-	vfd_is_open = 0;
-
+	clear_bit(0, &vfd_is_open);
 	return 0;
 }
 
+/* Note that single open doesn't mean single threaded read or write so all
+   this code is unsafe */
 static ssize_t briq_panel_read(struct file *file, char __user *buf, size_t count,
 			 loff_t *ppos)
 {
 	unsigned short c;
 	unsigned char cp;
 
-	if (!vfd_is_open)
-		return -ENODEV;
-
 	c = (inb(BRIQ_PANEL_LED_IOPORT) & 0x000c) | (ledpb & 0x0003);
 	set_led(' ');
 	/* upper button released */

@@ -137,9 +129,6 @@ static ssize_t briq_panel_write(struct file *file, const char __user *buf, size_
 	size_t indx = len;
 	int i, esc = 0;
 
-	if (!vfd_is_open)
-		return -EBUSY;
-
 	for (;;) {
 		char c;
 		if (!indx)
#1 lore raw
Quoted 2 lines · Alan Cox
drivers/char/briq_panel.c +0 -0

...

drivers/char/briq_panel.c static void set_led(char state) · line 72

Quoted 10 lines
drivers/char/briq_panel.c +2 -3
@@ -66,37 +67,28 @@ static void set_led(char state)
 
 static int briq_panel_open(struct inode *ino, struct file *filep)
 {
-	tty_lock();
-	/* enforce single access, vfd_is_open is protected by BKL */
-	if (vfd_is_open) {
+	/* enforce single open */
+	if (test_and_set_bit(0, &vfd_is_open)) {
 		tty_unlock();

This one should be removed too.

drivers/char/briq_panel.c static void set_led(char state) · line 76

Quoted 7 lines
drivers/char/briq_panel.c +0 -3
 		return -EBUSY;
 	}
-	vfd_is_open = 1;
-
-	tty_unlock();
 	return 0;
 }

thanks,

Signature · 2 lines
js
suse labs