[RFC PATCH v1 00/15] firmware loader: introduce cache/uncache firmware

[RFC PATCH v1 00/15] firmware loader: introduce cache/uncache firmware

[Ming Lei]

Hi,

In [1][2], the problem below has been discussed for some time:

        device's firmware may be lost during suspend/resume
        cycle because device might be unplugged and plugged again
        or device experiences system power loss during the period.
        But in resume path, system is still not ready(process
        frozen, rootfs not usable, ...) to complete loading firmware
        from user space for devices.

The conclusion is that caching firmware during suspend/resume cycle
is capable of solving the problem.

This patchset implements cache/uncache firmware mechanism,
and apply the mechnism to cache device's firmware in kernel memory
space automatically during suspend/resume cyclye, so device can
load its firmware easily in its resume path. When resume is completed
and system is ready, the cached firmware will be removed from
kernel memory later.

The patch 15/15 is one example to apply the firmware cache mechanism on
ath9k-htc driver.

Even there are some corener cases[3] which can't be solved by the
cache approach, but as discussed in[4], the problem can be easily
fixed by a simple patch written by Linus.

This patch set is against 3.6.0-rc1-next-20120803.

[1]. http://marc.info/?t=134278790800004&r=1&w=2
[2]. http://marc.info/?t=132528956000002&r=10&w=2
[3]. http://marc.info/?l=linux-usb&m=132554118928398&w=2
[4]. http://marc.info/?l=linux-kernel&m=134323730805443&w=2  

--
V1:
	-handle vmap failure case(1/15)
	-fix a memory leak of 'firmware_buf'(5/15)
	-fix oops during failure path of requesting firmware(6/15)
	-fix vmap more than one time(6/15)
	-introduce __fw_lookup_buf to avoid code duplication(7/15)
	-fix comment of request_firmware_nowait(9/15)
	-avoid releasing lock in devres_for_each_res(11/15)
	-use new devres iterator API to create fw cache entry(12/15)
	-rename some functions and data structures(12/15)
	-some code style fixes

	Thanks for Borislav's review!

--
 drivers/base/devres.c                    |   42 ++
 drivers/base/firmware_class.c            |  764 ++++++++++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/hif_usb.c |   34 +-
 drivers/net/wireless/ath/ath9k/hif_usb.h |    4 +-
 include/linux/device.h                   |    4 +
 include/linux/firmware.h                 |   15 +
 6 files changed, 747 insertions(+), 116 deletions(-)


Thanks,
--
Ming Lei

[RFC PATCH v1 01/15] firmware loader: simplify pages ownership transfer

[Ming Lei]

This patch doesn't transfer ownership of pages' buffer to the
instance of firmware until the firmware loading is completed,
which will simplify firmware_loading_store a lot, so help
to introduce the following cache_firmware and uncache_firmware
mechanism during system suspend-resume cycle.

In fact, this patch fixes one bug: if writing data into
firmware loader device is bypassed between writting 1 and 0 to
'loading', OOPS will be triggered without the patch.

Also handle the vmap failure case, and add some comments to make
code more readable.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |   62 ++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 803cfc1..1cbefcf 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -93,6 +93,8 @@ struct firmware_priv {
 	struct completion completion;
 	struct firmware *fw;
 	unsigned long status;
+	void *data;
+	size_t size;
 	struct page **pages;
 	int nr_pages;
 	int page_array_size;
@@ -156,9 +158,11 @@ static void fw_dev_release(struct device *dev)
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
 	int i;
 
+	/* free untransfered pages buffer */
 	for (i = 0; i < fw_priv->nr_pages; i++)
 		__free_page(fw_priv->pages[i]);
 	kfree(fw_priv->pages);
+
 	kfree(fw_priv);
 
 	module_put(THIS_MODULE);
@@ -194,6 +198,7 @@ static ssize_t firmware_loading_show(struct device *dev,
 	return sprintf(buf, "%d\n", loading);
 }
 
+/* firmware holds the ownership of pages */
 static void firmware_free_data(const struct firmware *fw)
 {
 	int i;
@@ -237,9 +242,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 
 	switch (loading) {
 	case 1:
-		firmware_free_data(fw_priv->fw);
-		memset(fw_priv->fw, 0, sizeof(struct firmware));
-		/* If the pages are not owned by 'struct firmware' */
+		/* discarding any previous partial load */
 		for (i = 0; i < fw_priv->nr_pages; i++)
 			__free_page(fw_priv->pages[i]);
 		kfree(fw_priv->pages);
@@ -250,20 +253,6 @@ static ssize_t firmware_loading_store(struct device *dev,
 		break;
 	case 0:
 		if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) {
-			vunmap(fw_priv->fw->data);
-			fw_priv->fw->data = vmap(fw_priv->pages,
-						 fw_priv->nr_pages,
-						 0, PAGE_KERNEL_RO);
-			if (!fw_priv->fw->data) {
-				dev_err(dev, "%s: vmap() failed\n", __func__);
-				goto err;
-			}
-			/* Pages are now owned by 'struct firmware' */
-			fw_priv->fw->pages = fw_priv->pages;
-			fw_priv->pages = NULL;
-
-			fw_priv->page_array_size = 0;
-			fw_priv->nr_pages = 0;
 			complete(&fw_priv->completion);
 			clear_bit(FW_STATUS_LOADING, &fw_priv->status);
 			break;
@@ -273,7 +262,6 @@ static ssize_t firmware_loading_store(struct device *dev,
 		dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
 		/* fallthrough */
 	case -1:
-	err:
 		fw_load_abort(fw_priv);
 		break;
 	}
@@ -299,12 +287,12 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
 		ret_count = -ENODEV;
 		goto out;
 	}
-	if (offset > fw->size) {
+	if (offset > fw_priv->size) {
 		ret_count = 0;
 		goto out;
 	}
-	if (count > fw->size - offset)
-		count = fw->size - offset;
+	if (count > fw_priv->size - offset)
+		count = fw_priv->size - offset;
 
 	ret_count = count;
 
@@ -396,6 +384,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
 		retval = -ENODEV;
 		goto out;
 	}
+
 	retval = fw_realloc_buffer(fw_priv, offset + count);
 	if (retval)
 		goto out;
@@ -418,7 +407,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
 		count -= page_cnt;
 	}
 
-	fw->size = max_t(size_t, offset, fw->size);
+	fw_priv->size = max_t(size_t, offset, fw_priv->size);
 out:
 	mutex_unlock(&fw_lock);
 	return retval;
@@ -504,6 +493,29 @@ static void _request_firmware_cleanup(const struct firmware **firmware_p)
 	*firmware_p = NULL;
 }
 
+/* transfer the ownership of pages to firmware */
+static int fw_set_page_data(struct firmware_priv *fw_priv)
+{
+	struct firmware *fw = fw_priv->fw;
+
+	fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages,
+				0, PAGE_KERNEL_RO);
+	if (!fw_priv->data)
+		return -ENOMEM;
+
+	fw->data = fw_priv->data;
+	fw->pages = fw_priv->pages;
+	fw->size = fw_priv->size;
+
+	WARN_ON(PFN_UP(fw->size) != fw_priv->nr_pages);
+
+	fw_priv->nr_pages = 0;
+	fw_priv->pages = NULL;
+	fw_priv->data = NULL;
+
+	return 0;
+}
+
 static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 				  long timeout)
 {
@@ -549,8 +561,12 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 	del_timer_sync(&fw_priv->timeout);
 
 	mutex_lock(&fw_lock);
-	if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status))
+	if (!fw_priv->size || test_bit(FW_STATUS_ABORT, &fw_priv->status))
 		retval = -ENOENT;
+
+	/* transfer pages ownership at the last minute */
+	if (!retval)
+		retval = fw_set_page_data(fw_priv);
 	fw_priv->fw = NULL;
 	mutex_unlock(&fw_lock);
 
-- 
1.7.9.5

[RFC PATCH v1 02/15] firmware loader: fix races during loading firmware

[Ming Lei]

This patch fixes two races in loading firmware:

1, FW_STATUS_DONE should be set before waking up the task waitting
on _request_firmware_load, otherwise FW_STATUS_ABORT may be
thought as DONE mistakenly.

2, Inside _request_firmware_load(), there is a small window between
wait_for_completion() and mutex_lock(&fw_lock), and 'echo 1 > loading'
still may happen during the period, so this patch checks FW_STATUS_DONE
to prevent pages' buffer completed from being freed in firmware_loading_store.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 1cbefcf..1915ad8 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -243,18 +243,21 @@ static ssize_t firmware_loading_store(struct device *dev,
 	switch (loading) {
 	case 1:
 		/* discarding any previous partial load */
-		for (i = 0; i < fw_priv->nr_pages; i++)
-			__free_page(fw_priv->pages[i]);
-		kfree(fw_priv->pages);
-		fw_priv->pages = NULL;
-		fw_priv->page_array_size = 0;
-		fw_priv->nr_pages = 0;
-		set_bit(FW_STATUS_LOADING, &fw_priv->status);
+		if (!test_bit(FW_STATUS_DONE, &fw_priv->status)) {
+			for (i = 0; i < fw_priv->nr_pages; i++)
+				__free_page(fw_priv->pages[i]);
+			kfree(fw_priv->pages);
+			fw_priv->pages = NULL;
+			fw_priv->page_array_size = 0;
+			fw_priv->nr_pages = 0;
+			set_bit(FW_STATUS_LOADING, &fw_priv->status);
+		}
 		break;
 	case 0:
 		if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) {
-			complete(&fw_priv->completion);
+			set_bit(FW_STATUS_DONE, &fw_priv->status);
 			clear_bit(FW_STATUS_LOADING, &fw_priv->status);
+			complete(&fw_priv->completion);
 			break;
 		}
 		/* fallthrough */
@@ -557,7 +560,6 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 
 	wait_for_completion(&fw_priv->completion);
 
-	set_bit(FW_STATUS_DONE, &fw_priv->status);
 	del_timer_sync(&fw_priv->timeout);
 
 	mutex_lock(&fw_lock);
-- 
1.7.9.5

[RFC PATCH v1 03/15] firmware loader: remove unnecessary wmb()

[Ming Lei]

The wmb() inside fw_load_abort is not necessary, since
complete() and wait_on_completion() has implied one pair
of memory barrier.

Also wmb() isn't a correct usage, so just remove it.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 1915ad8..0bd09c7 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -112,7 +112,6 @@ static struct firmware_priv *to_firmware_priv(struct device *dev)
 static void fw_load_abort(struct firmware_priv *fw_priv)
 {
 	set_bit(FW_STATUS_ABORT, &fw_priv->status);
-	wmb();
 	complete(&fw_priv->completion);
 }
 
