Re: [PATCH] hdparm_X.patch (was: Re: 2.6.4-rc-bk3: hdparm -X locks up IDE)

[Denis Vlasenko]

> > This patch survived cyclic switching of all DMA modes from mwdma0 to
> > udma4 and continuous write load of type
> > while(1) { dd if=/dev/zero of=file bs=1M count=<RAM size>; sync; }
> > for five minutes.
> >
> > It does not handle crippled/simplex chipset, it is a TODO.
> >
> > Things commented by C++ style comments (//) aren't meant to stay.
> >
> > I think original code was a bit buggy:
> > set_transfer() returned 0 for modes < XFER_SW_DMA_0, ie,
> > for all PIO modes. Later,
> >        if (set_transfer(drive, &tfargs)) {
> >                xfer_rate = args[1];
> >                if (ide_ata66_check(drive, &tfargs))
> >                        goto abort;
> >         }
> >         err = ide_wait_cmd(drive, args[0], args[1], args[2], args[3],
> > argbuf); if (!err && xfer_rate) {
> >                ide_set_xfer_rate(drive, xfer_rate);
> >                ide_driveid_update(drive);
> >        }
> > This will never execute ide_set_xfer_rate() for PIO modes
>
> It will also never execute ide_set_xfer_rate() for DMA on PIO only drive.
> I must check if "no PIO" thing is a bug or a design decision.

I think IDE driver will never ask for DMA on PIO only drive

Bartlomiej, do you have comments on my patch? So far, I myself spotted
some minor problems:

I forgot to remove now-extraneous if() from ide.c:

static int set_xfer_rate (ide_drive_t *drive, int arg)
{
        int err = ide_wait_cmd(drive,
                        WIN_SETFEATURES, (u8) arg,
                        SETFEATURES_XFER, 0, NULL);

        if (!err && arg) {
                ide_set_xfer_rate(drive, (u8) arg);
                ide_driveid_update(drive);
        }
        return err;
}

And second, in ide_do_drive_cmd(), mdelay(2000)
after WIN_SETFEATURES is a bit rude.
-- 
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/