Re: [PATCH] ide: convert ide_find_best_mode() users to use ide_max_dma_mode()

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

Hi,

On Tuesday 12 June 2007, Sergei Shtylyov wrote:
> Bartlomiej Zolnierkiewicz wrote:

> > Index: b/drivers/ide/mips/au1xxx-ide.c
> > ===================================================================
> > --- a/drivers/ide/mips/au1xxx-ide.c
> > +++ b/drivers/ide/mips/au1xxx-ide.c
> > @@ -381,9 +381,7 @@ static int auide_dma_setup(ide_drive_t *
> >  
> >  static int auide_dma_check(ide_drive_t *drive)
> >  {
> > -	u8 speed;
> > -
> > -#ifdef CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA
> > +	u8 speed = ide_max_dma_mode(drive);
> >  
> >  	if( dbdma_init_done == 0 ){
> 
>     Ugh, coding style isnn't kosher here. :-)

Ah, indeed I overlooked it.

Now looking at the au1xxx-ide.c I see that there are few other
non-kosher places worth fixing...  Care to send a patch?

> >  		auide_hwif.white_list = ide_in_drive_list(drive->id,
> > @@ -394,7 +392,6 @@ static int auide_dma_check(ide_drive_t *
> >  		auide_ddma_init(&auide_hwif);
> >  		dbdma_init_done = 1;
> >  	}
> > -#endif
> >  
> >  	/* Is the drive in our DMA black list? */
> >  
> > @@ -409,8 +406,6 @@ static int auide_dma_check(ide_drive_t *
> >  	else
> >  		drive->using_dma = 1;
> >  
> > -	speed = ide_find_best_mode(drive, XFER_PIO | XFER_MWDMA);
> > -	
> >  	if (drive->autodma && (speed & XFER_MODE) != XFER_PIO)
> 
>     ide_max_dma_mode() returns 0 if DMA not available, so that if should have 
> looked this way:

XFER_PIO is 0x00 so the patch is correct but

>   	if (drive->autodma && speed)

is of course simpler

I think that it is not worth re-spinning the whole patch just for that
(especially since the code in question is an old one and not introduced
in this patch).  However I'll happilly apply a follow-up fix.

> >  		return 0;
> 
>     But is this really equivalent? Why there's no fallback call to 
> ide_find_best_pio_mode() like in other cases?

Because of the

	if (drive->autodma && (speed & XFER_MODE) != XFER_PIO)

check.

In other places "speed" is always used, here it is only used if it denotes
a DMA mode (XFER_PIO mask was needlessly passed to ide_find_best_mode() call
since PIO modes were always filtered out later).

Thanks,
Bart