Andrey Borzenkov wrote:
The attached patch fixes 4K stack for me. I have not tested spectrum case.
Thanks for identifying the problem. The Agere case looks good - a few suggestions for the Symbol case though:
diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -549,12 +556,16 @@ symbol_dl_image(struct orinoco_private *priv, const struct fw_info *fw,
int secondary)
{
hermes_t *hw = &priv->hw;
- int ret;
+ int ret = 0;
const unsigned char *ptr;
const unsigned char *first_block;
/* Plug Data Area (PDA) */
- __le16 pda[256];
+ __le16 *pda;
Please initialise pda to NULL here...
+ pda = kzalloc(fw->pda_size, GFP_KERNEL);
+ if (!pda)
+ return -ENOMEM;
And move the allocation to
/* Binary block begins after the 0x1A marker */
ptr = image;
@@ -563,22 +574,22 @@ symbol_dl_image(struct orinoco_private *priv, const struct fw_info *fw,
/* Read the PDA from EEPROM */
if (secondary) {
... here.
- ret = hermes_read_pda(hw, pda, fw->pda_addr, sizeof(pda), 1);
+ ret = hermes_read_pda(hw, pda, fw->pda_addr, fw->pda_size, 1);
if (ret)
- return ret;
+ goto free;
}
/* Stop the firmware, so that it can be safely rewritten */
if (priv->stop_fw) {
ret = priv->stop_fw(priv, 1);
if (ret)
- return ret;
+ goto free;
}
/* Program the adapter with new firmware */
ret = hermes_program(hw, first_block, end);
if (ret)
- return ret;
+ goto free;
/* Write the PDA to the adapter */
if (secondary) {
@@ -586,28 +597,31 @@ symbol_dl_image(struct orinoco_private *priv, const struct fw_info *fw,
ptr = first_block + len;
ret = hermes_apply_pda(hw, ptr, pda);
if (ret)
- return ret;
+ goto free;
Then kfree(pda) here. We're done with it now.
}
/* Run the firmware */
if (priv->stop_fw) {
ret = priv->stop_fw(priv, 0);
if (ret)
- return ret;
+ goto free;
So this isn't needed.
}
/* Reset hermes chip and make sure it responds */
ret = hermes_init(hw);
/* hermes_reset() should return 0 with the secondary firmware */
- if (secondary && ret != 0)
- return -ENODEV;
+ if (secondary && ret != 0) {
+ ret = -ENODEV;
+ goto free;
+ }
nor this.
/* And this should work with any firmware */
if (!hermes_present(hw))
- return -ENODEV;
+ ret = -ENODEV;
- return 0;
or these.
+free:
But you absolutely have to kfree(pda) here. I would prefer the label be something like 'abort' (what we are doing), rather than 'free' - but there's merit in keeping with what you have called in in orinoco_dl_firmware which already has an 'abort' label.
+ return ret;
}
The net effect of the above suggestion is that we won't allocate memory when programming the primary firmware (which is just before we do the secondary).
Dan Williams wrote:
maybe you should not use priv->pda_size but
#define SYMBOL_PDA_SIZE 256 and use that for the hermes_read_pda()
length just to ensure the patched code is functionally the same as
before the patch.
Using fw->pda_size should be fine. The value comes from a const static, set to 0x100 for Symbol.
Regards,
Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
