On Sun, May 05 2002, Martin Dalecki wrote:
> - Split up the TCQ UDMA handling stuff in to proper functions. Jens must has
> been dreaming as he introduced them ;-).
This was pending stuff, and why I wrote that the start command tcq = 1
stuff was a gross hack.
> - int tcq = 0;
>
> if (!drive->using_dma)
> return ide_started;
>
> /* for dma commands we don't set the handler */
> - if (args->taskfile.command == WIN_WRITEDMA || args->taskfile.command == WIN_WRITEDMA_EXT)
> + if (args->taskfile.command == WIN_WRITEDMA
> + || args->taskfile.command == WIN_WRITEDMA_EXT)
> dma_act = ide_dma_write;
> - else if (args->taskfile.command == WIN_READDMA || args->taskfile.command == WIN_READDMA_EXT)
> + else if (args->taskfile.command == WIN_READDMA
> + || args->taskfile.command == WIN_READDMA_EXT)
> dma_act = ide_dma_read;
> - else if (args->taskfile.command == WIN_WRITEDMA_QUEUED || args->taskfile.command == WIN_WRITEDMA_QUEUED_EXT) {
> - tcq = 1;
> - dma_act = ide_dma_write_queued;
> - } else if (args->taskfile.command == WIN_READDMA_QUEUED || args->taskfile.command == WIN_READDMA_QUEUED_EXT) {
> - tcq = 1;
> - dma_act = ide_dma_read_queued;
> - } else {
> +#ifdef CONFIG_BLK_DEV_IDE_TCQ
> + else if (args->taskfile.command == WIN_WRITEDMA_QUEUED
> + || args->taskfile.command == WIN_WRITEDMA_QUEUED_EXT
> + || args->taskfile.command == WIN_READDMA_QUEUED
> + || args->taskfile.command == WIN_READDMA_QUEUED_EXT)
> + return udma_tcq_taskfile(drive, rq);
> +#endif
> + else {
> printk("ata_taskfile: unknown command %x\n", args->taskfile.command);
> return ide_stopped;
> }
>
> - /*
> - * FIXME: this is a gross hack, need to unify tcq dma proc and
> - * regular dma proc -- basically split stuff that needs to act
> - * on a request from things like ide_dma_check etc.
> - */
> - if (tcq)
> - return drive->channel->udma(dma_act, drive, rq);
> - else {
> - if (drive->channel->udma(dma_act, drive, rq))
> - return ide_stopped;
> - }
> +
> + if (drive->channel->udma(dma_act, drive, rq))
> + return ide_stopped;
This is still ugly, IMHO. What I wanted was to split the udma->
function into two parts, one that acts on a request (ide_dma_read,
ide_dma_being, etc -- and then also ide_dma_read_queued) ad one that
does the silly stuff like ide_dma_check etc. Then unify the tcq and
non-tcq stuff so that udma_rw->(dma_act, drive, rq) always returns
ide_started or ide_stopped (or ide_released) and kill the #ifdef above.
I don't think udma_tcq_taskfile() should be public like this, and btw I
think the name really SUCKS! :-). ata_tcq_dma() is much better for
instance, even though it should be a private stratey. BTW, this goes for
all your tcq.c renaming.
-- Jens Axboe- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue May 07 2002 - 22:00:24 EST