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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
