Skip to contentSkip to conversation

hpt366: don't check enablebits for HPT36x

PATCH Applied Sergei Shtylyov 4 participants 13 messages May 4, 2007 – May 28, 2007 libata

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hpt366: don't check enablebits for HPT36x

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

---
Michal, sorry for unintended breakage, please verify the patch... :-)

 drivers/ide/pci/hpt366.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)
drivers/ide/pci/hpt366.c +6 -1
Index: linux-2.6/drivers/ide/pci/hpt366.c
===================================================================
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 1.02	Apr 18, 2007
+ * linux/drivers/ide/pci/hpt366.c		Version 1.03	May 4, 2007
  *
  * Copyright (C) 1999-2003		Andre Hedrick <[email protected]>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.
@@ -1527,7 +1527,12 @@ static int __devinit init_setup_hpt366(s
 	if (rev > 2)
 		goto init_single;
 
+	/*
+	 * HPT36x chips are single channel and
+	 * do not seem to have the channel enable bit...
+	 */
 	d->channels = 1;
+	d->enablebits[0].reg = 0;
 
 	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
 	  	u8  pin1 = 0, pin2 = 0;
#2 lore raw

Hello.

Quoted 6 lines · Bartlomiej Zolnierkiewicz
Quoted 4 lines

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

applied

I'm getting "403 Forbidden" trying to browse this patch (and
ide-cs-recognize-2gb-compactflash-from-transcend.patch as well).

MBR, Sergei

#3 lore raw
Quoted 7 lines · Sergei Shtylyov

Hello.

Quoted 2 lines · Bartlomiej Zolnierkiewicz
Quoted 2 lines

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

applied

I'm getting "403 Forbidden" trying to browse this patch (and
ide-cs-recognize-2gb-compactflash-from-transcend.patch as well).

Should be fixed now - sorry for that.

While at it I replaced mine version of "the hpt366: simplify UltraDMA filtering (take 2)" with the official one ;) and fixed my damn scripts to always do "chmod 644 pata-2.6/patches/*.patch" before pushing updates to kernel.org.

Bart

#5 lore raw
In reply to Sergei Shtylyov
Quoted 17 lines · Hello, I wrote:

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

Index: linux-2.6/drivers/ide/pci/hpt366.c
===================================================================

drivers/ide/pci/hpt366.c +1 -1
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/hpt366.c		Version 1.02	Apr 18, 2007
+ * linux/drivers/ide/pci/hpt366.c		Version 1.03	May 4, 2007
  *
  * Copyright (C) 1999-2003		Andre Hedrick <[email protected]>
  * Portions Copyright (C) 2001	        Sun Microsystems, Inc.

