* [PATCH 3/15] qd65xx: fix deadlock on error handling
@ 2007-10-01 21:33 Bartlomiej Zolnierkiewicz
2007-10-02 12:59 ` Sergei Shtylyov
0 siblings, 1 reply; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-01 21:33 UTC (permalink / raw)
To: linux-ide
Stop abusing ide_lock lock (switch to a private locking).
Fixes same issue as fixed by Alan Cox in atiixp host driver with
commit 6c5f8cc33eb2e10b6ab788bbe259fc142a068627.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/legacy/qd65xx.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
Index: b/drivers/ide/legacy/qd65xx.c
===================================================================
--- a/drivers/ide/legacy/qd65xx.c
+++ b/drivers/ide/legacy/qd65xx.c
@@ -89,13 +89,15 @@
static int timings[4]={-1,-1,-1,-1}; /* stores current timing for each timer */
+static DEFINE_SPINLOCK(qd65xx_lock);
+
static void qd_write_reg (u8 content, unsigned long reg)
{
unsigned long flags;
- spin_lock_irqsave(&ide_lock, flags);
+ spin_lock_irqsave(&qd65xx_lock, flags);
outb(content,reg);
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irqrestore(&qd65xx_lock, flags);
}
static u8 __init qd_read_reg (unsigned long reg)
@@ -103,9 +105,9 @@ static u8 __init qd_read_reg (unsigned l
unsigned long flags;
u8 read;
- spin_lock_irqsave(&ide_lock, flags);
+ spin_lock_irqsave(&qd65xx_lock, flags);
read = inb(reg);
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irqrestore(&qd65xx_lock, flags);
return read;
}
@@ -301,16 +303,15 @@ static void qd6580_set_pio_mode(ide_driv
static int __init qd_testreg(int port)
{
- u8 savereg;
- u8 readreg;
unsigned long flags;
+ u8 savereg, readreg;
- spin_lock_irqsave(&ide_lock, flags);
+ spin_lock_irqsave(&qd65xx_lock, flags);
savereg = inb_p(port);
outb_p(QD_TESTVAL, port); /* safe value */
readreg = inb_p(port);
outb(savereg, port);
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irqrestore(&qd65xx_lock, flags);
if (savereg == QD_TESTVAL) {
printk(KERN_ERR "Outch ! the probe for qd65xx isn't reliable !\n");
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/15] qd65xx: fix deadlock on error handling
2007-10-01 21:33 [PATCH 3/15] qd65xx: fix deadlock on error handling Bartlomiej Zolnierkiewicz
@ 2007-10-02 12:59 ` Sergei Shtylyov
2007-10-02 13:04 ` Jeff Garzik
0 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2007-10-02 12:59 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide
Bartlomiej Zolnierkiewicz wrote:
> Stop abusing ide_lock lock (switch to a private locking).
> Fixes same issue as fixed by Alan Cox in atiixp host driver with
> commit 6c5f8cc33eb2e10b6ab788bbe259fc142a068627.
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Index: b/drivers/ide/legacy/qd65xx.c
> ===================================================================
> --- a/drivers/ide/legacy/qd65xx.c
> +++ b/drivers/ide/legacy/qd65xx.c
> @@ -89,13 +89,15 @@
>
> static int timings[4]={-1,-1,-1,-1}; /* stores current timing for each timer */
>
> +static DEFINE_SPINLOCK(qd65xx_lock);
> +
> static void qd_write_reg (u8 content, unsigned long reg)
> {
> unsigned long flags;
>
> - spin_lock_irqsave(&ide_lock, flags);
> + spin_lock_irqsave(&qd65xx_lock, flags);
> outb(content,reg);
> - spin_unlock_irqrestore(&ide_lock, flags);
> + spin_unlock_irqrestore(&qd65xx_lock, flags);
> }
>
> static u8 __init qd_read_reg (unsigned long reg)
> @@ -103,9 +105,9 @@ static u8 __init qd_read_reg (unsigned l
> unsigned long flags;
> u8 read;
>
> - spin_lock_irqsave(&ide_lock, flags);
> + spin_lock_irqsave(&qd65xx_lock, flags);
> read = inb(reg);
> - spin_unlock_irqrestore(&ide_lock, flags);
> + spin_unlock_irqrestore(&qd65xx_lock, flags);
> return read;
> }
I don't see why all the locking above is needed at all -- isn't these
atomic functions? :-/
> @@ -301,16 +303,15 @@ static void qd6580_set_pio_mode(ide_driv
>
> static int __init qd_testreg(int port)
> {
> - u8 savereg;
> - u8 readreg;
> unsigned long flags;
> + u8 savereg, readreg;
>
> - spin_lock_irqsave(&ide_lock, flags);
> + spin_lock_irqsave(&qd65xx_lock, flags);
> savereg = inb_p(port);
> outb_p(QD_TESTVAL, port); /* safe value */
> readreg = inb_p(port);
> outb(savereg, port);
> - spin_unlock_irqrestore(&ide_lock, flags);
> + spin_unlock_irqrestore(&qd65xx_lock, flags);
>
> if (savereg == QD_TESTVAL) {
> printk(KERN_ERR "Outch ! the probe for qd65xx isn't reliable !\n");
Heh, 486 with VLB hardly needed spinlocks at all... although there *were*
486 SMP machines -- with external LAPICs.
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/15] qd65xx: fix deadlock on error handling
2007-10-02 12:59 ` Sergei Shtylyov
@ 2007-10-02 13:04 ` Jeff Garzik
2007-10-02 22:15 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Garzik @ 2007-10-02 13:04 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
Sergei Shtylyov wrote:
> Bartlomiej Zolnierkiewicz wrote:
>
>> Stop abusing ide_lock lock (switch to a private locking).
>
>> Fixes same issue as fixed by Alan Cox in atiixp host driver with
>> commit 6c5f8cc33eb2e10b6ab788bbe259fc142a068627.
>
>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>
>> Index: b/drivers/ide/legacy/qd65xx.c
>> ===================================================================
>> --- a/drivers/ide/legacy/qd65xx.c
>> +++ b/drivers/ide/legacy/qd65xx.c
>> @@ -89,13 +89,15 @@
>>
>> static int timings[4]={-1,-1,-1,-1}; /* stores current timing for
>> each timer */
>>
>> +static DEFINE_SPINLOCK(qd65xx_lock);
>> +
>> static void qd_write_reg (u8 content, unsigned long reg)
>> {
>> unsigned long flags;
>>
>> - spin_lock_irqsave(&ide_lock, flags);
>> + spin_lock_irqsave(&qd65xx_lock, flags);
>> outb(content,reg);
>> - spin_unlock_irqrestore(&ide_lock, flags);
>> + spin_unlock_irqrestore(&qd65xx_lock, flags);
>> }
>>
>> static u8 __init qd_read_reg (unsigned long reg)
>> @@ -103,9 +105,9 @@ static u8 __init qd_read_reg (unsigned l
>> unsigned long flags;
>> u8 read;
>>
>> - spin_lock_irqsave(&ide_lock, flags);
>> + spin_lock_irqsave(&qd65xx_lock, flags);
>> read = inb(reg);
>> - spin_unlock_irqrestore(&ide_lock, flags);
>> + spin_unlock_irqrestore(&qd65xx_lock, flags);
>> return read;
>> }
>
> I don't see why all the locking above is needed at all -- isn't these
> atomic functions? :-/
Agreed.
Jeff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/15] qd65xx: fix deadlock on error handling
2007-10-02 13:04 ` Jeff Garzik
@ 2007-10-02 22:15 ` Bartlomiej Zolnierkiewicz
2007-10-02 22:19 ` Jeff Garzik
0 siblings, 1 reply; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-02 22:15 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Sergei Shtylyov, linux-ide
On Tuesday 02 October 2007, Jeff Garzik wrote:
> Sergei Shtylyov wrote:
> > Bartlomiej Zolnierkiewicz wrote:
> >
> >> Stop abusing ide_lock lock (switch to a private locking).
> >
> >> Fixes same issue as fixed by Alan Cox in atiixp host driver with
> >> commit 6c5f8cc33eb2e10b6ab788bbe259fc142a068627.
> >
> >> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >
> >> Index: b/drivers/ide/legacy/qd65xx.c
> >> ===================================================================
> >> --- a/drivers/ide/legacy/qd65xx.c
> >> +++ b/drivers/ide/legacy/qd65xx.c
> >> @@ -89,13 +89,15 @@
> >>
> >> static int timings[4]={-1,-1,-1,-1}; /* stores current timing for
> >> each timer */
> >>
> >> +static DEFINE_SPINLOCK(qd65xx_lock);
> >> +
> >> static void qd_write_reg (u8 content, unsigned long reg)
> >> {
> >> unsigned long flags;
> >>
> >> - spin_lock_irqsave(&ide_lock, flags);
> >> + spin_lock_irqsave(&qd65xx_lock, flags);
> >> outb(content,reg);
> >> - spin_unlock_irqrestore(&ide_lock, flags);
> >> + spin_unlock_irqrestore(&qd65xx_lock, flags);
> >> }
> >>
> >> static u8 __init qd_read_reg (unsigned long reg)
> >> @@ -103,9 +105,9 @@ static u8 __init qd_read_reg (unsigned l
> >> unsigned long flags;
> >> u8 read;
> >>
> >> - spin_lock_irqsave(&ide_lock, flags);
> >> + spin_lock_irqsave(&qd65xx_lock, flags);
> >> read = inb(reg);
> >> - spin_unlock_irqrestore(&ide_lock, flags);
> >> + spin_unlock_irqrestore(&qd65xx_lock, flags);
> >> return read;
> >> }
> >
> > I don't see why all the locking above is needed at all -- isn't these
> > atomic functions? :-/
>
> Agreed.
Indeed, incremental patch follows.
[PATCH] qd65xx: remove pointless qd_{read,write}_reg()
These functions are atomic so locking is pointless (noticed by Sergei).
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/legacy/qd65xx.c | 43 +++++++++++--------------------------------
1 file changed, 11 insertions(+), 32 deletions(-)
Index: b/drivers/ide/legacy/qd65xx.c
===================================================================
--- a/drivers/ide/legacy/qd65xx.c
+++ b/drivers/ide/legacy/qd65xx.c
@@ -89,28 +89,6 @@
static int timings[4]={-1,-1,-1,-1}; /* stores current timing for each timer */
-static DEFINE_SPINLOCK(qd65xx_lock);
-
-static void qd_write_reg (u8 content, unsigned long reg)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&qd65xx_lock, flags);
- outb(content,reg);
- spin_unlock_irqrestore(&qd65xx_lock, flags);
-}
-
-static u8 __init qd_read_reg (unsigned long reg)
-{
- unsigned long flags;
- u8 read;
-
- spin_lock_irqsave(&qd65xx_lock, flags);
- read = inb(reg);
- spin_unlock_irqrestore(&qd65xx_lock, flags);
- return read;
-}
-
/*
* qd_select:
*
@@ -123,7 +101,7 @@ static void qd_select (ide_drive_t *driv
(QD_TIMREG(drive) & 0x02);
if (timings[index] != QD_TIMING(drive))
- qd_write_reg(timings[index] = QD_TIMING(drive), QD_TIMREG(drive));
+ outb(timings[index] = QD_TIMING(drive), QD_TIMREG(drive));
}
/*
@@ -286,7 +264,7 @@ static void qd6580_set_pio_mode(ide_driv
}
if (!HWIF(drive)->channel && drive->media != ide_disk) {
- qd_write_reg(0x5f, QD_CONTROL_PORT);
+ outb(0x5f, QD_CONTROL_PORT);
printk(KERN_WARNING "%s: ATAPI: disabled read-ahead FIFO "
"and post-write buffer on %s.\n",
drive->name, HWIF(drive)->name);
@@ -303,6 +281,7 @@ static void qd6580_set_pio_mode(ide_driv
static int __init qd_testreg(int port)
{
+ static DEFINE_SPINLOCK(qd65xx_lock);
unsigned long flags;
u8 savereg, readreg;
@@ -365,13 +344,13 @@ static void __exit qd_unsetup(ide_hwif_t
if (set_pio_mode == (void *)qd6500_set_pio_mode) {
// will do it for both
- qd_write_reg(QD6500_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
+ outb(QD6500_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
} else if (set_pio_mode == (void *)qd6580_set_pio_mode) {
if (QD_CONTROL(hwif) & QD_CONTR_SEC_DISABLED) {
- qd_write_reg(QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
- qd_write_reg(QD6580_DEF_DATA2, QD_TIMREG(&hwif->drives[1]));
+ outb(QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
+ outb(QD6580_DEF_DATA2, QD_TIMREG(&hwif->drives[1]));
} else {
- qd_write_reg(hwif->channel ? QD6580_DEF_DATA2 : QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
+ outb(hwif->channel ? QD6580_DEF_DATA2 : QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
}
} else {
printk(KERN_WARNING "Unknown qd65xx tuning fonction !\n");
@@ -394,7 +373,7 @@ static int __init qd_probe(int base)
u8 config;
u8 unit;
- config = qd_read_reg(QD_CONFIG_PORT);
+ config = inb(QD_CONFIG_PORT);
if (! ((config & QD_CONFIG_BASEPORT) >> 1 == (base == 0xb0)) )
return 1;
@@ -438,7 +417,7 @@ static int __init qd_probe(int base)
/* qd6580 found */
- control = qd_read_reg(QD_CONTROL_PORT);
+ control = inb(QD_CONTROL_PORT);
printk(KERN_NOTICE "qd6580 at %#x\n", base);
printk(KERN_DEBUG "qd6580: config=%#x, control=%#x, ID3=%u\n",
@@ -459,7 +438,7 @@ static int __init qd_probe(int base)
ide_device_add(idx);
- qd_write_reg(QD_DEF_CONTR, QD_CONTROL_PORT);
+ outb(QD_DEF_CONTR, QD_CONTROL_PORT);
return 1;
} else {
@@ -486,7 +465,7 @@ static int __init qd_probe(int base)
ide_device_add(idx);
- qd_write_reg(QD_DEF_CONTR, QD_CONTROL_PORT);
+ outb(QD_DEF_CONTR, QD_CONTROL_PORT);
return 0; /* no other qd65xx possible */
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/15] qd65xx: fix deadlock on error handling
2007-10-02 22:15 ` Bartlomiej Zolnierkiewicz
@ 2007-10-02 22:19 ` Jeff Garzik
2007-10-03 22:41 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Garzik @ 2007-10-02 22:19 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Sergei Shtylyov, linux-ide
Bartlomiej Zolnierkiewicz wrote:
ACK everything else (that I snipped)...
> @@ -303,6 +281,7 @@ static void qd6580_set_pio_mode(ide_driv
>
> static int __init qd_testreg(int port)
> {
> + static DEFINE_SPINLOCK(qd65xx_lock);
> unsigned long flags;
> u8 savereg, readreg;
At this point, the lock is largely pointless, isn't it?
Seems like you might as well use local_irq_save/restore.
Jeff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/15] qd65xx: fix deadlock on error handling
2007-10-02 22:19 ` Jeff Garzik
@ 2007-10-03 22:41 ` Bartlomiej Zolnierkiewicz
2007-10-03 22:51 ` Jeff Garzik
0 siblings, 1 reply; 7+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-10-03 22:41 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Sergei Shtylyov, linux-ide
On Wednesday 03 October 2007, Jeff Garzik wrote:
> Bartlomiej Zolnierkiewicz wrote:
>
> ACK everything else (that I snipped)...
>
> > @@ -303,6 +281,7 @@ static void qd6580_set_pio_mode(ide_driv
> >
> > static int __init qd_testreg(int port)
> > {
> > + static DEFINE_SPINLOCK(qd65xx_lock);
> > unsigned long flags;
> > u8 savereg, readreg;
>
> At this point, the lock is largely pointless, isn't it?
>
> Seems like you might as well use local_irq_save/restore.
Yeah.
[PATCH] qd65xx: remove pointless qd_{read,write}_reg() (take 2)
These functions are atomic so locking is pointless (noticed by Sergei).
v2:
We can now just use local_irq_save/restore() in qd_testreg() (noticed by Jeff).
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/legacy/qd65xx.c | 46 +++++++++++---------------------------------
1 file changed, 12 insertions(+), 34 deletions(-)
Index: b/drivers/ide/legacy/qd65xx.c
===================================================================
--- a/drivers/ide/legacy/qd65xx.c
+++ b/drivers/ide/legacy/qd65xx.c
@@ -89,28 +89,6 @@
static int timings[4]={-1,-1,-1,-1}; /* stores current timing for each timer */
-static DEFINE_SPINLOCK(qd65xx_lock);
-
-static void qd_write_reg (u8 content, unsigned long reg)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&qd65xx_lock, flags);
- outb(content,reg);
- spin_unlock_irqrestore(&qd65xx_lock, flags);
-}
-
-static u8 __init qd_read_reg (unsigned long reg)
-{
- unsigned long flags;
- u8 read;
-
- spin_lock_irqsave(&qd65xx_lock, flags);
- read = inb(reg);
- spin_unlock_irqrestore(&qd65xx_lock, flags);
- return read;
-}
-
/*
* qd_select:
*
@@ -123,7 +101,7 @@ static void qd_select (ide_drive_t *driv
(QD_TIMREG(drive) & 0x02);
if (timings[index] != QD_TIMING(drive))
- qd_write_reg(timings[index] = QD_TIMING(drive), QD_TIMREG(drive));
+ outb(timings[index] = QD_TIMING(drive), QD_TIMREG(drive));
}
/*
@@ -286,7 +264,7 @@ static void qd6580_set_pio_mode(ide_driv
}
if (!HWIF(drive)->channel && drive->media != ide_disk) {
- qd_write_reg(0x5f, QD_CONTROL_PORT);
+ outb(0x5f, QD_CONTROL_PORT);
printk(KERN_WARNING "%s: ATAPI: disabled read-ahead FIFO "
"and post-write buffer on %s.\n",
drive->name, HWIF(drive)->name);
@@ -306,12 +284,12 @@ static int __init qd_testreg(int port)
unsigned long flags;
u8 savereg, readreg;
- spin_lock_irqsave(&qd65xx_lock, flags);
+ local_irq_save(flags);
savereg = inb_p(port);
outb_p(QD_TESTVAL, port); /* safe value */
readreg = inb_p(port);
outb(savereg, port);
- spin_unlock_irqrestore(&qd65xx_lock, flags);
+ local_irq_restore(flags);
if (savereg == QD_TESTVAL) {
printk(KERN_ERR "Outch ! the probe for qd65xx isn't reliable !\n");
@@ -365,13 +343,13 @@ static void __exit qd_unsetup(ide_hwif_t
if (set_pio_mode == (void *)qd6500_set_pio_mode) {
// will do it for both
- qd_write_reg(QD6500_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
+ outb(QD6500_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
} else if (set_pio_mode == (void *)qd6580_set_pio_mode) {
if (QD_CONTROL(hwif) & QD_CONTR_SEC_DISABLED) {
- qd_write_reg(QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
- qd_write_reg(QD6580_DEF_DATA2, QD_TIMREG(&hwif->drives[1]));
+ outb(QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
+ outb(QD6580_DEF_DATA2, QD_TIMREG(&hwif->drives[1]));
} else {
- qd_write_reg(hwif->channel ? QD6580_DEF_DATA2 : QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
+ outb(hwif->channel ? QD6580_DEF_DATA2 : QD6580_DEF_DATA, QD_TIMREG(&hwif->drives[0]));
}
} else {
printk(KERN_WARNING "Unknown qd65xx tuning fonction !\n");
@@ -394,7 +372,7 @@ static int __init qd_probe(int base)
u8 config;
u8 unit;
- config = qd_read_reg(QD_CONFIG_PORT);
+ config = inb(QD_CONFIG_PORT);
if (! ((config & QD_CONFIG_BASEPORT) >> 1 == (base == 0xb0)) )
return 1;
@@ -438,7 +416,7 @@ static int __init qd_probe(int base)
/* qd6580 found */
- control = qd_read_reg(QD_CONTROL_PORT);
+ control = inb(QD_CONTROL_PORT);
printk(KERN_NOTICE "qd6580 at %#x\n", base);
printk(KERN_DEBUG "qd6580: config=%#x, control=%#x, ID3=%u\n",
@@ -459,7 +437,7 @@ static int __init qd_probe(int base)
ide_device_add(idx);
- qd_write_reg(QD_DEF_CONTR, QD_CONTROL_PORT);
+ outb(QD_DEF_CONTR, QD_CONTROL_PORT);
return 1;
} else {
@@ -486,7 +464,7 @@ static int __init qd_probe(int base)
ide_device_add(idx);
- qd_write_reg(QD_DEF_CONTR, QD_CONTROL_PORT);
+ outb(QD_DEF_CONTR, QD_CONTROL_PORT);
return 0; /* no other qd65xx possible */
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/15] qd65xx: fix deadlock on error handling
2007-10-03 22:41 ` Bartlomiej Zolnierkiewicz
@ 2007-10-03 22:51 ` Jeff Garzik
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Garzik @ 2007-10-03 22:51 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: Sergei Shtylyov, linux-ide
Bartlomiej Zolnierkiewicz wrote:
> On Wednesday 03 October 2007, Jeff Garzik wrote:
>> Bartlomiej Zolnierkiewicz wrote:
>>
>> ACK everything else (that I snipped)...
>>
>>> @@ -303,6 +281,7 @@ static void qd6580_set_pio_mode(ide_driv
>>>
>>> static int __init qd_testreg(int port)
>>> {
>>> + static DEFINE_SPINLOCK(qd65xx_lock);
>>> unsigned long flags;
>>> u8 savereg, readreg;
>> At this point, the lock is largely pointless, isn't it?
>>
>> Seems like you might as well use local_irq_save/restore.
>
> Yeah.
>
>
> [PATCH] qd65xx: remove pointless qd_{read,write}_reg() (take 2)
>
> These functions are atomic so locking is pointless (noticed by Sergei).
>
> v2:
> We can now just use local_irq_save/restore() in qd_testreg() (noticed by Jeff).
>
> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Cc: Jeff Garzik <jeff@garzik.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
ACK
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-10-03 22:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-01 21:33 [PATCH 3/15] qd65xx: fix deadlock on error handling Bartlomiej Zolnierkiewicz
2007-10-02 12:59 ` Sergei Shtylyov
2007-10-02 13:04 ` Jeff Garzik
2007-10-02 22:15 ` Bartlomiej Zolnierkiewicz
2007-10-02 22:19 ` Jeff Garzik
2007-10-03 22:41 ` Bartlomiej Zolnierkiewicz
2007-10-03 22:51 ` Jeff Garzik
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).