Re: [RFC] Echoaudio driver

[Takashi Iwai <tiwai@suse.de>]

At Thu, 23 Sep 2004 10:22:31 +0200 (CEST),
Giuliano Pochini wrote:
> 
> > I'd suggest either to ask Echo about the permission for BSD/GPL dual
> > license or to rewrite the BSD part.  Above all, the generic wrapper
> > is hated by kernel developers, so we'll need to rewrite them later
> > anyway.  So, I'd prefer rewriting them.
> 
> The overall structure of the driver is about the same of the
> au88x0 driver (I took it as model).

Well, I don't like the current implementation of au88x0 drivers, too
;)  As a result, you got bunch of ifdefs in the code.

In the case of au88x0, it was hard to merge because they use different
addresses in each chip, while echo boards seem to have the same
register offsets, right?
But I don't insist on this rewrite if you prefer keeping this, of
course.

BTW, let's kill SWAP32/SWAP16 macro.  They are always le16_to_cpu()
and le32_to_cpu() regardless of endianness.

Also, checking of sizeof(dma_addr_t) is not correct.  You should check
that the value itself is in 32bit, not the variable type.


>  Some functions are indeed
> too ugly (eg. the enumerated controls) and I already planned to
> change them. But I don't like the idea of including lowlevel
> functions inside the control and pcm functions because they
> would become overly big and ugly.

At least, all functions and variables should have the same style.
The combination of HowLooksThis and how_looks_that is ugly, IMO.


>  If you're talking about the
> *eg thing, well, it's not a problem at all moving the EchoGals
> structure inside chip_t and passing it to the lowlevel
> functions in place of &eg just like au88x0 does. That would
> also allow some cleanup and maybe tighter locking.

Having a common struct as a super calss is ok.


> >> - Userspace firmware loading is still missing.
> >
> > The problem is similar, the license of this firmware is unclear.
> > Let's push it to user-space.  You can replace it easily with
> > request_firmware() (although it's for 2.6 kernels only).
> 
> I don't thing the licence is a big problem: the people at
> Echoaudio are very friendly. I'll contact them.

That's really appreciated.  To be noted, it's not recommended to
include such binary data into the kernel tree, especially if it's
avoidable easily.  (I see you already started coding
request_firmware().)


Takashi


-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php