-- 
1.7.9.5

[RFC PATCH v1 04/15] firmware loader: fix creation failure of fw loader device

[Ming Lei]

If one device driver calls request_firmware_nowait() to request
several different firmwares' loading, device_add() will return
failure since all firmware loader device use same name of the
device who is requesting firmware.

This patch always use the name of firmware image as the firmware
loader device name to fix the problem since the following patches
for caching firmware will make sure only one loading for same
firmware is alllowd at the same time.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 0bd09c7..04c75b5 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -452,7 +452,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 	f_dev = &fw_priv->dev;
 
 	device_initialize(f_dev);
-	dev_set_name(f_dev, "%s", dev_name(device));
+	dev_set_name(f_dev, "%s", fw_name);
 	f_dev->parent = device;
 	f_dev->class = &firmware_class;
 
-- 
1.7.9.5

[RFC PATCH v1 05/15] firmware loader: introduce firmware_buf

[Ming Lei]

This patch introduces struct firmware_buf to describe the buffer
which holds the firmware data, which will make the following
cache_firmware/uncache_firmware implemented easily.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |  180 +++++++++++++++++++++++------------------
 1 file changed, 102 insertions(+), 78 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 04c75b5..5f2076e 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -89,7 +89,7 @@ static inline long firmware_loading_timeout(void)
  * guarding for corner cases a global lock should be OK */
 static DEFINE_MUTEX(fw_lock);
 
