All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: gregkh@linuxfoundation.org, ming.lei@canonical.com
Cc: bp@alien8.de, wagi@monom.org, teg@jklm.no,
	mchehab@osg.samsung.com, zajec5@gmail.com,
	linux-kernel@vger.kernel.org, markivx@codeaurora.org,
	stephen.boyd@linaro.org, broonie@kernel.org,
	zohar@linux.vnet.ibm.com, tiwai@suse.de,
	johannes@sipsolutions.net, chunkeey@googlemail.com,
	hauke@hauke-m.de, jwboyer@fedoraproject.org,
	dmitry.torokhov@gmail.com, dwmw2@infradead.org, jslaby@suse.com,
	torvalds@linux-foundation.org, luto@amacapital.net,
	fengguang.wu@intel.com, rpurdie@rpsys.net,
	j.anaszewski@samsung.com, Abhay_Salunke@dell.com,
	Julia.Lawall@lip6.fr, Gilles.Muller@lip6.fr,
	nicolas.palix@imag.fr, dhowells@redhat.com,
	bjorn.andersson@linaro.org, arend.vanspriel@broadcom.com,
	kvalo@codeaurora.org, "Luis R. Rodriguez" <mcgrof@kernel.org>,
	linux-leds@vger.kernel.org
Subject: [PATCH v4 1/2] firmware: add SmPL report for custom fallback mechanism
Date: Thu, 12 Jan 2017 06:42:49 -0800	[thread overview]
Message-ID: <20170112144250.12376-2-mcgrof@kernel.org> (raw)
In-Reply-To: <20170112144250.12376-1-mcgrof@kernel.org>

Even though most distributions today disable the fallback mechanism
by default we've determined that we cannot remove them from the kernel.
This is not well understood so document the reason and logic behind that.

Recent discussions suggest some future userspace development prospects which
may enable fallback mechanisms to become more useful while avoiding some
historical issues. These discussions have made it clear though that there
is less value to the custom fallback mechanism and an alternative can be
provided in the future. Its also clear that some old users of the custom
fallback mechanism were using it as a copy and paste error. Because of
all this add a Coccinelle SmPL patch to help maintainers police for new
incorrect users of the custom fallback mechanism.

Best we can do for now then is police for new users of the custom
fallback mechanism and and fix incorrect users when they are spotted.
Drivers can only be transitioned out of the custom fallback mechanism
once we know old userspace cannot be not be broken by a kernel change.

The current SmPL patch reports:

$ export COCCI=scripts/coccinelle/api/request_firmware-custom-fallback.cocci
$ make coccicheck MODE=report

drivers/leds/leds-lp55xx-common.c:227:8-31: WARNING: please check if driver really needs a custom fallback mechanism
drivers/firmware/dell_rbu.c:622:17-40: WARNING: please check if driver really needs a custom fallback mechanism

Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
Cc: linux-leds@vger.kernel.org
Cc: Abhay Salunke <Abhay_Salunke@dell.com>
Acked-by: Julia.Lawall@lip6.fr
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 .../driver-api/firmware/fallback-mechanisms.rst    | 17 +++++++++++
 .../api/request_firmware-custom-fallback.cocci     | 35 ++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 scripts/coccinelle/api/request_firmware-custom-fallback.cocci

diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index d19354794e67..b87a292153c6 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -28,6 +28,12 @@ CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n
 the kobject uevent fallback mechanism will never take effect even
 for request_firmware_nowait() when uevent is set to true.
 
+Although the fallback mechanisms are not used widely today they cannot be
+removed from the kernel since some old userspace may exist which could
+entirely depend on the fallback mechanism enabled with the kernel config option
+CONFIG_FW_LOADER_USER_HELPER_FALLBACK. In the future though drivers may opt
+to embrace a different API which provides alternative fallback mechanisms.
+
 Justifying the firmware fallback mechanism
 ==========================================
 
@@ -176,6 +182,17 @@ but you want to suppress kobject uevents, as you have a custom solution which
 will monitor for your device addition into the device hierarchy somehow and
 load firmware for you through a custom path.
 
+The custom fallback mechanism can often be enabled by mistake. We currently
+have only 2 users of it, and little justification to enable it for other users.
+Since it is a common driver developer mistake to enable it, help police for
+new users of the custom fallback mechanism with::
+
+        $ export COCCI=scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
+        $ make coccicheck MODE=report
+
+Drivers can only be transitioned out of the custom fallback mechanism
+once we know old userspace cannot be not be broken by a kernel change.
+
 Firmware fallback timeout
 =========================
 
