All of lore.kernel.org
 help / color / mirror / Atom feed
* [1/2] firmware: add more flexible request_firmware_async function
  2018-06-22 14:49 [PATCH 0/2] Avoid firmware warning in imx-sdma Sebastian Reichel
@ 2018-06-22 14:49 ` Sebastian Reichel
  0 siblings, 0 replies; 8+ messages in thread
From: Sebastian Reichel @ 2018-06-22 14:49 UTC (permalink / raw)
  To: Luis R. Rodriguez, Greg Kroah-Hartman, Dan Williams, Vinod Koul
  Cc: dmaengine, linux-kernel, Rafał Miłecki, Sebastian Reichel

From: Rafał Miłecki <rafal@milecki.pl>

So far we got only one function for loading firmware asynchronously:
request_firmware_nowait. It didn't allow much customization of firmware
loading process - there is only one bool uevent argument. Moreover this
bool also controls user helper in an unclear way.

Some drivers need more flexible helper providing more options. This
includes control over uevent or warning for the missing firmware. Of
course this list may grow up in the future.

To handle this easily this patch adds a generic request_firmware_async
function. It takes struct with options as an argument which will allow
extending it in the future without massive changes.

This is a really cheap change (no new independent API) which works
nicely with the existing code. The old request_firmware_nowait is kept
as a simple helper calling new helper.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
[rebased to v4.18-rc1]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/base/firmware_loader/main.c | 41 ++++++++++++++++++++++++-----
 include/linux/firmware.h            | 20 ++++++++++++++
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 0943e7065e0e..e1d2ba570ea3 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -774,7 +774,7 @@ static void request_firmware_work_func(struct work_struct *work)
 	_request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0,
 			  fw_work->opt_flags);
 	fw_work->cont(fw, fw_work->context);
-	put_device(fw_work->device); /* taken in request_firmware_nowait() */
+	put_device(fw_work->device); /* taken in __request_firmware_nowait() */
 
 	module_put(fw_work->module);
 	kfree_const(fw_work->name);
@@ -782,8 +782,9 @@ static void request_firmware_work_func(struct work_struct *work)
 }
 
 /**
- * request_firmware_nowait() - asynchronous version of request_firmware
+ * __request_firmware_nowait() - asynchronous version of request_firmware
  * @module: module requesting the firmware
+ * @opt_flags: flags that control firmware loading process, see FW_OPT_*
  * @uevent: sends uevent to copy the firmware image if this flag
  *	is non-zero else the firmware copy must be done manually.
  * @name: name of firmware file
@@ -804,13 +805,14 @@ static void request_firmware_work_func(struct work_struct *work)
  *
  *		- can't sleep at all if @gfp is GFP_ATOMIC.
  **/
-int
-request_firmware_nowait(
-	struct module *module, bool uevent,
+static int
+__request_firmware_nowait(
+	struct module *module, unsigned int opt_flags,
 	const char *name, struct device *device, gfp_t gfp, void *context,
 	void (*cont)(const struct firmware *fw, void *context))
 {
 	struct firmware_work *fw_work;
+	bool uevent = !!(opt_flags & FW_OPT_UEVENT);
 
 	fw_work = kzalloc(sizeof(struct firmware_work), gfp);
 	if (!fw_work)
@@ -825,8 +827,7 @@ request_firmware_nowait(
 	fw_work->device = device;
 	fw_work->context = context;
 	fw_work->cont = cont;
-	fw_work->opt_flags = FW_OPT_NOWAIT |
-		(uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
+	fw_work->opt_flags = FW_OPT_NOWAIT | opt_flags;
 
 	if (!uevent && fw_cache_is_setup(device, name)) {
 		kfree_const(fw_work->name);
@@ -845,8 +846,34 @@ request_firmware_nowait(
 	schedule_work(&fw_work->work);
 	return 0;
 }
+
+int request_firmware_nowait(struct module *module, bool uevent,
+			    const char *name, struct device *device, gfp_t gfp,
+			    void *context,
+			    void (*cont)(const struct firmware *fw, void *context))
+{
+	unsigned int opt_flags = (uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
+
+	return __request_firmware_nowait(module, opt_flags, name, device, gfp,
+					 context, cont);
+}
 EXPORT_SYMBOL(request_firmware_nowait);
 
+int __request_firmware_async(struct module *module, const char *name,
+			     struct firmware_opts *fw_opts, struct device *dev,
+			     void *context,
+			     void (*cont)(const struct firmware *fw, void *context))
+{
+	unsigned int opt_flags = FW_OPT_UEVENT;
+
+	if (fw_opts->optional)
+		opt_flags |= FW_OPT_NO_WARN;
+
+	return __request_firmware_nowait(module, opt_flags, name, dev,
+					 GFP_KERNEL, context, cont);
+}
+EXPORT_SYMBOL(__request_firmware_async);
+
 #ifdef CONFIG_PM_SLEEP
 static ASYNC_DOMAIN_EXCLUSIVE(fw_cache_domain);
 
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 2dd566c91d44..ad2fc6e7fee7 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -27,6 +27,10 @@ struct builtin_fw {
 	unsigned long size;
 };
 
+struct firmware_opts {
+	bool optional;
+};
+
 /* We have to play tricks here much like stringify() to get the
    __COUNTER__ macro to be expanded as we want it */
 #define __fw_concat1(x, y) x##y
@@ -52,6 +56,10 @@ int request_firmware_direct(const struct firmware **fw, const char *name,
 			    struct device *device);
 int request_firmware_into_buf(const struct firmware **firmware_p,
 	const char *name, struct device *device, void *buf, size_t size);
+int __request_firmware_async(struct module *module, const char *name,
+			     struct firmware_opts *fw_opts, struct device *dev,
+			     void *context,
+			     void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
 #else
@@ -94,8 +102,20 @@ static inline int request_firmware_into_buf(const struct firmware **firmware_p,
 	return -EINVAL;
 }
 
+int __request_firmware_async(struct module *module, const char *name,
+			     struct firmware_opts *fw_opts, struct device *dev,
+			     void *context,
+			     void (*cont)(const struct firmware *fw, void *context))
+{
+	return -EINVAL;
+}
+
 #endif
 
 int firmware_request_cache(struct device *device, const char *name);
 
+#define request_firmware_async(name, fw_opts, dev, context, cont)	\
+	__request_firmware_async(THIS_MODULE, name, fw_opts, dev,	\
+				 context, cont)
+
 #endif

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [1/2] firmware: add more flexible request_firmware_async function
  2018-06-22 14:49 ` [PATCH 1/2] " Sebastian Reichel
