On 09/12/2009 04:59 AM, Jung-Ik (John) Lee wrote: (snip) Looks mostly reasonable to me, other than a few issues:
+static void atp867x_set_piomode(struct ata_port *ap, struct ata_device *adev) +{ + struct ata_device *peer = ata_dev_pair(adev); + struct atp867x_priv *dp = ap->private_data; + u8 speed = adev->pio_mode; + struct ata_timing t, p; + int T, UT; + u8 b; + + T = 1000000000 / 33333; + UT = T/4; + + switch (speed) { + case XFER_PIO_4: + case XFER_PIO_3: + case XFER_PIO_2: + case XFER_PIO_1: + case XFER_PIO_0: + case XFER_PIO_SLOW: + break; + default: + printk(KERN_WARNING "ATP867X: Unsupported speed %#x." + " Default to XFER_PIO_0.\n", (unsigned)speed); + speed = XFER_PIO_0; + } + + ata_timing_compute(adev, speed,&t, T, UT); + if (peer&& peer->pio_mode) { + ata_timing_compute(peer, peer->pio_mode,&p, T, UT); + ata_timing_merge(&p,&t,&t, ATA_TIMING_8BIT); + } + + b = inb(dp->dma_mode); + if (adev->devno& 1) + b = (b& ~ATP867X_IO_DMAMODE_SLAVE_MASK); + else + b = (b& ~ATP867X_IO_DMAMODE_MSTR_MASK); + outb(b, dp->dma_mode); + +#ifdef ATP867X_NO_HACK_PIOMODE + b = atp867x_get_active_clocks_shifted(t.active) | + atp867x_get_recover_clocks_shifted(t.recover); +#else + /* + * magic value that works (from doc 6.4, 6.6.9) + */ + b = 0x31; +#endif
What's the purpose of this ifdef?
+ if (dp->pci66mhz) + b += 0x10; + + if (adev->devno& 1) + outb(b, dp->slave_piospd); + else + outb(b, dp->mstr_piospd); + + /* + * use the same value for comand timing as for PIO timimg + */ + outb(b, dp->eightb_piospd); +} + +static int atp867x_cable_detect(struct ata_port *ap) +{ + return ATA_CBL_PATA40_SHORT; +}
Doesn't the controller have a way to do proper 80-wire cable detection? -- 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