Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

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@linux.intel.com>
> > 
> > [...]
> > 
> > > +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