Date: Thu, 21 Aug 1997 23:49:58 -0700 From: Amancio Hasty <hasty@rah.star-gate.com> To: Luigi Rizzo <luigi@labinfo.iet.unipi.it> Cc: multimedia@FreeBSD.ORG Subject: Re: [snd] For your review: isa.c Message-ID: <199708220649.XAA01467@rah.star-gate.com> In-Reply-To: Your message of "Fri, 22 Aug 1997 07:19:57 %2B0200." <199708220519.HAA15088@labinfo.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
The warning about isa.c not supporting bounce buffers for auto mode is fine and may serve a reminder for someone to clean up the isa's dma interface. If auto dma calls are made for references higher than 16mb then the system should panic. Given that isa_dmastart is of type void and that we are the only ones so far using auto dma it should not be a problem cause we are responsible for allocating memory for the sound driver. The isa interface should be revised to be able to return error codes however I am not going to do it nor is recommended that we do it for the sound driver release thats a function of the "core group" because it involves changing drivers and is not easy to get the freebsd hackers to agree on anything. Probably the later is the most overriding factor. So please just clean up isa_dmastatus & isa_dmastop and the rest of your suggestions I will be happy to do them. The last thing is to get it check in and I hope that they don't put up a strong fight. Tnks, Amancio >From The Desk Of Luigi Rizzo : > About the old vs. new isa.c: > > my original isa_dmadone, although it works in most cases, is very buggy: > * it cannot handle situations where the counter changes while > reading the dma registers; > * it uses splhigh() whereas it should use disable_intr() to block > interrupts; > * it uselessly reads the data pointer register. > > As a temporary fix for supporting auto mode, I second amancio's > approach of using the variable dma_auto_mode to mark the state of the > channel when using auto mode. Probably it could be a good idea to > check, upon the call to isa_dmastart, that the address passed to the > channel is compatible with the operating mode (i.e. is below 16M when > auto mode is used) and return an error otherwise. Since the test (a > call to isa_dmarangecheck() ) is already there, one could just add a > test within that conditional block (and then either return an error > message, or panic). > > I also would like that the code had a big warning on the fact that > auto-mode does not work with bounce buffers. E.g. something like > > /* > * For use a dma channel in normal mode, you should call > * the following routines: > * > * isa_dma_acquire() first, to register the use of the channel; > * isa_dmastart()/isa_dmadone on each transfer. Both calls > * are necessary to let the bounce buffer mechanism work. > * isa_dma_release() to unregister the use of the channel. > * > * For AUTO mode, you need to call isa_dmastart() only once, and > * isa_dmadone() when operation is complete. WARNING: bounce > * buffers are not supported in auto mode. > * > */ > > Cheers > Luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199708220649.XAA01467>