Re: + fix-ide-dma-resource-managment.patch added to -mm tree

[Sergei Shtylylov <sshtylyov@ru.mvista.com>]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> On 3/27/06, Sergei Shtylylov <sshtylyov@ru.mvista.com> wrote:

>>>>>>>Resolve several IDE DMA resource management issues:

>>>>>>>- release DMA engine for memory mapped DMA as well

>>>>>>NAK - this won't work, see siimage.c how these resources were
>>>>>>really reserved in the first place [ request_mem_region() ]

>>>>   NB: it's reserved this way only when available -- this driver can work
>>>>with both I/O and memory mapped regs...

>>>>>  Oops, looks like I missed that missing ide_setup_dma() call in that
>>>>>driver...

>>>>   No, I've just forgotten that it's called implicitly from setup-pci.c since
>>>>init_dma() "method" in not defined in that driver. So, looks like you're wrong
>>>>here -- it should work currently BUT the DMA engine is not released since
>>>>(mmio == 2) check and return precedes call to ide_release_dma_engine() in
>>>>ide_release_dma().

>>>please take a look at siimage.c:init_iops_siimage():

>>>      if (pci_get_drvdata(dev) == NULL)
>>>              return;

>>    So what? It just leaves the default I/O mapped iops intact and the driver
>>init. continues...

> ->mmio stays 0 and the driver continues in the standard I/O mode...

    I don't really see the difference in regard to our case. Am I missing 
something?

>>>      init_mmio_iops_siimage(hwif);

>>>and siimage.c:init_mmio_iops_siimage():

>>>      hwif->mmio                      = 2;

>>>so ->mmio == 2 IFF MMIO is used

>>    Yes, but ide_setup_dma() is called in _both_ cases with the current IDE
>>core. And therefore, ide_release_dma() should've undone what ide_setup_dma()

> and it does it for ->mmio == 0 case

    But not for mmio == 2. Why?

>>did... As I understand now, both the current behavior and what I'm proposing
>>are wrong.

    Not really, at least in this part. :-)

> yes (->mmio == 2 case)

    We've just concluded that allocating and freeing the DMA engine for that 
case is still perfectly valid, so where was I wrong?

[skipped]

> Bartlomiej

WBR, Sergei