Skip to contentSkip to conversation

ata_bmdma_* calls in libata-core.c

DISCUSSION Mat Loikkanen 2 participants 2 messages Feb 2, 2005 1 message not in the archive libata

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Mat Loikkanen

author
#1 lore raw

Jeff-

My driver (and our controller) actually does do DMA, using an off-controller DMAC (the ARM PL080 on the Versatile board -- our controller talks the DMA protocol over the AMBA/AHB bus). I have successfully implemented ->qc_prep(), ->bmdma_setup(), and ->bmdma_start(). These functions setup scatter-gather lists, configure the DMAC and kick off the DMAC respectively. All works. That said, I do run into ata_bmdma_status() and ata_bmdma_stop() in ata_qc_timeout().

I suggest all BMDMA calls become callbacks that the driver can set to its own functions or assign to wrapper functions implemented in libata-core.c that call the inline functions in libata.h. Should I do this?

-mat

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Wednesday, February 02, 2005 11:25 AM
To: [email protected]
Subject: Re: ata_bmdma_* calls in libata-core.c

Quoted 5 lines · Mat Loikkanen

Jeff-

Could we somehow eliminate the direct calls to ata_bmdma_*() functions in libata-core.c? Specifically, what's getting in my way currently are the calls within ata_qc_timeout(), called by ata_eng_timeout(). I can't use

the

Quoted 2 lines

Libata callback ata_eng_timeout() since I'm not BMDMA. What would be the correct fix: Make these BMDMA calls no-ops? Have the functions check

that

Quoted 6 lines

ap->ioaddr.bmdma_addr doesn't equal some bad value (0xcdcdcdcd)? Or make all these ata_bmdma_stop/ack_irq/status() overridable callbacks like bmdma_setup/start()? The last option may be the most flexible, in case a non-BMDMA driver (like mine) would like to do something relevant in their place. I will change the code and send a patch to you if you tell me what your preference is.

Quite honestly, libata error handling is very basic, and could stand to be improved.

On the "other side" of the transaction, the command submission side of things, libata is design so that it provides a standard IDE BMDMA interface. If drivers do not have a purely BMDMA interface (AHCI is a good example here), the driver overrides library functions with its own.

However, for non-DMA controllers (your situation), there does not appear to be a big problem: ata_eng_timeout() and ata_qc_timeout() correctly check qc->tf.protocol before calling BMDMA functions, except for one case:

                 /* ack bmdma irq events */
                 ata_bmdma_ack_irq(ap);

This is called in the PIO/non-data paths. This is correct for most controllers, who want an IRQ ack on the BMDMA register even though its not a DMA command... but in your case, this is incorrect.

So,

1) Do you see problems other than the call to ata_bmdma_ack_irq() ?

2) Feel free to suggest a patch which works around this. Maybe an ATA_FLAG_BMDMA_ACK_IRQ for those controllers that must call ata_bmdma_ack_irq() on non-DMA paths.

Jeff

#2 lore raw
Quoted 3 lines · Mat Loikkanen

I suggest all BMDMA calls become callbacks that the driver can set to its own functions or assign to wrapper functions implemented in libata-core.c that call the inline functions in libata.h. Should I do this?

Yeah, that works for me.

Jeff