diff --git a/scripts/coccinelle/api/request_firmware-custom-fallback.cocci b/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
new file mode 100644
index 000000000000..0188d446b611
--- /dev/null
+++ b/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
@@ -0,0 +1,35 @@
+// Avoid the firmware custom fallback mechanism at all costs
+//
+// request_firmware_nowait() API enables explicit request for use of the custom
+// fallback mechanism if firmware is not found. Chances are high its use is
+// just a copy and paste bug. Before you fix the driver be sure to *verify* no
+// custom firmware loading tool exists that would otherwise break if we replace
+// the driver to use the uevent fallback mechanism.
+//
+// Confidence: High
+//
+// Copyright: (C) 2017 Luis R. Rodriguez <mcgrof@kernel.org> GPLv2.
+//
+// Options: --include-headers
+
+virtual report
+virtual context
+
+@ r1 depends on report || context @
+expression mod, name, dev, gfp, drv, cb;
+position p;
+@@
+
+(
+*request_firmware_nowait@p(mod, false, name, dev, gfp, drv, cb)
+|
+*request_firmware_nowait@p(mod, 0, name, dev, gfp, drv, cb)
+|
+*request_firmware_nowait@p(mod, FW_ACTION_NOHOTPLUG, name, dev, gfp, drv, cb)
+)
+
+@script:python depends on report@
+p << r1.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: please check if driver really needs a custom fallback mechanism")
-- 
2.11.0

  reply	other threads:[~2017-01-12 14:43 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-13  3:08 [PATCH 0/5] firmware: doc revamp Luis R. Rodriguez
2016-12-13  3:08 ` [PATCH 1/5] selftests: firmware: only modprobe if driver is missing Luis R. Rodriguez
2016-12-13  3:08 ` [PATCH 2/5] selftests: firmware: send expected errors to /dev/null Luis R. Rodriguez
2016-12-13  3:08 ` [PATCH 3/5] firmware: revamp firmware documentation Luis R. Rodriguez
2016-12-13  7:26   ` Rafał Miłecki
2016-12-16  9:09     ` Luis R. Rodriguez
2016-12-13 13:26   ` Daniel Wagner
2016-12-13 13:30     ` Rafał Miłecki
2016-12-16  9:18       ` Luis R. Rodriguez
2016-12-16  9:34         ` Johannes Berg
2016-12-16  9:16     ` Luis R. Rodriguez
2017-01-12 14:42   ` [PATCH v4 0/2] firmware: fw doc revamp follow up Luis R. Rodriguez
2017-01-12 14:42     ` Luis R. Rodriguez [this message]
2017-01-12 14:42     ` [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Luis R. Rodriguez
2017-01-19 11:31       ` Greg KH
2017-01-19 16:08         ` Luis R. Rodriguez
2017-01-19 16:08           ` Luis R. Rodriguez
2017-01-19 16:14           ` Greg KH
2017-01-19 21:38             ` Luis R. Rodriguez
2017-01-19 21:38               ` Luis R. Rodriguez
2016-12-13  3:08 ` [PATCH 4/5] firmware: add SmPL report for custom fallback mechanism Luis R. Rodriguez
2016-12-13  6:13   ` Julia Lawall
2016-12-13  9:44   ` Jacek Anaszewski
2016-12-13  9:44     ` Jacek Anaszewski
2016-12-14  1:48     ` Milo Kim
2016-12-14  1:48       ` Milo Kim
2016-12-16  9:29       ` Luis R. Rodriguez
2016-12-16  9:29         ` Luis R. Rodriguez
2017-01-11 18:51         ` Luis R. Rodriguez
2016-12-13  3:08 ` [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Luis R. Rodriguez
2016-12-13 19:04   ` Pavel Machek
2016-12-16  9:22     ` Luis R. Rodriguez
2016-12-16  9:22       ` Luis R. Rodriguez
2016-12-16  9:29       ` Pavel Machek
2016-12-16  9:59         ` Luis R. Rodriguez
2016-12-16  9:59           ` Luis R. Rodriguez
2016-12-16 10:14           ` Pavel Machek
2016-12-16 10:56             ` Luis R. Rodriguez
2016-12-16 10:56               ` Luis R. Rodriguez
2016-12-16 11:27               ` Pavel Machek
2016-12-16 15:19                 ` Luis R. Rodriguez
2016-12-16 15:19                   ` Luis R. Rodriguez
2016-12-16 16:10                 ` Luis R. Rodriguez
2016-12-16 16:10                   ` Luis R. Rodriguez
2016-12-16 16:14                   ` Luis R. Rodriguez
2016-12-16 16:14                     ` Luis R. Rodriguez
2016-12-18  3:50                     ` Milo Kim
2016-12-18  3:50                       ` Milo Kim
2016-12-19 20:08                       ` Pavel Machek
2016-12-19 20:08                         ` Pavel Machek
2016-12-19 20:46                         ` Jacek Anaszewski
2016-12-19 20:46                           ` Jacek Anaszewski
2016-12-21 18:49                           ` Pavel Machek
2016-12-21 18:49                             ` Pavel Machek
2016-12-21 20:33                             ` Jacek Anaszewski
2016-12-21 20:33                               ` Jacek Anaszewski
2016-12-15  9:32   ` Jacek Anaszewski
2016-12-16  9:26     ` Luis R. Rodriguez
2016-12-16  9:26       ` Luis R. Rodriguez
2016-12-13 12:58 ` [PATCH 0/5] firmware: doc revamp Daniel Wagner

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=20170112144250.12376-2-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=Abhay_Salunke@dell.com \
    --cc=Gilles.Muller@lip6.fr \
    --cc=Julia.Lawall@lip6.fr \
    --cc=arend.vanspriel@broadcom.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=bp@alien8.de \
    --cc=broonie@kernel.org \
    --cc=chunkeey@googlemail.com \
    --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=j.anaszewski@samsung.com \
    --cc=johannes@sipsolutions.net \
    --cc=jslaby@suse.com \
    --cc=jwboyer@fedoraproject.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=markivx@codeaurora.org \
    --cc=mchehab@osg.samsung.com \
    --cc=ming.lei@canonical.com \
    --cc=nicolas.palix@imag.fr \
    --cc=rpurdie@rpsys.net \
    --cc=stephen.boyd@linaro.org \
    --cc=teg@jklm.no \
    --cc=tiwai@suse.de \
    --cc=torvalds@linux-foundation.org \
    --cc=wagi@monom.org \
    --cc=zajec5@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.