Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1540812 > unrolled thread

[PATCH 0/5] firmware: doc revamp

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] firmware: doc revamp "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-13 04:10 +0100
    [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-13 04:10 +0100
      Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Pavel Machek <pavel@ucw.cz> - 2016-12-13 20:10 +0100
        Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 10:30 +0100
          Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Pavel Machek <pavel@ucw.cz> - 2016-12-16 10:30 +0100
            Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 11:00 +0100
              Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Pavel Machek <pavel@ucw.cz> - 2016-12-16 11:20 +0100
                Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 12:00 +0100
                  Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Pavel Machek <pavel@ucw.cz> - 2016-12-16 12:30 +0100
                    Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 16:30 +0100
                    Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 17:20 +0100
                      Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Milo Kim <woogyom.kim@gmail.com> - 2016-12-18 05:00 +0100
                        Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Pavel Machek <pavel@ucw.cz> - 2016-12-19 21:10 +0100
                          Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-12-19 21:50 +0100
                            Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Pavel Machek <pavel@ucw.cz> - 2016-12-21 19:50 +0100
                              Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-12-21 21:40 +0100
                    Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 17:20 +0100
      Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-12-15 10:40 +0100
        Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-12-16 10:30 +0100
    Re: [PATCH 0/5] firmware: doc revamp Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-12-13 14:10 +0100

#1540812 — [PATCH 0/5] firmware: doc revamp

Greg,

here's a few simple changes for documentation revamp and a few
simple fixes for the firmware test script. I'm pretty sure this is
too late for the next release so I am happy with this getting queued in
until the merge window closes, posting now to iron out kinks once
the merge window closes.

I'll soon also post the extensible firmware API as per feedback, much of
which was just name change preferences and also a huge clarification on
roadmap on fallback mechanisms. The documentation should help iron out
tons of kinks I think folks have on this front, but more on all this later
once those patches get posted.

You may notice I've dropped the SmPL patches which complain on use of the
API on init and probe -- although valid the context was off given the only
valid use case was if you don't use initramfs, and that's a corner case.
Fortunatley Daniel Wagner and Tom Gundersen have come up with some ideas
that should help correct these issues, so I've dropped that grammar patch.

Luis R. Rodriguez (5):
  selftests: firmware: only modprobe if driver is missing
  selftests: firmware: send expected errors to /dev/null
  firmware: revamp firmware documentation
  firmware: add SmPL report for custom fallback mechanism
  firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

 Documentation/driver-api/firmware/built-in-fw.rst  |  36 ++++
 Documentation/driver-api/firmware/core.rst         |  16 ++
 .../driver-api/firmware/direct-fs-lookup.rst       |  30 +++
 .../driver-api/firmware/fallback-mechanisms.rst    | 215 +++++++++++++++++++++
 .../driver-api/firmware/firmware_cache.rst         |  51 +++++
 .../driver-api/firmware/fw_search_path.rst         |  26 +++
 Documentation/driver-api/firmware/index.rst        |  16 ++
 Documentation/driver-api/firmware/introduction.rst |  27 +++
 Documentation/driver-api/firmware/lookup-order.rst |  18 ++
 .../driver-api/firmware/request_firmware.rst       |  56 ++++++
 Documentation/driver-api/index.rst                 |   1 +
 Documentation/firmware_class/README                | 128 ------------
 drivers/firmware/dell_rbu.c                        |   1 +
 drivers/leds/leds-lp55xx-common.c                  |   1 +
 include/linux/firmware.h                           |   7 +
 .../api/request_firmware-custom-fallback.cocci     |  44 +++++
 tools/testing/selftests/firmware/fw_filesystem.sh  |  25 ++-
 17 files changed, 565 insertions(+), 133 deletions(-)
 create mode 100644 Documentation/driver-api/firmware/built-in-fw.rst
 create mode 100644 Documentation/driver-api/firmware/core.rst
 create mode 100644 Documentation/driver-api/firmware/direct-fs-lookup.rst
 create mode 100644 Documentation/driver-api/firmware/fallback-mechanisms.rst
 create mode 100644 Documentation/driver-api/firmware/firmware_cache.rst
 create mode 100644 Documentation/driver-api/firmware/fw_search_path.rst
 create mode 100644 Documentation/driver-api/firmware/index.rst
 create mode 100644 Documentation/driver-api/firmware/introduction.rst
 create mode 100644 Documentation/driver-api/firmware/lookup-order.rst
 create mode 100644 Documentation/driver-api/firmware/request_firmware.rst
 delete mode 100644 Documentation/firmware_class/README
 create mode 100644 scripts/coccinelle/api/request_firmware-custom-fallback.cocci

-- 
2.10.1

[toc] | [next] | [standalone]

#1540813 — [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

We need to ensure that when driver developers use the custom firmware
fallback mechanism it was not a copy and paste bug. These use cases on
upstream drivers are rare, we only have 2 upstream users and its for
really old drivers. Since valid uses are rare but possible enable a
white-list for its use, and use this same white-list annotation to refer
to the documentation covering the custom use case.

New faulty users can be reported via 0-day now.

Cc: Fengguang Wu <fengguang.wu@intel.com>
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>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 Documentation/driver-api/firmware/fallback-mechanisms.rst     | 7 +++++--
 drivers/firmware/dell_rbu.c                                   | 1 +
 drivers/leds/leds-lp55xx-common.c                             | 1 +
 include/linux/firmware.h                                      | 7 +++++++
 scripts/coccinelle/api/request_firmware-custom-fallback.cocci | 9 ++++++++-
 5 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index 955c11d6ff9d..b51673e40439 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -184,8 +184,11 @@ 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::
+Since it is a common driver developer mistake to enable it, driver developers
+should use DECLARE_FW_CUSTOM_FALLBACK() to both white-list and validate their
+use and also refer to the documentation for the custom loading solution.
+
+Invalid users of the custom fallback mechanism can be policed using::
 
         $ export COCCI=scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
         $ make coccicheck MODE=report
diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c
index 2f452f1f7c8a..3f2aa35bc54d 100644
--- a/drivers/firmware/dell_rbu.c
+++ b/drivers/firmware/dell_rbu.c
@@ -586,6 +586,7 @@ static ssize_t read_rbu_image_type(struct file *filp, struct kobject *kobj,
 	return size;
 }
 
+DECLARE_FW_CUSTOM_FALLBACK("Documentation/dell_rbu.txt");
 static ssize_t write_rbu_image_type(struct file *filp, struct kobject *kobj,
 				    struct bin_attribute *bin_attr,
 				    char *buffer, loff_t pos, size_t count)
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 5377f22ff994..04161428ee3b 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
 	release_firmware(chip->fw);
 }
 
+DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
 static int lp55xx_request_firmware(struct lp55xx_chip *chip)
 {
 	const char *name = chip->cl->name;
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index b1f9f0ccb8ac..e6ca19c03dcc 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -8,6 +8,13 @@
 #define FW_ACTION_NOHOTPLUG 0
 #define FW_ACTION_HOTPLUG 1
 
+/*
+ * Helper for scripts/coccinelle/api/request_firmware-custom-fallback.cocci
+ * and so users can also easily search for the documentation for the
+ * respectively needed custom fallback mechanism.
+ */
+#define DECLARE_FW_CUSTOM_FALLBACK(__usermode_helper)
+
 struct firmware {
 	size_t size;
 	const u8 *data;
diff --git a/scripts/coccinelle/api/request_firmware-custom-fallback.cocci b/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
index c7598cfc4780..68cacab35b76 100644
--- a/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
+++ b/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
@@ -17,6 +17,13 @@
 virtual report
 virtual context
 
+@ r0 depends on report || context @
+declarer name DECLARE_FW_CUSTOM_FALLBACK;
+expression E;
+@@
+
+DECLARE_FW_CUSTOM_FALLBACK(E);
+
 @ r1 depends on report || context @
 expression mod, name, dev, gfp, drv, cb;
 position p;
@@ -30,7 +37,7 @@ position p;
 *request_firmware_nowait@p(mod, FW_ACTION_NOHOTPLUG, name, dev, gfp, drv, cb)
 )
 
-@script:python depends on report@
+@script:python depends on report && !r0 @
 p << r1.p;
 @@
 
-- 
2.10.1

[toc] | [prev] | [next] | [standalone]

#1541402 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Multipart message — attachments visible in raw view] — view raw

Hi!

> We need to ensure that when driver developers use the custom firmware
> fallback mechanism it was not a copy and paste bug. These use cases on
> upstream drivers are rare, we only have 2 upstream users and its for
> really old drivers. Since valid uses are rare but possible enable a
> white-list for its use, and use this same white-list annotation to refer
> to the documentation covering the custom use case.

> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
>  	release_firmware(chip->fw);
>  }
>  
> +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
>  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
>  {
>  	const char *name = chip->cl->name;

The driver does:

static void lp55xx_firmware_loaded(const struct firmware *fw, void
*context)
{
	struct lp55xx_chip *chip = context;
	        struct device *dev = &chip->cl->dev;
		        enum lp55xx_engine_index idx =
			chip->engine_idx;

        if (!fw) {
	                dev_err(dev, "firmware request failed\n");
			                goto out;
					     }
        ...
out:
        /* firmware should be released for other channel use */
	        release_firmware(chip->fw);
}


Does that match the "custom fallback" definition?


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1543303 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Tue, Dec 13, 2016 at 08:04:29PM +0100, Pavel Machek wrote:
> Hi!
> 
> > We need to ensure that when driver developers use the custom firmware
> > fallback mechanism it was not a copy and paste bug. These use cases on
> > upstream drivers are rare, we only have 2 upstream users and its for
> > really old drivers. Since valid uses are rare but possible enable a
> > white-list for its use, and use this same white-list annotation to refer
> > to the documentation covering the custom use case.
> 
> > --- a/drivers/leds/leds-lp55xx-common.c
> > +++ b/drivers/leds/leds-lp55xx-common.c
> > @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
> >  	release_firmware(chip->fw);
> >  }
> >  
> > +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
> >  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
> >  {
> >  	const char *name = chip->cl->name;
> 
> The driver does:
> 
> static void lp55xx_firmware_loaded(const struct firmware *fw, void
> *context)
> {
> 	struct lp55xx_chip *chip = context;
> 	        struct device *dev = &chip->cl->dev;
> 		        enum lp55xx_engine_index idx =
> 			chip->engine_idx;
> 
>         if (!fw) {
> 	                dev_err(dev, "firmware request failed\n");
> 			                goto out;
> 					     }
>         ...
> out:
>         /* firmware should be released for other channel use */
> 	        release_firmware(chip->fw);
> }
> 
> 
> Does that match the "custom fallback" definition?

Refer to the documentation I supplied, and also to the grammar rule, in
particular the patch "firmware: add SmPL report for custom fallback mechanism",
it captures the SmPL form for the custom fallback mechanism as:

@ 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)   
)     