drivers/ide/pci/hpt366.c static int __devinit init_setup_hpt366(s · line 1533

Quoted 7 lines
drivers/ide/pci/hpt366.c +4 -0
@@ -1527,7 +1527,12 @@ static int __devinit init_setup_hpt366(s
 	if (rev > 2)
 		goto init_single;
 
+	/*
+	 * HPT36x chips are single channel and
+	 * do not seem to have the channel enable bit...
+	 */

Actually it does, but the bit is different. :-/

drivers/ide/pci/hpt366.c static int __devinit init_setup_hpt366(s · line 1535

Quoted 2 lines
drivers/ide/pci/hpt366.c +1 -0
 	d->channels = 1;
+	d->enablebits[0].reg = 0;

Linas, Andries, Michal, cound you try this instead:

	d->enablebits[0].mask = d->enablebits[0].val = 0x10;

drivers/ide/pci/hpt366.c static int __devinit init_setup_hpt366(s · line 1538

Quoted 2 lines
drivers/ide/pci/hpt366.c +0 -0
 	if ((dev2 = pci_get_slot(dev->bus, dev->devfn + 1)) != NULL) {
 	  	u8  pin1 = 0, pin2 = 0;

MBR, Sergei

#6 lore raw

Hi,

Quoted 8 lines · Sergei Shtylyov

drivers/ide/pci/hpt366.c static int __devinit init_setup_hpt366(s · line 1535

Quoted 7 lines · Hello, I wrote:

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

	d->channels = 1;
drivers/ide/pci/hpt366.c +1 -0
+	d->enablebits[0].reg = 0;

The original patch worked for me.

Quoted 3 lines

Linas, Andries, Michal, cound you try this instead:

	d->enablebits[0].mask = d->enablebits[0].val = 0x10;

Based on the printk's from my system, this should work fine. The config register had 0x33 in it, so 0x33 & mask == val for me. I'll reply tommorrow if this doesn't work.

--linas

#7 lore raw
Quoted 9 lines · Linas Vepstas
Quoted 6 lines

drivers/ide/pci/hpt366.c static int __devinit init_setup_hpt366(s · line 1535

Quoted 6 lines

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

	d->channels = 1;
drivers/ide/pci/hpt366.c +1 -0
+	d->enablebits[0].reg = 0;

The original patch worked for me.

Quoted 7 lines
Quoted 3 lines

Linas, Andries, Michal, cound you try this instead:

	d->enablebits[0].mask = d->enablebits[0].val = 0x10;

Based on the printk's from my system, this should work fine. The config register had 0x33 in it, so 0x33 & mask == val for me. I'll reply tommorrow if this doesn't work.

It probably won't work the way it should anyway -- the secondary channel
(and controller in this case) uses another bit in this register and the controllers get registered with IDE core "in pair". Highpoint knows how to make broken hardware. :-)

Quoted 1 line

--linas

MBR, Sergei

#8 lore raw
Quoted 2 lines · Sergei Shtylyov

knows how to make broken hardware. :-)

You mean, "value-added hardware" ;->

--linas

#9 lore raw
In reply to Sergei Shtylyov
Quoted 11 lines · Hello, I wrote:
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

    d->channels = 1;

+ d->enablebits[0].reg = 0;

The original patch worked for me.

Quoted 2 lines

Linas, Andries, Michal, cound you try this instead:

Quoted 8 lines
Quoted 5 lines
Quoted 1 line
    d->enablebits[0].mask = d->enablebits[0].val = 0x10;

Based on the printk's from my system, this should work fine. The config register had 0x33 in it, so 0x33 & mask == val for me. I'll reply tommorrow if this doesn't work.

It probably won't work the way it should anyway -- the secondary
channel (and controller in this case) uses another bit in this register and the controllers get registered with IDE core "in pair".

Maybe it will though, after reading some more "secret" stuff. :-)

Quoted 1 line

Highpoint knows how to make broken hardware. :-)

It's alos known for lousy documentation. And even that they're not readily
giving out. :-)

Quoted 1 line
Quoted 1 line

--linas

MBR, Sergei

#10 lore raw
Quoted 9 lines · Sergei Shtylyov
Quoted 7 lines · Hello, I wrote:
Quoted 3 lines
Quoted 3 lines

Linas, Andries, Michal, cound you try this instead:

   d->enablebits[0].mask = d->enablebits[0].val = 0x10;

It probably won't work the way it should anyway -- the secondary
channel (and controller in this case) uses another bit in this register and the controllers get registered with IDE core "in pair".

Setting d->enablebits[0].mask = d->enablebits[0].val = 0x10; makes my system bootable, and so this works well enough for me. Without this patch, mainline 2.6.21.1 is broken, and so I'll say it again:

Please submit a patch to the stable branch so that this gets generically fixed! I'll happily Ack it.

--linas

#11 lore raw
Quoted 7 lines · Linas Vepstas
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Linas, Andries, Michal, cound you try this instead:

  d->enablebits[0].mask = d->enablebits[0].val = 0x10;

It probably won't work the way it should anyway -- the secondary
channel (and controller in this case) uses another bit in this register and the controllers get registered with IDE core "in pair".

Quoted 6 lines

Setting d->enablebits[0].mask = d->enablebits[0].val = 0x10; makes my system bootable, and so this works well enough for me. Without this patch, mainline 2.6.21.1 is broken, and so I'll say it again:

Please submit a patch to the stable branch so that this gets generically fixed! I'll happily Ack it.

Already done. I hope it'll be in 2.6.21.3...

Quoted 1 line

--linas

WBR, Sergei

#12 lore raw
In reply to Sergei Shtylyov
Quoted 11 lines · Sergei Shtylyov
Quoted 2 lines

HPT36x chip don't seem to have the channel enable bits, so prevent the IDE core from checking them...

Actually it does, but the bit is different. :-/

drivers/ide/pci/hpt366.c static int __devinit init_setup_hpt366(s · line 1535

Quoted 2 lines
drivers/ide/pci/hpt366.c +1 -0
 	d->channels = 1;
+	d->enablebits[0].reg = 0;

Linas, Andries, Michal, cound you try this instead:

	d->enablebits[0].mask = d->enablebits[0].val = 0x10;

OK, tried, works fine.

Andries