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

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

Bartlomiej Zolnierkiewicz wrote:
> ide-timing.h:

> * remove handling of DMA modes from ide_find_best_mode() and rename it to
>   ide_find_best_pio_mode()

> * drop no longer needed "map" argument from ide_find_best_pio_mode()
>   and delete needless ->id check

> * remove no longer needed XFER_SWDMA and XFER_UDMA* defines

> au1xxx-ide.c:

> * use ide_max_dma_mode() instead of ide_find_best_mode()

> * remove needless CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA #ifdef

> amd74xx.c:
> 
> * store UDMA masks in amd_ide_chip[] and while at it make "base" field
>   to be u8 instead of unsigned long
> 
> * convert the driver to use UDMA masks from amd_ide_chip[]
> 
> * use ide_max_dma_mode() and ide_find_best_pio_mode() instead
>   of ide_find_best_mode()
> 
> * delete stale comment from amd74xx_ide_dma_check()
> 
> * remove no longer needed AMD_UDMA* defines
> 
> via82cxxx.c:

> * remove unused DISPLAY_VIA_TIMINGS define

> * store UDMA masks in via_isa_bridges[] and while at it make "flags" field
>   to be u8 instead of u16

> * convert the driver to use UDMA masks from via_isa_bridges[]

> * use ide_max_dma_mode() and ide_find_best_pio_mode() instead
>   of ide_find_best_mode()

> * remove no longer needed VIA_UDMA* defines

> pmac.c:

> * use ide_max_dma_mode() instead of ide_find_best_mode()

> There should be no functionality changes caused by this patch.

    I'm afraid this hasn't been accomplished yet. ;-)

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

> 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. :-)

>  		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:

  	if (drive->autodma && speed)

>  		return 0;

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

MBR, Sergei