Re: SoundScape driver... FIXMEs

[Takashi Iwai <tiwai@suse.de>]

At Mon, 23 Sep 2002 13:28:56 +0100 (BST),
Chris Rankin wrote:
> 
> > i don't see any particular part that restricts to
> > module (except that
> > the driver needs firmware to be loaded via ioctl).
> > am i missing something?
> 
> Only that it's OBSCENELY more difficult to develop a
> driver if you compile it into the kernel every time...
> Unless you ENJOY rebooting your machine, of course. 

yes, but it should not be the reason to stop compiling the built-in
driver via ifdef.  why not just put a warning something like "this
driver is not tested as built-in", if you are not sure?

> > > - I was not aware that "verify_area()" had been
> > > deprecated.
> 
> > verify_area() is simply not necessary
> > if you use copy_from/to_user().
> 
> Aha, but that's not the same as "deprecated" at all
> ;-).
 
no.  it's still alive even on 2.5 tree.

> I only do one big "verify_area()" up front to simplify
> the DMA-uploading loop. This way, I know before I even
> begin that *entire* address range is valid, and so I
> don't need to worry about __copy_from_user() being
> passed an invalid address.
> 
> There is only one verify_area() call, but several
> __copy_from_user() calls because the firmware is 64K
> long, but I upload it in contiguous chunks.
> 
> > generally it's recommended to check the return value
> > from
> > copy_from/to_user() at each call and to return
> > -EFAULT, because it can be faster.
> 
> Note that copy_from_user() effectively calls
> verify_area() and so may return -EFAULT. However, I
> call __copy_from_user() instead, having already
> checked the entire address range. In other words, I
> perform 1 call to verify_area() and N calls to
> __copy_from_user(), rather than the equivalent of N
> calls to verify_area() and N calls to
> __copy_from_user(). Also, I don't need to keep on
> checking for -EFAULT
>  inside the loop.

yeah, i know.  that's why i wrote "generally", above. 
i don't think putting these checks into one verify_area() improves the
performance so much.  checking the return value of copy_from/to_user()
is more obvious, i think.  anyway, it's a matter of taste.

> > another point:  doesn't the busy-loop in
> > host_read/write_ctrl_unsafe
> > need udelay() or something to produce a certain
> > delay length?
> > otherwise the timeout is very dependent on a
> > machine.
> 
> Yes, possibly. Provided udelay() isn't deprecated and
> doesn't schedule or anything.

no and no.  it's a simply busy delay for the given time length
with _relatively_ good accuracy (up to 1ms).

> I tested this driver on
> a P120 - not exactly a speed daemon... ;-)!

ah, then the current driver will fail definitely on 2GHz P4 ;)


ciao,

Takashi


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf