Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752614AbdFTPUz (ORCPT ); Tue, 20 Jun 2017 11:20:55 -0400 Received: from mga02.intel.com ([134.134.136.20]:43214 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751205AbdFTPUx (ORCPT ); Tue, 20 Jun 2017 11:20:53 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,364,1493708400"; d="scan'208";a="1184835121" Subject: Re: [PATCH v9 1/5] firmware: add extensible driver data params To: AKASHI Takahiro , Greg KH , "Luis R. Rodriguez" , wagi@monom.org, dwmw2@infradead.org, rafal@milecki.pl, arend.vanspriel@broadcom.com, rjw@rjwysocki.net, 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, dhowells@redhat.com, pjones@redhat.com, hdegoede@redhat.com, alan@linux.intel.com, tytso@mit.edu, linux-kernel@vger.kernel.org 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> <20170620014813.GD4820@linaro.org> From: "Li, Yi" Message-ID: <6ce1a05b-60ba-7305-abff-405f208399ae@linux.intel.com> Date: Tue, 20 Jun 2017 10:20:43 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170620014813.GD4820@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2653 Lines: 65 On 6/19/2017 8:48 PM, AKASHI Takahiro wrote: > On Mon, Jun 19, 2017 at 05:51:08PM -0500, Li, Yi wrote: >> Hi Greg, >> >> On 6/17/2017 2:38 PM, Greg KH wrote: >>> On Tue, Jun 13, 2017 at 09:40:11PM +0200, Luis R. Rodriguez wrote: >>>> On Tue, Jun 13, 2017 at 11:05:48AM +0200, Greg KH wrote: >>>>> On Mon, Jun 05, 2017 at 02:39:33PM -0700, Luis R. Rodriguez wrote: >>> >>>> What you have to ask yourself really is if this makes it *less complex* and >>>> helps *clean things up* in a much better way than it was before. Also does it >>>> allow us to *pave the way for new functionality easily*, without creating >>>> further mess? >>> >>> I agree, that's what I'm saying here. I just do not see that happening >>> with your patch set at all. It's adding more code, a more complex way >>> to interact with the subsystem, and not making driver writer lives any >>> easier at all that I can see. >>> >>> Again, the code is now bigger, does more, with not even any real benefit >>> for existing users. >> >> I am still new to the upstreaming world, pardon me if my understanding is >> naive. :) My take with Luis's driver data API is that it adds a wrapper on >> top of the old request_firmware APIs, so the new features can be >> added/disabled by the parameters structures instead of adding/changing API >> functions. Agree that there is not much new for existing users. It adds more >> codes (not necessary more complex) but create a consistent way for extension >> IMO. > > Most of code of my feature, firmware signing, is implemented in common > place between old and new APIs, while only new API has a parameter, > DRIVER_DATA_REQ_NO_SIG_CHECK, which allow users to enable/disable > this feature per-driver-datum. Simple enough. I meant to say more code does NOT necessary equal to more complex, sorry for the confusion. > > So what matters is adding yet another variant of request_firmware_xx() > vs. adding a mere parameter? Agree, I also prefer the parameter approach. > > Thanks, > -Takahiro AKASHI > >> Below are 3 examples I tried to add streaming support to load large firmware >> files. >> Adding streaming with driver data API: >> https://patchwork.kernel.org/patch/9738503 . This patch series depends on >> non-cache patch series https://patchwork.kernel.org/patch/9793825 , which is >> bigger than it should be since it added some codes to test firmware caching. >> and pre-allocate buffer patch series >> https://patchwork.kernel.org/patch/9738487/ >> >> By comparison, here is my old streaming RFC with original firmware class: >> https://lkml.org/lkml/2017/3/9/872 >> Do you think this is the better way? >> >> Thanks, >> Yi