linux-mips
[Top] [All Lists]

Re: [alsa-devel] [RFC] SGI O2 MACE audio ALSA module

To: TJ <tj.trevelyan@gmail.com>
Subject: Re: [alsa-devel] [RFC] SGI O2 MACE audio ALSA module
From: Takashi Iwai <tiwai@suse.de>
Date: Tue, 03 Jul 2007 12:20:05 +0200
Cc: "Linux MIPS List" <linux-mips@linux-mips.org>, ALSA Dev List <alsa-devel@alsa-project.org>
In-reply-to: <6849c8890707020427q47704326od05ebb8241c3cf@mail.gmail.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <6849c8890707020427q47704326od05ebb8241c3cf@mail.gmail.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Wanderlust/2.15.5 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (Sanjō) APEL/10.6 MULE XEmacs/21.5 (beta27) (fiddleheads) (+CVS-20060704) (i386-suse-linux)
At Mon, 2 Jul 2007 12:27:58 +0100,
TJ wrote:
> 
> Hi,
> 
> It's been a while since I last posted something. Here is the latest
> version of my code. It is somewhat of a mess at the moment, but I plan
> to tidy it all up when it works better, any // comments are not
> permanent features. I have been using trial and error to find why it
> play too fast.
> 
> I am glad to say that it does play sound correctly, bar one little
> issue that has me stuck at the moment, it plays too fast (for any
> sample frequency). Also how fast appears to depend on the player
> (tried aplay and ogg123).
> 
> I have cc the ALSA dev too in the hope that between the two lists
> someone may spot something. Please can anyone who replies cc me.
> 
> The patch was built against (applies to) linux-2.6.19.7 from
> linux-mips.org. It'll probably work on other versions.

The patch includes old typdefed structs which were already removed
from the upstream, such as, snd_card_t.  Please replace them
appropriately.  You can find the replacement in
include/sound/typedefs.h (in the old kernel tree).
Similarly, I'd recommend to avoid typedefs in your own code, too.
People don't like it :)

Other things I noticed through a quick glance:

- Follow the standard coding style, e.g. 80 chars in a line, don't put
  if-block in a single line, etc.
- Avoid non-ASCII letters, especially outside the comments
- You don't need *_irqsave() in get/put callbacks of the control API.
  It's always schedulable, so, spin_lock_irq() suffices.
- ad1843_lock could be better implemented with mutex if you have long
  delays inside the spinlock (except for the calls from irq handler)
- please remove uneeded debug printks.  If they are useful, keep it in
  another macro form.

Could you fix these and repost?

thanks,

Takashi

<Prev in Thread] Current Thread [Next in Thread>