Re: [RFC/CFT] cmd640 irqlocking fixes

[Andre Hedrick <andre@linux-ide.org>]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]


On 25 Jul 2002, Alan Cox wrote:

> On Thu, 2002-07-25 at 09:56, Marcin Dalecki wrote:
> > OK. Right. We have to touch this code anyway. Do you know first hand how
> > to detect programmatically which configuration method is in charge? If 
> > not I can look it up on my own..
> 
> Just copy the code from 2.4.19-rc3-ac3. Andre didnt write it so you
> don't have to pretend it doesn't exist. I'll do this and test it since I
> did the original fixes in 2.4. Expect a patch later today

I am sorry Alan, but I fixed all of the locking code in 2.4 a long time
ago, and you adopted it somewhere around this patch.

Please check your patch revisions, and I can retrive out of archives
the date and time when I finally fixed it for all of x86 and then when I
started reworking all the arch specifics.  Noting that I had broken the
ia64 locking and DM of HP replied to me offering help to solve the need
for the local_irq_set() calls which are need currently.

I have now fix all the asm-*/system.h asm-*/hw_irq.h etc with proper
assembley calls.  

Again I have to call this patch and fix and take credit and full ownership
of removing all the save/cli/sti/restore which littered the driver and
were spread like cow patties through a chopper gun.

Cheers,

Andre Hedrick
LAD Storage Consulting Group

[-- Attachment #2: Type: text/plain, Size: 5910 bytes --]

diff -urN linux-2.4.19-p8-ac1-pristine/drivers/ide/cmd640.c linux-2.4.19-p8-ac1/drivers/ide/cmd640.c
--- linux-2.4.19-p8-ac1-pristine/drivers/ide/cmd640.c	Fri May 10 10:10:21 2002
+++ linux-2.4.19-p8-ac1/drivers/ide/cmd640.c	Fri May 10 05:15:51 2002
@@ -217,11 +217,10 @@
 {
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
-	outl_p((reg & 0xfc) | cmd640_key, 0xcf8);
+	spin_lock_irqsave(&io_request_lock, flags);
+	outb_p((reg & 0xfc) | cmd640_key, 0xcf8);
 	outb_p(val, (reg & 3) | 0xcfc);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 }
 
 static byte get_cmd640_reg_pci1 (unsigned short reg)
@@ -229,11 +228,10 @@
 	byte b;
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
-	outl_p((reg & 0xfc) | cmd640_key, 0xcf8);
+	spin_lock_irqsave(&io_request_lock, flags);
+	outb_p((reg & 0xfc) | cmd640_key, 0xcf8);
 	b = inb_p((reg & 3) | 0xcfc);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 	return b;
 }
 
@@ -243,12 +241,11 @@
 {
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 	outb_p(0x10, 0xcf8);
 	outb_p(val, cmd640_key + reg);
 	outb_p(0, 0xcf8);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 }
 
 static byte get_cmd640_reg_pci2 (unsigned short reg)
@@ -256,12 +253,11 @@
 	byte b;
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 	outb_p(0x10, 0xcf8);
 	b = inb_p(cmd640_key + reg);
 	outb_p(0, 0xcf8);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 	return b;
 }
 
@@ -271,11 +267,10 @@
 {
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 	outb_p(reg, cmd640_key);
 	outb_p(val, cmd640_key + 4);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 }
 
 static byte get_cmd640_reg_vlb (unsigned short reg)
@@ -283,11 +278,10 @@
 	byte b;
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 	outb_p(reg, cmd640_key);
 	b = inb_p(cmd640_key + 4);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 	return b;
 }
 
