Re: [PATCH v2 0/9] Add DSP topology management for SKL

From: Mark Brown <broonie@kernel.org>
To: Vinod Koul <vinod.koul@intel.com>
Cc: liam.r.girdwood@linux.intel.com, patches.audio@intel.com,
	alsa-devel@alsa-project.org
Subject: Re: [PATCH v2 0/9] Add DSP topology management for SKL
Date: Sat, 19 Sep 2015 09:56:12 -0700	[thread overview]
Message-ID: <20150919165612.GA32072@sirena.org.uk> (raw)
In-Reply-To: <20150911114500.GK12027@sirena.org.uk>

[-- Attachment #1.1: Type: text/plain, Size: 1742 bytes --]

On Fri, Sep 11, 2015 at 12:45:00PM +0100, Mark Brown wrote:
> On Thu, Sep 03, 2015 at 01:44:54PM +0530, Vinod Koul wrote:

> > I was wondering if you have any feedback on this series ?

> Please don't send content free pings (as I'm sure I've mentioned
> before).  I've not looked at this yet because it's another large,
> invasive patch series which you sent at the very end of the development
> cycle.

So, I've now looked at these, though not fully.  To expand on the above
a bit: these are big bits of very device specific code that's really
hard to read and understand and when I do read it I'm seeing similar
issues again and again both in terms of big picture stuff and the
trivial details (including the inconsistencies in things like commenting
style that I'm not specifically pulling up).  I'm really worried that
I'm often seeing fragments of things that may or may not work well
together.

These Intel devices not only have a lot more code than most devices do,
they also get a lot more exposure than most devices do to end users so I
actually have to try to make some effort to understand what's going on
and can't skimp on review even though I feel like I am.  I'm really
concerned about where we're heading here and how robustly the results
are going to work.

This all really slows down review, both directly in requiring detailed
checks of hard to follow the code and as a result of making almost all
other code review seem much more productive.

As well as the low level code quality issues it would really help if
rather than continually sending large blocks of functionality we were
starting from something that was a minimally viable but functional
system and then extending that.  That should be a lot easier to follow.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]