More IDE hackery

More IDE hackery

[Alan Cox]

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

Our SI3112 driver fixups don't work in some cases. I found this while
trying to work out what was going on in IT8212. At the point we check
for the mod15 bug we may not have drive data. So we need a "fixups"
callback. 

I've also played with a pile of flash adapters - it seems the slave
decode bug is PCMCIA specific - so we need a fixup hook for
pcmcia/delkin/..

I ended up with the following which solves all three in one go and
allows us to fix anything else that turns up

1. Turned ide_undecoded_slave into a routine of its own
2. Changed probe_hwif_init to take a third argument a "fixup" callback
which is run after hwif_init but before ata_attach
3. Modify ide-cs, delkin to pass ide_undecoded_slave as their fixup
4. Added "fixup" as a method in the ide_pci_device_t and made the
pci-setup code pass this as the fixup argument to probe_hwif_init
5. Made si3112 pass the mod15 check this way, made it8212 pass the
geometry demnagler this way
6. Fix up the other callers to pass NULL

I've attached the code change, if you think its sane I'll split you out
a set of patches versus the base Linus tree which do this

Alan



[-- Attachment #2: a1 --]
[-- Type: text/x-patch, Size: 2987 bytes --]

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.9/drivers/ide/ide-probe.c linux-2.6.9/drivers/ide/ide-probe.c
--- linux.vanilla-2.6.9/drivers/ide/ide-probe.c	2004-10-20 23:16:53.000000000 +0100
+++ linux-2.6.9/drivers/ide/ide-probe.c	2004-11-02 18:04:58.000000000 +0000
@@ -662,6 +662,42 @@
 	return rc;
 }
 
