Message-ID: <20170624123951.GA10622@kroah.com>
Date:   Sat, 24 Jun 2017 14:39:51 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Julia Lawall <julia.lawall@...6.fr>,
        Daniel Wagner <wagi@...om.org>,
        David Woodhouse <dwmw2@...radead.org>, rafal@...ecki.pl,
        Arend Van Spriel <arend.vanspriel@...adcom.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "Li, Yi" <yi1.li@...ux.intel.com>, atull@...nsource.altera.com,
        Moritz Fischer <moritz.fischer@...us.com>,
        Petr Mladek <pmladek@...e.com>,
        Johannes Berg <johannes.berg@...el.com>,
        Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
        "Coelho, Luciano" <luciano.coelho@...el.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Andrew Lutomirski <luto@...nel.org>,
        Jiri Kosina <jkosina@...e.cz>,
        Kees Cook <keescook@...omium.org>,
        "AKASHI, Takahiro" <takahiro.akashi@...aro.org>,
        David Howells <dhowells@...hat.com>,
        Peter Jones <pjones@...hat.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Alan Cox <alan@...ux.intel.com>, Theodore Ts'o <tytso@....edu>,
        NeilBrown <neilb@...e.com>, Christoph Hellwig <hch@....de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 1/5] firmware: add extensible driver data params

On Sat, Jun 24, 2017 at 02:48:28AM +0200, Luis R. Rodriguez wrote:
> On Fri, Jun 23, 2017 at 04:09:29PM -0700, Linus Torvalds wrote:
> > On Fri, Jun 23, 2017 at 3:43 PM, Luis R. Rodriguez <mcgrof@...nel.org> wrote:
> > >
> > > Ah, yes! Here is what I believe seems to be the *crux* issue of these patch
> > > series and I'm happy we have finally landed on it. Yes, indeed the new API
> > > proposed here provides more flexibility, and it does so by embracing a
> > > "data driven" API Vs the traditional procedural APIs we have seen for
> > > *the firmware API*.
> > 
> > This has been going on forever. Everybody hates your data-driven one.
> 
> Before you, the only person who had expressed disdain here was Greg.

Very few people actually review code, you know that.

> > Things like that may be ok as an internal implementation, but even
> > there it's questionable if it then means a big disconnect between what
> > people actually use (the normal functional model) and the
> > implementation.
> 
> A vendor tree implemented their *own* solution and were willing to maintain
> it despite this likely making it hard to port stable fixes. That I think says
> a lot for a need...

What vendor tree?  Where was it shipped?  Why was it external and how is
it different from your patches?  Was it used because your version has
taken so long to be submitted/reviwed?

> There are still other requirements and features in the pipeline for which we
> can consider parameters to parse for, rather than adding new API. Case in
> point, do we want *one* API just to disable the firmware cache? Specially
> knowing that another feature in the pipeline later would make use of this as a
> requirement?

Again, I do not care!  You can not justify patches today with some
mythical thing in the future that might never even happen.

Again, as it stands, this patch series is unacceptable, and the added
complexity of a crazy api that goes against almost all normal in-kernel
apis, is only one part of the reason.  The other being the loads of
added code for no apparent benifit at all.

So please both fix the api to be "normal", and show as to why these
patches are actually needed _today_, otherwise we can just live with
what we have now just fine and muddle along like always.

thanks,

greg k-h

Powered by blists - more mailing lists