@ 2018-06-23  0:52 ` kbuild test robot
  -1 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2018-06-23  0:52 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: kbuild-all, Luis R. Rodriguez, Greg Kroah-Hartman, Dan Williams,
	Vinod Koul, dmaengine, linux-kernel, Rafał Miłecki

Hi Rafał,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc1 next-20180622]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Reichel/firmware-add-more-flexible-request_firmware_async-function/20180623-025750
config: powerpc-mpc834x_mds_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/intel/e100.o: In function `__request_firmware_async':
>> e100.c:(.text+0x4f40): multiple definition of `__request_firmware_async'
   drivers/base/firmware_loader/fallback_table.o:fallback_table.c:(.text+0x0): first defined here
---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [2/2] dmaengine: imx-sdma: request firmware with FW_OPT_NO_WARN
  2018-06-22 14:49 [PATCH 0/2] Avoid firmware warning in imx-sdma Sebastian Reichel
@ 2018-06-22 14:49 ` Sebastian Reichel
  0 siblings, 0 replies; 8+ messages in thread
From: Sebastian Reichel @ 2018-06-22 14:49 UTC (permalink / raw)
  To: Luis R. Rodriguez, Greg Kroah-Hartman, Dan Williams, Vinod Koul
  Cc: dmaengine, linux-kernel, Rafał Miłecki, Sebastian Reichel

Request firmware with FW_OPT_NO_WARN. The driver works without the
firmware by using the one supplied in ROM. There is already an info
message, that informs about this.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/dma/imx-sdma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index f077992635c2..415ffe68ff77 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1599,11 +1599,13 @@ static int sdma_event_remap(struct sdma_engine *sdma)
 static int sdma_get_firmware(struct sdma_engine *sdma,
 		const char *fw_name)
 {
+	struct firmware_opts fw_opts = {
+		.optional = true,
+	};
 	int ret;
 
-	ret = request_firmware_nowait(THIS_MODULE,
-			FW_ACTION_HOTPLUG, fw_name, sdma->dev,
-			GFP_KERNEL, sdma, sdma_load_firmware);
+	ret = request_firmware_async(fw_name, &fw_opts, sdma->dev,
+				     sdma, sdma_load_firmware);
 
 	return ret;
 }

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 0/2] Avoid firmware warning in imx-sdma
@ 2018-06-22 14:49 Sebastian Reichel
  2018-06-28  3:13 ` Luis R. Rodriguez
  0 siblings, 1 reply; 8+ messages in thread
From: Sebastian Reichel @ 2018-06-22 14:49 UTC (permalink / raw)
  To: Luis R. Rodriguez, Greg Kroah-Hartman, Dan Williams, Vinod Koul
  Cc: dmaengine, linux-kernel, Rafał Miłecki, Sebastian Reichel

Subject: Avoid firmware warning in imx-sdma

Hi,

I grabbed the first patch from patchwork from an 2017 patch series. As far as I
could see, their usecase vanished due to switching to sync FW API (that already
supports NOWARN). This series fixes a kernel warning in imx-sdma driver, which
works fine with ROM firmware (and already prints an info message about ROM
firmware being used due to missing firmware file). This has been tested on
arch/arm/boot/dts/imx53-ppd.dts.

-- Sebastian

Rafał Miłecki (1):
  firmware: add more flexible request_firmware_async function

Sebastian Reichel (1):
  dmaengine: imx-sdma: request firmware with FW_OPT_NO_WARN

 drivers/base/firmware_loader/main.c | 41 ++++++++++++++++++++++++-----
 drivers/dma/imx-sdma.c              |  8 +++---
 include/linux/firmware.h            | 20 ++++++++++++++
 3 files changed, 59 insertions(+), 10 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-06-28  3:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-22 14:49 [1/2] firmware: add more flexible request_firmware_async function Sebastian Reichel
2018-06-22 14:49 ` [PATCH 1/2] " Sebastian Reichel
  -- strict thread matches above, loose matches on Subject: below --
2018-06-23  0:52 [1/2] " kbuild test robot
2018-06-23  0:52 ` [PATCH 1/2] " kbuild test robot
2018-06-22 14:49 [2/2] dmaengine: imx-sdma: request firmware with FW_OPT_NO_WARN Sebastian Reichel
2018-06-22 14:49 ` [PATCH 2/2] " Sebastian Reichel
2018-06-22 14:49 [PATCH 0/2] Avoid firmware warning in imx-sdma Sebastian Reichel
2018-06-28  3:13 ` Luis R. Rodriguez

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.