alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
Cc: alsa-devel@alsa-project.org, patches@opensource.wolfsonmicro.com,
	Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [PATCH 2/4] ASoC: soc-cache: Add support for standard register caching
Date: Fri, 22 Oct 2010 09:03:51 -0700	[thread overview]
Message-ID: <20101022160351.GE16521@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <1287757702-9573-3-git-send-email-dp@opensource.wolfsonmicro.com>

On Fri, Oct 22, 2010 at 03:28:20PM +0100, Dimitris Papastamos wrote:

> +static int snd_soc_cache_default_sync(struct snd_soc_codec *codec)
> +{
> +       const u8 *cache;
> +       struct snd_soc_codec_driver *codec_drv;
> +       unsigned int val;
> 
> +	codec_drv = codec->driver;
> +	for (n = 0; n < codec_drv->reg_cache_size; ++n) {

Please use i as an array index unless using something meaningful.

> +                       if (!memcmp(&val, cache, codec_drv->reg_word_size))
> +                               continue;

This memcmp() looks very suspicious - we're copying from an unsigned int
into a variable of another type.  That seems to have a bit of an
endianness assumption, doesn't it?  It certainly needs comments
explaining how it works; a similar thing applies to the other memcpy()
and memcmp() operations in the code.

> +static int snd_soc_cache_default_deinit(struct snd_soc_codec *codec)
> +{

_exit or _free would be traditional.

> +	kfree(codec->reg_cache);
> +	return 0;
> +};

Extra ; (I'm surprised nothing warns).

  reply	other threads:[~2010-10-22 16:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-22 14:28 [PATCH 0/4] ASoC: Implement new caching API Dimitris Papastamos
2010-10-22 14:28 ` [PATCH 1/4] ASoC: soc.h: Add new caching API prototypes and hooks Dimitris Papastamos
2010-10-22 15:44   ` Mark Brown
2010-10-22 14:28 ` [PATCH 2/4] ASoC: soc-cache: Add support for standard register caching Dimitris Papastamos
2010-10-22 16:03   ` Mark Brown [this message]
2010-10-23 18:24     ` Dimitris Papastamos
2010-10-24 13:18       ` Timur Tabi
2010-10-24 21:35         ` Mark Brown
2010-10-25  8:09           ` Dimitris Papastamos
2010-10-22 14:28 ` [PATCH 3/4] ASoC: soc-core: Adapt soc-core to fit the new caching API Dimitris Papastamos
2010-10-22 14:28 ` [PATCH 4/4] ASoC: soc-cache: Add support for LZO based register caching Dimitris Papastamos
2010-10-22 16:18   ` Mark Brown
2010-10-23 18:28     ` Dimitris Papastamos
2010-10-25 18:13       ` Mark Brown

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=20101022160351.GE16521@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=dp@opensource.wolfsonmicro.com \
    --cc=lrg@slimlogic.co.uk \
    --cc=patches@opensource.wolfsonmicro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).