Date: Tue, 13 Jun 2017 17:50:41 +0200
From: Greg KH <gregkh@linuxfoundation.org>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>, wagi@monom.org,
        dwmw2@infradead.org, arend.vanspriel@broadcom.com, rjw@rjwysocki.net,
        yi1.li@linux.intel.com, atull@opensource.altera.com,
        moritz.fischer@ettus.com, pmladek@suse.com, johannes.berg@intel.com,
        emmanuel.grumbach@intel.com, luciano.coelho@intel.com,
        kvalo@codeaurora.org, luto@kernel.org, torvalds@linux-foundation.org,
        keescook@chromium.org, takahiro.akashi@linaro.org, dhowells@redhat.com,
        pjones@redhat.com, hdegoede@redhat.com, alan@linux.intel.com,
        tytso@mit.edu, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 1/5] firmware: add extensible driver data params
Message-ID: <20170613155041.GB24483@kroah.com>
References: <20170605213314.GR8951@wotan.suse.de>
 <20170605213937.26215-1-mcgrof@kernel.org>
 <20170605213937.26215-2-mcgrof@kernel.org>
 <20170613090548.GA31421@kroah.com>
 <4c7b15d3b9e7853faa0e928a6ab160f2@milecki.pl>
 <20170613131743.GC23757@kroah.com>
 <20170613153249.GH27288@wotan.suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20170613153249.GH27288@wotan.suse.de>
User-Agent: Mutt/1.8.3 (2017-05-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2508
Lines: 54

On Tue, Jun 13, 2017 at 05:32:49PM +0200, Luis R. Rodriguez wrote:
> On Tue, Jun 13, 2017 at 03:17:43PM +0200, Greg KH wrote:
> > On Tue, Jun 13, 2017 at 12:31:04PM +0200, Rafał Miłecki wrote:
> > > On 2017-06-13 11:05, Greg KH wrote:
> > > > On Mon, Jun 05, 2017 at 02:39:33PM -0700, Luis R. Rodriguez wrote:
> > > > > As the firmware API evolves we keep extending functions with more
> > > > > arguments.
> > > > > Stop this nonsense by proving an extensible data structure which can
> > > > > be used
> > > > > to represent both user parameters and private internal parameters.
> > > > 
> > > > Let's take a simple C function interface and make it a more complex
> > > > data-driven interface that is impossible to understand and obviously
> > > > understand how it is to be used and works!
> > > > 
> > > > :(
> > > > 
> > > > Seriously, why?  Why are we extending any of this at all?  This series
> > > > adds a ton of new "features" and complexity, but for absolutely no gain.
> > > > 
> > > > Oh, I take it back, you removed 29 lines from the iwlwifi driver.
> > > > 
> > > > That's still not worth it at all, you have yet to sell me on this whole
> > > > complex beast.  I can't see why we need it, and if I, one of the few
> > > > people who thinks they actually understand this kernel interface, can't
> > > > see it, how can you sell it to someone else?
> > > > 
> > > > Sorry, but no, I'm still not going to take this series until you show
> > > > some _REAL_ benefit for it.
> > > 
> > > FWIW I saw (or maybe still see?) a need to extend request_firmware* API to
> > > allow silencing a warning if firmware file is missing.
> > > 
> > > I even sent a trivial patch adding support for this:
> > > [PATCH V4 1/2] firmware: add more flexible request_firmware_async function
> > > https://patchwork.kernel.org/patch/9588787/
> > > (I think it still applies) but it got rejected due to Luis's big rework.
> > 
> > Can you resend this series if it still does apply?
> 
> FWIW just some notes on Rafał's series:
> 
> someone else brought up second that his second no longer should be applied as
> some devices do need what seems to be today's optional request. Also note that
> the approach follows the same I take, just struct a firmware_opts instead of
> driver params... and it does not mesh up the old options as I did in my first
> patch in this series.

As I have no idea what his series looks like at the moment, why not wait
until they are posted again to review them? :)

thanks,

greg k-h