Messages in this thread | | | Date | Wed, 1 Sep 1999 16:41:58 -0700 (PDT) | From | Andre Hedrick <> | Subject | Re: PATCH: IDE PCI init cleanup |
| |
It will be late friday before I can catch up to the rest of you on this issue. I am out of reach and no access to my hardware. If the init process is identical in effect and the feel is different, that is expected. What are the advantages/robustness in the changes? I have an IDE chipset that is really dependent upon getting the device order correct. One mistake will result in hda/b/c/d detection order of the devices, becomes hdc/d/a/b. This is a real bad thing.
Andre Hedrick The Linux IDE guy
On Tue, 31 Aug 1999, Martin Mares wrote:
> Hi Andre, > > When rewriting the PCI subsystem, I've also taken a look at initialization > routines of the IDE drivers and there is a small patch trying to clean them > up a bit (I didn't have a chance to test the patches, so please examine > them carefully). > > Have a nice fortnight > -- > Martin `MJ' Mares <mj@ucw.cz> http://atrey.karlin.mff.cuni.cz/~mj/ > Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth > "A mathematician is a machine for converting coffee into theorems." > > > Changes: > > o Removed enabling of expansion ROMs. Given the principle PCI works, > it cannot make any difference and the sole fact the ROM has assigned > some address doesn't mean at all that the address won't collide with > other devices. > o Changed global cli() to local __cli() in alim15x3.c init. (I think > the cli can be removed at all, but I'm not 100% sure.) > o Replaced setting of PCI_LATENCY_TIMER by a call to pci_set_master(). > o Removed PCI_CACHE_LINE_SIZE setting on __sparc_v9__ as DaveM has > said he'll implement these fixups in his new Sparc64 PCI code. > o hpt34x: cleaned up resource handling. > o Removed ide_special_settings() as it's not used. > o Use pci_dev->name when printing messages. > o pdc202xx: #if 0'd out the INTERRUPT_LINE check. INTERRUPT_LINE > is not used at all (neither in the kernel nor by the hardware), so > there is no need to touch it. > o Base addresses in dev->resource[] don't contain the type bits, > so it isn't needed to mask them out. > > > diff -uN linux-2.3.16-pre1/drivers/block/aec6210.c rsrc/drivers/block/aec6210.c > --- linux-2.3.16-pre1/drivers/block/aec6210.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/aec6210.c Tue Aug 31 10:27:33 1999 > @@ -52,9 +52,5 @@ > > unsigned int __init pci_init_aec6210 (struct pci_dev *dev, const char *name) > { > - if (dev->resource[PCI_ROM_RESOURCE].start) { > - pci_write_config_dword(dev, PCI_ROM_ADDRESS, dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE); > - printk("%s: ROM enabled at 0x%08lx\n", name, dev->resource[PCI_ROM_RESOURCE].start); > - } > return dev->irq; > } > diff -uN linux-2.3.16-pre1/drivers/block/alim15x3.c rsrc/drivers/block/alim15x3.c > --- linux-2.3.16-pre1/drivers/block/alim15x3.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/alim15x3.c Tue Aug 31 10:40:55 1999 > @@ -92,11 +92,11 @@ > if (r_clc >= 16) > r_clc = 0; > } > - save_flags(flags); > - cli(); > + __save_flags(flags); > + __cli(); > pci_write_config_byte(dev, port, s_clc); > pci_write_config_byte(dev, port+drive->select.b.unit+2, (a_clc << 4) | r_clc); > - restore_flags(flags); > + __restore_flags(flags); > > /* > * setup active rec > diff -uN linux-2.3.16-pre1/drivers/block/cmd646.c rsrc/drivers/block/cmd646.c > --- linux-2.3.16-pre1/drivers/block/cmd646.c Sat Aug 14 12:27:53 1999 > +++ rsrc/drivers/block/cmd646.c Tue Aug 31 13:15:33 1999 > @@ -251,12 +251,6 @@ > > hwif->chipset = ide_cmd646; > > - /* Set a good latency timer and cache line size value. */ > - (void) pci_write_config_byte(dev, PCI_LATENCY_TIMER, 64); > -#ifdef __sparc_v9__ > - (void) pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, 0x10); > -#endif > - > /* Setup interrupts. */ > (void) pci_read_config_byte(dev, 0x71, &mrdmode); > mrdmode &= ~(0x30); > diff -uN linux-2.3.16-pre1/drivers/block/hpt34x.c rsrc/drivers/block/hpt34x.c > --- linux-2.3.16-pre1/drivers/block/hpt34x.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/hpt34x.c Tue Aug 31 13:15:41 1999 > @@ -343,37 +343,22 @@ > unsigned int __init pci_init_hpt34x (struct pci_dev *dev, const char *name) > { > unsigned short cmd; > + int i; > + unsigned long hpt34xIoBase = dev->resource[4].start; > > pci_write_config_byte(dev, HPT34X_PCI_INIT_REG, 0x00); > pci_read_config_word(dev, PCI_COMMAND, &cmd); > - if (cmd & PCI_COMMAND_MEMORY) { > - if (dev->resource[PCI_ROM_RESOURCE].start) { > - pci_write_config_byte(dev, PCI_ROM_ADDRESS, dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE); > - printk(KERN_INFO "HPT345: ROM enabled at 0x%08lx\n", dev->resource[PCI_ROM_RESOURCE].start); > - } > - pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0xF0); > - } else { > - int i = 0; > - unsigned long hpt34xIoBase = dev->resource[4].start; > - > - pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_IO); > - dev->resource[0].start = (hpt34xIoBase + 0x20); > - dev->resource[1].start = (hpt34xIoBase + 0x34); > - dev->resource[2].start = (hpt34xIoBase + 0x28); > - dev->resource[3].start = (hpt34xIoBase + 0x3c); > - for(i=0; i<4; i++) > - dev->resource[i].flags |= PCI_BASE_ADDRESS_SPACE_IO; > - /* > - * Since 20-23 can be assigned and are R/W, we correct them. > - */ > - pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, dev->resource[0].start); > - pci_write_config_dword(dev, PCI_BASE_ADDRESS_1, dev->resource[1].start); > - pci_write_config_dword(dev, PCI_BASE_ADDRESS_2, dev->resource[2].start); > - pci_write_config_dword(dev, PCI_BASE_ADDRESS_3, dev->resource[3].start); > - > - pci_write_config_word(dev, PCI_COMMAND, cmd); > - pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x20); > + pci_write_config_word(dev, PCI_COMMAND, cmd & ~PCI_COMMAND_IO); > + for(i=0; i<4; i++) { > + static int hpt34x_offsets[] = { 0x20, 0x34, 0x28, 0x3c }; > + static int hpt34x_sizes[] = { 8, 1, 8, 1 }; > + struct resource *r = &dev->resource[i]; > + r->start = hpt34xIoBase + hpt34x_offsets[i]; > + r->end = r->start + hpt34x_sizes[i] - 1; > + r->flags |= PCI_BASE_ADDRESS_SPACE_IO | IORESOURCE_IO; > + pci_write_config_dword(dev, PCI_BASE_ADDRESS_0 + 4*i, r->start); > } > + pci_write_config_word(dev, PCI_COMMAND, cmd); > return dev->irq; > } > > @@ -386,6 +371,7 @@ > pci_read_config_word(hwif->pci_dev, PCI_COMMAND, &pcicmd); > #ifdef CONFIG_BLK_DEV_HPT34X_DMA > #if 0 > + /* This check looks bogus --mj */ > hwif->autodma = (pcicmd & PCI_COMMAND_MEMORY) ? 1 : 0; > #endif > #endif /* CONFIG_BLK_DEV_HPT34X_DMA */ > diff -uN linux-2.3.16-pre1/drivers/block/hpt366.c rsrc/drivers/block/hpt366.c > --- linux-2.3.16-pre1/drivers/block/hpt366.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/hpt366.c Tue Aug 31 10:22:17 1999 > @@ -466,8 +466,6 @@ > byte ata66 = 0; > > pci_read_config_byte(dev, 0x5a, &ata66); > - if (dev->rom_address) > - pci_write_config_byte(dev, PCI_ROM_ADDRESS, dev->rom_address | PCI_ROM_ADDRESS_ENABLE); > printk("%s: reg5ah=0x%02x ATA-%s Cable Port%d\n", name, ata66, (ata66 & 0x02) ? "33" : "66", PCI_FUNC(dev->devfn)); > return dev->irq; > } > diff -uN linux-2.3.16-pre1/drivers/block/ide-pci.c rsrc/drivers/block/ide-pci.c > --- linux-2.3.16-pre1/drivers/block/ide-pci.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/ide-pci.c Tue Aug 31 13:40:11 1999 > @@ -250,45 +250,6 @@ > > static byte hpt363_shared_irq = 0; > > -/* > - * This allows offboard ide-pci cards the enable a BIOS, verify interrupt > - * settings of split-mirror pci-config space, place chipset into init-mode, > - * and/or preserve an interrupt if the card is not native ide support. > - */ > -static unsigned int __init ide_special_settings (struct pci_dev *dev, const char *name) > -{ > - switch(dev->device) { > - case PCI_DEVICE_ID_TTI_HPT343: > - { > - unsigned short pcicmd = 0; > - > - pci_write_config_byte(dev, 0x80, 0x00); > - pci_read_config_word(dev, PCI_COMMAND, &pcicmd); > - if (!(pcicmd & PCI_COMMAND_MEMORY)) { > - int i; > - unsigned long hpt34xIoBase = dev->resource[4].start; > - > - dev->resource[0].start = (hpt34xIoBase + 0x20); > - dev->resource[1].start = (hpt34xIoBase + 0x34); > - dev->resource[2].start = (hpt34xIoBase + 0x28); > - dev->resource[3].start = (hpt34xIoBase + 0x3c); > - for(i=0; i<4; i++) > - dev->resource[i].flags |= PCI_BASE_ADDRESS_SPACE_IO; > - pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x20); > - } else { > - pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0xF0); > - } > - } > - case PCI_DEVICE_ID_TTI_HPT366: > - case PCI_DEVICE_ID_PROMISE_20246: > - case PCI_DEVICE_ID_PROMISE_20262: > - case PCI_DEVICE_ID_ARTOP_ATP850UF: > - return dev->irq; > - default: > - break; > - } > - return 0; > -} > > /* > * Match a PCI IDE port against an entry in ide_hwifs[], > @@ -446,7 +407,7 @@ > * space, place chipset into init-mode, and/or preserve > * an interrupt if the card is not native ide support. > */ > - pciirq = (d->init_chipset) ? d->init_chipset(dev, d->name) : ide_special_settings(dev, d->name); > + pciirq = d->init_chipset ? d->init_chipset(dev, d->name) : 0; > } else if (tried_config) { > printk("%s: will probe irqs later\n", d->name); > pciirq = 0; > @@ -552,12 +513,8 @@ > /* > * Set up BM-DMA capability (PnP BIOS should have done this) > */ > + pci_set_master(dev); > hwif->autodma = 0; /* default DMA off if we had to configure it here */ > - (void) pci_write_config_word(dev, PCI_COMMAND, pcicmd | PCI_COMMAND_MASTER); > - if (pci_read_config_word(dev, PCI_COMMAND, &pcicmd) || !(pcicmd & PCI_COMMAND_MASTER)) { > - printk("%s: %s error updating PCICMD\n", hwif->name, d->name); > - dma_base = 0; > - } > } > if (dma_base) { > if (d->dma_init) { > @@ -646,10 +603,10 @@ > hpt366_device_order_fixup(dev, d); > else if (!IDE_PCI_DEVID_EQ(d->devid, IDE_PCI_DEVID_NULL) || (dev->class >> 8) == PCI_CLASS_STORAGE_IDE) { > if (IDE_PCI_DEVID_EQ(d->devid, IDE_PCI_DEVID_NULL)) > - printk("%s: unknown IDE controller on PCI bus %02x device %02x, VID=%04x, DID=%04x\n", > - d->name, dev->bus->number, dev->devfn, devid.vid, devid.did); > + printk("%s: unknown IDE controller on %s, VID=%04x, DID=%04x\n", > + d->name, dev->name, devid.vid, devid.did); > else > - printk("%s: IDE controller on PCI bus %02x dev %02x\n", d->name, dev->bus->number, dev->devfn); > + printk("%s: IDE controller on %s\n", d->name, dev->name); > ide_setup_pci_device(dev, d); > } > } > diff -uN linux-2.3.16-pre1/drivers/block/ns87415.c rsrc/drivers/block/ns87415.c > --- linux-2.3.16-pre1/drivers/block/ns87415.c Sat Aug 14 12:27:56 1999 > +++ rsrc/drivers/block/ns87415.c Tue Aug 31 13:15:24 1999 > @@ -112,12 +112,6 @@ > byte stat; > #endif > > - /* Set a good latency timer and cache line size value. */ > - (void) pci_write_config_byte(dev, PCI_LATENCY_TIMER, 64); > -#ifdef __sparc_v9__ > - (void) pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, 0x10); > -#endif > - > /* > * We cannot probe for IRQ: both ports share common IRQ on INTA. > * Also, leave IRQ masked during drive probing, to prevent infinite > diff -uN linux-2.3.16-pre1/drivers/block/pdc202xx.c rsrc/drivers/block/pdc202xx.c > --- linux-2.3.16-pre1/drivers/block/pdc202xx.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/pdc202xx.c Tue Aug 31 10:27:20 1999 > @@ -498,11 +498,7 @@ > byte primary_mode = inb(high_16 + 0x001a); > byte secondary_mode = inb(high_16 + 0x001b); > > - if (dev->resource[PCI_ROM_RESOURCE].start) { > - pci_write_config_dword(dev, PCI_ROM_ADDRESS, dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE); > - printk("%s: ROM enabled at 0x%08lx\n", name, dev->resource[PCI_ROM_RESOURCE].start); > - } > - > +#if 0 /* Should not have any effect. --mj */ > if ((dev->class >> 8) != PCI_CLASS_STORAGE_IDE) { > byte irq = 0, irq2 = 0; > pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq); > @@ -512,6 +508,7 @@ > printk("%s: pci-config space interrupt mirror fixed.\n", name); > } > } > +#endif > > printk("%s: (U)DMA Burst Bit %sABLED " \ > "Primary %s Mode " \ > diff -uN linux-2.3.16-pre1/drivers/block/sis5513.c rsrc/drivers/block/sis5513.c > --- linux-2.3.16-pre1/drivers/block/sis5513.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/sis5513.c Tue Aug 31 13:16:08 1999 > @@ -333,16 +333,13 @@ > unsigned int __init pci_init_sis5513 (struct pci_dev *dev, const char *name) > { > struct pci_dev *host; > - byte latency = 0, reg48h = 0; > + byte reg48h = 0; > > - pci_read_config_byte(dev, PCI_LATENCY_TIMER, &latency); > pci_read_config_byte(dev, 0x48, ®48h); > > for (host = pci_devices; host; host=host->next) { > if (host->vendor == PCI_VENDOR_ID_SI && > host->device == PCI_DEVICE_ID_SI_620) { > - if (latency != 0x10) > - pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x10); > host_dev = host; > printk("%s: Chipset Core ATA-66, SiS620\n", name); > printk("%s: Primary ATA-%s, Secondary ATA-%s Cable Detect\n", > diff -uN linux-2.3.16-pre1/drivers/block/trm290.c rsrc/drivers/block/trm290.c > --- linux-2.3.16-pre1/drivers/block/trm290.c Tue Aug 31 15:31:46 1999 > +++ rsrc/drivers/block/trm290.c Tue Aug 31 10:36:46 1999 > @@ -223,7 +223,7 @@ > cfgbase = dev->resource[4].start; > if ((dev->class & 5) && cfgbase) > { > - hwif->config_data = cfgbase & PCI_BASE_ADDRESS_IO_MASK; > + hwif->config_data = cfgbase; > printk("TRM290: chip config base at 0x%04lx\n", hwif->config_data); > } else { > hwif->config_data = 0x3df0; > > > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.rutgers.edu > Please read the FAQ at http://www.tux.org/lkml/ >
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |