Date: Tue, 13 Jun 2017 17:32:49 +0200
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>,
        "Luis R. Rodriguez" <mcgrof@kernel.org>, 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: <20170613153249.GH27288@wotan.suse.de>
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>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20170613131743.GC23757@kroah.com>
User-Agent: Mutt/1.6.0 (2016-04-01)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2217
Lines: 48

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.

  Luis