* [PATCH] tty: the briq panel isn't a tty, make it use its own locking
@ 2012-02-23 13:39 Alan Cox
2012-02-23 20:42 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 8+ messages in thread
From: Alan Cox @ 2012-02-23 13:39 UTC (permalink / raw)
To: benh, linux-kernel
From: Alan Cox <alan@linux.intel.com>
The driver appears to be terminall broken, if nobody is maintaining it then
perhaps it should go into staging and the out-tray.
(Fixed bug noted by Jiri)
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/char/briq_panel.c | 26 +++++++-------------------
1 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/char/briq_panel.c b/drivers/char/briq_panel.c
index 095ab90..3335d07 100644
--- a/drivers/char/briq_panel.c
+++ b/drivers/char/briq_panel.c
@@ -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,27 @@ 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) {
- tty_unlock();
+ /* enforce single open */
+ if (test_and_set_bit(0, &vfd_is_open))
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 +128,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)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: the briq panel isn't a tty, make it use its own locking
2012-02-23 13:39 [PATCH] tty: the briq panel isn't a tty, make it use its own locking Alan Cox
@ 2012-02-23 20:42 ` Benjamin Herrenschmidt
2012-02-23 22:56 ` Alan Cox
0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2012-02-23 20:42 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-kernel
On Thu, 2012-02-23 at 13:39 +0000, Alan Cox wrote:
> From: Alan Cox <alan@linux.intel.com>
>
> The driver appears to be terminall broken, if nobody is maintaining it then
> perhaps it should go into staging and the out-tray.
Hrm, I don't think anybody maintains it but I do have a BriQ somewhere
in storage, so I suppose I could pick it up, not that I need another
crap driver to deal with right now though...
Cheers,
Ben.
> (Fixed bug noted by Jiri)
>
> Signed-off-by: Alan Cox <alan@linux.intel.com>
> ---
>
> drivers/char/briq_panel.c | 26 +++++++-------------------
> 1 files changed, 7 insertions(+), 19 deletions(-)
>
>
> diff --git a/drivers/char/briq_panel.c b/drivers/char/briq_panel.c
> index 095ab90..3335d07 100644
> --- a/drivers/char/briq_panel.c
> +++ b/drivers/char/briq_panel.c
> @@ -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,27 @@ 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) {
> - tty_unlock();
> + /* enforce single open */
> + if (test_and_set_bit(0, &vfd_is_open))
> 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 +128,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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: the briq panel isn't a tty, make it use its own locking
2012-02-23 20:42 ` Benjamin Herrenschmidt
@ 2012-02-23 22:56 ` Alan Cox
2012-02-24 1:08 ` Josh Boyer
2012-02-24 1:37 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 8+ messages in thread
From: Alan Cox @ 2012-02-23 22:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linux-kernel
On Fri, 24 Feb 2012 07:42:27 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Thu, 2012-02-23 at 13:39 +0000, Alan Cox wrote:
> > From: Alan Cox <alan@linux.intel.com>
> >
> > The driver appears to be terminall broken, if nobody is maintaining it then
> > perhaps it should go into staging and the out-tray.
>
> Hrm, I don't think anybody maintains it but I do have a BriQ somewhere
> in storage, so I suppose I could pick it up, not that I need another
> crap driver to deal with right now though...
In which case can we just stuff it into the tree, or take the briq driver
out into staging as this is the last blocker on tackling the tty_lock
entirely in the tty layer
Alan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: the briq panel isn't a tty, make it use its own locking
2012-02-23 22:56 ` Alan Cox
@ 2012-02-24 1:08 ` Josh Boyer
2012-02-24 1:37 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 8+ messages in thread
From: Josh Boyer @ 2012-02-24 1:08 UTC (permalink / raw)
To: Alan Cox; +Cc: Benjamin Herrenschmidt, linux-kernel
On Thu, Feb 23, 2012 at 5:56 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Fri, 24 Feb 2012 07:42:27 +1100
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> On Thu, 2012-02-23 at 13:39 +0000, Alan Cox wrote:
>> > From: Alan Cox <alan@linux.intel.com>
>> >
>> > The driver appears to be terminall broken, if nobody is maintaining it then
>> > perhaps it should go into staging and the out-tray.
>>
>> Hrm, I don't think anybody maintains it but I do have a BriQ somewhere
>> in storage, so I suppose I could pick it up, not that I need another
>> crap driver to deal with right now though...
>
> In which case can we just stuff it into the tree, or take the briq driver
> out into staging as this is the last blocker on tackling the tty_lock
> entirely in the tty layer
I'd vote for out to staging. Don't rely on BenH to make sane decisions when
it comes to deprecating stuff nobody uses anymore. I mean... he's only just
now dropping iSeries support.
josh
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: the briq panel isn't a tty, make it use its own locking
2012-02-23 22:56 ` Alan Cox
2012-02-24 1:08 ` Josh Boyer
@ 2012-02-24 1:37 ` Benjamin Herrenschmidt
2012-02-29 20:21 ` Greg KH
1 sibling, 1 reply; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2012-02-24 1:37 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-kernel
On Thu, 2012-02-23 at 22:56 +0000, Alan Cox wrote:
> > Hrm, I don't think anybody maintains it but I do have a BriQ somewhere
> > in storage, so I suppose I could pick it up, not that I need another
> > crap driver to deal with right now though...
>
> In which case can we just stuff it into the tree, or take the briq driver
> out into staging as this is the last blocker on tackling the tty_lock
> entirely in the tty layer
Just remove the driver, I don't think anybody cares.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: the briq panel isn't a tty, make it use its own locking
2012-02-24 1:37 ` Benjamin Herrenschmidt
@ 2012-02-29 20:21 ` Greg KH
0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2012-02-29 20:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Alan Cox, linux-kernel
On Fri, Feb 24, 2012 at 12:37:17PM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2012-02-23 at 22:56 +0000, Alan Cox wrote:
> > > Hrm, I don't think anybody maintains it but I do have a BriQ somewhere
> > > in storage, so I suppose I could pick it up, not that I need another
> > > crap driver to deal with right now though...
> >
> > In which case can we just stuff it into the tree, or take the briq driver
> > out into staging as this is the last blocker on tackling the tty_lock
> > entirely in the tty layer
>
> Just remove the driver, I don't think anybody cares.
Wonderful, I'll go do that right now :)
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tty: the briq panel isn't a tty, make it use its own locking
2012-02-23 12:34 Alan Cox
@ 2012-02-23 12:37 ` Jiri Slaby
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Slaby @ 2012-02-23 12:37 UTC (permalink / raw)
To: Alan Cox; +Cc: benh, linux-kernel
On 02/23/2012 01:34 PM, Alan Cox wrote:
> --- a/drivers/char/briq_panel.c
> +++ b/drivers/char/briq_panel.c
...
> @@ -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.
> return -EBUSY;
> }
> - vfd_is_open = 1;
> -
> - tty_unlock();
> return 0;
> }
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] tty: the briq panel isn't a tty, make it use its own locking
@ 2012-02-23 12:34 Alan Cox
2012-02-23 12:37 ` Jiri Slaby
0 siblings, 1 reply; 8+ messages in thread
From: Alan Cox @ 2012-02-23 12:34 UTC (permalink / raw)
To: benh, linux-kernel
From: Alan Cox <alan@linux.intel.com>
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 <alan@linux.intel.com>
---
drivers/char/briq_panel.c | 25 +++++++------------------
1 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/char/briq_panel.c b/drivers/char/briq_panel.c
index 8abfd26..1c323f4 100644
--- a/drivers/char/briq_panel.c
+++ b/drivers/char/briq_panel.c
@@ -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)
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-02-29 20:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-23 13:39 [PATCH] tty: the briq panel isn't a tty, make it use its own locking Alan Cox
2012-02-23 20:42 ` Benjamin Herrenschmidt
2012-02-23 22:56 ` Alan Cox
2012-02-24 1:08 ` Josh Boyer
2012-02-24 1:37 ` Benjamin Herrenschmidt
2012-02-29 20:21 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2012-02-23 12:34 Alan Cox
2012-02-23 12:37 ` Jiri Slaby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).