+/**
+ *	ide_undecoded_slave	-	look for bad CF adapters
+ *	@hwif: interface
+ *
+ *	Analyse the drives on the interface and attempt to decide if we
+ *	have the same drive viewed twice. This occurs with crap CF adapters
+ *	and PCMCIA sometimes.
+ */
+
+void ide_undecoded_slave(ide_hwif_t *hwif) 
+{
+	ide_drive_t *drive0 = &hwif->drives[0];
+	ide_drive_t *drive1 = &hwif->drives[1];
+	
+	if (drive0->present == 0 || drive1->present == 0)
+			return;
+		
+	/* If the models don't match they are not the same product */	
+	if (strcmp(drive0->id->model, drive1->id->model))
+		return;
+	/* Serial numbers do not match */
+	if(strncmp(drive0->id->serial_no, drive1->id->serial_no, 20))
+		return;
+	/* No serial number, thankfully very rare for CF */
+	if (drive0->id->serial_no[0] == 0)
+		return;
+	/* Has a serial number but is warped */
+	if (!strstr(drive0->id->model, "Integrated Technology Express"))
+		return;
+	/* Appears to be an IDE flash adapter with decode bugs */
+	printk(KERN_WARNING "ide-probe: ignoring undecoded slave\n");
+	drive1->present = 0;
+}
+
+EXPORT_SYMBOL(ide_undecoded_slave);
+
 /*
  * This routine only knows how to look for drive units 0 and 1
  * on an interface, so any setting of MAX_DRIVES > 2 won't work here.
@@ -736,18 +772,6 @@
 		ide_drive_t *drive = &hwif->drives[unit];
 		drive->dn = (hwif->channel ? 2 : 0) + unit;
 		(void) probe_for_drive(drive);
-		if (drive->present && hwif->present && unit == 1) {
-			if (strcmp(hwif->drives[0].id->model, drive->id->model) == 0 &&
-			    /* Don't do this for noprobe or non ATA */
-			    strcmp(drive->id->model, "UNKNOWN") &&
-			    /* And beware of confused Maxtor drives that go "M0000000000"
-			      "The SN# is garbage in the ID block..." [Eric] */
-			    strncmp(drive->id->serial_no, "M0000000000000000000", 20) &&
-			    strncmp(hwif->drives[0].id->serial_no, drive->id->serial_no, 20) == 0) {
-				printk(KERN_WARNING "ide-probe: ignoring undecoded slave\n");
-				drive->present = 0;
-			}
-		}
 		if (drive->present && !hwif->present) {
 			hwif->present = 1;
 			if (hwif->chipset != ide_4drives ||
@@ -821,13 +845,18 @@
 }
 
 static int hwif_init(ide_hwif_t *hwif);
-int probe_hwif_init (ide_hwif_t *hwif)
+
+int probe_hwif_init (ide_hwif_t *hwif, void (*fixup)(ide_hwif_t *hwif))
 {
 	probe_hwif(hwif);
 	hwif_init(hwif);
 
 	if (hwif->present) {
 		u16 unit = 0;
+
+		if(fixup != NULL)
+			fixup(hwif);
+			
 		for (unit = 0; unit < MAX_DRIVES; ++unit) {
 			ide_drive_t *drive = &hwif->drives[unit];
 			/* For now don't attach absent drives, we may

Re: More IDE hackery

[Alan Cox]

> It won't work unless you modify ide_register_hw().
> See ide_register_hw() vs "initializing == 1".

I changed that to allow a fixup to be passed as well. That causes most
of the noise changes but I could make that ide_register_hw_with_fixup()
to avoid changes elsewhere adding NULL to ide_register_hw calls.  Which
would you prefer and I'll go make full patches

> +	/* Has a serial number but is warped */
> +	if (!strstr(drive0->id->model, "Integrated Technology Express"))
> +		return;
> 
> This shouldn't be needed now, no?

Probably not.

Re: More IDE hackery

[Bartlomiej Zolnierkiewicz]

Hi!

On Wed, 03 Nov 2004 13:16:50 +0000, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Our SI3112 driver fixups don't work in some cases. I found this while

brown paper bug of mine

> trying to work out what was going on in IT8212. At the point we check
> for the mod15 bug we may not have drive data. So we need a "fixups"
> callback.
> 
> I've also played with a pile of flash adapters - it seems the slave
> decode bug is PCMCIA specific - so we need a fixup hook for
> pcmcia/delkin/..

Cool.

> I ended up with the following which solves all three in one go and
> allows us to fix anything else that turns up
> 
> 1. Turned ide_undecoded_slave into a routine of its own

fine

> 2. Changed probe_hwif_init to take a third argument a "fixup" callback
> which is run after hwif_init but before ata_attach

What about ide_hwif_t->fixup instead?

> 3. Modify ide-cs, delkin to pass ide_undecoded_slave as their fixup

It won't work unless you modify ide_register_hw().
See ide_register_hw() vs "initializing == 1".

> 4. Added "fixup" as a method in the ide_pci_device_t and made the
> pci-setup code pass this as the fixup argument to probe_hwif_init
> 5. Made si3112 pass the mod15 check this way, made it8212 pass the
> geometry demnagler this way
> 6. Fix up the other callers to pass NULL

I prefer minimal changes to existing code...

> I've attached the code change, if you think its sane I'll split you out
> a set of patches versus the base Linus tree which do this

Only patch for 1. is attached.

It is hard to speak about changes without seeing actual code.

+	/* Has a serial number but is warped */
+	if (!strstr(drive0->id->model, "Integrated Technology Express"))
+		return;

This shouldn't be needed now, no?

Bartlomiej

Re: More IDE hackery

[Bartlomiej Zolnierkiewicz]

On Wed, 03 Nov 2004 17:08:35 +0000, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > It won't work unless you modify ide_register_hw().
> > See ide_register_hw() vs "initializing == 1".
> 
> I changed that to allow a fixup to be passed as well. That causes most
> of the noise changes but I could make that ide_register_hw_with_fixup()
> to avoid changes elsewhere adding NULL to ide_register_hw calls.  Which
> would you prefer and I'll go make full patches

Sounds good, please make ide_register_hw_with_fixup().