[ added Akira & Kou to cc: ] On Tuesday 08 April 2008, Sergei Shtylyov wrote: > Hi, I just wrote: > > >>> This function didn't actually check if a given BAR is in I/O space > >>> because of > >>> using the bogus PCI_BASE_ADDRESS_IO_MASK (which equals ~3) to test > >>> the resource > >>> flags instead of IORESOURCE_IO -- fix this, make ide_hwif_configure() > >>> check the > >>> results failing if necessary, and move the printk() call to the > >>> failure path. > > >> This change is OK in itself but I worry that ide_pci_check_iomem() may > >> now > >> return "false" errors (bogus PCI_BASE_ADDRESS_IO_MASK check resulted > >> in MEM > >> resources always surviving ide_pci_check_iomem() calls before the fix) > >> for > >> some host drivers (siimage, scc_pata...) resulting in failed > >> initialization. > > > The SiI chips do have normal I/O resources at BAR0..BAR3. As for > > scc_pata, the control should not even get there because BAR0..BAR3 are > > *not* IDE command/control block bases on this chip (BAR0/1 are > > control/DMA bases if you look into setup_mmio_scc()) but they are > > treated as such by the code immediately following ide_pci_check_iomem() > > calls in ide_hwif_configure(), i.e. we might have an error here. The > > same can be said about the PowerMAC driver which has all its MMIO > > registers at BAR0. > > >> How's about removing this dead/broken function instead for now? > > > If we indeed have a MMIO problem here, it's not in this function but > > in its callers. > > Looks like we actually have this problem with scc_pata -- it calls > ide_setup_pci_device() which should lead to calling ide_hwif_configure(). But > this is broken since this call chain expects a normal PCI IDE controller with > BAR0..BAR3 either non-existant or being primary/secondary port bases in I/O space. Yep, scc_pata needs fixing before your patch can be applied. Looks like it just needs to do all the needed setup itself in init_setup_scc() instead of calling ide_setup_pci_device() [ similarly to how cs5520 host driver handles this in cs5520.c::cs5520_init_one() ]. PS it would be also nice to call pci_enable_device() before setup_mmio_scc() (which accesses PCI BARs, calls pci_set_master() etc.) while we are at it (driver seems to work fine without it ATM but it may break in future). Thanks, Bart -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
