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.