From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750764AbdA0SYc (ORCPT ); Fri, 27 Jan 2017 13:24:32 -0500 Received: from mx2.suse.de ([195.135.220.15]:40295 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750735AbdA0SXk (ORCPT ); Fri, 27 Jan 2017 13:23:40 -0500 Date: Fri, 27 Jan 2017 19:23:34 +0100 From: "Luis R. Rodriguez" To: Linus Torvalds Cc: "Luis R. Rodriguez" , Greg KH , Ming Lei , Borislav Petkov , Daniel Wagner , Tom Gundersen , Mauro Carvalho Chehab , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , Linux Kernel Mailing List , Vikram Mulukutla , Stephen Boyd , Mark Brown , Mimi Zohar , Takashi Iwai , Johannes Berg , Christian Lamparter , Hauke Mehrtens , Josh Boyer , Dmitry Torokhov , David Woodhouse , Jiri Slaby , Andy Lutomirski , Wu Fengguang , Richard Purdie , Jacek Anaszewski , Abhay_Salunke@dell.com, Julia Lawall , Gilles.Muller@lip6.fr, nicolas.palix@imag.fr, David Howells , Bjorn Andersson , arend.vanspriel@broadcom.com, Kalle Valo Subject: Re: [PATCH v4 3/3] p54: convert to sysdata API Message-ID: <20170127182333.GB24047@wotan.suse.de> References: <20161216114632.22559-1-mcgrof@kernel.org> <20170112150244.12700-1-mcgrof@kernel.org> <20170112150244.12700-4-mcgrof@kernel.org> <20170119113857.GQ28024@kroah.com> <20170119162751.GJ13946@wotan.suse.de> <20170126215005.GU13946@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 On Thu, Jan 26, 2017 at 01:54:20PM -0800, Linus Torvalds wrote: > On Thu, Jan 26, 2017 at 1:50 PM, Luis R. Rodriguez wrote: > > > > OK I've added a respective helper call which would map 1-1 with the > > old sync mechanism to enable a 1-1 change, this will be called > > driver_data_request_simple(), but let me know if there is a preference > > for something else. > > So just looking at this patch, what's the *advantage* to the driver writer? So for the driver writer it provides a clean way to logically split up what is to be done for certain situations if the firmware is not present or is present. Without this the code is a bit unruly, and this is actually a mild case. There are much crazier chained conditionals (iwlwifi is one that has a long chain of firmwares) but a goal here was to just provide only the most basic bump in logic so that further enhancements/functionality is added later. > Apart from the actual new feature, this patch seems to actively make > the driver uglier. > > I mentioned this before, but replacing "request_firmware()" with > "driver_data_request_simple()" is SIMPLY NOT AN IMPROVEMENT. I strongly agree with this. > The new name is longer and _less_ descriptive. > > So I'm really not seeing why you want to make these conversions that > just make code worse. The real goal here was first to actually provide a flexible API to enable more advanced features to be added without having to affect existing callers, as has been done before. So hence the const struct driver_data_req_params approach and only two basic calls -- a sync and async call. This was after long ago we had revised how we would go about adding firmware signing support to the kernel. My first approach in addressing firmware signing was to mimic how we handle have module signing: everyone gets it (even those on the old API), using one default key. The flexible API was then a secondary step, to enable users to customize signature requirements. As we discussed things it was clear that we wanted the ability to support firmware signing with the ability to provide alternative key requirements from the very start. Having an extensible firmware API in place first would enable the flexibility to let us decide what requirements we want to put in place for firmware signing without concern for making a slew of collateral evolutions as requirements change. The flexible API then would be, as is in this series, completely optional. Only if you want to reap benefit of some of the new features would you use it. So unless the flexible API is reproachable in and of itself perhaps the thing to do is leave all drivers as-is (without no conversion) and only convert once we have a full gain value-add. For instance later adding support to easily chain a series of firmware requests (not just 2), or once we have firmware signing support and a driver want to reap benefit from it. Thoughts ? Luis