alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Mike Frysinger <vapier@gentoo.org>
Cc: Scott Jiang <scott.jiang@analog.com>,
	alsa-devel@alsa-project.org, Cliff Cai <cliff.cai@analog.com>,
	device-drivers-devel@blackfin.uclinux.org,
	Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [Device-drivers-devel] [PATCH] ASoC: ad74111: new codec driver
Date: Sat, 26 Mar 2011 18:09:42 +0000	[thread overview]
Message-ID: <20110326180942.GA13129@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <AANLkTinAs+3g1KHDyxBTDze8MnFZn3X7b5vuQSM+yLsN@mail.gmail.com>

On Sat, Mar 26, 2011 at 01:52:48PM -0400, Mike Frysinger wrote:
> On Sat, Mar 26, 2011 at 08:21, Mark Brown wrote:

> > A few of your patches have this sort of additional change in them.
> > While the cleanup is good it would be better to split it into a separate
> > patch.  Seeing the unrelated change slows down review and it'll also
> > create extra merge issues when applying or cherry picking the driver
> > back to older kernels.

> in general i agree, but for moving literally one line i dont generally
> worry about it

It's a one line patch that totally changes the shape of the diff for
that hunk.  As I said above this slows down review, it's jarring as one
has to stop and reverse engineer from the change if was intentional and
if it is sensible.  It could be some unrelated cleanup, it could be (and
frequently is) context that got included in the diff by mistake.

> > Again, drop the -codec unless this is part of a MFD.

> when i looked at the history of things, it seemed like it's the ASoC
> maintainers sticking "-codec" on stuff.  so should i go through
> existing drivers and revert that back ?

Yes, they were included by mistake as a result of automated changes in
the multi-component merges.

  reply	other threads:[~2011-03-26 18:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-26  8:33 [PATCH] ASoC: ad74111: new codec driver Mike Frysinger
2011-03-26 12:21 ` Mark Brown
2011-03-26 17:52   ` [Device-drivers-devel] " Mike Frysinger
2011-03-26 18:09     ` Mark Brown [this message]
2011-03-27  4:10       ` Mike Frysinger
2011-03-27 13:43         ` Mark Brown
2011-03-28  7:22           ` Mike Frysinger
2011-03-29 21: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=20110326180942.GA13129@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=cliff.cai@analog.com \
    --cc=device-drivers-devel@blackfin.uclinux.org \
    --cc=lrg@slimlogic.co.uk \
    --cc=scott.jiang@analog.com \
    --cc=vapier@gentoo.org \
    /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).