[toc] | [prev] | [next] | [standalone]

#1543310 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Multipart message — attachments visible in raw view] — view raw

On Fri 2016-12-16 10:22:41, Luis R. Rodriguez wrote:
> On Tue, Dec 13, 2016 at 08:04:29PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > We need to ensure that when driver developers use the custom firmware
> > > fallback mechanism it was not a copy and paste bug. These use cases on
> > > upstream drivers are rare, we only have 2 upstream users and its for
> > > really old drivers. Since valid uses are rare but possible enable a
> > > white-list for its use, and use this same white-list annotation to refer
> > > to the documentation covering the custom use case.
> > 
> > > --- a/drivers/leds/leds-lp55xx-common.c
> > > +++ b/drivers/leds/leds-lp55xx-common.c
> > > @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
> > >  	release_firmware(chip->fw);
> > >  }
> > >  
> > > +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
> > >  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
> > >  {
> > >  	const char *name = chip->cl->name;
> > 
> > The driver does:
> > 
> > static void lp55xx_firmware_loaded(const struct firmware *fw, void
> > *context)
> > {
> > 	struct lp55xx_chip *chip = context;
> > 	        struct device *dev = &chip->cl->dev;
> > 		        enum lp55xx_engine_index idx =
> > 			chip->engine_idx;
> > 
> >         if (!fw) {
> > 	                dev_err(dev, "firmware request failed\n");
> > 			                goto out;
> > 					     }
> >         ...
> > out:
> >         /* firmware should be released for other channel use */
> > 	        release_firmware(chip->fw);
> > }
> > 
> > 
> > Does that match the "custom fallback" definition?
> 
> Refer to the documentation I supplied, and also to the grammar rule, in
> particular the patch "firmware: add SmPL report for custom fallback mechanism",
> it captures the SmPL form for the custom fallback mechanism as:

I don't much care what the rule says. If you believe the code is
buggy, submit a patch.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1543325 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Fri, Dec 16, 2016 at 10:29:20AM +0100, Pavel Machek wrote:
> On Fri 2016-12-16 10:22:41, Luis R. Rodriguez wrote:
> > On Tue, Dec 13, 2016 at 08:04:29PM +0100, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > We need to ensure that when driver developers use the custom firmware
> > > > fallback mechanism it was not a copy and paste bug. These use cases on
> > > > upstream drivers are rare, we only have 2 upstream users and its for
> > > > really old drivers. Since valid uses are rare but possible enable a
> > > > white-list for its use, and use this same white-list annotation to refer
> > > > to the documentation covering the custom use case.
> > > 
> > > > --- a/drivers/leds/leds-lp55xx-common.c
> > > > +++ b/drivers/leds/leds-lp55xx-common.c
> > > > @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
> > > >  	release_firmware(chip->fw);
> > > >  }
> > > >  
> > > > +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
> > > >  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
> > > >  {
> > > >  	const char *name = chip->cl->name;
> > > 
> > > The driver does:
> > > 
> > > static void lp55xx_firmware_loaded(const struct firmware *fw, void
> > > *context)
> > > {
> > > 	struct lp55xx_chip *chip = context;
> > > 	        struct device *dev = &chip->cl->dev;
> > > 		        enum lp55xx_engine_index idx =
> > > 			chip->engine_idx;
> > > 
> > >         if (!fw) {
> > > 	                dev_err(dev, "firmware request failed\n");
> > > 			                goto out;
> > > 					     }
> > >         ...
> > > out:
> > >         /* firmware should be released for other channel use */
> > > 	        release_firmware(chip->fw);
> > > }
> > > 
> > > 
> > > Does that match the "custom fallback" definition?
> > 
> > Refer to the documentation I supplied, and also to the grammar rule, in
> > particular the patch "firmware: add SmPL report for custom fallback mechanism",
> > it captures the SmPL form for the custom fallback mechanism as:
> 
> I don't much care what the rule says. If you believe the code is
> buggy, submit a patch.

Huh? No, its an old API and valid uses are scarce. The point is to avoid folks
adding yet other users by mistake by using grammar to help white-list actual
valid users.

  Luis

[toc] | [prev] | [next] | [standalone]

#1543335 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Multipart message — attachments visible in raw view] — view raw

On Fri 2016-12-16 10:59:06, Luis R. Rodriguez wrote:
> On Fri, Dec 16, 2016 at 10:29:20AM +0100, Pavel Machek wrote:
> > On Fri 2016-12-16 10:22:41, Luis R. Rodriguez wrote:
> > > On Tue, Dec 13, 2016 at 08:04:29PM +0100, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > > We need to ensure that when driver developers use the custom firmware
> > > > > fallback mechanism it was not a copy and paste bug. These use cases on
> > > > > upstream drivers are rare, we only have 2 upstream users and its for
> > > > > really old drivers. Since valid uses are rare but possible enable a
> > > > > white-list for its use, and use this same white-list annotation to refer
> > > > > to the documentation covering the custom use case.
> > > > 
> > > > > --- a/drivers/leds/leds-lp55xx-common.c
> > > > > +++ b/drivers/leds/leds-lp55xx-common.c
> > > > > @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
> > > > >  	release_firmware(chip->fw);
> > > > >  }
> > > > >  
> > > > > +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
> > > > >  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
> > > > >  {
> > > > >  	const char *name = chip->cl->name;
> > > > 
> > > > The driver does:
> > > > 
> > > > static void lp55xx_firmware_loaded(const struct firmware *fw, void
> > > > *context)
> > > > {
> > > > 	struct lp55xx_chip *chip = context;
> > > > 	        struct device *dev = &chip->cl->dev;
> > > > 		        enum lp55xx_engine_index idx =
> > > > 			chip->engine_idx;
> > > > 
> > > >         if (!fw) {
> > > > 	                dev_err(dev, "firmware request failed\n");
> > > > 			                goto out;
> > > > 					     }
> > > >         ...
> > > > out:
> > > >         /* firmware should be released for other channel use */
> > > > 	        release_firmware(chip->fw);
> > > > }
> > > > 
> > > > 
> > > > Does that match the "custom fallback" definition?
> > > 
> > > Refer to the documentation I supplied, and also to the grammar rule, in
> > > particular the patch "firmware: add SmPL report for custom fallback mechanism",
> > > it captures the SmPL form for the custom fallback mechanism as:
> > 
> > I don't much care what the rule says. If you believe the code is
> > buggy, submit a patch.
> 
> Huh? No, its an old API and valid uses are scarce. The point is to avoid folks
> adding yet other users by mistake by using grammar to help white-list actual
> valid users.

Well, I was asking if the above snipped looks like valid use. Because
AFAICT, the "custom fallback" is just dev_err(), see above. Coccinelle
rules don't help me...
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1543368 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Fri, Dec 16, 2016 at 11:14:05AM +0100, Pavel Machek wrote:
> 
> Well, I was asking if the above snipped looks like valid use. Because
> AFAICT, the "custom fallback" is just dev_err(), see above. Coccinelle
> rules don't help me...

Its not. Its when you ask for no uevent. Only 2 drivers do this.

  Luis

[toc] | [prev] | [next] | [standalone]

#1543391 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Multipart message — attachments visible in raw view] — view raw

On Fri 2016-12-16 11:56:48, Luis R. Rodriguez wrote:
> On Fri, Dec 16, 2016 at 11:14:05AM +0100, Pavel Machek wrote:
> > 
> > Well, I was asking if the above snipped looks like valid use. Because
> > AFAICT, the "custom fallback" is just dev_err(), see above. Coccinelle
> > rules don't help me...
> 
> Its not. Its when you ask for no uevent. Only 2 drivers do this.

That was one of two you listed. If that is not valid use, perhaps it
should be removed, not annotated?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1543522 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Fri, Dec 16, 2016 at 5:27 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Fri 2016-12-16 11:56:48, Luis R. Rodriguez wrote:
>> On Fri, Dec 16, 2016 at 11:14:05AM +0100, Pavel Machek wrote:
>> >
>> > Well, I was asking if the above snipped looks like valid use. Because
>> > AFAICT, the "custom fallback" is just dev_err(), see above. Coccinelle
>> > rules don't help me...
>>
>> Its not. Its when you ask for no uevent. Only 2 drivers do this.
>
> That was one of two you listed. If that is not valid use, perhaps it
> should be removed, not annotated?

Pavel, the annotation was added on top of:

static int lp55xx_request_firmware(struct lp55xx_chip *chip)
{
        const char *name = chip->cl->name;
        struct device *dev = &chip->cl->dev;

        return request_firmware_nowait(THIS_MODULE, false, name, dev,
                                GFP_KERNEL, chip, lp55xx_firmware_loaded);
}

Note the second argument is false. This matches the grammar and the
definition for a custom fallback mechanism since uevents are not used.
What am I missing?

 Luis

[toc] | [prev] | [next] | [standalone]

#1543560 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Fri, Dec 16, 2016 at 05:10:18PM +0100, Luis R. Rodriguez wrote:
> Ah, well Milo Kim replied and described that the custom fallback is used as to
> help load LED effect manually, and suggested a sysfs interface is more ideal [0]. I
> agree however its also may be too late, and it depends how wide spread this "userspace"
> that relies on this is, we just can't break it. Granted the custom fallback
> mechanism was broken since v4.0 (see the fix "firmware: fix usermode helper
> fallback loading") so one may argue no one seems to care...
> 
> So this is a judgement call, and the declaration is to point to documentation
> to white list uses, as terrible as this one is userspace exists for it. but
> more importantly to also help the SmPL grammar report to avoid reporting
> already vetted cases. The alarm / cases for the 2 drivers has been issueed,
> moving forward the lack of declaration with the custom fallback should trigger
> a rant through 0-day so we don't run into the same stupid situation.
> 
> [0] https://marc.info/?l=linux-kernel&m=148168024112445

Milo if sysfs is used can't the old userspace be mapped to use the new 
sysfs interface through a wrapper of some sort ? What exactly would be
needed to ensure old userspace will not break? Why has no one cried
after the v4.0 custom fallback mechanism breaking ? How wide spread is
this custom userspace ?

  Luis

[toc] | [prev] | [next] | [standalone]

#1544088 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

Hi Luis,

On 12/17/2016 01:14 AM, Luis R. Rodriguez wrote:
> Milo if sysfs is used can't the old userspace be mapped to use the new
> sysfs interface through a wrapper of some sort ? What exactly would be
> needed to ensure old userspace will not break?

LP5521 and LP5523 have two ways to load hex code from the userspace - 
the sysfs and firmware I/F. So user program supports both interfaces. 
Even if the firmware I/F is not available, user can still run LED effect 
through the sysfs.

However, LP5562 and LP8501 support only single way which is the firmware 
I/F. So user-space program for LP5562/8501 should be modified if lp55xx 
removes the interface. My idea is

   Phase 1)
   - create sysfs in LP5562 and LP8501
   - use new sysfs inside the firmware I/F loading callback
   - mark the firmware callback as a deprecated interface

   Phase 2)
   - remove the firmware I/F after all user program fixes the interface
     (but the problem is how can we get to know when this is done?)

 > Why has no one cried
 > after the v4.0 custom fallback mechanism breaking ?

