On Sat, Sep 12, 2009 at 9:49 AM, Robert Hancock <hancockrwd@xxxxxxxxx> wrote:
> 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?
The magic value part must go. I'll update the patch.
>
>> + 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?
No programmatic way. libata.force should be used for other configurations.
Thanks,
-John
--
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
