mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@novell.com>
To: Andrew Morton <akpm@osdl.org>, Tim Waugh <twaugh@redhat.com>
Cc: linux-kernel@vger.kernel.org
Subject: parport_pc superio chip fixes
Date: Thu, 30 Sep 2004 08:08:51 +0200	[thread overview]
Message-ID: <20040930060851.GJ22008@dualathlon.random> (raw)

This patch fixes some troubles that somebody reported me with the
superio chips.

In short rmmod parport_pc && cat /proc/iomem was good enough for
crashing the box hard on some machine (and hwscan --printer was doing
just that). The way the oops triggers is that iomem tries to vsprintf
the p->name, but the p->name was a static string in the module address
(now unloaded).

The reason is that the superio chip scanning leaves up to two persistent
ranges claimed. But the second (legacy) pass has no way to notice the
resources are already reclaimed. Plus if the superio->io was different
than the "io" variable (the range to scan for superio chips) the "io"
range would generate a leak of the original "io" range too.

I simply make sure to always release the requested space during the
superio scan, and I make sure not to istantiate new ranges in the
p->base that would cause the later parport scan to fail too (plus
leaving up to leaked resources).

The previous code that was returning values and was leaving garbage in
there made no sense to me. My best guess (assuming I didn't misread it ;)
is that probably somebody added the request_region without realizing
they're pointing to the very same address that would be requested later
(and nobody does accesses on those ranges until later, so it was very
safe to claim it later).

Disclaimer: I don't have the specs of the winbond and smsc at hand, I
just guessed what they do from the code (nothing checks superio->io
except get_superio_dma get_superio_irq, which made the thing enough self
explainatory to fix it without specs)

Signed-off-by: Andrea Arcangeli <andrea@novell.com>

--- sles/drivers/parport/parport_pc.c.~1~	2004-09-30 05:56:36.000000000 +0200
+++ sles/drivers/parport/parport_pc.c	2004-09-30 07:54:56.469113152 +0200
@@ -1194,7 +1194,7 @@ struct parport_operations parport_pc_ops
 
 #ifdef CONFIG_PARPORT_PC_SUPERIO
 /* Super-IO chipset detection, Winbond, SMSC */
-static int __devinit show_parconfig_smsc37c669(int io, int key)
+static void __devinit show_parconfig_smsc37c669(int io, int key)
 {
 	int cr1,cr4,cra,cr23,cr26,cr27,i=0;
 	static const char *modes[]={ "SPP and Bidirectional (PS/2)",	
@@ -1261,26 +1261,17 @@ static int __devinit show_parconfig_smsc
 					superios[i].io = 0x278;
 					superios[i].irq = 5;
 			}
-			if (io != superios[i].io) {
-				/* how many bytes? */
-				if (!request_region(superios[i].io, 3, "smsc parport")) {
-					superios[i].io = 0;
-					return 0;
-				}
-			}
 			d=(cr26 &0x0f);
 			if((d==1) || (d==3)) 
 				superios[i].dma= d;
 			else
 				superios[i].dma= PARPORT_DMA_NONE;
-			return 1;
 		}
  	}
-	return 0;
 }
 
 
-static int __devinit show_parconfig_winbond(int io, int key)
+static void __devinit show_parconfig_winbond(int io, int key)
 {
 	int cr30,cr60,cr61,cr70,cr74,crf0,i=0;
 	static const char *modes[] = {
@@ -1336,23 +1327,14 @@ static int __devinit show_parconfig_winb
 			printk(KERN_INFO "Super-IO: too many chips!\n");
 		else {
 			superios[i].io = (cr60<<8)|cr61;
-			if (io != superios[i].io) {
-				/* how many bytes? */
-				if (!request_region(superios[i].io, 3, "winbond parport")) {
-					superios[i].io = 0;
-					return 0;
-				}
-			}
 			superios[i].irq = cr70&0x0f;
 			superios[i].dma = (((cr74 & 0x07) > 3) ?
 					   PARPORT_DMA_NONE : (cr74 & 0x07));
-			return 1;
 		}
 	}
-	return 0;
 }
 
-static int __devinit decode_winbond(int efer, int key, int devid, int devrev, int oldid)
+static void __devinit decode_winbond(int efer, int key, int devid, int devrev, int oldid)
 {
 	const char *type = "unknown";
 	int id,progif=2;
@@ -1360,7 +1342,7 @@ static int __devinit decode_winbond(int 
 	if (devid == devrev)
 		/* simple heuristics, we happened to read some
                    non-winbond register */
-		return 0;
+		return;
 
 	id=(devid<<8) | devrev;
 
@@ -1385,20 +1367,19 @@ static int __devinit decode_winbond(int 
 		       efer, key, devid, devrev, oldid, type);
 
 	if (progif == 2)
-		return show_parconfig_winbond(efer,key);
-	return 0;
+		show_parconfig_winbond(efer,key);
 }
 
-static int __devinit decode_smsc(int efer, int key, int devid, int devrev)
+static void __devinit decode_smsc(int efer, int key, int devid, int devrev)
 {
         const char *type = "unknown";
-	int (*func)(int io, int key);
+	void (*func)(int io, int key);
         int id;
 
         if (devid == devrev)
 		/* simple heuristics, we happened to read some
                    non-smsc register */
-		return 0;
+		return;
 
 	func=NULL;
         id=(devid<<8) | devrev;
@@ -1414,8 +1395,7 @@ static int __devinit decode_smsc(int efe
 		       efer, key, devid, devrev, type);
 
 	if (func)
-		return func(efer,key);
-	return 0;
+		func(efer,key);
 }
 
 
@@ -1448,8 +1428,7 @@ static void __devinit winbond_check(int 
 	if ((x_devid == devid) && (x_devrev == devrev) && (x_oldid == oldid))
 		goto out; /* protection against false positives */
 
-	if (decode_winbond(io,key,devid,devrev,oldid));
-		return;
+	decode_winbond(io,key,devid,devrev,oldid);
 out:
 	release_region(io, 3);
 }
@@ -1482,8 +1461,7 @@ static void __devinit winbond_check2(int
 	if ((x_devid == devid) && (x_devrev == devrev) && (x_oldid == oldid))
 		goto out; /* protection against false positives */
 
-        if (decode_winbond(io,key,devid,devrev,oldid))
-		return;
+	decode_winbond(io,key,devid,devrev,oldid);
 out:
 	release_region(io, 3);
 }
@@ -1522,8 +1500,7 @@ static void __devinit smsc_check(int io,
 	    (x_oldid == oldid) && (x_rev == rev))
 		goto out; /* protection against false positives */
 
-        if (decode_smsc(io,key,oldid,oldrev))
-		return;
+        decode_smsc(io,key,oldid,oldrev);
 out:
 	release_region(io, 3);
 }


             reply	other threads:[~2004-09-30  6:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-30  6:08 Andrea Arcangeli [this message]
     [not found] ` <20040930172650.4301ec92.akpm@osdl.org>
     [not found]   ` <20041001003613.GE32279@dualathlon.random>
     [not found]     ` <20040930175232.4d65ef50.akpm@osdl.org>
2004-10-11 22:11       ` ptep_establish smp race x86 PAE >4G Andrea Arcangeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040930060851.GJ22008@dualathlon.random \
    --to=andrea@novell.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=twaugh@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®