On Monday 07 December 2009 04:05:28 pm Bartlomiej Zolnierkiewicz wrote:
> On Monday 07 December 2009 02:26:00 pm Sergei Shtylyov wrote:
> > Hello.
> >
> > Alan Cox wrote:
> >
> > > Bartlomiej pointed out that while this got fixed in the old driver whoever
> > > did it didn't port it across.
> >
> > > Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
> >
> > [...]
> >
> > > +static int mwdma_clip_to_pio(struct ata_device *adev)
> > > +{
> > > + const int mwdma_to_pio[3] = {
> > > + XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
> > > + };
> > > + return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
> > > + adev->pio_mode - XFER_PIO_0);
> > > +}
> >
> > You call min() on uncomparables, i.e. mwdma_to_pio[] contains XFER_PIO_*
> > and adev->pio_mode - XFER_PIO_0 yields you a mode number. Thus the second
> > argument will always "win" as a minimal one.
>
> There are more issues with the patch related to mwdma_clip_to_pio().
Hmm.. why cannot ->qc_issue be used here (like in all other host drivers with
shared PIO/MWDMA timings) instead of some new 'clipping'?
->qc_issue is the standard way to do it, takes care of more corner cases
and would also allow for faster MWDMA0 timings.
--
Bartlomiej Zolnierkiewicz
--
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