Well, I don't know the reason exactly but my guess is they maybe still 
using old kernel.

 > How wide spread is this custom userspace ?

Device manufactures in Asia & North America requested lp55xx drivers, 
but I don't know how many vendors uses the firmware I/F. Some vendors 
embeds the binary code inside the driver instead of using user-program.

I understood it's a kind of troublesome work in terms of the 
maintenance. Sorry for that. I hope we have a consensus to resolve it.
Thanks!

Best regards,
Milo

[toc] | [prev] | [next] | [standalone]

#1544753 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Multipart message — attachments visible in raw view] — view raw

Hi!

> On 12/17/2016 01:14 AM, Luis R. Rodriguez wrote:
> >Milo if sysfs is used can't the old userspace be mapped to use the new
> >sysfs interface through a wrapper of some sort ? What exactly would be
> >needed to ensure old userspace will not break?
> 
> LP5521 and LP5523 have two ways to load hex code from the userspace - the
> sysfs and firmware I/F. So user program supports both interfaces. Even if
> the firmware I/F is not available, user can still run LED effect through the
> sysfs.
> 
> However, LP5562 and LP8501 support only single way which is the firmware
> I/F. So user-space program for LP5562/8501 should be modified if lp55xx
> removes the interface. My idea is

Actually... it would be good to have some reasonable interface for RGB
LEDs. This way, we need separate "firmware" for each LED
controller. It would be good to have common format for LED effects.

