linux-kernel - Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130406182957.GA11630@roeck-us.net>
Date:	Sat, 6 Apr 2013 11:30:17 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	Wolfram Sang <wsa@...-dreams.de>, Simon Glass <sjg@...omium.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Daniel Kurtz <djkurtz@...omium.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

On Fri, Apr 05, 2013 at 02:03:52PM -0600, Stephen Warren wrote:
> On 04/05/2013 01:37 PM, Simon Glass wrote:
> > HI Wolfram,
> > 
> > On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang <wsa@...-dreams.de> wrote:
> >> Doug,
> >>
> >>> Separately from a discussion of the technical merits, I'd say that
> >>> this patch is needed because the Embedded Controller (EC) on the ARM
> >>> Chromebook shipped expecting to communicate with this scheme.  While
> >>
> >> Uhrm, with all respect, "we already shipped it" is not a convincing
> >> argument regarding inclusion. Benefit for the kernel is.
> 
> I'm not quite sure why that isn't a convincing argument.
> 
> The hardware has shipped. I don't know whether the EC microcode can be
> updated in the field; it seems risky to do so even if it's possible. So,
> it either gets supported or not; the HW/ucode isn't going to change I
> suspect.
> 
> Hence, it seems that the decision would be:
> 
> a) Disallow the implementation of the arbitration scheme in the kernel,
> and hence don't support this board in the kernel. (or at least some very
> core features of this board)
> 
> b) Allow the implementation of the arbitration scheme in the kernel, and
> hence make possible support this board in the kernel.
> 
> From that perspective, the benefit for the kernel question comes down
> to: do we see a benefit for the kernel to support this board? I can't
> see why that wouldn't be a benefit.
> 
> The only disadvantage would be having to carrying code to support that
> board. That same argument can be made for any board, and I think
> typically doesn't cause any issue. The code for this I2C mux seems
> pretty self-contained, so even if it was absolutely terrible (which I
> don't think it is), it still wouldn't cause any wide-spread issues, I think.

Very interesting discussion, especially the argument that "we already shipped"
would not be a convincing argument.

I had senior kernel maintainers tell me and the company I work for that we should
submit _all_ our platform specific kernel code and drivers for inclusion into
the upstream kernel.

This exchange suggests that "it is a shipping product" does not count for such
submissions, and that "Benefit for the kernel" would be the deciding factor
instead. Which of course is a very vague statement - if code supporting
Chromebookis is of no benefit for the kernel, support for my company's products
for sure is much less so.

Which kind of leaves me in a bind. On one side I promote that we should submit
all our kernel code, on the other side there is a very compelling case to be
made that it won't be accepted anyway. That doesn't make my life easier -
essentially since it supports those who say that we should not submit anything
in the first place. And believe me, there are many of those. 

Just to give some examples:
- I2C multiplexers. We have a bunch of those. Looking at this exchange,
  it doesn't look good to get that code included.
- Custom multi-function FPGAs and CPLDs, amongst others implementing I2C
  controllers, I2C muxes, GPIO access, Flash access, and other functions. Same
  as above.
- Devicetree support for UIO devices (mostly forwarding ASICs), including gpio
  bindings, interrupt bindings, and clock bindings. Looking at older exchanges,
  that doesn't look good either. And please dont expect me to implement hacks
  around a clean solution because any devicetree binding for UIO drivers
  "does not describe hardware but its use".

Now, I can understand that there may be technical or architectural issues
preventing one driver or another from being accepted. For example, I can
understand if a driver for an USB-I2C adapder isn't accepted because the adapter
reports itself to the USB subsystem as serial driver. But "Benefit for the
kernel" is vague enough to reject anything for no real reason other than
someone not liking it (or the submitter, or the company the submitter
works for, or the hardware architecture).

It would be nice have to get some well defined guidelines for "acceptable"
contributions. "Benefit for the kernel" sure isn't one.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/