-struct firmware_priv {
+struct firmware_buf {
 	struct completion completion;
 	struct firmware *fw;
 	unsigned long status;
@@ -98,10 +98,14 @@ struct firmware_priv {
 	struct page **pages;
 	int nr_pages;
 	int page_array_size;
+	char fw_id[];
+};
+
+struct firmware_priv {
 	struct timer_list timeout;
-	struct device dev;
 	bool nowait;
-	char fw_id[];
+	struct device dev;
+	struct firmware_buf *buf;
 };
 
 static struct firmware_priv *to_firmware_priv(struct device *dev)
@@ -111,8 +115,10 @@ static struct firmware_priv *to_firmware_priv(struct device *dev)
 
 static void fw_load_abort(struct firmware_priv *fw_priv)
 {
-	set_bit(FW_STATUS_ABORT, &fw_priv->status);
-	complete(&fw_priv->completion);
+	struct firmware_buf *buf = fw_priv->buf;
+
+	set_bit(FW_STATUS_ABORT, &buf->status);
+	complete(&buf->completion);
 }
 
 static ssize_t firmware_timeout_show(struct class *class,
@@ -152,15 +158,21 @@ static struct class_attribute firmware_class_attrs[] = {
 	__ATTR_NULL
 };
 
-static void fw_dev_release(struct device *dev)
+static void fw_free_buf(struct firmware_buf *buf)
 {
-	struct firmware_priv *fw_priv = to_firmware_priv(dev);
 	int i;
 
-	/* free untransfered pages buffer */
-	for (i = 0; i < fw_priv->nr_pages; i++)
-		__free_page(fw_priv->pages[i]);
-	kfree(fw_priv->pages);
+	if (!buf)
+		return;
+
+	for (i = 0; i < buf->nr_pages; i++)
+		__free_page(buf->pages[i]);
+	kfree(buf->pages);
+}
+
+static void fw_dev_release(struct device *dev)
+{
+	struct firmware_priv *fw_priv = to_firmware_priv(dev);
 
 	kfree(fw_priv);
 
@@ -171,7 +183,7 @@ static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
 
-	if (add_uevent_var(env, "FIRMWARE=%s", fw_priv->fw_id))
+	if (add_uevent_var(env, "FIRMWARE=%s", fw_priv->buf->fw_id))
 		return -ENOMEM;
 	if (add_uevent_var(env, "TIMEOUT=%i", loading_timeout))
 		return -ENOMEM;
@@ -192,7 +204,7 @@ static ssize_t firmware_loading_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
-	int loading = test_bit(FW_STATUS_LOADING, &fw_priv->status);
+	int loading = test_bit(FW_STATUS_LOADING, &fw_priv->buf->status);
 
 	return sprintf(buf, "%d\n", loading);
 }
@@ -231,32 +243,33 @@ static ssize_t firmware_loading_store(struct device *dev,
 				      const char *buf, size_t count)
 {
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
+	struct firmware_buf *fw_buf = fw_priv->buf;
 	int loading = simple_strtol(buf, NULL, 10);
 	int i;
 
 	mutex_lock(&fw_lock);
 
-	if (!fw_priv->fw)
+	if (!fw_buf)
 		goto out;
 
 	switch (loading) {
 	case 1:
 		/* discarding any previous partial load */
-		if (!test_bit(FW_STATUS_DONE, &fw_priv->status)) {
-			for (i = 0; i < fw_priv->nr_pages; i++)
-				__free_page(fw_priv->pages[i]);
-			kfree(fw_priv->pages);
-			fw_priv->pages = NULL;
-			fw_priv->page_array_size = 0;
-			fw_priv->nr_pages = 0;
-			set_bit(FW_STATUS_LOADING, &fw_priv->status);
+		if (!test_bit(FW_STATUS_DONE, &fw_buf->status)) {
+			for (i = 0; i < fw_buf->nr_pages; i++)
+				__free_page(fw_buf->pages[i]);
+			kfree(fw_buf->pages);
+			fw_buf->pages = NULL;
+			fw_buf->page_array_size = 0;
+			fw_buf->nr_pages = 0;
+			set_bit(FW_STATUS_LOADING, &fw_buf->status);
 		}
 		break;
 	case 0:
-		if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) {
-			set_bit(FW_STATUS_DONE, &fw_priv->status);
-			clear_bit(FW_STATUS_LOADING, &fw_priv->status);
-			complete(&fw_priv->completion);
+		if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
+			set_bit(FW_STATUS_DONE, &fw_buf->status);
+			clear_bit(FW_STATUS_LOADING, &fw_buf->status);
+			complete(&fw_buf->completion);
 			break;
 		}
 		/* fallthrough */
@@ -280,21 +293,21 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
 {
 	struct device *dev = kobj_to_dev(kobj);
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
-	struct firmware *fw;
+	struct firmware_buf *buf;
 	ssize_t ret_count;
 
 	mutex_lock(&fw_lock);
-	fw = fw_priv->fw;
-	if (!fw || test_bit(FW_STATUS_DONE, &fw_priv->status)) {
+	buf = fw_priv->buf;
+	if (!buf || test_bit(FW_STATUS_DONE, &buf->status)) {
 		ret_count = -ENODEV;
 		goto out;
 	}
-	if (offset > fw_priv->size) {
+	if (offset > buf->size) {
 		ret_count = 0;
 		goto out;
 	}
-	if (count > fw_priv->size - offset)
-		count = fw_priv->size - offset;
+	if (count > buf->size - offset)
+		count = buf->size - offset;
 
 	ret_count = count;
 
@@ -304,11 +317,11 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
 		int page_ofs = offset & (PAGE_SIZE-1);
 		int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
 
-		page_data = kmap(fw_priv->pages[page_nr]);
+		page_data = kmap(buf->pages[page_nr]);
 
 		memcpy(buffer, page_data + page_ofs, page_cnt);
 
-		kunmap(fw_priv->pages[page_nr]);
+		kunmap(buf->pages[page_nr]);
 		buffer += page_cnt;
 		offset += page_cnt;
 		count -= page_cnt;
@@ -320,12 +333,13 @@ out:
 
 static int fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size)
 {
+	struct firmware_buf *buf = fw_priv->buf;
 	int pages_needed = ALIGN(min_size, PAGE_SIZE) >> PAGE_SHIFT;
 
 	/* If the array of pages is too small, grow it... */
-	if (fw_priv->page_array_size < pages_needed) {
+	if (buf->page_array_size < pages_needed) {
 		int new_array_size = max(pages_needed,
-					 fw_priv->page_array_size * 2);
+					 buf->page_array_size * 2);
 		struct page **new_pages;
 
 		new_pages = kmalloc(new_array_size * sizeof(void *),
@@ -334,24 +348,24 @@ static int fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size)
 			fw_load_abort(fw_priv);
 			return -ENOMEM;
 		}
-		memcpy(new_pages, fw_priv->pages,
-		       fw_priv->page_array_size * sizeof(void *));
-		memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) *
-		       (new_array_size - fw_priv->page_array_size));
-		kfree(fw_priv->pages);
-		fw_priv->pages = new_pages;
-		fw_priv->page_array_size = new_array_size;
+		memcpy(new_pages, buf->pages,
+		       buf->page_array_size * sizeof(void *));
+		memset(&new_pages[buf->page_array_size], 0, sizeof(void *) *
+		       (new_array_size - buf->page_array_size));
+		kfree(buf->pages);
+		buf->pages = new_pages;
+		buf->page_array_size = new_array_size;
 	}
 
-	while (fw_priv->nr_pages < pages_needed) {
-		fw_priv->pages[fw_priv->nr_pages] =
+	while (buf->nr_pages < pages_needed) {
+		buf->pages[buf->nr_pages] =
 			alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
 
-		if (!fw_priv->pages[fw_priv->nr_pages]) {
+		if (!buf->pages[buf->nr_pages]) {
 			fw_load_abort(fw_priv);
 			return -ENOMEM;
 		}
-		fw_priv->nr_pages++;
+		buf->nr_pages++;
 	}
 	return 0;
 }
@@ -374,15 +388,15 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
 {
 	struct device *dev = kobj_to_dev(kobj);
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
-	struct firmware *fw;
+	struct firmware_buf *buf;
 	ssize_t retval;
 
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
 
 	mutex_lock(&fw_lock);
-	fw = fw_priv->fw;
-	if (!fw || test_bit(FW_STATUS_DONE, &fw_priv->status)) {
+	buf = fw_priv->buf;
+	if (!buf || test_bit(FW_STATUS_DONE, &buf->status)) {
 		retval = -ENODEV;
 		goto out;
 	}
@@ -399,17 +413,17 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
 		int page_ofs = offset & (PAGE_SIZE - 1);
 		int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
 
-		page_data = kmap(fw_priv->pages[page_nr]);
+		page_data = kmap(buf->pages[page_nr]);
 
 		memcpy(page_data + page_ofs, buffer, page_cnt);
 
-		kunmap(fw_priv->pages[page_nr]);
+		kunmap(buf->pages[page_nr]);
 		buffer += page_cnt;
 		offset += page_cnt;
 		count -= page_cnt;
 	}
 
-	fw_priv->size = max_t(size_t, offset, fw_priv->size);
+	buf->size = max_t(size_t, offset, buf->size);
 out:
 	mutex_unlock(&fw_lock);
 	return retval;
@@ -434,20 +448,31 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 		   struct device *device, bool uevent, bool nowait)
 {
 	struct firmware_priv *fw_priv;
+	struct firmware_buf *buf;
 	struct device *f_dev;
 
-	fw_priv = kzalloc(sizeof(*fw_priv) + strlen(fw_name) + 1 , GFP_KERNEL);
+	fw_priv = kzalloc(sizeof(*fw_priv), GFP_KERNEL);
 	if (!fw_priv) {
 		dev_err(device, "%s: kmalloc failed\n", __func__);
-		return ERR_PTR(-ENOMEM);
+		fw_priv = ERR_PTR(-ENOMEM);
+		goto exit;
+	}
+
+	buf = kzalloc(sizeof(*buf) + strlen(fw_name) + 1, GFP_KERNEL);
+	if (!buf) {
+		dev_err(device, "%s: kmalloc failed\n", __func__);
+		kfree(fw_priv);
+		fw_priv = ERR_PTR(-ENOMEM);
+		goto exit;
 	}
 
-	fw_priv->fw = firmware;
+	buf->fw = firmware;
+	fw_priv->buf = buf;
 	fw_priv->nowait = nowait;
-	strcpy(fw_priv->fw_id, fw_name);
-	init_completion(&fw_priv->completion);
 	setup_timer(&fw_priv->timeout,
 		    firmware_class_timeout, (u_long) fw_priv);
+	strcpy(buf->fw_id, fw_name);
+	init_completion(&buf->completion);
 
 	f_dev = &fw_priv->dev;
 
@@ -455,7 +480,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 	dev_set_name(f_dev, "%s", fw_name);
 	f_dev->parent = device;
 	f_dev->class = &firmware_class;
-
+exit:
 	return fw_priv;
 }
 
@@ -496,24 +521,18 @@ static void _request_firmware_cleanup(const struct firmware **firmware_p)
 }
 
 /* transfer the ownership of pages to firmware */
-static int fw_set_page_data(struct firmware_priv *fw_priv)
+static int fw_set_page_data(struct firmware_buf *buf)
 {
-	struct firmware *fw = fw_priv->fw;
+	struct firmware *fw = buf->fw;
 
-	fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages,
-				0, PAGE_KERNEL_RO);
-	if (!fw_priv->data)
+	buf->data = vmap(buf->pages, buf->nr_pages, 0, PAGE_KERNEL_RO);
+	if (!buf->data)
 		return -ENOMEM;
 
-	fw->data = fw_priv->data;
-	fw->pages = fw_priv->pages;
-	fw->size = fw_priv->size;
-
-	WARN_ON(PFN_UP(fw->size) != fw_priv->nr_pages);
-
-	fw_priv->nr_pages = 0;
-	fw_priv->pages = NULL;
-	fw_priv->data = NULL;
+	fw->data = buf->data;
+	fw->pages = buf->pages;
+	fw->size = buf->size;
+	WARN_ON(PFN_UP(fw->size) != buf->nr_pages);
 
 	return 0;
 }
@@ -523,6 +542,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 {
 	int retval = 0;
 	struct device *f_dev = &fw_priv->dev;
+	struct firmware_buf *buf = fw_priv->buf;
 
 	dev_set_uevent_suppress(f_dev, true);
 
@@ -549,7 +569,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 
 	if (uevent) {
 		dev_set_uevent_suppress(f_dev, false);
-		dev_dbg(f_dev, "firmware: requesting %s\n", fw_priv->fw_id);
+		dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id);
 		if (timeout != MAX_SCHEDULE_TIMEOUT)
 			mod_timer(&fw_priv->timeout,
 				  round_jiffies_up(jiffies + timeout));
@@ -557,18 +577,22 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 		kobject_uevent(&fw_priv->dev.kobj, KOBJ_ADD);
 	}
 
-	wait_for_completion(&fw_priv->completion);
+	wait_for_completion(&buf->completion);
 
 	del_timer_sync(&fw_priv->timeout);
 
 	mutex_lock(&fw_lock);
-	if (!fw_priv->size || test_bit(FW_STATUS_ABORT, &fw_priv->status))
+	if (!buf->size || test_bit(FW_STATUS_ABORT, &buf->status))
 		retval = -ENOENT;
 
 	/* transfer pages ownership at the last minute */
 	if (!retval)
-		retval = fw_set_page_data(fw_priv);
-	fw_priv->fw = NULL;
+		retval = fw_set_page_data(buf);
+	if (retval)
+		fw_free_buf(buf); /* free untransfered pages buffer */
+
+	kfree(buf);
+	fw_priv->buf = NULL;
 	mutex_unlock(&fw_lock);
 
 	device_remove_file(f_dev, &dev_attr_loading);
-- 
1.7.9.5

[RFC PATCH v1 06/15] firmware loader: always let firmware_buf own the pages buffer

[Ming Lei]

This patch always let firmware_buf own the pages buffer allocated
inside firmware_data_write, and add all instances of firmware_buf
into the firmware cache global list. Also introduce one private field
in 'struct firmware', so release_firmware will see the instance of
firmware_buf associated with the current firmware instance, then just
'free' the instance of firmware_buf.

The firmware_buf instance represents one pages buffer for one
firmware image, so lots of firmware loading requests can share
the same firmware_buf instance if they request the same firmware
image file.

This patch will make implementation of the following cache_firmware/
uncache_firmware very easy and simple.

In fact, the patch improves request_formware/release_firmware:

        - only request userspace to write firmware image once if
	several devices share one same firmware image and its drivers
	call request_firmware concurrently.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |  240 +++++++++++++++++++++++++++++------------
 include/linux/firmware.h      |    3 +
 2 files changed, 174 insertions(+), 69 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 5f2076e..848ad97 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -21,6 +21,7 @@
 #include <linux/firmware.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/list.h>
 
 MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
@@ -85,13 +86,17 @@ static inline long firmware_loading_timeout(void)
 	return loading_timeout > 0 ? loading_timeout * HZ : MAX_SCHEDULE_TIMEOUT;
 }
 
-/* fw_lock could be moved to 'struct firmware_priv' but since it is just
- * guarding for corner cases a global lock should be OK */
-static DEFINE_MUTEX(fw_lock);
+struct firmware_cache {
+	/* firmware_buf instance will be added into the below list */
+	spinlock_t lock;
+	struct list_head head;
+};
 
 struct firmware_buf {
+	struct kref ref;
+	struct list_head list;
 	struct completion completion;
-	struct firmware *fw;
+	struct firmware_cache *fwc;
 	unsigned long status;
 	void *data;
 	size_t size;
@@ -106,8 +111,94 @@ struct firmware_priv {
 	bool nowait;
 	struct device dev;
 	struct firmware_buf *buf;
+	struct firmware *fw;
 };
 
+#define to_fwbuf(d) container_of(d, struct firmware_buf, ref)
+
+/* fw_lock could be moved to 'struct firmware_priv' but since it is just
+ * guarding for corner cases a global lock should be OK */
+static DEFINE_MUTEX(fw_lock);
+
+static struct firmware_cache fw_cache;
+
+static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
+					      struct firmware_cache *fwc)
+{
+	struct firmware_buf *buf;
+
+	buf = kzalloc(sizeof(*buf) + strlen(fw_name) + 1 , GFP_ATOMIC);
+
+	if (!buf)
+		return buf;
+
+	kref_init(&buf->ref);
+	strcpy(buf->fw_id, fw_name);
+	buf->fwc = fwc;
+	init_completion(&buf->completion);
+
+	pr_debug("%s: fw-%s buf=%p\n", __func__, fw_name, buf);
+
+	return buf;
+}
+
+static int fw_lookup_and_allocate_buf(const char *fw_name,
+				      struct firmware_cache *fwc,
+				      struct firmware_buf **buf)
+{
+	struct firmware_buf *tmp;
+
+	spin_lock(&fwc->lock);
+	list_for_each_entry(tmp, &fwc->head, list)
+		if (!strcmp(tmp->fw_id, fw_name)) {
+			kref_get(&tmp->ref);
+			spin_unlock(&fwc->lock);
+			*buf = tmp;
+			return 1;
+		}
+
+	tmp = __allocate_fw_buf(fw_name, fwc);
+	if (tmp)
+		list_add(&tmp->list, &fwc->head);
+	spin_unlock(&fwc->lock);
+
+	*buf = tmp;
+
+	return tmp ? 0 : -ENOMEM;
+}
+
+static void __fw_free_buf(struct kref *ref)
+{
+	struct firmware_buf *buf = to_fwbuf(ref);
+	struct firmware_cache *fwc = buf->fwc;
+	int i;
+
+	pr_debug("%s: fw-%s buf=%p data=%p size=%u\n",
+		 __func__, buf->fw_id, buf, buf->data,
+		 (unsigned int)buf->size);
+
+	spin_lock(&fwc->lock);
+	list_del(&buf->list);
+	spin_unlock(&fwc->lock);
+
+	vunmap(buf->data);
+	for (i = 0; i < buf->nr_pages; i++)
+		__free_page(buf->pages[i]);
+	kfree(buf->pages);
+	kfree(buf);
+}
+
+static void fw_free_buf(struct firmware_buf *buf)
+{
+	kref_put(&buf->ref, __fw_free_buf);
+}
+
+static void __init fw_cache_init(void)
+{
+	spin_lock_init(&fw_cache.lock);
+	INIT_LIST_HEAD(&fw_cache.head);
+}
+
 static struct firmware_priv *to_firmware_priv(struct device *dev)
 {
 	return container_of(dev, struct firmware_priv, dev);
@@ -118,7 +209,7 @@ static void fw_load_abort(struct firmware_priv *fw_priv)
 	struct firmware_buf *buf = fw_priv->buf;
 
 	set_bit(FW_STATUS_ABORT, &buf->status);
-	complete(&buf->completion);
+	complete_all(&buf->completion);
 }
 
 static ssize_t firmware_timeout_show(struct class *class,
@@ -158,18 +249,6 @@ static struct class_attribute firmware_class_attrs[] = {
 	__ATTR_NULL
 };
 
-static void fw_free_buf(struct firmware_buf *buf)
-{
-	int i;
-
-	if (!buf)
-		return;
-
-	for (i = 0; i < buf->nr_pages; i++)
-		__free_page(buf->pages[i]);
-	kfree(buf->pages);
-}
-
 static void fw_dev_release(struct device *dev)
 {
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
@@ -212,13 +291,8 @@ static ssize_t firmware_loading_show(struct device *dev,
 /* firmware holds the ownership of pages */
 static void firmware_free_data(const struct firmware *fw)
 {
-	int i;
-	vunmap(fw->data);
-	if (fw->pages) {
-		for (i = 0; i < PFN_UP(fw->size); i++)
-			__free_page(fw->pages[i]);
-		kfree(fw->pages);
-	}
+	WARN_ON(!fw->priv);
+	fw_free_buf(fw->priv);
 }
 
 /* Some architectures don't have PAGE_KERNEL_RO */
@@ -269,7 +343,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 		if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) {
 			set_bit(FW_STATUS_DONE, &fw_buf->status);
 			clear_bit(FW_STATUS_LOADING, &fw_buf->status);
-			complete(&fw_buf->completion);
+			complete_all(&fw_buf->completion);
 			break;
 		}
 		/* fallthrough */
@@ -448,7 +522,6 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 		   struct device *device, bool uevent, bool nowait)
 {
 	struct firmware_priv *fw_priv;
-	struct firmware_buf *buf;
 	struct device *f_dev;
 
 	fw_priv = kzalloc(sizeof(*fw_priv), GFP_KERNEL);
@@ -458,21 +531,10 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 		goto exit;
 	}
 
-	buf = kzalloc(sizeof(*buf) + strlen(fw_name) + 1, GFP_KERNEL);
-	if (!buf) {
-		dev_err(device, "%s: kmalloc failed\n", __func__);
-		kfree(fw_priv);
-		fw_priv = ERR_PTR(-ENOMEM);
-		goto exit;
-	}
-
-	buf->fw = firmware;
-	fw_priv->buf = buf;
 	fw_priv->nowait = nowait;
+	fw_priv->fw = firmware;
 	setup_timer(&fw_priv->timeout,
 		    firmware_class_timeout, (u_long) fw_priv);
-	strcpy(buf->fw_id, fw_name);
-	init_completion(&buf->completion);
 
 	f_dev = &fw_priv->dev;
 
@@ -484,12 +546,42 @@ exit:
 	return fw_priv;
 }
 
+/* one pages buffer is mapped/unmapped only once */
+static int fw_map_pages_buf(struct firmware_buf *buf)
+{
+	buf->data = vmap(buf->pages, buf->nr_pages, 0, PAGE_KERNEL_RO);
+	if (!buf->data)
+		return -ENOMEM;
+	return 0;
+}
+
+/* store the pages buffer info firmware from buf */
+static void fw_set_page_data(struct firmware_buf *buf, struct firmware *fw)
+{
+	fw->priv = buf;
+	fw->pages = buf->pages;
+	fw->size = buf->size;
+	fw->data = buf->data;
+
+	pr_debug("%s: fw-%s buf=%p data=%p size=%u\n",
+		 __func__, buf->fw_id, buf, buf->data,
+		 (unsigned int)buf->size);
+}
+
+static void _request_firmware_cleanup(const struct firmware **firmware_p)
+{
+	release_firmware(*firmware_p);
+	*firmware_p = NULL;
+}
+
 static struct firmware_priv *
 _request_firmware_prepare(const struct firmware **firmware_p, const char *name,
 			  struct device *device, bool uevent, bool nowait)
 {
 	struct firmware *firmware;
-	struct firmware_priv *fw_priv;
+	struct firmware_priv *fw_priv = NULL;
+	struct firmware_buf *buf;
+	int ret;
 
 	if (!firmware_p)
 		return ERR_PTR(-EINVAL);
@@ -506,35 +598,45 @@ _request_firmware_prepare(const struct firmware **firmware_p, const char *name,
 		return NULL;
 	}
 
-	fw_priv = fw_create_instance(firmware, name, device, uevent, nowait);
-	if (IS_ERR(fw_priv)) {
-		release_firmware(firmware);
+	ret = fw_lookup_and_allocate_buf(name, &fw_cache, &buf);
+	if (!ret)
+		fw_priv = fw_create_instance(firmware, name, device,
+				uevent, nowait);
+
+	if (IS_ERR(fw_priv) || ret < 0) {
+		kfree(firmware);
 		*firmware_p = NULL;
+		return ERR_PTR(-ENOMEM);
+	} else if (fw_priv) {
+		fw_priv->buf = buf;
+
+		/*
+		 * bind with 'buf' now to avoid warning in failure path
+		 * of requesting firmware.
+		 */
+		firmware->priv = buf;
+		return fw_priv;
 	}
-	return fw_priv;
-}
-
-static void _request_firmware_cleanup(const struct firmware **firmware_p)
-{
-	release_firmware(*firmware_p);
-	*firmware_p = NULL;
-}
 
-/* transfer the ownership of pages to firmware */
-static int fw_set_page_data(struct firmware_buf *buf)
-{
-	struct firmware *fw = buf->fw;
-
-	buf->data = vmap(buf->pages, buf->nr_pages, 0, PAGE_KERNEL_RO);
-	if (!buf->data)
-		return -ENOMEM;
-
-	fw->data = buf->data;
-	fw->pages = buf->pages;
-	fw->size = buf->size;
-	WARN_ON(PFN_UP(fw->size) != buf->nr_pages);
+	/* share the cached buf, which is inprogessing or completed */
+ check_status:
+	mutex_lock(&fw_lock);
+	if (test_bit(FW_STATUS_ABORT, &buf->status)) {
+		fw_priv = ERR_PTR(-ENOENT);
+		_request_firmware_cleanup(firmware_p);
+		goto exit;
+	} else if (test_bit(FW_STATUS_DONE, &buf->status)) {
+		fw_priv = NULL;
+		fw_set_page_data(buf, firmware);
+		goto exit;
+	}
+	mutex_unlock(&fw_lock);
+	wait_for_completion(&buf->completion);
+	goto check_status;
 
-	return 0;
+exit:
+	mutex_unlock(&fw_lock);
+	return fw_priv;
 }
 
 static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
@@ -585,13 +687,12 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 	if (!buf->size || test_bit(FW_STATUS_ABORT, &buf->status))
 		retval = -ENOENT;
 
-	/* transfer pages ownership at the last minute */
 	if (!retval)
-		retval = fw_set_page_data(buf);
-	if (retval)
-		fw_free_buf(buf); /* free untransfered pages buffer */
+		retval = fw_map_pages_buf(buf);
+
+	/* pass the pages buffer to driver at the last minute */
+	fw_set_page_data(buf, fw_priv->fw);
 
-	kfree(buf);
 	fw_priv->buf = NULL;
 	mutex_unlock(&fw_lock);
 
@@ -753,6 +854,7 @@ request_firmware_nowait(
 
 static int __init firmware_class_init(void)
 {
+	fw_cache_init();
 	return class_register(&firmware_class);
 }
 
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 1e7c011..e85b771 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -12,6 +12,9 @@ struct firmware {
 	size_t size;
 	const u8 *data;
 	struct page **pages;
+
+	/* firmware loader private fields */
+	void *priv;
 };
 
 struct module;
-- 
1.7.9.5

[RFC PATCH v1 07/15] firmware loader: introduce cache_firmware and uncache_firmware

[Ming Lei]

This patches introduce two kernel APIs of cache_firmware and
uncache_firmware, both of which take the firmware file name
as the only parameter.

So any drivers can call cache_firmware to cache the specified
firmware file into kernel memory, and can use the cached firmware
in situations which can't request firmware from user space.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |  100 +++++++++++++++++++++++++++++++++++++----
 include/linux/firmware.h      |   12 +++++
 2 files changed, 104 insertions(+), 8 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 848ad97..fc119ce 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -142,6 +142,17 @@ static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
 	return buf;
 }
 
+static struct firmware_buf *__fw_lookup_buf(const char *fw_name)
+{
+	struct firmware_buf *tmp;
+	struct firmware_cache *fwc = &fw_cache;
+
+	list_for_each_entry(tmp, &fwc->head, list)
+		if (!strcmp(tmp->fw_id, fw_name))
+			return tmp;
+	return NULL;
+}
+
 static int fw_lookup_and_allocate_buf(const char *fw_name,
 				      struct firmware_cache *fwc,
 				      struct firmware_buf **buf)
@@ -149,14 +160,13 @@ static int fw_lookup_and_allocate_buf(const char *fw_name,
 	struct firmware_buf *tmp;
 
 	spin_lock(&fwc->lock);
-	list_for_each_entry(tmp, &fwc->head, list)
-		if (!strcmp(tmp->fw_id, fw_name)) {
-			kref_get(&tmp->ref);
-			spin_unlock(&fwc->lock);
-			*buf = tmp;
-			return 1;
-		}
-
+	tmp = __fw_lookup_buf(fw_name);
+	if (tmp) {
+		kref_get(&tmp->ref);
+		spin_unlock(&fwc->lock);
+		*buf = tmp;
+		return 1;
+	}
 	tmp = __allocate_fw_buf(fw_name, fwc);
 	if (tmp)
 		list_add(&tmp->list, &fwc->head);
@@ -167,6 +177,18 @@ static int fw_lookup_and_allocate_buf(const char *fw_name,
 	return tmp ? 0 : -ENOMEM;
 }
 
+static struct firmware_buf *fw_lookup_buf(const char *fw_name)
+{
+	struct firmware_buf *tmp;
+	struct firmware_cache *fwc = &fw_cache;
+
+	spin_lock(&fwc->lock);
+	tmp = __fw_lookup_buf(fw_name);
+	spin_unlock(&fwc->lock);
+
+	return tmp;
+}
+
 static void __fw_free_buf(struct kref *ref)
 {
 	struct firmware_buf *buf = to_fwbuf(ref);
@@ -852,6 +874,66 @@ request_firmware_nowait(
 	return 0;
 }
 
+/**
+ * cache_firmware - cache one firmware image in kernel memory space
+ * @fw_name: the firmware image name
+ *
+ * Cache firmware in kernel memory so that drivers can use it when
+ * system isn't ready for them to request firmware image from userspace.
+ * Once it returns successfully, driver can use request_firmware or its
+ * nowait version to get the cached firmware without any interacting
+ * with userspace
+ *
+ * Return 0 if the firmware image has been cached successfully
+ * Return !0 otherwise
+ *
+ */
+int cache_firmware(const char *fw_name)
+{
+	int ret;
+	const struct firmware *fw;
+
+	pr_debug("%s: %s\n", __func__, fw_name);
+
+	ret = request_firmware(&fw, fw_name, NULL);
+	if (!ret)
+		kfree(fw);
+
+	pr_debug("%s: %s ret=%d\n", __func__, fw_name, ret);
+
+	return ret;
+}
+
+/**
+ * uncache_firmware - remove one cached firmware image
+ * @fw_name: the firmware image name
+ *
+ * Uncache one firmware image which has been cached successfully
+ * before.
+ *
+ * Return 0 if the firmware cache has been removed successfully
+ * Return !0 otherwise
+ *
+ */
+int uncache_firmware(const char *fw_name)
+{
+	struct firmware_buf *buf;
+	struct firmware fw;
+
+	pr_debug("%s: %s\n", __func__, fw_name);
+
+	if (fw_get_builtin_firmware(&fw, fw_name))
+		return 0;
+
+	buf = fw_lookup_buf(fw_name);
+	if (buf) {
+		fw_free_buf(buf);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int __init firmware_class_init(void)
 {
 	fw_cache_init();
@@ -869,3 +951,5 @@ module_exit(firmware_class_exit);
 EXPORT_SYMBOL(release_firmware);
 EXPORT_SYMBOL(request_firmware);
 EXPORT_SYMBOL(request_firmware_nowait);
+EXPORT_SYMBOL_GPL(cache_firmware);
+EXPORT_SYMBOL_GPL(uncache_firmware);
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index e85b771..e4279fe 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -47,6 +47,8 @@ int request_firmware_nowait(
 	void (*cont)(const struct firmware *fw, void *context));
 
 void release_firmware(const struct firmware *fw);
+int cache_firmware(const char *name);
+int uncache_firmware(const char *name);
 #else
 static inline int request_firmware(const struct firmware **fw,
 				   const char *name,
@@ -65,6 +67,16 @@ static inline int request_firmware_nowait(
 static inline void release_firmware(const struct firmware *fw)
 {
 }
+
+static inline int cache_firmware(const char *name)
+{
+	return -ENOENT;
+}
+
+static inline int uncache_firmware(const char *name)
+{
+	return -EINVAL;
+}
 #endif
 
 #endif
-- 
1.7.9.5

[RFC PATCH v1 08/15] firmware loader: fix device lifetime

[Ming Lei]

Callers of request_firmware* must hold the reference count of
@device, otherwise it is easy to trigger oops since the firmware
loader device is the child of @device.

This patch adds comments about the usage. In fact, most of drivers
call request_firmware* in its probe() or open(), so the constraint
should be reasonable and can be satisfied.

Also this patch holds the reference count of @device before
schedule_work() in request_firmware_nowait() to avoid that
the @device is released after request_firmware_nowait returns
and before the worker function is scheduled.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index fc119ce..7d3a83b 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -742,6 +742,8 @@ err_put_dev:
  *      @name will be used as $FIRMWARE in the uevent environment and
  *      should be distinctive enough not to be confused with any other
  *      firmware image for this or any other device.
+ *
+ *	Caller must hold the reference count of @device.
  **/
 int
 request_firmware(const struct firmware **firmware_p, const char *name,
@@ -823,6 +825,7 @@ static void request_firmware_work_func(struct work_struct *work)
 
  out:
 	fw_work->cont(fw, fw_work->context);
+	put_device(fw_work->device);
 
 	module_put(fw_work->module);
 	kfree(fw_work);
@@ -841,6 +844,8 @@ static void request_firmware_work_func(struct work_struct *work)
  * @cont: function will be called asynchronously when the firmware
  *	request is over.
  *
+ *	Caller must hold the reference count of @device.
+ *
  *	Asynchronous variant of request_firmware() for user contexts where
  *	it is not possible to sleep for long time. It can't be called
  *	in atomic contexts.
@@ -869,6 +874,7 @@ request_firmware_nowait(
 		return -EFAULT;
 	}
 
+	get_device(fw_work->device);
 	INIT_WORK(&fw_work->work, request_firmware_work_func);
 	schedule_work(&fw_work->work);
 	return 0;
-- 
1.7.9.5

[RFC PATCH v1 09/15] firmware loader: fix comments on request_firmware_nowait

[Ming Lei]

request_firmware_nowait is allowed to be called in atomic
context now if @gfp is GFP_ATOMIC, so fix the obsolete
comments and states which situations are suitable for using
it.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 7d3a83b..a47266c 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -846,9 +846,13 @@ static void request_firmware_work_func(struct work_struct *work)
  *
  *	Caller must hold the reference count of @device.
  *
- *	Asynchronous variant of request_firmware() for user contexts where
- *	it is not possible to sleep for long time. It can't be called
- *	in atomic contexts.
+ *	Asynchronous variant of request_firmware() for user contexts:
+ *		- sleep for as small periods as possible since it may
+ *		increase kernel boot time of built-in device drivers
+ *		requesting firmware in their ->probe() methods, if
+ *		@gfp is GFP_KERNEL.
+ *
+ *		- can't sleep at all if @gfp is GFP_ATOMIC.
  **/
 int
 request_firmware_nowait(
-- 
1.7.9.5

[RFC PATCH v1 10/15] firmware loader: store firmware name into devres list

[Ming Lei]

This patch will store firmware name into devres list of the device
which is requesting firmware loading, so that we can implement
auto cache and uncache firmware for devices in need.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |   64 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index a47266c..65c6066 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -114,6 +114,11 @@ struct firmware_priv {
 	struct firmware *fw;
 };
 
+struct fw_name_devm {
+	unsigned long magic;
+	char name[];
+};
+
 #define to_fwbuf(d) container_of(d, struct firmware_buf, ref)
 
 /* fw_lock could be moved to 'struct firmware_priv' but since it is just
@@ -590,6 +595,55 @@ static void fw_set_page_data(struct firmware_buf *buf, struct firmware *fw)
 		 (unsigned int)buf->size);
 }
 
+static void fw_name_devm_release(struct device *dev, void *res)
+{
+	struct fw_name_devm *fwn = res;
+
+	if (fwn->magic == (unsigned long)&fw_cache)
+		pr_debug("%s: fw_name-%s devm-%p released\n",
+				__func__, fwn->name, res);
+}
+
+static int fw_devm_match(struct device *dev, void *res,
+		void *match_data)
+{
+	struct fw_name_devm *fwn = res;
+
+	return (fwn->magic == (unsigned long)&fw_cache) &&
+		!strcmp(fwn->name, match_data);
+}
+
+static struct fw_name_devm *fw_find_devm_name(struct device *dev,
+		const char *name)
+{
+	struct fw_name_devm *fwn;
+
+	fwn = devres_find(dev, fw_name_devm_release,
+			  fw_devm_match, (void *)name);
+	return fwn;
+}
+
+/* add firmware name into devres list */
+static int fw_add_devm_name(struct device *dev, const char *name)
+{
+	struct fw_name_devm *fwn;
+
+	fwn = fw_find_devm_name(dev, name);
+	if (fwn)
+		return 1;
+
+	fwn = devres_alloc(fw_name_devm_release, sizeof(struct fw_name_devm) +
+			   strlen(name) + 1, GFP_KERNEL);
+	if (!fwn)
+		return -ENOMEM;
+
+	fwn->magic = (unsigned long)&fw_cache;
+	strcpy(fwn->name, name);
+	devres_add(dev, fwn);
+
+	return 0;
+}
+
 static void _request_firmware_cleanup(const struct firmware **firmware_p)
 {
 	release_firmware(*firmware_p);
@@ -709,6 +763,16 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 	if (!buf->size || test_bit(FW_STATUS_ABORT, &buf->status))
 		retval = -ENOENT;
 
+	/*
+	 * add firmware name into devres list so that we can auto cache
+	 * and uncache firmware for device.
+	 *
+	 * f_dev->parent may has been deleted already, but the problem
+	 * should be fixed in devres or driver core.
+	 */
+	if (!retval && f_dev->parent)
+		fw_add_devm_name(f_dev->parent, buf->fw_id);
+
 	if (!retval)
 		retval = fw_map_pages_buf(buf);
 
-- 
1.7.9.5

[RFC PATCH v1 11/15] driver core: devres: introduce devres_for_each_res

[Ming Lei]

This patch introduces one devres API of devres_for_each_res
so that the device's driver can iterate each resource it has
interest in.

The firmware loader will use the API to get each firmware name
from the device instance.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/devres.c  |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    4 ++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 2360adb..8731979 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -144,6 +144,48 @@ EXPORT_SYMBOL_GPL(devres_alloc);
 #endif
 
 /**
+ * devres_for_each_res - Resource iterator
+ * @dev: Device to iterate resource from
+ * @release: Look for resources associated with this release function
+ * @match: Match function (optional)
+ * @match_data: Data for the match function
+ * @fn: Function to be called for each matched resource.
+ * @data: Data for @fn, the 3rd parameter of @fn
+ *
+ * Call @fn for each devres of @dev which is associated with @release
+ * and for which @match returns 1.
+ *
+ * RETURNS:
+ * 	void
+ */
+void devres_for_each_res(struct device *dev, dr_release_t release,
+			dr_match_t match, void *match_data,
+			void (*fn)(struct device *, void *, void *),
+			void *data)
+{
+	struct devres_node *node;
+	struct devres_node *tmp;
+	unsigned long flags;
+
+	if (!fn)
+		return;
+
+	spin_lock_irqsave(&dev->devres_lock, flags);
+	list_for_each_entry_safe_reverse(node, tmp,
+			&dev->devres_head, entry) {
+		struct devres *dr = container_of(node, struct devres, node);
+
+		if (node->release != release)
+			continue;
+		if (match && !match(dev, dr->data, match_data))
+			continue;
+		fn(dev, dr->data, data);
+	}
+	spin_unlock_irqrestore(&dev->devres_lock, flags);
+}
+EXPORT_SYMBOL_GPL(devres_for_each_res);
+
+/**
  * devres_free - Free device resource data
  * @res: Pointer to devres data to free
  *
diff --git a/include/linux/device.h b/include/linux/device.h
index 52a5f15..ecd9006 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -536,6 +536,10 @@ extern void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
 #else
 extern void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp);
 #endif
+extern void devres_for_each_res(struct device *dev, dr_release_t release,
+				dr_match_t match, void *match_data,
+				void (*fn)(struct device *, void *, void *),
+				void *data);
 extern void devres_free(void *res);
 extern void devres_add(struct device *dev, void *res);
 extern void *devres_find(struct device *dev, dr_release_t release,
-- 
1.7.9.5

[RFC PATCH v1 12/15] firmware: introduce device_cache/uncache_fw_images

[Ming Lei]

This patch introduces the three helpers below:

	void device_cache_fw_images(void)
	void device_uncache_fw_images(void)
	void device_uncache_fw_images_delay(unsigned long)

so we can use device_cache_fw_images() to cache firmware for
all devices which need firmware to work, and the device driver
can get the firmware easily from kernel memory when system isn't
ready for completing requests of loading firmware.

After system is ready for completing firmware loading, driver core
will call device_uncache_fw_images() or its delay version to free
the cached firmware.

The above helpers will be used to cache device firmware during
system suspend/resume cycle in the following patches.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |  221 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 215 insertions(+), 6 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 65c6066..68fd4c6 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -22,6 +22,11 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/list.h>
+#include <linux/async.h>
+#include <linux/pm.h>
+
+#include "base.h"
+#include "power/power.h"
 
 MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
@@ -90,6 +95,19 @@ struct firmware_cache {
 	/* firmware_buf instance will be added into the below list */
 	spinlock_t lock;
 	struct list_head head;
+
+	/*
+	 * Names of firmware images which have been cached successfully
+	 * will be added into the below list so that device uncache
+	 * helper can trace which firmware images have been cached
+	 * before.
+	 */
+	spinlock_t name_lock;
+	struct list_head fw_names;
+
+	wait_queue_head_t wait_queue;
+	int cnt;
+	struct delayed_work work;
 };
 
 struct firmware_buf {
@@ -106,6 +124,11 @@ struct firmware_buf {
 	char fw_id[];
 };
 
+struct fw_cache_entry {
+	struct list_head list;
+	char name[];
+};
+
 struct firmware_priv {
 	struct timer_list timeout;
 	bool nowait;
@@ -220,12 +243,6 @@ static void fw_free_buf(struct firmware_buf *buf)
 	kref_put(&buf->ref, __fw_free_buf);
 }
 
-static void __init fw_cache_init(void)
-{
-	spin_lock_init(&fw_cache.lock);
-	INIT_LIST_HEAD(&fw_cache.head);
-}
-
 static struct firmware_priv *to_firmware_priv(struct device *dev)
 {
 	return container_of(dev, struct firmware_priv, dev);
@@ -1008,6 +1025,198 @@ int uncache_firmware(const char *fw_name)
 	return -EINVAL;
 }
 
+static struct fw_cache_entry *alloc_fw_cache_entry(const char *name)
+{
+	struct fw_cache_entry *fce;
+
+	fce = kzalloc(sizeof(*fce) + strlen(name) + 1, GFP_ATOMIC);
+	if (!fce)
+		goto exit;
+
+	strcpy(fce->name, name);
+exit:
+	return fce;
+}
+
+static void free_fw_cache_entry(struct fw_cache_entry *fce)
+{
+	kfree(fce);
+}
+
+static void __async_dev_cache_fw_image(void *fw_entry,
+				       async_cookie_t cookie)
+{
+	struct fw_cache_entry *fce = fw_entry;
+	struct firmware_cache *fwc = &fw_cache;
+	int ret;
+
+	ret = cache_firmware(fce->name);
+	if (ret)
+		goto free;
+
+	spin_lock(&fwc->name_lock);
+	list_add(&fce->list, &fwc->fw_names);
+	spin_unlock(&fwc->name_lock);
+	goto drop_ref;
+
+free:
+	free_fw_cache_entry(fce);
+drop_ref:
+	spin_lock(&fwc->name_lock);
+	fwc->cnt--;
+	spin_unlock(&fwc->name_lock);
+
+	wake_up(&fwc->wait_queue);
+}
+
+/* called with dev->devres_lock held */
+static void dev_create_fw_entry(struct device *dev, void *res,
+				void *data)
+{
+	struct fw_name_devm *fwn = res;
+	const char *fw_name = fwn->name;
+	struct list_head *head = data;
+	struct fw_cache_entry *fce;
+
+	fce = alloc_fw_cache_entry(fw_name);
+	if (fce)
+		list_add(&fce->list, head);
+}
+
+static int devm_name_match(struct device *dev, void *res,
+			   void *match_data)
+{
+	struct fw_name_devm *fwn = res;
+	return (fwn->magic == (unsigned long)match_data);
+}
+
+static void dev_cache_fw_image(struct device *dev)
+{
+	LIST_HEAD(todo);
+	struct fw_cache_entry *fce;
+	struct fw_cache_entry *fce_next;
+	struct firmware_cache *fwc = &fw_cache;
+
+	devres_for_each_res(dev, fw_name_devm_release,
+			    devm_name_match, &fw_cache,
+			    dev_create_fw_entry, &todo);
+
+	list_for_each_entry_safe(fce, fce_next, &todo, list) {
+		list_del(&fce->list);
+
+		spin_lock(&fwc->name_lock);
+		fwc->cnt++;
+		spin_unlock(&fwc->name_lock);
+
+		async_schedule(__async_dev_cache_fw_image, (void *)fce);
+	}
+}
+
+static void __device_uncache_fw_images(void)
+{
+	struct firmware_cache *fwc = &fw_cache;
+	struct fw_cache_entry *fce;
+
+	spin_lock(&fwc->name_lock);
+	while (!list_empty(&fwc->fw_names)) {
+		fce = list_entry(fwc->fw_names.next,
+				struct fw_cache_entry, list);
+		list_del(&fce->list);
+		spin_unlock(&fwc->name_lock);
+
+		uncache_firmware(fce->name);
+		free_fw_cache_entry(fce);
+
+		spin_lock(&fwc->name_lock);
+	}
+	spin_unlock(&fwc->name_lock);
+}
+
+/**
+ * device_cache_fw_images - cache devices' firmware
+ *
+ * If one device called request_firmware or its nowait version
+ * successfully before, the firmware names are recored into the
+ * device's devres link list, so device_cache_fw_images can call
+ * cache_firmware() to cache these firmwares for the device,
+ * then the device driver can load its firmwares easily at
+ * time when system is not ready to complete loading firmware.
+ */
+static void device_cache_fw_images(void)
+{
+	struct firmware_cache *fwc = &fw_cache;
+	struct device *dev;
+	DEFINE_WAIT(wait);
+
+	pr_debug("%s\n", __func__);
+
+	device_pm_lock();
+	list_for_each_entry(dev, &dpm_list, power.entry)
+		dev_cache_fw_image(dev);
+	device_pm_unlock();
+
+	/* wait for completion of caching firmware for all devices */
+	spin_lock(&fwc->name_lock);
+	for (;;) {
+		prepare_to_wait(&fwc->wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (!fwc->cnt)
+			break;
+
+		spin_unlock(&fwc->name_lock);
+
+		schedule();
+
+		spin_lock(&fwc->name_lock);
+	}
+	spin_unlock(&fwc->name_lock);
+	finish_wait(&fwc->wait_queue, &wait);
+}
+
+/**
+ * device_uncache_fw_images - uncache devices' firmware
+ *
+ * uncache all firmwares which have been cached successfully
+ * by device_uncache_fw_images earlier
+ */
+static void device_uncache_fw_images(void)
+{
+	pr_debug("%s\n", __func__);
+	__device_uncache_fw_images();
+}
+
+static void device_uncache_fw_images_work(struct work_struct *work)
+{
+	device_uncache_fw_images();
+}
+
+/**
+ * device_uncache_fw_images_delay - uncache devices firmwares
+ * @delay: number of milliseconds to delay uncache device firmwares
+ *
+ * uncache all devices's firmwares which has been cached successfully
+ * by device_cache_fw_images after @delay milliseconds.
+ */
+static void device_uncache_fw_images_delay(unsigned long delay)
+{
+	schedule_delayed_work(&fw_cache.work,
+			msecs_to_jiffies(delay));
+}
+
+static void __init fw_cache_init(void)
+{
+	spin_lock_init(&fw_cache.lock);
+	INIT_LIST_HEAD(&fw_cache.head);
+
+	spin_lock_init(&fw_cache.name_lock);
+	INIT_LIST_HEAD(&fw_cache.fw_names);
+	fw_cache.cnt = 0;
+
+	init_waitqueue_head(&fw_cache.wait_queue);
+	INIT_DELAYED_WORK(&fw_cache.work,
+			  device_uncache_fw_images_work);
+}
+
 static int __init firmware_class_init(void)
 {
 	fw_cache_init();
-- 
1.7.9.5

Re: [RFC PATCH v1 12/15] firmware: introduce device_cache/uncache_fw_images

[Andrew Morton]

On Sat,  4 Aug 2012 12:01:27 +0800
Ming Lei <ming.lei@canonical.com> wrote:

> This patch introduces the three helpers below:
> 
> 	void device_cache_fw_images(void)
> 	void device_uncache_fw_images(void)
> 	void device_uncache_fw_images_delay(unsigned long)

CONFIG_PM=n:

drivers/base/firmware_class.c:1147: warning: 'device_cache_fw_images' defined but not used
drivers/base/firmware_class.c:1212: warning: 'device_uncache_fw_images_delay' defined but not used

the fix is not trivial - this patch added quite a lot of code and data
structure bloat which will never be used on CONFIG_PM=n kernels.


(Well, a trivial fix would be to eradicate CONFIG_PM=n, but...)

Re: [RFC PATCH v1 12/15] firmware: introduce device_cache/uncache_fw_images

[Ming Lei]

On Fri, Sep 7, 2012 at 6:44 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Sat,  4 Aug 2012 12:01:27 +0800
> Ming Lei <ming.lei@canonical.com> wrote:
>
>> This patch introduces the three helpers below:
>>
>>       void device_cache_fw_images(void)
>>       void device_uncache_fw_images(void)
>>       void device_uncache_fw_images_delay(unsigned long)
>
> CONFIG_PM=n:
>
> drivers/base/firmware_class.c:1147: warning: 'device_cache_fw_images' defined but not used
> drivers/base/firmware_class.c:1212: warning: 'device_uncache_fw_images_delay' defined but not used

Thanks for your report.

>
> the fix is not trivial - this patch added quite a lot of code and data
> structure bloat which will never be used on CONFIG_PM=n kernels.

The below patch should fix the warning.

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 95f6851..6e21080 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -96,7 +96,9 @@ struct firmware_cache {
 	/* firmware_buf instance will be added into the below list */
 	spinlock_t lock;
 	struct list_head head;
+	int state;

+#ifdef CONFIG_PM_SLEEP
 	/*
 	 * Names of firmware images which have been cached successfully
 	 * will be added into the below list so that device uncache
@@ -106,13 +108,12 @@ struct firmware_cache {
 	spinlock_t name_lock;
 	struct list_head fw_names;

-	int state;
-
 	wait_queue_head_t wait_queue;
 	int cnt;
 	struct delayed_work work;

 	struct notifier_block   pm_notify;
+#endif
 };

 struct firmware_buf {
@@ -622,6 +623,7 @@ static void fw_set_page_data(struct firmware_buf
*buf, struct firmware *fw)
 		 (unsigned int)buf->size);
 }

+#ifdef CONFIG_PM_SLEEP
 static void fw_name_devm_release(struct device *dev, void *res)
 {
 	struct fw_name_devm *fwn = res;
@@ -670,6 +672,12 @@ static int fw_add_devm_name(struct device *dev,
const char *name)

 	return 0;
 }
+#else
+static int fw_add_devm_name(struct device *dev, const char *name)
+{
+	return 0;
+}
+#endif

 static void _request_firmware_cleanup(const struct firmware **firmware_p)
 {
@@ -1046,6 +1054,7 @@ int uncache_firmware(const char *fw_name)
 	return -EINVAL;
 }

+#ifdef CONFIG_PM_SLEEP
 static struct fw_cache_entry *alloc_fw_cache_entry(const char *name)
 {
 	struct fw_cache_entry *fce;
@@ -1258,7 +1267,6 @@ static void
device_uncache_fw_images_delay(unsigned long delay)
 			msecs_to_jiffies(delay));
 }

-#ifdef CONFIG_PM
 static int fw_pm_notify(struct notifier_block *notify_block,
 			unsigned long mode, void *unused)
 {
@@ -1285,13 +1293,6 @@ static int fw_pm_notify(struct notifier_block
*notify_block,

 	return 0;
 }
-#else
-static int fw_pm_notify(struct notifier_block *notify_block,
-			unsigned long mode, void *unused)
-{
-	return 0;
-}
-#endif

 /* stop caching firmware once syscore_suspend is reached */
 static int fw_suspend(void)
@@ -1303,16 +1304,23 @@ static int fw_suspend(void)
 static struct syscore_ops fw_syscore_ops = {
 	.suspend = fw_suspend,
 };
+#else
+static int fw_cache_piggyback_on_request(const char *name)
+{
+	return 0;
+}
+#endif

 static void __init fw_cache_init(void)
 {
 	spin_lock_init(&fw_cache.lock);
 	INIT_LIST_HEAD(&fw_cache.head);
+	fw_cache.state = FW_LOADER_NO_CACHE;

+#ifdef CONFIG_PM_SLEEP
 	spin_lock_init(&fw_cache.name_lock);
 	INIT_LIST_HEAD(&fw_cache.fw_names);
 	fw_cache.cnt = 0;
-	fw_cache.state = FW_LOADER_NO_CACHE;

 	init_waitqueue_head(&fw_cache.wait_queue);
 	INIT_DELAYED_WORK(&fw_cache.work,
@@ -1322,6 +1330,7 @@ static void __init fw_cache_init(void)
 	register_pm_notifier(&fw_cache.pm_notify);

 	register_syscore_ops(&fw_syscore_ops);
+#endif
 }

 static int __init firmware_class_init(void)
@@ -1332,8 +1341,10 @@ static int __init firmware_class_init(void)

 static void __exit firmware_class_exit(void)
 {
+#ifdef CONFIG_PM_SLEEP
 	unregister_syscore_ops(&fw_syscore_ops);
 	unregister_pm_notifier(&fw_cache.pm_notify);
+#endif
 	class_unregister(&firmware_class);
 }


Thanks,
--
Ming Lei

Re: [RFC PATCH v1 12/15] firmware: introduce device_cache/uncache_fw_images

[Greg Kroah-Hartman]

On Fri, Sep 07, 2012 at 11:32:36AM +0800, Ming Lei wrote:
> On Fri, Sep 7, 2012 at 6:44 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Sat,  4 Aug 2012 12:01:27 +0800
> > Ming Lei <ming.lei@canonical.com> wrote:
> >
> >> This patch introduces the three helpers below:
> >>
> >>       void device_cache_fw_images(void)
> >>       void device_uncache_fw_images(void)
> >>       void device_uncache_fw_images_delay(unsigned long)
> >
> > CONFIG_PM=n:
> >
> > drivers/base/firmware_class.c:1147: warning: 'device_cache_fw_images' defined but not used
> > drivers/base/firmware_class.c:1212: warning: 'device_uncache_fw_images_delay' defined but not used
> 
> Thanks for your report.
> 
> >
> > the fix is not trivial - this patch added quite a lot of code and data
> > structure bloat which will never be used on CONFIG_PM=n kernels.
> 
> The below patch should fix the warning.

Can you send it to me in a format that I can apply it in?

greg k-h

Re: [RFC PATCH v1 12/15] firmware: introduce device_cache/uncache_fw_images

[Ming Lei]

On Fri, Sep 7, 2012 at 11:16 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> Can you send it to me in a format that I can apply it in?

OK, sent out, :-)

Thanks,
--
Ming Lei

[RFC PATCH v1 13/15] firmware loader: use small timeout for cache device firmware

[Ming Lei]

Because device_cache_fw_images only cache the firmware which has been
loaded sucessfully at leat once, using a small loading timeout should
be reasonable.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 68fd4c6..8ca0052 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1146,10 +1146,22 @@ static void device_cache_fw_images(void)
 {
 	struct firmware_cache *fwc = &fw_cache;
 	struct device *dev;
+	int old_timeout;
 	DEFINE_WAIT(wait);
 
 	pr_debug("%s\n", __func__);
 
+	/*
+	 * use small loading timeout for caching devices' firmware
+	 * because all these firmware images have been loaded
+	 * successfully at lease once, also system is ready for
+	 * completing firmware loading now. The maximum size of
+	 * firmware in current distributions is about 2M bytes,
+	 * so 10 secs should be enough.
+	 */
+	old_timeout = loading_timeout;
+	loading_timeout = 10;
+
 	device_pm_lock();
 	list_for_each_entry(dev, &dpm_list, power.entry)
 		dev_cache_fw_image(dev);
@@ -1171,6 +1183,8 @@ static void device_cache_fw_images(void)
 	}
 	spin_unlock(&fwc->name_lock);
 	finish_wait(&fwc->wait_queue, &wait);
+
+	loading_timeout = old_timeout;
 }
 
 /**
-- 
1.7.9.5

[RFC PATCH v1 14/15] firmware loader: cache devices firmware during suspend/resume cycle

[Ming Lei]

This patch implements caching devices' firmware automatically
during system syspend/resume cycle, so any device drivers can
call request_firmware or request_firmware_nowait inside resume
path to get the cached firmware if they have loaded firmwares
successfully at least once before entering suspend.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/base/firmware_class.c |   32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8ca0052..5bd2100 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -24,6 +24,7 @@
 #include <linux/list.h>
 #include <linux/async.h>
 #include <linux/pm.h>
+#include <linux/suspend.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -108,6 +109,8 @@ struct firmware_cache {
 	wait_queue_head_t wait_queue;
 	int cnt;
 	struct delayed_work work;
+
+	struct notifier_block   pm_notify;
 };
 
 struct firmware_buf {
@@ -1217,6 +1220,31 @@ static void device_uncache_fw_images_delay(unsigned long delay)
 			msecs_to_jiffies(delay));
 }
 
+#ifdef CONFIG_PM
+static int fw_pm_notify(struct notifier_block *notify_block,
+			unsigned long mode, void *unused)
+{
+	switch (mode) {
+	case PM_HIBERNATION_PREPARE:
+	case PM_SUSPEND_PREPARE:
+		device_cache_fw_images();
+		break;
+
+	case PM_POST_SUSPEND:
+	case PM_POST_HIBERNATION:
+	case PM_POST_RESTORE:
+		device_uncache_fw_images_delay(10 * MSEC_PER_SEC);
+		break;
+	}
+
+	return 0;
+}
+#else
+static int fw_pm_notify(struct notifier_block *notify_block,
+			unsigned long mode, void *unused)
+{}
+#endif
+
 static void __init fw_cache_init(void)
 {
 	spin_lock_init(&fw_cache.lock);
@@ -1229,6 +1257,9 @@ static void __init fw_cache_init(void)
 	init_waitqueue_head(&fw_cache.wait_queue);
 	INIT_DELAYED_WORK(&fw_cache.work,
 			  device_uncache_fw_images_work);
+
+	fw_cache.pm_notify.notifier_call = fw_pm_notify;
+	register_pm_notifier(&fw_cache.pm_notify);
 }
 
 static int __init firmware_class_init(void)
@@ -1239,6 +1270,7 @@ static int __init firmware_class_init(void)
 
 static void __exit firmware_class_exit(void)
 {
+	unregister_pm_notifier(&fw_cache.pm_notify);
 	class_unregister(&firmware_class);
 }
 
-- 
1.7.9.5

[RFC PATCH v1 15/15] wireless: ath9k-htc: only load firmware in need

[Ming Lei]

It is not necessary to hold the firmware memory during the whole
driver lifetime, and obviously it does waste memory. Suppose there
are 4 ath9k-htc usb dongles working, kernel has to consume about
4*50KBytes RAM to cache firmware for all dongles. After applying the
patch, kernel only caches one single firmware image in RAM for
all ath9k-htc devices just during system suspend/resume cycle.

When system is ready for loading firmware, ath9k-htc can request
the loading from usersapce. During system resume, ath9k-htc still
can load the firmware which was cached in kernel memory before
system suspend.

Cc: linux-wireless@vger.kernel.org
Cc: "Luis R. Rodriguez" <mcgrof@qca.qualcomm.com>
Cc: Jouni Malinen <jouni@qca.qualcomm.com>
Cc: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
Cc: Senthil Balasubramanian <senthilb@qca.qualcomm.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c |   34 ++++++++++++++++++++----------
 drivers/net/wireless/ath/ath9k/hif_usb.h |    4 +++-
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index aa327ad..d35a19c 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -973,8 +973,8 @@ static void ath9k_hif_usb_dealloc_urbs(struct hif_device_usb *hif_dev)
 static int ath9k_hif_usb_download_fw(struct hif_device_usb *hif_dev)
 {
 	int transfer, err;
-	const void *data = hif_dev->firmware->data;
-	size_t len = hif_dev->firmware->size;
+	const void *data = hif_dev->fw_data;
+	size_t len = hif_dev->fw_size;
 	u32 addr = AR9271_FIRMWARE;
 	u8 *buf = kzalloc(4096, GFP_KERNEL);
 	u32 firm_offset;
@@ -1017,7 +1017,7 @@ static int ath9k_hif_usb_download_fw(struct hif_device_usb *hif_dev)
 		return -EIO;
 
 	dev_info(&hif_dev->udev->dev, "ath9k_htc: Transferred FW: %s, size: %ld\n",
-		 hif_dev->fw_name, (unsigned long) hif_dev->firmware->size);
+		 hif_dev->fw_name, (unsigned long) hif_dev->fw_size);
 
 	return 0;
 }
@@ -1099,11 +1099,11 @@ static void ath9k_hif_usb_firmware_cb(const struct firmware *fw, void *context)
 
 	hif_dev->htc_handle = ath9k_htc_hw_alloc(hif_dev, &hif_usb,
 						 &hif_dev->udev->dev);
-	if (hif_dev->htc_handle == NULL) {
-		goto err_fw;
-	}
+	if (hif_dev->htc_handle == NULL)
+		goto err_dev_alloc;
 
-	hif_dev->firmware = fw;
+	hif_dev->fw_data = fw->data;
+	hif_dev->fw_size = fw->size;
 
 	/* Proceed with initialization */
 
@@ -1111,6 +1111,8 @@ static void ath9k_hif_usb_firmware_cb(const struct firmware *fw, void *context)
 	if (ret)
 		goto err_dev_init;
 
+	release_firmware(fw);
+
 	ret = ath9k_htc_hw_init(hif_dev->htc_handle,
 				&hif_dev->interface->dev,
 				hif_dev->usb_device_id->idProduct,
@@ -1121,6 +1123,7 @@ static void ath9k_hif_usb_firmware_cb(const struct firmware *fw, void *context)
 		goto err_htc_hw_init;
 	}
 
+	hif_dev->flags |= HIF_USB_READY;
 	complete(&hif_dev->fw_done);
 
 	return;
@@ -1129,8 +1132,8 @@ err_htc_hw_init:
 	ath9k_hif_usb_dev_deinit(hif_dev);
 err_dev_init:
 	ath9k_htc_hw_free(hif_dev->htc_handle);
+err_dev_alloc:
 	release_firmware(fw);
-	hif_dev->firmware = NULL;
 err_fw:
 	ath9k_hif_usb_firmware_fail(hif_dev);
 }
@@ -1277,11 +1280,10 @@ static void ath9k_hif_usb_disconnect(struct usb_interface *interface)
 
 	wait_for_completion(&hif_dev->fw_done);
 
-	if (hif_dev->firmware) {
+	if (hif_dev->flags & HIF_USB_READY) {
 		ath9k_htc_hw_deinit(hif_dev->htc_handle, unplugged);
 		ath9k_htc_hw_free(hif_dev->htc_handle);
 		ath9k_hif_usb_dev_deinit(hif_dev);
-		release_firmware(hif_dev->firmware);
 	}
 
 	usb_set_intfdata(interface, NULL);
@@ -1317,13 +1319,23 @@ static int ath9k_hif_usb_resume(struct usb_interface *interface)
 	struct hif_device_usb *hif_dev = usb_get_intfdata(interface);
 	struct htc_target *htc_handle = hif_dev->htc_handle;
 	int ret;
+	const struct firmware *fw;
 
 	ret = ath9k_hif_usb_alloc_urbs(hif_dev);
 	if (ret)
 		return ret;
 
-	if (hif_dev->firmware) {
+	if (hif_dev->flags & HIF_USB_READY) {
+		/* request cached firmware during suspend/resume cycle */
+		ret = request_firmware(&fw, hif_dev->fw_name,
+				       &hif_dev->udev->dev);
+		if (ret)
+			goto fail_resume;
+
+		hif_dev->fw_data = fw->data;
+		hif_dev->fw_size = fw->size;
 		ret = ath9k_hif_usb_download_fw(hif_dev);
+		release_firmware(fw);
 		if (ret)
 			goto fail_resume;
 	} else {
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 487ff65..51496e7 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -85,12 +85,14 @@ struct cmd_buf {
 };
 
 #define HIF_USB_START BIT(0)
+#define HIF_USB_READY BIT(1)
 
 struct hif_device_usb {
 	struct usb_device *udev;
 	struct usb_interface *interface;
 	const struct usb_device_id *usb_device_id;
-	const struct firmware *firmware;
+	const void *fw_data;
+	size_t fw_size;
 	struct completion fw_done;
 	struct htc_target *htc_handle;
 	struct hif_usb_tx tx;
-- 
1.7.9.5

Re: [RFC PATCH v1 00/15] firmware loader: introduce cache/uncache firmware

[Ming Lei]

On Sat, Aug 4, 2012 at 12:01 PM, Ming Lei <ming.lei@canonical.com> wrote:
> Hi,
>
> In [1][2], the problem below has been discussed for some time:
>
>         device's firmware may be lost during suspend/resume
>         cycle because device might be unplugged and plugged again
>         or device experiences system power loss during the period.
>         But in resume path, system is still not ready(process
>         frozen, rootfs not usable, ...) to complete loading firmware
>         from user space for devices.
>
> The conclusion is that caching firmware during suspend/resume cycle
> is capable of solving the problem.
>
> This patchset implements cache/uncache firmware mechanism,
> and apply the mechnism to cache device's firmware in kernel memory
> space automatically during suspend/resume cyclye, so device can
> load its firmware easily in its resume path. When resume is completed
> and system is ready, the cached firmware will be removed from
> kernel memory later.
>
> The patch 15/15 is one example to apply the firmware cache mechanism on
> ath9k-htc driver.
>
> Even there are some corener cases[3] which can't be solved by the
> cache approach, but as discussed in[4], the problem can be easily
> fixed by a simple patch written by Linus.
>
> This patch set is against 3.6.0-rc1-next-20120803.
>
> [1]. http://marc.info/?t=134278790800004&r=1&w=2
> [2]. http://marc.info/?t=132528956000002&r=10&w=2
> [3]. http://marc.info/?l=linux-usb&m=132554118928398&w=2
> [4]. http://marc.info/?l=linux-kernel&m=134323730805443&w=2
>
> --
> V1:
>         -handle vmap failure case(1/15)
>         -fix a memory leak of 'firmware_buf'(5/15)
>         -fix oops during failure path of requesting firmware(6/15)
>         -fix vmap more than one time(6/15)
>         -introduce __fw_lookup_buf to avoid code duplication(7/15)
>         -fix comment of request_firmware_nowait(9/15)
>         -avoid releasing lock in devres_for_each_res(11/15)
>         -use new devres iterator API to create fw cache entry(12/15)
>         -rename some functions and data structures(12/15)
>         -some code style fixes
>
>         Thanks for Borislav's review!

Gentle ping on -V1, :-)

Thanks,
--
Ming Lei

Re: [RFC PATCH v1 00/15] firmware loader: introduce cache/uncache firmware

[Greg Kroah-Hartman]

On Fri, Aug 10, 2012 at 03:30:14PM +0800, Ming Lei wrote:
> On Sat, Aug 4, 2012 at 12:01 PM, Ming Lei <ming.lei@canonical.com> wrote:
> > Hi,
> >
> > In [1][2], the problem below has been discussed for some time:
> >
> >         device's firmware may be lost during suspend/resume
> >         cycle because device might be unplugged and plugged again
> >         or device experiences system power loss during the period.
> >         But in resume path, system is still not ready(process
> >         frozen, rootfs not usable, ...) to complete loading firmware
> >         from user space for devices.
> >
> > The conclusion is that caching firmware during suspend/resume cycle
> > is capable of solving the problem.
> >
> > This patchset implements cache/uncache firmware mechanism,
> > and apply the mechnism to cache device's firmware in kernel memory
> > space automatically during suspend/resume cyclye, so device can
> > load its firmware easily in its resume path. When resume is completed
> > and system is ready, the cached firmware will be removed from
> > kernel memory later.
> >
> > The patch 15/15 is one example to apply the firmware cache mechanism on
> > ath9k-htc driver.
> >
> > Even there are some corener cases[3] which can't be solved by the
> > cache approach, but as discussed in[4], the problem can be easily
> > fixed by a simple patch written by Linus.
> >
> > This patch set is against 3.6.0-rc1-next-20120803.
> >
> > [1]. http://marc.info/?t=134278790800004&r=1&w=2
> > [2]. http://marc.info/?t=132528956000002&r=10&w=2
> > [3]. http://marc.info/?l=linux-usb&m=132554118928398&w=2
> > [4]. http://marc.info/?l=linux-kernel&m=134323730805443&w=2
> >
> > --
> > V1:
> >         -handle vmap failure case(1/15)
> >         -fix a memory leak of 'firmware_buf'(5/15)
> >         -fix oops during failure path of requesting firmware(6/15)
> >         -fix vmap more than one time(6/15)
> >         -introduce __fw_lookup_buf to avoid code duplication(7/15)
> >         -fix comment of request_firmware_nowait(9/15)
> >         -avoid releasing lock in devres_for_each_res(11/15)
> >         -use new devres iterator API to create fw cache entry(12/15)
> >         -rename some functions and data structures(12/15)
> >         -some code style fixes
> >
> >         Thanks for Borislav's review!
> 
> Gentle ping on -V1, :-)

Ok, these look good to me, so I've applied the first 14 to the
driver-core-next tree to get some testing by others.

I didn't feel I could add the 15th patch without some acks from those
developers, or at least someone who says "it works for me!".

Also, what about those other patches you reference in your footnotes
above, what's the status of them?

thanks,

greg k-h

Re: [RFC PATCH v1 00/15] firmware loader: introduce cache/uncache firmware

[Ming Lei]

On Fri, Aug 17, 2012 at 4:46 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Ok, these look good to me, so I've applied the first 14 to the
> driver-core-next tree to get some testing by others.
>
> I didn't feel I could add the 15th patch without some acks from those
> developers, or at least someone who says "it works for me!".

OK, there is still one problem(double free in failure path) in the 15th patch,
and I will send a new version later and ask for comments from ath9k guys.

>
> Also, what about those other patches you reference in your footnotes
> above, what's the status of them?

There is only one patch in the reference below which need to be
merged, but it has nothing conflicted with these firmware loader 14
patches.

         http://marc.info/?l=linux-kernel&m=134323730805443&w=2

I think Linus will deal with it, :-)


Thanks,
--
Ming Lei