[PATCH 1/2] orinoco: reload firmware on resume

[PATCH 1/2] orinoco: reload firmware on resume

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]

On resume card state is likely lost so we have to reload firmware
again.

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>

---

This is non-functional without second patch. Currently you simply
have no way to load request firmware from user space in ->resume.

 drivers/net/wireless/orinoco.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)


diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 1f9dcc4..76c480b 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -2298,9 +2298,14 @@ int orinoco_reinit_firmware(struct net_device *dev)
 {
 	struct orinoco_private *priv = netdev_priv(dev);
 	struct hermes *hw = &priv->hw;
-	int err;
+	int err = 0;
 
 	err = hermes_init(hw);
+	if (priv->do_fw_download && !err) {
+		err = orinoco_download(priv);
+		if (err)
+			priv->do_fw_download = 0;
+	}
 	if (!err)
 		err = orinoco_allocate_fid(dev);
 
@@ -2926,12 +2931,6 @@ static void orinoco_reset(struct work_struct *work)
 		}
 	}
 
-	if (priv->do_fw_download) {
-		err = orinoco_download(priv);
-		if (err)
-			priv->do_fw_download = 0;
-	}
-
 	err = orinoco_reinit_firmware(dev);
 	if (err) {
 		printk(KERN_ERR "%s: orinoco_reset: Error %d re-initializing firmware\n",

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 2/2] orinoco: cache downloadable firmware image in memory for use during resume

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 4170 bytes --]

orinoco: cache downloadable firmware image in memory for use during resume

If card is using downloadable firmware (like Agere 9.x), firmware has
to be reloaded during resume. It is not possible to use request_firmware
for that, because tasks are still frozen, so request_firmware will
just timeout and fail. So cache firmware image in memory for later
reuse in ->resume method.

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>

---

Unfortunately this is the only way to do it given current infrastructure.
I think that extra memory cost (~60kb) does not warrant anything more
sophisticated - even if this is possible. Also users not using dowloadable
firmware won't be penalized at all.

 drivers/net/wireless/orinoco.c |   42 ++++++++++++++++++++++++++++++----------
 drivers/net/wireless/orinoco.h |    4 ++++
 2 files changed, 35 insertions(+), 11 deletions(-)


diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 76c480b..204b2a6 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -487,15 +487,25 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	if (err)
 		goto free;
 
-	err = request_firmware(&fw_entry, firmware, priv->dev);
-	if (err) {
-		printk(KERN_ERR "%s: Cannot find firmware %s\n",
-		       dev->name, firmware);
-		err = -ENOENT;
-		goto free;
+	if (!priv->cached_fw_data) {
+		err = request_firmware(&fw_entry, firmware, priv->dev);
+		if (err) {
+			printk(KERN_ERR "%s: Cannot find firmware %s\n",
+			       dev->name, firmware);
+			err = -ENOENT;
+			goto free;
+		}
+		priv->cached_fw_size = fw_entry->size;
+		priv->cached_fw_data = kmemdup(fw_entry->data, fw_entry->size,
+						GFP_KERNEL);
+		release_firmware(fw_entry);
+		if (!priv->cached_fw_data) {
+			err = -ENOMEM;
+			goto abort;
+		}
 	}
 
-	hdr = (const struct orinoco_fw_header *) fw_entry->data;
+	hdr = (const struct orinoco_fw_header *) priv->cached_fw_data;
 
 	/* Enable aux port to allow programming */
 	err = hermesi_program_init(hw, le32_to_cpu(hdr->entry_point));
@@ -504,10 +514,10 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 		goto abort;
 
 	/* Program data */
-	first_block = (fw_entry->data +
+	first_block = (priv->cached_fw_data +
 		       le16_to_cpu(hdr->headersize) +
 		       le32_to_cpu(hdr->block_offset));
-	end = fw_entry->data + fw_entry->size;
+	end = priv->cached_fw_data + priv->cached_fw_size;
 
 	err = hermes_program(hw, first_block, end);
 	printk(KERN_DEBUG "%s: Program returned %d\n", dev->name, err);
@@ -515,7 +525,7 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 		goto abort;
 
 	/* Update production data */
-	first_block = (fw_entry->data +
+	first_block = (priv->cached_fw_data +
 		       le16_to_cpu(hdr->headersize) +
 		       le32_to_cpu(hdr->pdr_offset));
 
@@ -535,7 +545,13 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	       dev->name, hermes_present(hw));
 
 abort:
-	release_firmware(fw_entry);
+
+	/* If downloading failed, destroy cached copy */
+	if (err) {
+		priv->cached_fw_size = 0;
+		kfree(priv->cached_fw_data);
+		priv->cached_fw_data = NULL;
+	}
 
 free:
 	kfree(pda);
@@ -3534,6 +3550,9 @@ struct net_device
 	netif_carrier_off(dev);
 	priv->last_linkstatus = 0xffff;
 
+	priv->cached_fw_size = 0;
+	priv->cached_fw_data = NULL;
+
 	return dev;
 }
 
@@ -3545,6 +3564,7 @@ void free_orinocodev(struct net_device *dev)
 	 * when we call tasklet_kill it will run one final time,
 	 * emptying the list */
 	tasklet_kill(&priv->rx_tasklet);
+	kfree(priv->cached_fw_data);
 	priv->wpa_ie_len = 0;
 	kfree(priv->wpa_ie);
 	orinoco_mic_free(priv);
diff --git a/drivers/net/wireless/orinoco.h b/drivers/net/wireless/orinoco.h
index 981570b..403fff8 100644
--- a/drivers/net/wireless/orinoco.h
+++ b/drivers/net/wireless/orinoco.h
@@ -164,6 +164,10 @@ struct orinoco_private {
 	unsigned int wpa_enabled:1;
 	unsigned int tkip_cm_active:1;
 	unsigned int key_mgmt:3;
+
+	/* Cached firmware to use in ->resume */
+	u8 *cached_fw_data;
+	size_t cached_fw_size;
 };
 
 #ifdef ORINOCO_DEBUG

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2] orinoco: cache downloadable firmware image in memory for use during resume

