Re: [GIT PULL] Greybus driver subsystem for 4.9-rc1

From: Mark Rutland
Date: Wed Sep 21 2016 - 09:02:38 EST


Apologies for the late reply to this; I don't subscribe to LKML with my
work address and didn't spot this sub-thread until now.

On Fri, Sep 16, 2016 at 08:05:19AM +0200, Greg KH wrote:
> On Thu, Sep 15, 2016 at 03:45:53PM +0100, Mark Brown wrote:
> > On Wed, Sep 14, 2016 at 12:09:49PM +0200, Greg KH wrote:
> >
> > > I'll send out a follow-up set of "simple" patches that just add the
> > > files to the kernel tree, to give people an idea of the code involved.
> > > Overall, it's a tiny stand-alone driver subsystem, only 37k lines, that
> > > implements a protocol which allows for "generic" cameras, audio devices,
> > > and other class type devices, as well as a bridged "physical" layer
> > > protocol to talk to serial, spi, uart, pwm, gpio, i2c, and even USB host
> >
> > Just emphasizing what Mark Rutland said complete NACK, in particular the
> > drivers for functions have not been posted upstream at all (and it's
> > concerning that they're all being added under drivers/greybus rather
> > than within the relevant subsystem like we do normally). I've not
> > looked at the code as it has not been submitted but given that and that
> > the reason I found this pull request was an ASoC contributor who had
> > seen it and looked at the code going "oh dear, greybus..." on IRC I'm
> > very concerned.
> >
> > Sending a pull request for code that's never been seen upstream seems
> > completely premature.
> Hey, how does code get upstream then? :)

As Mark Brown said, after review.

> Anyway, as vger seems to be marking all of the greybus patches I send
> out as "spam" according to a filter, it's making it a bit hard to send
> patches out, but I'll try it again "by hand" later today...
> As for the drivers all living under drivers/greybus/ I understand, but
> we need the greybus core present first before we can get the drivers in.
> How about we do what happened with IIO, we take the greybus core code in
> drivers/greybus/ and put the drivers in staging, and then move them out
> of staging into the "real" portion of the kernel as they get reviewed
> and accepted?
> Any objections to that workflow?

Personally, yes.

I would rather see the few core patches go through some level of review
first. It's vastly easier to handle that than to reverse-engineer an
understanding of the code when "move XXX out of staging/" patches

I'm more than happy to review a reasonably-size, linearised series for
that core code.

My concerns previously mentioned still apply to the patches queued in
linux-next, regardless of whether these patches are under staging.
Especially given the ABI implications of the devicetree bindings.