Hi Linus,
This trivially fixes the cyclades driver under ISA to get rid of the
cli()/sti() locking and use a spin lock as described in the
Documentation/cli-sti-removal.txt instructions. I'm using this on an
SMP system without any problems so far.
Thanks,
John
[email protected]
diff -ur l-2.5.72/drivers/char/cyclades.c l-2.5.72-cyclades/drivers/char/cyclades.c
--- l-2.5.72/drivers/char/cyclades.c Wed Jun 18 10:26:21 2003
+++ l-2.5.72-cyclades/drivers/char/cyclades.c Wed Jun 18 17:33:06 2003
@@ -867,6 +867,7 @@
static int cyz_issue_cmd(struct cyclades_card *, uclong, ucchar, uclong);
#ifdef CONFIG_ISA
static unsigned detect_isa_irq (volatile ucchar *);
+spinlock_t isa_card_lock = SPIN_LOCK_UNLOCKED;
#endif /* CONFIG_ISA */
static int cyclades_get_proc_info(char *, char **, off_t , int , int *, void *);
@@ -1050,14 +1051,14 @@
udelay(5000L);
/* Enable the Tx interrupts on the CD1400 */
- save_flags(flags); cli();
+ spin_lock_irqsave(&isa_card_lock,flags);
cy_writeb((u_long)address + (CyCAR<<index), 0);
cyy_issue_cmd(address, CyCHAN_CTL|CyENB_XMTR, index);
cy_writeb((u_long)address + (CyCAR<<index), 0);
cy_writeb((u_long)address + (CySRER<<index),
cy_readb(address + (CySRER<<index)) | CyTxRdy);
- restore_flags(flags);
+ spin_unlock_irqrestore(&isa_card_lock, flags);
/* Wait ... */
udelay(5000L);
@@ -5675,13 +5676,13 @@
}
#endif /* CONFIG_CYZ_INTR */
- save_flags(flags); cli();
+ spin_lock_irqsave(&isa_card_lock, flags);
if ((e1 = tty_unregister_driver(cy_serial_driver)))
printk("cyc: failed to unregister Cyclades serial driver(%d)\n",
e1);
- restore_flags(flags);
+ spin_unlock_irqrestore(&isa_card_lock,flags);
put_tty_driver(cy_serial_driver);
for (i = 0; i < NR_CARDS; i++) {
On Wed, Jun 18, 2003 at 05:48:17PM -0400, John Stoffel wrote:
> - save_flags(flags); cli();
> + spin_lock_irqsave(&isa_card_lock, flags);
>
> if ((e1 = tty_unregister_driver(cy_serial_driver)))
> printk("cyc: failed to unregister Cyclades serial driver(%d)\n",
> e1);
>
> - restore_flags(flags);
> + spin_unlock_irqrestore(&isa_card_lock,flags);
It doesn't fix the problem and only makes the compile trouble go away.
Not to mention anything else, you are relying on a lot of code being
non-blocking.
Could you explain what is protected by disabling interrupts and taking
a spinlock here?
viro> On Wed, Jun 18, 2003 at 05:48:17PM -0400, John Stoffel wrote:
>> - save_flags(flags); cli();
>> + spin_lock_irqsave(&isa_card_lock, flags);
>>
>> if ((e1 = tty_unregister_driver(cy_serial_driver)))
>> printk("cyc: failed to unregister Cyclades serial driver(%d)\n",
>> e1);
>>
>> - restore_flags(flags);
>> + spin_unlock_irqrestore(&isa_card_lock,flags);
viro> It doesn't fix the problem and only makes the compile trouble go
viro> away. Not to mention anything else, you are relying on a lot of
viro> code being non-blocking.
viro> Could you explain what is protected by disabling interrupts and
viro> taking a spinlock here?
Honestly, I'm not sure. My original patch was against 2.5.69, which
didn't have this section of code at all, I think there have been some
patches to the code between 2.5.69 and 2.5.72 which have mucked around
with stuff.
It looked suspicious to me as well, and I'd be happy to remove this
from my patch, since I don't know what locking is needed around this
section of code off hand. Let me look at some other drivers and see
how they unregister themselves. Maybe theres a tty_drive_spinlock
somewhere that should be used instead.
Thanks for your comments, good to see you back on l-k.
John