[Dave]

Andrey Borzenkov wrote:
> orinoco: cache downloadable firmware image in memory for use during resume
> 
> If card is using downloadable firmware (like Agere 9.x), firmware has
> to be reloaded during resume. It is not possible to use request_firmware
> for that, because tasks are still frozen, so request_firmware will
> just timeout and fail. So cache firmware image in memory for later
> reuse in ->resume method.
> 
> Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>
> 
> ---
> 
> Unfortunately this is the only way to do it given current infrastructure.
> I think that extra memory cost (~60kb) does not warrant anything more
> sophisticated - even if this is possible. Also users not using dowloadable
> firmware won't be penalized at all.

Spectrum_cs has had firmware download for a while. It achieves the firmware reload on resume by doing schedule_work(&priv->reset_work).

Would the same work for orinoco_cs?

If not, is there a way to avoid the caching when you've got the firmware built into the kernel image?



Regards,

Dave.

Re: [PATCH 2/2] orinoco: cache downloadable firmware image in memory for use during resume

[Andrey Borzenkov]

[-- Attachment #1.1: Type: text/plain, Size: 1424 bytes --]

On Saturday 11 October 2008, Dave wrote:
> Andrey Borzenkov wrote:
> > orinoco: cache downloadable firmware image in memory for use during resume
> > 
> > If card is using downloadable firmware (like Agere 9.x), firmware has
> > to be reloaded during resume. It is not possible to use request_firmware
> > for that, because tasks are still frozen, so request_firmware will
> > just timeout and fail. So cache firmware image in memory for later
> > reuse in ->resume method.
> > 
> > Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>
> > 
> > ---
> > 
> > Unfortunately this is the only way to do it given current infrastructure.
> > I think that extra memory cost (~60kb) does not warrant anything more
> > sophisticated - even if this is possible. Also users not using dowloadable
> > firmware won't be penalized at all.
> 
> Spectrum_cs has had firmware download for a while. It achieves the firmware reload on resume by doing schedule_work(&priv->reset_work).
> 
> Would the same work for orinoco_cs?
> 

I think it could result in hard to trace race condition, because you really
have no way to synchronize it with unfreezing of udev.

> If not, is there a way to avoid the caching when you've got the firmware built into the kernel image?
> 

Actually, yes. Patch attached. It needs some touching when (if) we start
supporting AP mode; right now only one kind of image is ever loaded.

[-- Attachment #1.2: 02-orinoco-cache-firmware-image-in-memory --]
[-- Type: text/x-diff, Size: 3035 bytes --]

Subject: [PATCH] orinoco: cache downloadable firmware image in memory for use during resume
From: Andrey Borzenkov <arvidjaar@mail.ru>

If card is using downloadable firmware (like Agere 9.x), firmware has
to be reloaded during resume. It is not possible to use request_firmware
for that, because tasks are still frozen, so request_firmware will
just timeout and fail. So cache firmware image in memory for later
reuse in ->resume method.

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>

---

 drivers/net/wireless/orinoco.c |   28 +++++++++++++++++++++-------
 drivers/net/wireless/orinoco.h |    5 +++++
 2 files changed, 26 insertions(+), 7 deletions(-)


diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 722619c..8a15230 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -487,12 +487,17 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	if (err)
 		goto free;
 
-	err = request_firmware(&fw_entry, firmware, priv->dev);
-	if (err) {
-		printk(KERN_ERR "%s: Cannot find firmware %s\n",
-		       dev->name, firmware);
-		err = -ENOENT;
-		goto free;
+	if (priv->cached_fw)
+		fw_entry = priv->cached_fw;
+	else {
+		err = request_firmware(&fw_entry, firmware, priv->dev);
+		if (err) {
+			printk(KERN_ERR "%s: Cannot find firmware %s\n",
+			       dev->name, firmware);
+			err = -ENOENT;
+			goto free;
+		}
+		priv->cached_fw = fw_entry;
 	}
 
 	hdr = (const struct orinoco_fw_header *) fw_entry->data;
@@ -535,7 +540,11 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	       dev->name, hermes_present(hw));
 
 abort:
-	release_firmware(fw_entry);
+	/* In case of error, assume firmware was bogus and release it */
+	if (err) {
+		priv->cached_fw = NULL;
+		release_firmware(fw_entry);
+	}
 
 free:
 	kfree(pda);
@@ -3534,6 +3543,8 @@ struct net_device
 	netif_carrier_off(dev);
 	priv->last_linkstatus = 0xffff;
 
+	priv->cached_fw = NULL;
+
 	return dev;
 }
 
@@ -3545,6 +3556,9 @@ void free_orinocodev(struct net_device *dev)
 	 * when we call tasklet_kill it will run one final time,
 	 * emptying the list */
 	tasklet_kill(&priv->rx_tasklet);
+	if (priv->cached_fw)
+		release_firmware(priv->cached_fw);
+	priv->cached_fw = NULL;
 	priv->wpa_ie_len = 0;
 	kfree(priv->wpa_ie);
 	orinoco_mic_free(priv);
diff --git a/drivers/net/wireless/orinoco.h b/drivers/net/wireless/orinoco.h
index 981570b..8c29538 100644
--- a/drivers/net/wireless/orinoco.h
+++ b/drivers/net/wireless/orinoco.h
@@ -66,6 +66,8 @@ struct orinoco_rx_data {
 	struct list_head list;
 };
 
+struct firmware;
+
 struct orinoco_private {
 	void *card;	/* Pointer to card dependent structure */
 	struct device *dev;
@@ -164,6 +166,9 @@ struct orinoco_private {
 	unsigned int wpa_enabled:1;
 	unsigned int tkip_cm_active:1;
 	unsigned int key_mgmt:3;
+
+	/* Cached in memory firmware to use in ->resume */
+	const struct firmware *cached_fw;
 };
 
 #ifdef ORINOCO_DEBUG

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/2] orinoco: reload firmware on resume

[Dave]

Andrey Borzenkov wrote:
> On resume card state is likely lost so we have to reload firmware
> again.
> 
> Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>
> 
> ---
> 
> This is non-functional without second patch. Currently you simply
> have no way to load request firmware from user space in ->resume.

Does it work when you compile the firmware into the kernel image
with CONFIG_FIRMWARE_KERNEL, CONFIG_EXTRA_FIRMWARE and
CONFIG_EXTRA_FIRMWARE_DIR?
 
> --- a/drivers/net/wireless/orinoco.c
> +++ b/drivers/net/wireless/orinoco.c
> @@ -2298,9 +2298,14 @@ int orinoco_reinit_firmware(struct net_device *dev)
>  {
>  	struct orinoco_private *priv = netdev_priv(dev);
>  	struct hermes *hw = &priv->hw;
> -	int err;
> +	int err = 0;

The explicit initialisation is unneccesary.

>  	err = hermes_init(hw);
> +	if (priv->do_fw_download && !err) {

Otherwise this looks right.


Regards,

Dave.

Re: [PATCH 1/2] orinoco: reload firmware on resume

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

On Saturday 11 October 2008, Dave wrote:
> Andrey Borzenkov wrote:
> > On resume card state is likely lost so we have to reload firmware
> > again.
> > 
> > Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>
> > 
> > ---
> > 
> > This is non-functional without second patch. Currently you simply
> > have no way to load request firmware from user space in ->resume.
> 
> Does it work when you compile the firmware into the kernel image
> with CONFIG_FIRMWARE_KERNEL, CONFIG_EXTRA_FIRMWARE and
> CONFIG_EXTRA_FIRMWARE_DIR?
>  

I assume yes - but the resulting binary cannot be redistributed, so it
of little help; it means, orinoco driver in distributions still won't
work out of the box.

[From another mail]
> Spectrum_cs has had firmware download for a while. It achieves the firmware
> reload on resume by doing schedule_work(&priv->reset_work).
>
> Would the same work for orinoco_cs?

You (currently) have no way to synchronize reset_work with unfreezing
other tasks. It may work most of the time but it may as well fail every
now and then. Also spectrum_cs is obviously broken - it claims hardware
and interface are available *before* they are actiually available. This
again is hard to debug race condition.

It is planned to add pre-freeze notifiers; in this case we could use them
to load firmware before suspend. But right now I do not see how this situation
is different from compiling firmware in module except that it is more clean
from legal point of view.

Personally I prefer to use it right now (ideally in 2.6.28; do not know who
decides) and return to it later when we have better interfaces.

> > +	int err = 0;
> 
> The explicit initialisation is unneccesary.

OK it is leftover; I tried first to load firmware before hermes_init; it
worked for STR and failed for STD. I'll remove it; thank you.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/2] orinoco: reload firmware on resume

[Andrey Borzenkov]

[-- Attachment #1.1: Type: text/plain, Size: 109 bytes --]

On Saturday 11 October 2008, Dave wrote:
> > -	int err;
> > +	int err = 0;
> 

Fixed version attached.

[-- Attachment #1.2: 01-orinoco-reload-firmware-on-resume --]
[-- Type: text/x-diff, Size: 1160 bytes --]

Subject: [PATCH] orinoco: reload firmware on resume
From: Andrey Borzenkov <arvidjaar@mail.ru>

On resume card state is likely lost so we have to reload firmware
again.

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>

---

 drivers/net/wireless/orinoco.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)


diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 3053605..7284ff5 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -2317,6 +2317,11 @@ int orinoco_reinit_firmware(struct net_device *dev)
 	int err;
 
 	err = hermes_init(hw);
+	if (priv->do_fw_download && !err) {
+		err = orinoco_download(priv);
+		if (err)
+			priv->do_fw_download = 0;
+	}
 	if (!err)
 		err = orinoco_allocate_fid(dev);
 
@@ -2942,12 +2947,6 @@ static void orinoco_reset(struct work_struct *work)
 		}
 	}
 
-	if (priv->do_fw_download) {
-		err = orinoco_download(priv);
-		if (err)
-			priv->do_fw_download = 0;
-	}
-
 	err = orinoco_reinit_firmware(dev);
 	if (err) {
 		printk(KERN_ERR "%s: orinoco_reset: Error %d re-initializing firmware\n",

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/2] orinoco: reload firmware on resume

[Dave]

Andrey Borzenkov wrote:
> On Saturday 11 October 2008, Dave wrote:
>>> -	int err;
>>> +	int err = 0;
> 
> Fixed version attached.

Acked-by: David Kilroy <kilroyd@googlemail.com>


Note that if the firmware download fails the card won't work because we no longer  allocate a fid. However even if we were to allocate a fid the driver flags would be inconsistent with the firmware capabilities, and the driver would just encounter a bunch of errors.

We should probably think about how we can fall back to the inbuilt firmware after a failed firmware download - but this can be done separately.


Regards,

Dave.

Re: [PATCH 1/2] orinoco: reload firmware on resume

[John W. Linville]

On Wed, Oct 15, 2008 at 08:31:26PM +0100, Dave wrote:
> Andrey Borzenkov wrote:
>> On Saturday 11 October 2008, Dave wrote:
>>>> -	int err;
>>>> +	int err = 0;
>>
>> Fixed version attached.
>
> Acked-by: David Kilroy <kilroyd@googlemail.com>

Please repost this series with the finalized patches, rebased on the
current wireless-testing.  Also, please include the patches inline,
as your MIME encoding is confusing my tools.

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

[PATCH 2/2] orinoco: cache downloadable firmware image in memory for use during resume

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]

From: Andrey Borzenkov <arvidjaar@mail.ru>
Date: Sun, 19 Oct 2008 12:06:11 +0400
Subject: [PATCH 2/2] orinoco: cache downloadable firmware image in memory for use during resume

If card is using downloadable firmware (like Agere 9.x), firmware has
to be reloaded during resume. It is not possible to use request_firmware
for that, because tasks are still frozen, so request_firmware will
just timeout and fail. So cache firmware image in memory for later
reuse in ->resume method.

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>
---

This version transparently works both with external and built-in firmware.
In case of built in extra overhead is one "struct firmware".

spectrum_cs has used delayed work to reset card after resume. After
reading multiple discussions on lkml I got impression, that this is
unreliable - there is no way to sycnronize delayed task with unfreezing
of user space.

 drivers/net/wireless/orinoco.c |   28 +++++++++++++++++++++-------
 drivers/net/wireless/orinoco.h |    5 +++++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 860803e..653306f 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -487,12 +487,17 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	if (err)
 		goto free;
 
-	err = request_firmware(&fw_entry, firmware, priv->dev);
-	if (err) {
-		printk(KERN_ERR "%s: Cannot find firmware %s\n",
-		       dev->name, firmware);
-		err = -ENOENT;
-		goto free;
+	if (priv->cached_fw)
+		fw_entry = priv->cached_fw;
+	else {
+		err = request_firmware(&fw_entry, firmware, priv->dev);
+		if (err) {
+			printk(KERN_ERR "%s: Cannot find firmware %s\n",
+			       dev->name, firmware);
+			err = -ENOENT;
+			goto free;
+		}
+		priv->cached_fw = fw_entry;
 	}
 
 	hdr = (const struct orinoco_fw_header *) fw_entry->data;
@@ -535,7 +540,11 @@ orinoco_dl_firmware(struct orinoco_private *priv,
 	       dev->name, hermes_present(hw));
 
 abort:
-	release_firmware(fw_entry);
+	/* In case of error, assume firmware was bogus and release it */
+	if (err) {
+		priv->cached_fw = NULL;
+		release_firmware(fw_entry);
+	}
 
 free:
 	kfree(pda);
@@ -3534,6 +3543,8 @@ struct net_device
 	netif_carrier_off(dev);
 	priv->last_linkstatus = 0xffff;
 
+	priv->cached_fw = NULL;
+
 	return dev;
 }
 
@@ -3545,6 +3556,9 @@ void free_orinocodev(struct net_device *dev)
 	 * when we call tasklet_kill it will run one final time,
 	 * emptying the list */
 	tasklet_kill(&priv->rx_tasklet);
+	if (priv->cached_fw)
+		release_firmware(priv->cached_fw);
+	priv->cached_fw = NULL;
 	priv->wpa_ie_len = 0;
 	kfree(priv->wpa_ie);
 	orinoco_mic_free(priv);
diff --git a/drivers/net/wireless/orinoco.h b/drivers/net/wireless/orinoco.h
index 981570b..8c29538 100644
--- a/drivers/net/wireless/orinoco.h
+++ b/drivers/net/wireless/orinoco.h
@@ -66,6 +66,8 @@ struct orinoco_rx_data {
 	struct list_head list;
 };
 
+struct firmware;
+
 struct orinoco_private {
 	void *card;	/* Pointer to card dependent structure */
 	struct device *dev;
@@ -164,6 +166,9 @@ struct orinoco_private {
 	unsigned int wpa_enabled:1;
 	unsigned int tkip_cm_active:1;
 	unsigned int key_mgmt:3;
+
+	/* Cached in memory firmware to use in ->resume */
+	const struct firmware *cached_fw;
 };
 
 #ifdef ORINOCO_DEBUG
-- 
1.6.0.2


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]