Bartlomiej Zolnierkiewicz wrote:
Good, that's what I lacked for hpt366.c! Were you planning to push it to Linus soon?
Not really but if needed I will extract MWDMA filter part and push it sooner.
Erm, may I just merge it to my patch (mentioning you of course)?
* Add ->mdma_filter to ide_hwif_t and use it in ide_get_mode_mask().
Hm, why not mwdma_filter()? That "mdma" word has unneeded connotation. ;-)
Ha! As predicted:
->mdma_filter name would make people more ecstatic about the code
;)
Nice one. 8-)
* Remove needless setting of drive->using_dma from auide_dma_check().
* Split off auide_mdma_filter() from auide_dma_check().
* Use ide_tune_dma() in auide_dma_check(), this fixes following issues: - device's DMA capability bit not being checked - device not being checked against generic DMA blacklist - transfer mode not being set on device/host
* Add PIO autotune fallback to auide_dma_check().
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Index: b/drivers/ide/mips/au1xxx-ide.c =================================================================== --- a/drivers/ide/mips/au1xxx-ide.c +++ b/drivers/ide/mips/au1xxx-ide.c @@ -351,11 +351,18 @@ static int auide_dma_setup(ide_drive_t * return 0; } -static int auide_dma_check(ide_drive_t *drive) +static u8 auide_mdma_filter(ide_drive_t *drive) { - u8 speed = ide_max_dma_mode(drive); + /* + * FIXME: ->white_list and ->black_list are based on completely bogus + * ->ide_dma_check implementation which didn't set neither the host + * controller timings nor the device for the desired transfer mode. + * + * They should be either removed or 0x00 MWDMA mask should be + * returned for devices on the ->black_list. + */
I don't get it -- why then introduce a method that does nothing?
It does something as you've noticed yourself:
Yeah, I saw that it does something that shouldn't be done there. :-) The more is the reason to move that method into my recent hpt366 filter patch.
- if( dbdma_init_done == 0 ){ + if (dbdma_init_done == 0) {
I wonder what this code is doing here at all...
auide_hwif.white_list = ide_in_drive_list(drive->id, dma_white_list); auide_hwif.black_list = ide_in_drive_list(drive->id,
Why the results of the drive list lockup gets tied to auide_hwif? :-O
Yet I was sure I'd typed "lookup". Probably a freudian slip. :-)
To use results in auide_ddma_init()... (which needs fixing of course).
The more interesting questions are: WTF is "safe MWDMA" mode (tsize == 1, devwidth == 16) and whether ->white/black_list is really needed.
No time to look into the manuals right now...
I planned to cc: AU1XXX platform maintainers on this patch but to my surprise MAINTAINERS lacks AU1XXX entry.
It's been solt out to Raza Microelectronics last year and those guys never sent a single patch to linux-mips...
Bart
MBR, Sergei - 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
