linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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>,
	markivx@codeaurora.org, stephen.boyd@linaro.org,
	chunkeey@googlemail.com, luto@amacapital.net, corbet@lwn.net,
	Julia.Lawall@lip6.fr, teg@jklm.no, dwmw2@infradead.org,
	akpm@linux-foundation.org, tj@kernel.org,
	jwboyer@fedoraproject.org, mmarek@suse.com, dhowells@redhat.com,
	zohar@linux.vnet.ibm.com, johannes@sipsolutions.net,
	daniel.vetter@ffwll.ch, Abhay_Salunke@dell.com,
	ming.lei@canonical.com, tiwai@suse.de, dmitry.torokhov@gmail.com,
	fengguang.wu@intel.com, broonie@kernel.org,
	keescook@chromium.org, gregkh@linuxfoundation.org,
	jslaby@suse.com, Gilles.Muller@lip6.fr,
	linux-kernel@vger.kernel.org, ki@samsung.com, bp@alien8.de,
	rpurdie@rpsys.net, nicolas.palix@imag.fr
Subject: Re: [PATCH v2 8/8] p54: convert to sysdata API
Date: Fri, 17 Jun 2016 03:36:57 +0200	[thread overview]
Message-ID: <20160617013657.GH11948@wotan.suse.de> (raw)
In-Reply-To: <CA+55aFyD+Xa2auP05=pBSxQc2qcuR858syMW5fiQKowbqbkDzQ@mail.gmail.com>

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

  parent reply	other threads:[~2016-06-17  1:37 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16 23:59 [PATCH v2 0/8] firmware: add new sysdata API Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 1/8] firmware: add new extensible firmware API - sysdata_file_request*() Luis R. Rodriguez
2016-08-11 21:15   ` Bjorn Andersson
2016-08-12 15:25     ` Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 2/8] lib/test_firmware.c: use late_initcall() Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 3/8] selftests: firmware: only modprobe if driver is missing Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 4/8] selftests: firmware: send expected errors to /dev/null Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 5/8] test: add new sysdata_file_request*() loader tester Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 6/8] Documentation/firmware_class: add sysdata API converter SmPL patch Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 7/8] x86/microcode: convert to use sysdata API Luis R. Rodriguez
2016-06-16 23:59 ` [PATCH v2 8/8] p54: convert to " Luis R. Rodriguez
     [not found]   ` <CA+55aFyD+Xa2auP05=pBSxQc2qcuR858syMW5fiQKowbqbkDzQ@mail.gmail.com>
2016-06-17  1:36     ` Luis R. Rodriguez [this message]
2016-06-17  3:09       ` Linus Torvalds
2016-06-17 18:35         ` Luis R. Rodriguez
2016-08-10 18:32           ` Luis R. Rodriguez
2016-08-10 19:04             ` Arend Van Spriel
2016-08-10 19:17               ` Mark Brown
2016-08-10 19:40                 ` Luis R. Rodriguez
2016-08-10 19:34               ` Luis R. Rodriguez
2016-06-17 23:40 ` [PATCH v2 0/8] firmware: add new " Luis R. Rodriguez
2016-06-18  0:32   ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160617013657.GH11948@wotan.suse.de \
    --to=mcgrof@kernel.org \
    --cc=Abhay_Salunke@dell.com \
    --cc=Gilles.Muller@lip6.fr \
    --cc=Julia.Lawall@lip6.fr \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=broonie@kernel.org \
    --cc=chunkeey@googlemail.com \
    --cc=corbet@lwn.net \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dhowells@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=fengguang.wu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hauke@hauke-m.de \
    --cc=johannes@sipsolutions.net \
    --cc=jslaby@suse.com \
    --cc=jwboyer@fedoraproject.org \
    --cc=keescook@chromium.org \
    --cc=ki@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=markivx@codeaurora.org \
    --cc=ming.lei@canonical.com \
    --cc=mmarek@suse.com \
    --cc=nicolas.palix@imag.fr \
    --cc=rpurdie@rpsys.net \
    --cc=stephen.boyd@linaro.org \
    --cc=teg@jklm.no \
    --cc=tiwai@suse.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).