>   Phase 1)
>   - create sysfs in LP5562 and LP8501
>   - use new sysfs inside the firmware I/F loading callback
>   - mark the firmware callback as a deprecated interface

Phase 1a)

stick WARN_ON() in the firmware callback.

>   Phase 2)
>   - remove the firmware I/F after all user program fixes the interface
>     (but the problem is how can we get to know when this is done?)
> 
> > Why has no one cried
> > after the v4.0 custom fallback mechanism breaking ?
> 
> Well, I don't know the reason exactly but my guess is they maybe still using
> old kernel.
> 
> > How wide spread is this custom userspace ?
> 
> Device manufactures in Asia & North America requested lp55xx drivers, but I
> don't know how many vendors uses the firmware I/F. Some vendors embeds the
> binary code inside the driver instead of using user-program.

Nokia N900 uses lp55xx, and I have custom scripts interfacing sysfs.

Maemo uses the LEDs, too, but maemo is not open source.

So no, I don't think there's anything important that could be broken.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1544762 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On 12/19/2016 09:08 PM, Pavel Machek wrote:
> Hi!
> 
>> On 12/17/2016 01:14 AM, Luis R. Rodriguez wrote:
>>> Milo if sysfs is used can't the old userspace be mapped to use the new
>>> sysfs interface through a wrapper of some sort ? What exactly would be
>>> needed to ensure old userspace will not break?
>>
>> LP5521 and LP5523 have two ways to load hex code from the userspace - the
>> sysfs and firmware I/F. So user program supports both interfaces. Even if
>> the firmware I/F is not available, user can still run LED effect through the
>> sysfs.
>>
>> However, LP5562 and LP8501 support only single way which is the firmware
>> I/F. So user-space program for LP5562/8501 should be modified if lp55xx
>> removes the interface. My idea is
> 
> Actually... it would be good to have some reasonable interface for RGB
> LEDs. This way, we need separate "firmware" for each LED
> controller. It would be good to have common format for LED effects.

