alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: "Reddy, MR Swami" <MR.Swami.Reddy@nsc.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [PATCH] ASoC: Add National Semiconductor LM49352 Codec Support
Date: Wed, 16 Mar 2011 10:08:13 +0000	[thread overview]
Message-ID: <20110316100353.GC2688@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <290463D19D2E064191F1F96ECA480A89432CDF2B49@EXMAIL02.scwf.nsc.com>

On Wed, Mar 16, 2011 at 12:38:57AM -0700, Reddy, MR Swami wrote:

> Tested this patch on SMDK6410 platform with Kernel- 2.6.24 and ALSA version is - 1.0.15.

As previously mentioned you need to submit code against current Linux
versions.  Linux 2.6.24 is over three years old and development of the
kernel has moved on substantially in that time.

> Please review and let me know the comments/suggestion on this patch.
> And also let me know the forward-porting (to the latest ALSA version APIs) steps/process. Thanks in advance.

As previously mentioned this is pretty much up to you - you can apply
a range of techniques, either from starting from scratch or reviewing
the kernel changelogs for the core and other drivers and making gradual
changes to forward port.  Which approach works best for you is largely a
matter of personal preference.  If you have questions on specific things
then please feel free to ask.

I've had a brief glance at the driver and there are a number of obvious
coding style issues, please before resubmitting review the coding style
you are using and ensure that your driver is following a similar coding
style to the existing kernel code.  For example comments like this:

> +/***************************************************************************
> + * Name:
> + *    lm49352_read_reg_cache- Reads specific register from reg cache.
> + *
> + * Synopsis:
> + *     static inline unsigned int
> + *     lm49352_read_reg_cache(struct snd_soc_codec *codec,unsigned int reg)

are not at all idiomatic for the kernel.

  reply	other threads:[~2011-03-16 10:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-07 12:03 [PATCH] ASoC: Add National Semiconductor LM49352 Audio Codec support Reddy, MR Swami
2011-03-07 13:18 ` Mark Brown
2011-03-08  4:31   ` Reddy, MR Swami
2011-03-16  7:38 ` [PATCH] ASoC: Add National Semiconductor LM49352 Codec Support Reddy, MR Swami
2011-03-16 10:08   ` Mark Brown [this message]
2011-03-16 13:38     ` Reddy, MR Swami
2011-03-16 14:35       ` 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=20110316100353.GC2688@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=MR.Swami.Reddy@nsc.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=lrg@slimlogic.co.uk \
    /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).