All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: davinci-linux-open-source@linux.davincidsp.com,
	clark.becker@ridgerun.com, santiago.nunez@ridgerun.com,
	diego.dompe@ridgerun.com, alsa-devel@alsa-project.org,
	nsnehaprabha@ti.com, todd.fischer@ridgerun.com
Subject: Re: [PATCH 2/2] Davinci: DM365: Enable DaVinci Voice Codec support for DM365 EVM
Date: Mon, 11 Jan 2010 09:11:19 -0600	[thread overview]
Message-ID: <4B4B3F97.8060508@ridgerun.com> (raw)
In-Reply-To: <20100108193720.GA3088@sirena.org.uk>

Mark Brown wrote:
> On Fri, Jan 08, 2010 at 11:36:59AM -0600, Miguel Aguilar wrote:
>> Mark Brown wrote:
>>> On Thu, Jan 07, 2010 at 04:17:21PM -0600, miguel.aguilar@ridgerun.com wrote:
> 
>>>> +		if (device == 0)
>>>> +			davinci_cfg_reg(DM365_EVT2_ASP_TX);
>>>> +		else
>>>> +			davinci_cfg_reg(DM365_EVT2_VC_TX);
> 
>>> I'd be a bit more comfortable with this if it were using something more
>>> symbolic like a #define or enum rather than checking a bare number to
>>> work out which device it's talking to.
> 
>> The idea of these function is check at runtime if the user space application is 
>> requesting the AIC3x or the voice codec, then it will set the proper source for 
>> the dma channels, since the ASP and the Voice Codec share the same dma channels, 
>> so that's why use a #define doesn't make sense.
> 
> I see what your code is doing but at the minute it's making this
> decision based on the device number that's being passed in by comparing
> it as a pure number.  This seems fragile - something symbolic that
> joined things up a bit more wouldn't raise eyebrows in the same way.
> 
>> Can you check the part of this patch related to registering both codecs AIC3x 
>> and the voice codec?
> 
> Like I say that all looks fine to me but I can't really check if the
> DaVinci code is idiomatic.

Is there any way to tell alsamixer to use the controls of one subdevice or the 
other?

  reply	other threads:[~2010-01-11 15:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-07 22:17 [PATCH 2/2] Davinci: DM365: Enable DaVinci Voice Codec support for DM365 EVM miguel.aguilar
2010-01-08 11:31 ` Mark Brown
2010-01-08 17:36   ` Miguel Aguilar
2010-01-08 19:37     ` Mark Brown
2010-01-11 15:11       ` Miguel Aguilar [this message]
2010-01-11 15:34         ` Mark Brown
2010-01-11 15:53           ` Miguel Aguilar
2010-01-11 16:21             ` Mark Brown
2010-01-19 20:47               ` Miguel Aguilar
2010-01-19 22:10                 ` Mark Brown
2010-01-20 14:32                   ` Miguel Aguilar
2010-01-20 15:01                     ` Mark Brown
2010-01-11 16:04 ` Kevin Hilman
2010-01-11 16:08   ` Miguel Aguilar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B4B3F97.8060508@ridgerun.com \
    --to=miguel.aguilar@ridgerun.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=clark.becker@ridgerun.com \
    --cc=davinci-linux-open-source@linux.davincidsp.com \
    --cc=diego.dompe@ridgerun.com \
    --cc=nsnehaprabha@ti.com \
    --cc=santiago.nunez@ridgerun.com \
    --cc=todd.fischer@ridgerun.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.