We still haven't tried trigger approach discussed over half a year ago.
If we used firmware approach we would still have to overcome the problem
of defining the LED class drivers affected by the firmware program.

>>   Phase 1)
>>   - create sysfs in LP5562 and LP8501
>>   - use new sysfs inside the firmware I/F loading callback
>>   - mark the firmware callback as a deprecated interface
> 
> Phase 1a)
> 
> stick WARN_ON() in the firmware callback.
> 
>>   Phase 2)
>>   - remove the firmware I/F after all user program fixes the interface
>>     (but the problem is how can we get to know when this is done?)
>>
>>> Why has no one cried
>>> after the v4.0 custom fallback mechanism breaking ?
>>
>> Well, I don't know the reason exactly but my guess is they maybe still using
>> old kernel.
>>
>>> How wide spread is this custom userspace ?
>>
>> Device manufactures in Asia & North America requested lp55xx drivers, but I
>> don't know how many vendors uses the firmware I/F. Some vendors embeds the
>> binary code inside the driver instead of using user-program.
> 
> Nokia N900 uses lp55xx, and I have custom scripts interfacing sysfs.
> 
> Maemo uses the LEDs, too, but maemo is not open source.
> 
> So no, I don't think there's anything important that could be broken.

We can't guarantee that. Is there any problem in just using the
currently introduced DECLARE_FW_CUSTOM_FALLBACK() in
drivers/leds/leds-lp55xx-common.c?

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1545947 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Multipart message — attachments visible in raw view] — view raw

Hi!

> >>> Milo if sysfs is used can't the old userspace be mapped to use the new
> >>> sysfs interface through a wrapper of some sort ? What exactly would be
> >>> needed to ensure old userspace will not break?
> >>
> >> LP5521 and LP5523 have two ways to load hex code from the userspace - the
> >> sysfs and firmware I/F. So user program supports both interfaces. Even if
> >> the firmware I/F is not available, user can still run LED effect through the
> >> sysfs.
> >>
> >> However, LP5562 and LP8501 support only single way which is the firmware
> >> I/F. So user-space program for LP5562/8501 should be modified if lp55xx
> >> removes the interface. My idea is
> > 
> > Actually... it would be good to have some reasonable interface for RGB
> > LEDs. This way, we need separate "firmware" for each LED
> > controller. It would be good to have common format for LED effects.
> 
> We still haven't tried trigger approach discussed over half a year ago.
> If we used firmware approach we would still have to overcome the problem
> of defining the LED class drivers affected by the firmware program.

The firmware approach is in the tree today :-(.

> >> Device manufactures in Asia & North America requested lp55xx drivers, but I
> >> don't know how many vendors uses the firmware I/F. Some vendors embeds the
> >> binary code inside the driver instead of using user-program.
> > 
> > Nokia N900 uses lp55xx, and I have custom scripts interfacing sysfs.
> > 
> > Maemo uses the LEDs, too, but maemo is not open source.
> > 
> > So no, I don't think there's anything important that could be broken.
> 
> We can't guarantee that. Is there any problem in just using the
> currently introduced DECLARE_FW_CUSTOM_FALLBACK() in
> drivers/leds/leds-lp55xx-common.c?

Well, it would be good to get rid of the custom fallback
functionality. And no, we don't need to "guarantee" that.  Removing
obscure functionality noone uses is far game... providing noone
complains ;-).

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1545998 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

Hi,

