Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754881AbdFXAsd (ORCPT ); Fri, 23 Jun 2017 20:48:33 -0400 Received: from mx2.suse.de ([195.135.220.15]:49193 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754075AbdFXAsc (ORCPT ); Fri, 23 Jun 2017 20:48:32 -0400 Date: Sat, 24 Jun 2017 02:48:28 +0200 From: "Luis R. Rodriguez" To: Linus Torvalds Cc: "Luis R. Rodriguez" , Greg KH , Julia Lawall , Daniel Wagner , David Woodhouse , rafal@milecki.pl, Arend Van Spriel , "Rafael J. Wysocki" , "Li, Yi" , atull@opensource.altera.com, Moritz Fischer , Petr Mladek , Johannes Berg , Emmanuel Grumbach , "Coelho, Luciano" , Kalle Valo , Andrew Lutomirski , Jiri Kosina , Kees Cook , "AKASHI, Takahiro" , David Howells , Peter Jones , Hans de Goede , Alan Cox , "Theodore Ts'o" , NeilBrown , Christoph Hellwig , Linux Kernel Mailing List Subject: Re: [PATCH v9 1/5] firmware: add extensible driver data params Message-ID: <20170624004828.GA21846@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> <20170613194011.GI27288@wotan.suse.de> <20170617193815.GI2974@kroah.com> <20170619193522.GH21846@wotan.suse.de> <20170623155123.GB3565@kroah.com> <20170623224338.GX21846@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2546 Lines: 56 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 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. > It's too indirect, it adds all those nasty "descriptors" of what to > do, and it doesn't match what the current model does at all. This is good feedback, I do accept deciding where to draw the line is hard. I decided to go with blocking/non-blocking as the fine line. > 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... > The thing is, it's much better to just have functions that load the > firmware data. Have them named that way ("load_firmware()"), and act > that way ("just load the damn firmware file") instead of having odd > descriptors that describe what is goign to be done and some state for > it, and then get passed around. > > Don't add this kind of crazy abstraction complexity. > > If somebody wants to veryify a signature on a firmware file, they > should *NOT* fill in a descriptor that says "check signature when > loading". Thats' complete BS. > > They should just do "load_firmware()" and then "check_signature()" or whatever. That's a fair suggestion for firmware signing! And I'll let AKASHI comment on whether or not that would suffice for his requirements given he's now addressing firmware signing. 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? Or let us just consider the very simple *optional* async firmware. Do we add *one* full new API call just for that? Luis