Hi Alan!
On Sunday 10 June 2007, Alan Cox wrote:
> > @@ -258,8 +259,14 @@ static void it821x_passthru_set_piomode(
> > static const u8 pio_want[] = { ATA_66, ATA_66, ATA_66, ATA_66, ATA_ANY };
> >
> > struct it821x_dev *itdev = ap->private_data;
> > + struct ata_device *pair = ata_dev_pair(adev);
> > int unit = adev->devno;
> > - int mode_wanted = adev->pio_mode - XFER_PIO_0;
> > + int mode_wanted = adev->pio_mode;
> > +
> > + if (pair && adev->pio_mode > pair->pio_mode)
> > + mode_wanted = pair->pio_mode;
> > +
> > + mode_wanted -= XFER_PIO_0;
>
> NAK this too
OK, I'm able to understand the meaning of "NAK" [1] but "this too"?
[1] http://ozlabs.org/~rusty/index.cgi/2007/05/04
;)
> Please see it821x_program.
>
> it821x_passthru_set_piomode computes the required mode and the preferred
> clock for the mode
> it821x_clock_strategy the computes the best clock to use
> it821x_program the programs the device
>
> Whenever we issue a command it821x_passthru_dev_select then does a timing
> mode switch and loads the PIO timing before doing the dev_select. The
> same is also done when doing a qc_issue_prot so each qc_issue will have
> the right timings loaded for the device.
>
> So if you've got a problem case your diagnosis isn't one that makes a lot
> of sense, and the fix is clearly wrong but I've not seen most of the
> discussion between you and whoever you worked with to judge why.
>
> One obvious possibility is that we are not currently doing timing
> clipping for the address setup and merge for the 8bit timings. However
> the docs I have strongly imply that the IT821x PIO/MWDMA timing register
> is for data cycles only.
>
> The other would be that there is some kind of pattern of interaction
> between the ATA midlayer and the driver causing the wrong timings to get
> loaded at some point.
>
> Your "fix" would happen to work for both of these cases, but is wrong for
> both of them.
>
> If the register is switching all the timings then the fix is to switch
> the PIO timing loaded to the lowest of the two initially (in _program)
> and flip it back to the correct value at the start of the data transfer
> sequence. We can then flip it back next qc_issue or indeed at the end of
> the data xfer
>
> If we've got an interaction problem then we need proper traces with
> LIBATA debug turned right up so we can fix the actual case where the
> switching occurs wrongly.
PIO fix was directly ported from my it821x.c patch. I now see that thanks
to it821x_passthru_dev_select() it is not really needeed in pata_it821x.c.
OTOH DMA fixes are pata_it821x.c unique, I will respin the patch.
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
