Re: [PATCH] libata: share PIO limits among devices sharing a channel

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Alan wrote:

>>PIO xfermask limits should be shared by all devices on the same
>>channel to avoid violating device selection timing.  libata used to

    This is not a good way to deal with this. Only command block (8-bit) 
timings  should be the same for both drives on channel, data register (16-bit) 
timings may be different.

> NAK, this is totally wrong

>>+	/* PIO xfermask limits are shared by all devices on the same
>>+	 * channel to avoid violating device selection timing.
>>+	 */
>>+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
>>+		struct ata_device *d = &ap->device[i];
>>+		unsigned int pio_mask;
>>+
>>+		if (ata_dev_absent(d))
>>+			continue;
>>+
>>+		ata_unpack_xfermask(ata_id_xfermask(d->id),
>>+				    &pio_mask, NULL, NULL);
>>+		pio_mask &= d->pio_mask;
>>+		xfer_mask &= ata_pack_xfermask(pio_mask, UINT_MAX, UINT_MAX);
>>+	}

> NAK

> This "guarantee" was deliberately removed long ago and is completely
> bogus.

    I'd agree...

> The good ATA chipsets do not suffer from selection timing limits of this
> form. The less smart ones do and the drivers correctly merge the timing
> values, including a whole chunk of functionality in the ata_timing
> interface to get it right when doing DMA modes.

    I need to have a look at this (when I have time :-)...

> Adding this patch is the regression. Even the ancient drivers/ide code
> does this properly.

    Not really, at least not all drivers.  Namely, hpt366.c (still) doesn't 
merge 8-bit timings (maybe this is handled in hardware but the datasheets 
don't tell about it then) -- I need to look at fixing this... Well, it was 
even worse before "the grand rewrite" since even setting UltraDMA modes 
changed PIO timings (both 8- and 16-bit) to match PIO4 -- piix/slc90e66 are 
still doing this kind of crap (I'm going to fix this at last).

MBR, Sergei