Re: [PATCH v2 8/8] p54: convert to sysdata API

From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	Vikram Mulukutla <markivx@codeaurora.org>,
	Stephen Boyd <stephen.boyd@linaro.org>,
	Christian Lamparter <chunkeey@googlemail.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Jonathan Corbet <corbet@lwn.net>,
	Julia Lawall <Julia.Lawall@lip6.fr>, Tom Gundersen <teg@jklm.no>,
	David Woodhouse <dwmw2@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>, Josh Boyer <jwboyer@fedoraproject.org>,
	Michal Marek <mmarek@suse.com>,
	David Howells <dhowells@redhat.com>,
	Mimi Zohar <zohar@linux.vnet.ibm.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Abhay_Salunke@dell.com, Ming Lei <ming.lei@canonical.com>,
	Takashi Iwai <tiwai@suse.de>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Wu Fengguang <fengguang.wu@intel.com>,
	Mark Brown <broonie@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	Gilles.Muller@lip6.fr,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Borislav Petkov <bp@alien8.de>,
	Richard Purdie <rpurdie@rpsys.net>,
	nicolas.palix@imag.fr
Subject: Re: [PATCH v2 8/8] p54: convert to sysdata API
Date: Fri, 17 Jun 2016 20:35:03 +0200	[thread overview]
Message-ID: <20160617183503.GV11948@wotan.suse.de> (raw)
In-Reply-To: <CA+55aFycG-YfukKpP3x7kbBpCzXGwg35SdTU2r4wBbxrfROLtQ@mail.gmail.com>

On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
> On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> >
> > Reason this could not wait is folks seem to want to keep extending the API,
> > which is another reason for this, do we want to put an end to an unflexible
> > API now or should we wait ?
> 
> So I absolutely abhor "changes for changes sake".
> 
> If the existing code works for existing drivers, let them keep it.
> 
> And if a new interface is truly more flexible, then it should be able
> to implement the old interface with no changes, so that drivers
> shouldn't need to be changed/upgraded.

Are we OK to leave only the usermode helper support in place for the
2 drivers I've noted that explicitly require the usermode helper in their
firmware request [0] ?

If so then I can do what you recommend below.

> Then, drivers that actually _want_ new features, or that can take
> advantage of new interfaces to actually make things *simpler*, can
> choose to make those changes. But those changes should have real
> advantages.

Sure agreed.

> Having to have a callback, 

This is because devm is not used, as such the consumer is needed prior to
freeing. I can give adding devm support a shot if folks seem to be generally 
agree its the right approach. I do expect this will mean tons of code
deletions and helping with bugs.

> or a magical "sysdata_desc" descriptor,

This is one way to make a flexible and extensible API without affecting drivers
with further collateral evolutions as it gets extended. Its no different than
the "flags" lesson learned from writing system calls, for instance.

Descriptor seemed, fitting, let me know if you have any other preference.

> and having a new name ("sysdata") that is less descriptive than the old one
> ("firmware")

Well no, the APIs are used in *a lot* of cases for things that are not firmware
already, and let's recall I originally started working on this to replace CRDA
from userspace to be able to just fetch the signed regulatory database file
from the kernel. Calling it firmware simply makes no sense anymore.

> are all in my opinion making the example patch be a
> step _backwards_ rather than an improvement. It does not look like a
> simpler or more natural interface for a driver.

Hope the above explains the current state. Feedback on desirable changes
welcomed.

[0] https://lkml.kernel.org/r/1466117661-22075-5-git-send-email-mcgrof@kernel.org

  Luis