All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	tiwai@suse.de, broonie@kernel.org,
	liam.r.girdwood@linux.intel.com, jank@cadence.com,
	joe@perches.com, srinivas.kandagatla@linaro.org,
	Sanyog Kale <sanyog.r.kale@intel.com>
Subject: Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues
Date: Tue, 30 Apr 2019 16:05:26 +0200	[thread overview]
Message-ID: <20190430140526.GB18986@kroah.com> (raw)
In-Reply-To: <9866ac8c-103d-22cd-a639-a71c39a685c2@linux.intel.com>

On Tue, Apr 30, 2019 at 08:38:01AM -0500, Pierre-Louis Bossart wrote:
> On 4/30/19 3:51 AM, Vinod Koul wrote:
> > On 15-04-19, 08:09, Pierre-Louis Bossart wrote:
> > > 
> > > > > 
> > > > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > > > ---
> > > > >    drivers/soundwire/Kconfig          |   2 +-
> > > > >    drivers/soundwire/bus.c            |  87 ++++++++--------
> > > > >    drivers/soundwire/bus.h            |  16 +--
> > > > >    drivers/soundwire/bus_type.c       |   4 +-
> > > > >    drivers/soundwire/cadence_master.c |  87 ++++++++--------
> > > > >    drivers/soundwire/cadence_master.h |  22 ++--
> > > > >    drivers/soundwire/intel.c          |  87 ++++++++--------
> > > > >    drivers/soundwire/intel.h          |   4 +-
> > > > >    drivers/soundwire/intel_init.c     |  12 +--
> > > > >    drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
> > > > >    drivers/soundwire/slave.c          |  10 +-
> > > > >    drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
> > > > 
> > > > I would prefer this to be a patch per module. It doesnt help to have a
> > > > single patch for all the files!
> > > > 
> > > > It would be great to have cleanup done per logical group, for example
> > > > typos in a patch, aligns in another etc...
> > > 
> > > You've got to be kidding. I've never seen people ask for this sort of
> > > detail.
> > 
> > Nope this is the way it should be. A patch is patch and which
> > should do one thing! Even if it is a cleanup one.
> > 
> > I dislike a patch which touches everything, core, modules, so please
> > split up. As a said in review it takes guesswork to find why a change
> > was done, was it whitespace fix, indentation or not, so please split up
> > based on type of fixes.
> 
> With all due respect, you are not helping here but rather slowing things
> down. I've done dozens of cleanups in the ALSA tree and I didn't go in this
> sort of details. The fact that the series was tagged as Reviewed by Takashi
> on April 11 and we are still discussing trivial changes tells me the
> integration model is broken. It's not just me, the patches related to
> runtime-pm from your own Linaro colleagues posted on March 28 went nowhere
> either.

My patch-bot would reject a patch that tried to do multiple types of
different cleanups on the same file(s).  Has done so for _years_, this
is not a new thing.

Remember, maintainer/reviewer time is scarce, engineer time is prolific,
we optimize for reviewers, not the people writing the patches.

thanks,

greg k-h

  reply	other threads:[~2019-04-30 14:05 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11  3:16 [PATCH v3 0/5] soundwire: code cleanup Pierre-Louis Bossart
2019-04-11  3:16 ` [PATCH v3 1/5] soundwire: intel: fix inversion in devm_kcalloc parameters Pierre-Louis Bossart
2019-04-11  8:43   ` Takashi Iwai
2019-04-11  3:16 ` [PATCH v3 2/5] soundwire: fix style issues Pierre-Louis Bossart
2019-04-14  9:58   ` Vinod Koul
2019-04-15 13:09     ` [alsa-devel] " Pierre-Louis Bossart
2019-04-17  9:33       ` Johan Hovold
2019-04-17 17:18         ` Pierre-Louis Bossart
2019-04-18 17:29           ` Johan Hovold
2019-04-19 17:14       ` Pierre-Louis Bossart
2019-04-19 17:14         ` Pierre-Louis Bossart
2019-04-30  8:57         ` [alsa-devel] " Vinod Koul
2019-04-30  8:51       ` Vinod Koul
2019-04-30 13:38         ` Pierre-Louis Bossart
2019-04-30 14:05           ` Greg KH [this message]
2019-04-30 14:13             ` Pierre-Louis Bossart
2019-04-30 14:25               ` Greg KH
2019-04-30 14:54           ` Vfi
2019-04-30 16:29             ` Pierre-Louis Bossart
2019-04-11  3:16 ` [PATCH v3 3/5] soundwire: bus: remove useless initializations Pierre-Louis Bossart
2019-04-11  3:17 ` [PATCH v3 4/5] soundwire: stream: remove useless initialization of local variable Pierre-Louis Bossart
2019-04-11  3:17 ` [PATCH v3 5/5] soundwire: add missing newlines in dynamic debug logs Pierre-Louis Bossart
2019-04-11  8:43 ` [PATCH v3 0/5] soundwire: code cleanup Takashi Iwai
2019-04-14 10:04 ` Vinod Koul
2019-04-15 12:57   ` [alsa-devel] " Pierre-Louis Bossart
2019-04-19 17:07     ` Pierre-Louis Bossart
2019-04-19 17:07       ` Pierre-Louis Bossart

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=20190430140526.GB18986@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=jank@cadence.com \
    --cc=joe@perches.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=sanyog.r.kale@intel.com \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=tiwai@suse.de \
    --cc=vkoul@kernel.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 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.