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

On 30-04-19, 08:38, 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. 

Thats fine, it is upto people, everyone has different views, mine is
different from Takashi's. We all know for example networking has
different stable and code style rule. That is how it is and I dont think
we would have one rule for all kernel.

All I ask is to be able to review and split up accordingly, I guess that
is a fair request

> 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. 

Is it? you got feedback on 15th (that too after my 2 week conf/vacation
break) and I got called crazy for that, not helping!!


> It's not just me the patches related to
> runtime-pm from your own Linaro colleagues posted on March 28 went nowhere
> either.

Does it matter it was a Linaro colleague or not, a patch was posted,
feedback given (similar to cadence one) we agreed that the fix
is not correct and so patch was not applied. I don't think Srini cried
over it!

> Moving forward, I suggest we merge SoundWire-related patches through the
> sound tree. There will be dependencies in the coming weeks between SOF and
> SoundWire and it makes no sense to have separate maintainers and make the
> life of early adopters more complicated than it needs to be. If we have
> 3-week delays for trivial stuff, I can't imagine what the pace will be when
> I publish the next 20-odd patches I am still working on, and the code needed
> for the SoundWire audio device class being standardized as we speak. Things
> were fine up to now since no one was actually using the code, we are in a
> different model now.

I disagree and wont accept it. I dont think you understand that you are
not the most important person in the whole world, the 20 patches series
you are cooking would sure be greatest ever, but that is not the point.
The kernel has a process, you got a feedback, please fix that and post
v2 rather than cribbing, complaining and calling crazy. The energy would
have been better spent on fixing the feedback provided.

Dependencies are _always_ there in kernel development and we know how to
deal with it. Am sure Takashi, Mark and me can come to reasonable
agreement, I wouldn't worry about that!

What we dont do is create new model for your 20 patches.

And I guess I dont have anything more to say on this thread, so I wont
bother replying, please feel free to post v2 and I shall review.

-- 
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  parent reply	other threads:[~2019-04-30 14:54 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
2019-04-30 14:13             ` Pierre-Louis Bossart
2019-04-30 14:25               ` Greg KH
2019-04-30 14:54           ` Vfi [this message]
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=20190430145444.GU3845@vkoul-mobl.Dlink \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jank@cadence.com \
    --cc=joe@perches.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sanyog.r.kale@intel.com \
    --cc=srinivas.kandagatla@linaro.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 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.