On 12/21/2016 07:49 PM, Pavel Machek wrote:
> Hi!
> 
>>>>> Milo if sysfs is used can't the old userspace be mapped to use the new
>>>>> sysfs interface through a wrapper of some sort ? What exactly would be
>>>>> needed to ensure old userspace will not break?
>>>>
>>>> LP5521 and LP5523 have two ways to load hex code from the userspace - the
>>>> sysfs and firmware I/F. So user program supports both interfaces. Even if
>>>> the firmware I/F is not available, user can still run LED effect through the
>>>> sysfs.
>>>>
>>>> However, LP5562 and LP8501 support only single way which is the firmware
>>>> I/F. So user-space program for LP5562/8501 should be modified if lp55xx
>>>> removes the interface. My idea is
>>>
>>> Actually... it would be good to have some reasonable interface for RGB
>>> LEDs. This way, we need separate "firmware" for each LED
>>> controller. It would be good to have common format for LED effects.
>>
>> We still haven't tried trigger approach discussed over half a year ago.
>> If we used firmware approach we would still have to overcome the problem
>> of defining the LED class drivers affected by the firmware program.
> 
> The firmware approach is in the tree today :-(.

to RGB LEDs? What exactly do you have on mind?

> 
>>>> Device manufactures in Asia & North America requested lp55xx drivers, but I
>>>> don't know how many vendors uses the firmware I/F. Some vendors embeds the
>>>> binary code inside the driver instead of using user-program.
>>>
>>> Nokia N900 uses lp55xx, and I have custom scripts interfacing sysfs.
>>>
>>> Maemo uses the LEDs, too, but maemo is not open source.
>>>
>>> So no, I don't think there's anything important that could be broken.
>>
>> We can't guarantee that. Is there any problem in just using the
>> currently introduced DECLARE_FW_CUSTOM_FALLBACK() in
>> drivers/leds/leds-lp55xx-common.c?
> 
> Well, it would be good to get rid of the custom fallback
> functionality. And no, we don't need to "guarantee" that.  Removing
> obscure functionality noone uses is far game... providing noone
> complains ;-).

As Milo explained:

> Why has no one cried
> after the v4.0 custom fallback mechanism breaking ?

"Well, I don't know the reason exactly but my guess is they maybe still
using old kernel."

and after that:

"Device manufactures in Asia & North America requested lp55xx drivers"

These should be sufficient arguments for us for keeping the API
unchanged. If the users decided to upgrade their kernel then they
would be surprised by the API change.

DECLARE_FW_CUSTOM_FALLBACK macro seems to have been designed for
handling exactly this type of cases.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1543563 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Fri, Dec 16, 2016 at 12:27:00PM +0100, Pavel Machek wrote:
> On Fri 2016-12-16 11:56:48, Luis R. Rodriguez wrote:
> > On Fri, Dec 16, 2016 at 11:14:05AM +0100, Pavel Machek wrote:
> > > 
> > > Well, I was asking if the above snipped looks like valid use. Because
> > > AFAICT, the "custom fallback" is just dev_err(), see above. Coccinelle
> > > rules don't help me...
> > 
> > Its not. Its when you ask for no uevent. Only 2 drivers do this.
> 
> That was one of two you listed. If that is not valid use, perhaps it
> should be removed, not annotated?

