Message-ID: <20160617013657.GH11948@wotan.suse.de>
Date:	Fri, 17 Jun 2016 03:36:57 +0200
From:	"Luis R. Rodriguez" <mcgrof@...nel.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	"Luis R. Rodriguez" <mcgrof@...nel.org>,
	Hauke Mehrtens <hauke@...ke-m.de>, markivx@...eaurora.org,
	stephen.boyd@...aro.org, chunkeey@...glemail.com,
	luto@...capital.net, corbet@....net, Julia.Lawall@...6.fr,
	teg@...m.no, dwmw2@...radead.org, akpm@...ux-foundation.org,
	tj@...nel.org, jwboyer@...oraproject.org, mmarek@...e.com,
	dhowells@...hat.com, zohar@...ux.vnet.ibm.com,
	johannes@...solutions.net, daniel.vetter@...ll.ch,
	Abhay_Salunke@...l.com, ming.lei@...onical.com, tiwai@...e.de,
	dmitry.torokhov@...il.com, fengguang.wu@...el.com,
	broonie@...nel.org, keescook@...omium.org,
	gregkh@...uxfoundation.org, jslaby@...e.com, Gilles.Muller@...6.fr,
	linux-kernel@...r.kernel.org, ki@...sung.com, bp@...en8.de,
	rpurdie@...ys.net, nicolas.palix@...g.fr
Subject: Re: [PATCH v2 8/8] p54: convert to sysdata API

On Thu, Jun 16, 2016 at 02:21:02PM -1000, Linus Torvalds wrote:
> So what is the advantage of this, since it needs to add more lines of code
> than it removes?
>
> It doesn't seem to be a simplification. Quite the reverse.
> 
> Your diffstat of the whole automated conversion did that too.

A lot of the diff stat of automated changes is space changes which consists of
not wanted new lines in one case caused by Coccinelle.  To get an idea by
comparison one would have to cleanup the output. Typically its on par,
sometimes you save, sometimes a bit more code due to the syn case needing a
callback. In this case its more given the non-keep cases which need a callback,
and also some changes around an #ifdef to make code cleaner.

> Why would we ever want to apply a patch like this?

Well, it depends, in terms of API it helps us extend it without having to do
more collateral evolutions. The p54 driver happens to have both sync and async
calls, and also has a few "KEEP" cases, the SmPL grammar does not deal with the
keep cases -- this change to p54 was just a demo, but what I'd much prefer is
to deal with the keep cases by folding the fw into devm to also avoid the
free'ing in drivers just as with the non-keep cases. If that's desirable it
needs discussion given its a significant change. The grammar still produces
more changes than before for the sync cases given a callback is needed, its
just that simple.

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 ?

If we want to wait for devm integration -- that's cool with me, its however
better to release often and release early. So the point of this particular
series is to show where development is at on the front of a new flexible API that
also avoids the usermode helper. I posted this also *now* given I saw the old
API being extended further. So another option is to evaluate a merge now, and
evolve the devm integration later.

Alternatively, since its only 2 drivers that explicitly require the usermode
helper, another option is to just compartamentalize the usermode helper
explicit callers with a its own API now and save the others with a strategy
devised by the current sysdata API, without requiring any changes at all.
This is a significant change though, so still requires discussion. As I
see it this is worth considering just because we get the same end result
if we transform drivers to sysdata or we just compartamentalize the usermode
helper now to the 2 callers.

Ideas, patches, feedback, rants welcomed.

  Luis

Powered by blists - more mailing lists