2008-10-11 14:16:31

by Andrey Borzenkov

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

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

Signed-off-by: Andrey Borzenkov <[email protected]>

---

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",


Attachments:
(No filename) (1.23 kB)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-11 17:42:49

by Dave Kilroy

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

Andrey Borzenkov wrote:
> On resume card state is likely lost so we have to reload firmware
> again.
>
> Signed-off-by: Andrey Borzenkov <[email protected]>
>
> ---
>
> 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.

2008-10-11 18:35:19

by Andrey Borzenkov

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

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 <[email protected]>
> >
> > ---
> >
> > 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.


Attachments:
(No filename) (1.78 kB)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-11 17:59:37

by Dave Kilroy

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

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 <[email protected]>
>
> ---
>
> 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.

2008-10-11 14:21:38

by Andrey Borzenkov

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

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 <[email protected]>

---

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


Attachments:
(No filename) (3.95 kB)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-12 17:43:04

by Andrey Borzenkov

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

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 <[email protected]>
> >
> > ---
> >
> > 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.


Attachments:
(No filename) (0.00 B)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2008-10-12 12:36:39

by Andrey Borzenkov

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

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

Fixed version attached.


Attachments:
(No filename) (0.00 B)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments