Linux ATA/IDE development
 help / color / mirror / Atom feed
From: Grant Grundler <grundler@parisc-linux.org>
To: parisc-linux@lists.parisc-linux.org
Cc: linux-ide@vger.kernel.org
Subject: NS87415 on C3K broken
Date: Thu, 16 Sep 2004 10:17:49 -0600	[thread overview]
Message-ID: <20040916161749.GA17660@colo.lackof.org> (raw)

Hi,
Willy pointed out the 2.6.9-rc2 NS87415 driver for C3000 machine
(uses SuckyIO IDE) is causing HPMC (system crash). This was not
a problem with 2.6.8.1.
But while trying to fix MMIO resource management problems, I ran
into this when testing on C3000.

The HPMC stack trace looks like this:
	ide_inb
	ide_wait_not_busy
	wait_hwif_ready
	probe_hwif
	probe_hwif_init
	ide_setup_pci_device
	ns87415_init_one
	ide_scan_pcidev
	ide_scan_pcibus
	ide_init
	do_initcalls

Changes to ide-probe.c cause this HPMC.
wait_hwif_ready() is inside #ifdef CONFIG_PPC in 2.6.8.1 and the
ifdef/endif was removed in 2.6.9-rcX. The result is inb() is invoked
*BEFORE* ide_setup_pci_device() (indirectly) calls pci_enabled_device() or
pci_set_master(). This is broken because IDE controllers NOT enabled
by firmware will simply not respond to inb() (even if the system
doesn't crash like on parisc).

I'm a wimp. I don't dare to restructure IDE init sequence.
Anyone less of a wimp?
Or are IDE pci drivers required to call pci_enable_device()
in their (mostly non-extistent) init_chipset entry point?


Anyway, ns87415 driver has more problems. The patch below adds
"init_chipset" entry point and init_chipset_ns87415() calls
pci_enable_device() and pci_set_master() before the probe.
And my C3k still HPMCs. My guess is more of the code from
init_hwif_ns87415() needs to be moved to init_chipset_ns87415().
And possible call some special suckyio init routines.
I won't be able to touch this for a few days.
Anyone else on parisc-linux ml want to take a whack at this?

BTW, normally a PCI Master Abort to IO Port space to should NOT fail.
But on parisc is does because the chipset will go "fatal" on
any Master Abort - special hacks enable config space to work.
I strongly prefere not to add those hacks to IO Port space
accessor functions since it's normally not a problem.
And while inconvenient, this behavior does expose programming
problems like the one above.


thanks,
grant


Index: drivers/ide/pci/ns87415.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/ide/pci/ns87415.c,v
retrieving revision 1.17
diff -u -p -r1.17 ns87415.c
--- drivers/ide/pci/ns87415.c	13 Sep 2004 15:23:04 -0000	1.17
+++ drivers/ide/pci/ns87415.c	16 Sep 2004 15:06:06 -0000
@@ -133,8 +133,29 @@ static int ns87415_ide_dma_check (ide_dr
 	return __ide_dma_check(drive);
 }
 
+static unsigned int __devinit init_chipset_ns87415(struct pci_dev *dev, const char *name)
+{
+	unsigned short w;
+printk("init_chipset_ns87415(%s,%s)\n", dev->slot_name, name);
+
+	pci_enable_device(dev);
+	pci_set_master(dev);
+
+(void) pci_read_config_word(dev, PCI_COMMAND, &w);
+printk("	cmd 0x%x\n", w);
+
+	if ((w & PCI_COMMAND_IO) == 0) {
+		w |= PCI_COMMAND_IO;
+		(void) pci_write_config_word(dev, PCI_COMMAND, w);
+printk("	enabling IO space\n");
+	}
+	return 0;
+}
+
+
 static void __init init_iops_ns87415(ide_hwif_t *hwif)
 {
+printk("init_iops_ns87415(%s)\n", hwif->pci_dev->slot_name);
 #ifdef CONFIG_SUPERIO
 	superio_ide_init_iops(hwif);
 #endif
@@ -150,12 +171,14 @@ static void __init init_hwif_ns87415 (id
 	u8 stat;
 #endif
 
+printk("init_hwif_ns87415(%s)\n", hwif->pci_dev->slot_name);
 	hwif->autodma = 0;
 	hwif->selectproc = &ns87415_selectproc;
 
+#if 0
 	/* Set a good latency timer and cache line size value. */
 	(void) pci_write_config_byte(dev, PCI_LATENCY_TIMER, 64);
-	/* FIXME: use pci_set_master() to ensure good latency timer value */
+#endif
 
 	/*
 	 * We cannot probe for IRQ: both ports share common IRQ on INTA.
@@ -227,6 +250,8 @@ static void __init init_hwif_ns87415 (id
 
 static ide_pci_device_t ns87415_chipset __devinitdata = {
 	.name		= "NS87415",
+	.init_chipset	= init_chipset_ns87415,
+	.init_iops	= init_iops_ns87415,
 	.init_hwif	= init_hwif_ns87415,
 	.channels	= 2,
 	.autodma	= AUTODMA,

             reply	other threads:[~2004-09-16 16:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-16 16:17 Grant Grundler [this message]
2004-09-16 22:50 ` NS87415 on C3K broken Bartlomiej Zolnierkiewicz
2004-09-16 23:25   ` Grant Grundler
2004-09-17  0:43     ` Bartlomiej Zolnierkiewicz
2004-09-22 10:41       ` a fix for " Joel Soete
2004-09-22 16:37         ` Grant Grundler
2004-09-22 16:53           ` [parisc-linux] " Matthew Wilcox
2004-09-22 17:21           ` Joel Soete
2004-09-22 17:41             ` [parisc-linux] " Matthew Wilcox
2004-09-23 16:21             ` Joel Soete
2004-09-23 16:27               ` Randolph Chung
2004-09-24  7:04                 ` Joel Soete
2004-09-24 16:17                   ` Joel Soete

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=20040916161749.GA17660@colo.lackof.org \
    --to=grundler@parisc-linux.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=parisc-linux@lists.parisc-linux.org \
    /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