From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Taylor Hutt <thutt@chromium.org>
Cc: Liam Girdwood <lrg@slimlogic.co.uk>,
Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.de>,
Peter Hsiang <Peter.Hsiang@maxim-ic.com>,
Olof Johansson <olofj@chromium.org>,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ASoC: codecs: Max98095: Fix logging of hardware revision.
Date: Tue, 21 Jun 2011 01:23:39 +0100 [thread overview]
Message-ID: <20110621002322.GE1905@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <1308596072-17158-1-git-send-email-thutt@chromium.org>
On Mon, Jun 20, 2011 at 11:54:32AM -0700, Taylor Hutt wrote:
> The base hardware revision of the Maxim 98095 part is 0x40; the code
> which outputs the revision of the hardware has been updated to
> properly use uppercase alphabetic values for the revision numbers.
Are you sure that this is true for all devices that might be supported
by the driver (I'm guessing there may be other variants)? There's often
a drift between silicon and package revisions which gets papered over by
datasheets and ignored by drivers.
> Also, the use of a constant for the length 'max98095_dai' has been
> replaced with ARRAY_SIZE().
Don't include a series of random unrelated changes in a single patch,
split them up into separate patches. This makes review much easier if
nothing else. There's no overlap at all between this change and the one
above. The change is sensible.
> ret = snd_soc_read(codec, M98095_0FF_REV_ID);
> if (ret < 0) {
> - dev_err(codec->dev, "Failed to read device revision: %d\n",
> + dev_err(codec->dev, "Failure reading hardware revision: %d\n",
> ret);
You've also got this again unrelated change which isn't mentioned in the
changelog at all.
next prev parent reply other threads:[~2011-06-21 0:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-20 18:54 [PATCH] ASoC: codecs: Max98095: Fix logging of hardware revision Taylor Hutt
2011-06-20 20:06 ` Liam Girdwood
2011-06-21 0:23 ` Mark Brown [this message]
2011-06-21 0:43 ` Peter Hsiang
2011-06-21 1:10 ` Mark Brown
2011-06-21 11:55 ` Mark Brown
[not found] ` <BANLkTinfL8uSdCvrQYPJb1TQH3HSCu=wAPFZv-AcWRw1_jspXg@mail.gmail.com>
2011-06-21 17:10 ` Mark Brown
2011-06-21 20:16 ` Peter Hsiang
2011-06-21 20:48 ` Peter Hsiang
[not found] ` <BANLkTikH6XpGNamh8z4zspxteH3K4mNmew@mail.gmail.com>
2011-06-21 10:06 ` Mark Brown
2011-06-21 23:28 ` 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=20110621002322.GE1905@opensource.wolfsonmicro.com \
--to=broonie@opensource.wolfsonmicro.com \
--cc=Peter.Hsiang@maxim-ic.com \
--cc=alsa-devel@alsa-project.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lrg@slimlogic.co.uk \
--cc=olofj@chromium.org \
--cc=perex@perex.cz \
--cc=thutt@chromium.org \
--cc=tiwai@suse.de \
/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