@@ -315,7 +309,9 @@
 {
 	get_cmd640_reg = get_cmd640_reg_pci1;
 	put_cmd640_reg = put_cmd640_reg_pci1;
-	for (cmd640_key = 0x80000000; cmd640_key <= 0x8000f800; cmd640_key += 0x800) {
+	for (cmd640_key = 0x80000000;
+	     cmd640_key <= 0x8000f800;
+	     cmd640_key += 0x800) {
 		if (match_pci_cmd640_device())
 			return 1; /* success */
 	}
@@ -364,8 +360,7 @@
 {
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 
 	outb_p(0x0a, 0x170 + IDE_SELECT_OFFSET);	/* select drive0 */
 	udelay(100);
@@ -373,11 +368,11 @@
 		outb_p(0x1a, 0x170 + IDE_SELECT_OFFSET); /* select drive1 */
 		udelay(100);
 		if ((inb_p(0x170 + IDE_SELECT_OFFSET) & 0x1f) != 0x1a) {
-			restore_flags(flags);
+			spin_unlock_irqrestore(&io_request_lock, flags);
 			return 0; /* nothing responded */
 		}
 	}
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 	return 1; /* success */
 }
 
@@ -458,8 +453,7 @@
 	byte b;
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 	b = get_cmd640_reg(reg);
 	if (mode) {	/* want prefetch on? */
 #if CMD640_PREFETCH_MASKS
@@ -475,7 +469,7 @@
 		b |= prefetch_masks[index];	/* disable prefetch */
 	}
 	put_cmd640_reg(reg, b);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 }
 
 /*
@@ -576,8 +570,7 @@
 	/*
 	 * Now that everything is ready, program the new timings
 	 */
-	save_flags (flags);
-	cli();
+	spin_lock_irqsave(&io_request_lock, flags);
 	/*
 	 * Program the address_setup clocks into ARTTIM reg,
 	 * and then the active/recovery counts into the DRWTIM reg
@@ -586,7 +579,7 @@
 	setup_count |= get_cmd640_reg(arttim_regs[index]) & 0x3f;
 	put_cmd640_reg(arttim_regs[index], setup_count);
 	put_cmd640_reg(drwtim_regs[index], pack_nibbles(active_count, recovery_count));
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 }
 
 /*
@@ -697,19 +690,17 @@
 	u32 tmp;
 	unsigned long flags;
 
-	save_flags(flags);
-	__cli();
-	outb(0x01, 0xCFB);
+	spin_lock_irqsave(&io_request_lock, flags);
+	OUT_BYTE(0x01, 0xCFB);
 	tmp = inl(0xCF8);
 	outl(0x80000000, 0xCF8);
-	if(inl(0xCF8) == 0x80000000)
-	{
+	if (inl(0xCF8) == 0x80000000) {
 		outl(tmp, 0xCF8);
-		restore_flags(flags);
+		spin_unlock_irqrestore(&io_request_lock, flags);
 		return 1;
 	}
 	outl(tmp, 0xCF8);
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 	return 0;
 }
 
@@ -717,17 +708,15 @@
 {
 	unsigned long flags;
 
-	save_flags(flags);
-	__cli();
-	outb(0x00, 0xCFB);
-	outb(0x00, 0xCF8);
-	outb(0x00, 0xCFA);
-	if(inb(0xCF8) == 0x00 && inb(0xCF8) == 0x00)
-	{
-		restore_flags(flags);
+	spin_lock_irqsave(&io_request_lock, flags);
+	OUT_BYTE(0x00, 0xCFB);
+	OUT_BYTE(0x00, 0xCF8);
+	OUT_BYTE(0x00, 0xCFA);
+	if (IN_BYTE(0xCF8) == 0x00 && IN_BYTE(0xCF8) == 0x00) {
+		spin_unlock_irqrestore(&io_request_lock, flags);
 		return 1;
 	}
-	restore_flags(flags);
+	spin_unlock_irqrestore(&io_request_lock, flags);
 	return 0;
 }
 
@@ -748,10 +737,8 @@
 		bus_type = "VLB";
 	} else {
 		cmd640_vlb = 0;
-		/*
-		 * Don't leak I/O cycles on the PCI bus by blindly attempting
-		 * a configuration cycle type that is not supported by the hardware.
-		 */
+		/* Find out what kind of PCI probing is supported otherwise
+		   Justin Gibbs will sulk.. */
 		if (pci_conf1() && probe_for_cmd640_pci1())
 			bus_type = "PCI (type1)";
 		else if (pci_conf2() && probe_for_cmd640_pci2())