Ah, well Milo Kim replied and described that the custom fallback is used as to
help load LED effect manually, and suggested a sysfs interface is more ideal [0]. I
agree however its also may be too late, and it depends how wide spread this "userspace"
that relies on this is, we just can't break it. Granted the custom fallback
mechanism was broken since v4.0 (see the fix "firmware: fix usermode helper
fallback loading") so one may argue no one seems to care...

So this is a judgement call, and the declaration is to point to documentation
to white list uses, as terrible as this one is userspace exists for it. but
more importantly to also help the SmPL grammar report to avoid reporting
already vetted cases. The alarm / cases for the 2 drivers has been issueed,
moving forward the lack of declaration with the custom fallback should trigger
a rant through 0-day so we don't run into the same stupid situation.

[0] https://marc.info/?l=linux-kernel&m=148168024112445

  Luis

[toc] | [prev] | [next] | [standalone]

#1542593 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

Hi Luis,

Thanks for the patch.

On 12/13/2016 04:08 AM, Luis R. Rodriguez wrote:
> We need to ensure that when driver developers use the custom firmware
> fallback mechanism it was not a copy and paste bug. These use cases on
> upstream drivers are rare, we only have 2 upstream users and its for
> really old drivers. Since valid uses are rare but possible enable a
> white-list for its use, and use this same white-list annotation to refer
> to the documentation covering the custom use case.
>
> New faulty users can be reported via 0-day now.
>
> Cc: Fengguang Wu <fengguang.wu@intel.com>
> 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>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  Documentation/driver-api/firmware/fallback-mechanisms.rst     | 7 +++++--
>  drivers/firmware/dell_rbu.c                                   | 1 +
>  drivers/leds/leds-lp55xx-common.c                             | 1 +
>  include/linux/firmware.h                                      | 7 +++++++
>  scripts/coccinelle/api/request_firmware-custom-fallback.cocci | 9 ++++++++-
>  5 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> index 955c11d6ff9d..b51673e40439 100644
> --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> @@ -184,8 +184,11 @@ 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::
> +Since it is a common driver developer mistake to enable it, driver developers
> +should use DECLARE_FW_CUSTOM_FALLBACK() to both white-list and validate their
> +use and also refer to the documentation for the custom loading solution.
> +
> +Invalid users of the custom fallback mechanism can be policed using::

double colon at the end of line

>
>          $ export COCCI=scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
>          $ make coccicheck MODE=report
> diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c
> index 2f452f1f7c8a..3f2aa35bc54d 100644
> --- a/drivers/firmware/dell_rbu.c
> +++ b/drivers/firmware/dell_rbu.c
> @@ -586,6 +586,7 @@ static ssize_t read_rbu_image_type(struct file *filp, struct kobject *kobj,
>  	return size;
>  }
>
> +DECLARE_FW_CUSTOM_FALLBACK("Documentation/dell_rbu.txt");
>  static ssize_t write_rbu_image_type(struct file *filp, struct kobject *kobj,
>  				    struct bin_attribute *bin_attr,
>  				    char *buffer, loff_t pos, size_t count)
> diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
> index 5377f22ff994..04161428ee3b 100644
> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
>  	release_firmware(chip->fw);
>  }
>
> +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
>  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
>  {

For this LED class driver:

Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>


>  	const char *name = chip->cl->name;
> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> index b1f9f0ccb8ac..e6ca19c03dcc 100644
> --- a/include/linux/firmware.h
> +++ b/include/linux/firmware.h
> @@ -8,6 +8,13 @@
>  #define FW_ACTION_NOHOTPLUG 0
>  #define FW_ACTION_HOTPLUG 1
>
> +/*
> + * Helper for scripts/coccinelle/api/request_firmware-custom-fallback.cocci
> + * and so users can also easily search for the documentation for the
> + * respectively needed custom fallback mechanism.
> + */
> +#define DECLARE_FW_CUSTOM_FALLBACK(__usermode_helper)
> +
>  struct firmware {
>  	size_t size;
>  	const u8 *data;
> diff --git a/scripts/coccinelle/api/request_firmware-custom-fallback.cocci b/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
> index c7598cfc4780..68cacab35b76 100644
> --- a/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
> +++ b/scripts/coccinelle/api/request_firmware-custom-fallback.cocci
> @@ -17,6 +17,13 @@
>  virtual report
>  virtual context
>
> +@ r0 depends on report || context @
> +declarer name DECLARE_FW_CUSTOM_FALLBACK;
> +expression E;
> +@@
> +
> +DECLARE_FW_CUSTOM_FALLBACK(E);
> +
>  @ r1 depends on report || context @
>  expression mod, name, dev, gfp, drv, cb;
>  position p;
> @@ -30,7 +37,7 @@ position p;
>  *request_firmware_nowait@p(mod, FW_ACTION_NOHOTPLUG, name, dev, gfp, drv, cb)
>  )
>
> -@script:python depends on report@
> +@script:python depends on report && !r0 @
>  p << r1.p;
>  @@
>
>

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1543304 — Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

On Thu, Dec 15, 2016 at 10:32:12AM +0100, Jacek Anaszewski wrote:
> > diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> > index 955c11d6ff9d..b51673e40439 100644
> > --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> > +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> > @@ -184,8 +184,11 @@ 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::
> > +Since it is a common driver developer mistake to enable it, driver developers
> > +should use DECLARE_FW_CUSTOM_FALLBACK() to both white-list and validate their
> > +use and also refer to the documentation for the custom loading solution.
> > +
> > +Invalid users of the custom fallback mechanism can be policed using::
> 
> double colon at the end of line

That is on purpose for rst files, for use with the new trendy hipster
Sphinx documentation format.

> > 
> >          $ export COCCI=scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
> >          $ make coccicheck MODE=report

It will kind of blockquote the above.

> > diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
> > index 5377f22ff994..04161428ee3b 100644
> > --- a/drivers/leds/leds-lp55xx-common.c
> > +++ b/drivers/leds/leds-lp55xx-common.c
> > @@ -219,6 +219,7 @@ static void lp55xx_firmware_loaded(const struct firmware *fw, void *context)
> >  	release_firmware(chip->fw);
> >  }
> > 
> > +DECLARE_FW_CUSTOM_FALLBACK("Documentation/leds/leds-lp55xx.txt");
> >  static int lp55xx_request_firmware(struct lp55xx_chip *chip)
> >  {
> 
> For this LED class driver:
> 
> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>

Thanks, amended!

  Luis

[toc] | [prev] | [next] | [standalone]

#1541052

Hi Luis,

On 12/13/2016 04:08 AM, Luis R. Rodriguez wrote:
> You may notice I've dropped the SmPL patches which complain on use of the
> API on init and probe -- although valid the context was off given the only
> valid use case was if you don't use initramfs, and that's a corner case.
> Fortunatley Daniel Wagner and Tom Gundersen have come up with some ideas
> that should help correct these issues, so I've dropped that grammar patch.

My BMW email address will soon be shutdown. I guess best to use my 
private instead: wagi@monom.org

cheers,
daniel

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web