[Qemu-devel] [RFC] Floppy controller: break relation with PC-style DMA

[Qemu-devel] [RFC] Floppy controller: break relation with PC-style DMA

[Hervé Poussineau]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

Hi,

Currently, floppy disk controller is tied to PC-style DMA controller.
Attached patch is an attempt to break the link between those, by 
introducing function pointers for dma operations (read, write, hold, 
release).

This will allow me to fix fdc DMA handling on MIPS Magnum/Acer Pica 61, 
where fdc is connected to rc4030 DMA controller.

Please comment.

Hervé

[-- Attachment #2: floppy_dma.diff --]
[-- Type: plain/text, Size: 13827 bytes --]

Re: [Qemu-devel] [RFC] Floppy controller: break relation with PC-style DMA

[Blue Swirl]

On 11/22/08, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Hi,
>
>  Currently, floppy disk controller is tied to PC-style DMA controller.
>  Attached patch is an attempt to break the link between those, by
> introducing function pointers for dma operations (read, write, hold,
> release).
>
>  This will allow me to fix fdc DMA handling on MIPS Magnum/Acer Pica 61,
> where fdc is connected to rc4030 DMA controller.

Isn't it possible to move the i8257 DMA helpers to for example dma.c?
Then I could eliminate some dummy DMA_xx functions from sun4m.c and I
think fdctrl_i8257_init would not be needed.

You are calling fdctrl->dma_memory_write and _read without checking
that they are not NULL.

Re: [Qemu-devel] [RFC] Floppy controller: break relation with PC-style DMA

[Hervé Poussineau]

Blue Swirl a écrit :
> On 11/22/08, Hervé Poussineau <hpoussin@reactos.org> wrote:
>> Hi,
>>
>>  Currently, floppy disk controller is tied to PC-style DMA controller.
>>  Attached patch is an attempt to break the link between those, by
>> introducing function pointers for dma operations (read, write, hold,
>> release).
> 
> Isn't it possible to move the i8257 DMA helpers to for example dma.c?
> Then I could eliminate some dummy DMA_xx functions from sun4m.c and I
> think fdctrl_i8257_init would not be needed.

That's a little bit hard for 3 reasons:
a) i8257_dma_hold() checks if DMA is well programmed before doing the 
transfer. I suppose the check should be moved to DMA engine, or at least 
to the _read and _write handlers.
b) There is also the problem at initialization, where each i8257 client 
calls DMA_register_channel() to register itself, and gives a callback 
function (fdctrl_transfer_handler for fdc.c).
c) This callback will then be called to feed destination DMA buffer part 
by part (see dma_pos and dma_len in fdctrl_transfer_handler), instead of 
in one big transfer. That also explains the data_pos parameter when 
calling DMA_read_memory/DMA_write_memory

Yet, I've no good idea to solve problem b) and c).

I suppose we really need a generic DMA framework, as it seems each DMA 
controller is using its own scheme.
For me, a first step in this direction is to break links between devices 
and DMA engines, like it has already be done for esp adapter. Then, 
something good can emerge.
You should also notice that I chose _read and _write callbacks for fdc 
to be exactly the same as in esp.

> You are calling fdctrl->dma_memory_write and _read without checking
> that they are not NULL.

Ok, I'll add checks for fdctrl->dma_memory_write and _read.
In all cases, one without the other should be an error at 
initialization, and then, DMA should be avoided/prevented if they are 
not defined.

Hervé

Re: [Qemu-devel] [RFC] Floppy controller: break relation with PC-style DMA

[Blue Swirl]

On 11/22/08, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Blue Swirl a écrit :
>
> > On 11/22/08, Hervé Poussineau <hpoussin@reactos.org> wrote:
> >
> > > Hi,
> > >
> > >  Currently, floppy disk controller is tied to PC-style DMA controller.
> > >  Attached patch is an attempt to break the link between those, by
> > > introducing function pointers for dma operations (read, write, hold,
> > > release).
> > >
> >
> > Isn't it possible to move the i8257 DMA helpers to for example dma.c?
> > Then I could eliminate some dummy DMA_xx functions from sun4m.c and I
> > think fdctrl_i8257_init would not be needed.
> >
>
>  That's a little bit hard for 3 reasons:
>  a) i8257_dma_hold() checks if DMA is well programmed before doing the
> transfer. I suppose the check should be moved to DMA engine, or at least to
> the _read and _write handlers.

The DMA engine should be a better place.

>  b) There is also the problem at initialization, where each i8257 client
> calls DMA_register_channel() to register itself, and gives a callback
> function (fdctrl_transfer_handler for fdc.c).

How about pushing registering of the callback to board level, for
example introduce a function for getting the transfer callback.

>  c) This callback will then be called to feed destination DMA buffer part by
> part (see dma_pos and dma_len in fdctrl_transfer_handler), instead of in one
> big transfer. That also explains the data_pos parameter when calling
> DMA_read_memory/DMA_write_memory

There is something fishy with the interface, real devices using DMA
have no idea about DMA position and they can't give DMA controller any
instructions.

>  Yet, I've no good idea to solve problem b) and c).
>
>  I suppose we really need a generic DMA framework, as it seems each DMA
> controller is using its own scheme.
>  For me, a first step in this direction is to break links between devices
> and DMA engines, like it has already be done for esp adapter. Then,
> something good can emerge.
>  You should also notice that I chose _read and _write callbacks for fdc to
> be exactly the same as in esp.

Great! Maybe I should revive some of my generic DMA patches.