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
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to