From: Samuel Ortiz <[email protected]>
We were sending the fragmentation threshold value to the wrong table,
causing an LMAC assert when setting it from wext.
Signed-off-by: Samuel Ortiz <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwmc3200wifi/cfg80211.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/iwmc3200wifi/cfg80211.c b/drivers/net/wireless/iwmc3200wifi/cfg80211.c
index 3256ad2..96f714e 100644
--- a/drivers/net/wireless/iwmc3200wifi/cfg80211.c
+++ b/drivers/net/wireless/iwmc3200wifi/cfg80211.c
@@ -268,7 +268,7 @@ static int iwm_cfg80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
iwm->conf.frag_threshold = wiphy->frag_threshold;
- ret = iwm_umac_set_config_fix(iwm, UMAC_PARAM_TBL_CFG_FIX,
+ ret = iwm_umac_set_config_fix(iwm, UMAC_PARAM_TBL_FA_CFG_FIX,
CFG_FRAG_THRESHOLD,
iwm->conf.frag_threshold);
if (ret < 0)
--
1.6.0.4
From: Marcel Holtmann <[email protected]>
Date: Tue, 26 May 2009 06:43:11 +0200
> Hi guys,
>
>> Firmware names can not be longer than 30 characters, and
>> iwmc3200wifi-lmac-calib-sdio.bin is too long.
>> Renaming it to iwmc3200wifi-calib-sdio.bin.
>
> hold on with this one. This limit should _NOT_ exist. Is this an issue
> in the driver model?
linux/firmware.h:FIRMWARE_NAME_MAX == 30
drivers/base/firmware_class.c:struct firmware_priv{ char fw_id[FIRMWARE_NAME_MAX]; ... };
On Tue, May 26, 2009 at 08:33:03AM +0200, Marcel Holtmann wrote:
> I think we need to fix this. Especially since all the other name length
> limits in the driver model are gone.
Yeah, but at the same time the driver should get a shorter and actually
pronouncable name. Why not just iwl3200 to follow the model set by
the other drivers?
Hi guys,
> Firmware names can not be longer than 30 characters, and
> iwmc3200wifi-lmac-calib-sdio.bin is too long.
> Renaming it to iwmc3200wifi-calib-sdio.bin.
hold on with this one. This limit should _NOT_ exist. Is this an issue
in the driver model?
Regards
Marcel
Hi Christoph,
> > I think we need to fix this. Especially since all the other name length
> > limits in the driver model are gone.
>
> Yeah, but at the same time the driver should get a shorter and actually
> pronouncable name. Why not just iwl3200 to follow the model set by
> the other drivers?
that beast is a multi-comm device with WiFi, WiMAX, Bluetooth and GPS. I
think you can guess which pieces are coming next.
Regards
Marcel
Hi Yi,
> > > FIRMWARE_NAME_MAX is defined 30 at this time.
> >
> > I think we need to fix this. Especially since all the other name
> > length limits in the driver model are gone.
>
> I agree. Should we fix this by simply increasing FIRMWARE_NAME_MAX to
> some acceptable length (what should it be? btw) or there are other
> better ideas?
I think just increasing the length of the static array is bad. Can we
just not allocated the needed length for the firmware filename?
> Anyway, I think whatever how we change FW_LOADER, this patch should
> still go. Because the "lmac" string doesn't make any sense in this file
> name due to calibration is only done in LMAC.
If you think that the lmac string is pointless anyway, then that is
fine. Changing the name, because of a FW_LOADER limitation should not be
done of course.
Regards
Marcel
On Tue, 2009-05-26 at 14:33 +0800, Marcel Holtmann wrote:
> > FIRMWARE_NAME_MAX is defined 30 at this time.
>
> I think we need to fix this. Especially since all the other name
> length limits in the driver model are gone.
I agree. Should we fix this by simply increasing FIRMWARE_NAME_MAX to
some acceptable length (what should it be? btw) or there are other
better ideas?
Anyway, I think whatever how we change FW_LOADER, this patch should
still go. Because the "lmac" string doesn't make any sense in this file
name due to calibration is only done in LMAC.
Thanks,
-yi
On Tue, 2009-05-26 at 14:36 +0800, Christoph Hellwig wrote:
> On Tue, May 26, 2009 at 08:33:03AM +0200, Marcel Holtmann wrote:
> > I think we need to fix this. Especially since all the other name length
> > limits in the driver model are gone.
>
> Yeah, but at the same time the driver should get a shorter and actually
> pronouncable name. Why not just iwl3200 to follow the model set by
> the other drivers?
There are totally 3 firmware images required by the iwmc3200wifi driver:
one used for calibration, one upper mac (UMAC) and a lower mac (LMAC).
This iwmc3200 is a multicomm device, WiFi is only one of its functions.
Finally we have to add the "-sdio" suffix to distinguish with other form
factors (e.g. USB, mini PCIe, etc).
Thanks,
-yi
Hi Marcel,
On Tue, May 26, 2009 at 01:51:34AM -0700, Marcel Holtmann wrote:
> > > > FIRMWARE_NAME_MAX is defined 30 at this time.
> > >
> > > I think we need to fix this. Especially since all the other name
> > > length limits in the driver model are gone.
> >
> > I agree. Should we fix this by simply increasing FIRMWARE_NAME_MAX to
> > some acceptable length (what should it be? btw) or there are other
> > better ideas?
>
> I think just increasing the length of the static array is bad. Can we
> just not allocated the needed length for the firmware filename?
I think we can, assuming all firmware name strings passed through
request_firmware() are \0 terminated. Based on that assumption, this is what I
propose:
--
drivers/base/firmware_class.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index d3a59c6..e1e69de 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -40,7 +40,7 @@ static int loading_timeout = 60; /* In seconds */
static DEFINE_MUTEX(fw_lock);
struct firmware_priv {
- char fw_id[FIRMWARE_NAME_MAX];
+ char *fw_id;
struct completion completion;
struct bin_attribute attr_data;
struct firmware *fw;
@@ -278,6 +278,7 @@ static void fw_dev_release(struct device *dev)
{
struct firmware_priv *fw_priv = dev_get_drvdata(dev);
+ kfree(fw_priv->fw_id);
kfree(fw_priv);
kfree(dev);
@@ -309,7 +310,14 @@ static int fw_register_device(struct device **dev_p, const char *fw_name,
init_completion(&fw_priv->completion);
fw_priv->attr_data = firmware_attr_data_tmpl;
- strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
+ fw_priv->fw_id = kzalloc(strlen(fw_name) + 1, GFP_KERNEL);
+ if (!fw_priv->fw_id) {
+ dev_err(device, "%s: Firmware name allocation failed\n",
+ __func__);
+ retval = -ENOMEM;
+ goto error_kfree;
+ }
+ strcpy(fw_priv->fw_id, fw_name);
fw_priv->timeout.function = firmware_class_timeout;
fw_priv->timeout.data = (u_long) fw_priv;
@@ -323,11 +331,14 @@ static int fw_register_device(struct device **dev_p, const char *fw_name,
retval = device_register(f_dev);
if (retval) {
dev_err(device, "%s: device_register failed\n", __func__);
- goto error_kfree;
+ goto error_kfree_fw_id;
}
*dev_p = f_dev;
return 0;
+error_kfree_fw_id:
+ kfree(fw_priv->fw_id);
+
error_kfree:
kfree(fw_priv);
kfree(f_dev);
--
Intel Open Source Technology Centre
http://oss.intel.com/
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
From: Zhu Yi <[email protected]>
Date: Tue, 26 May 2009 11:10:48 +0800
> The patch makes iwmc3200wifi select CFG80211 instead of LIB80211.
> This fixed module link error reported by Randy Dunlap
> <[email protected]> when compiling iwmc3200wifi without
> cfg80211 selected. WIRELESS_EXT is also selected by iwmc3200wifi.
>
> Signed-off-by: Zhu Yi <[email protected]>
I'm applying this directly so that the -next build failure
gets fixed.
On Tue, May 26, 2009 at 04:06:40AM -0700, Kay Sievers wrote:
> On Tue, May 26, 2009 at 13:04, Samuel Ortiz <[email protected]> wrote:
> > - ? ? ? strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
> > + ? ? ? fw_priv->fw_id = kzalloc(strlen(fw_name) + 1, GFP_KERNEL);
>
> kstrdup()?
Indeed, thanks.
I'm offloading this thread to lkml, as I dont want to continue polluting the
linux-wireless one with unrelated matters.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On Tue, 2009-05-26 at 12:43 +0800, Marcel Holtmann wrote:
> Hi guys,
>
> > Firmware names can not be longer than 30 characters, and
> > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > Renaming it to iwmc3200wifi-calib-sdio.bin.
>
> hold on with this one. This limit should _NOT_ exist. Is this an issue
> in the driver model?
It should be FW_LOADER framework implementation limitation.
request_firmware()
-> fw_setup_device()
-> fw_register_device()
fw_register_device()
{
...
strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
...
}
FIRMWARE_NAME_MAX is defined 30 at this time.
Thanks,
-yi
The patch makes iwmc3200wifi select CFG80211 instead of LIB80211.
This fixed module link error reported by Randy Dunlap
<[email protected]> when compiling iwmc3200wifi without
cfg80211 selected. WIRELESS_EXT is also selected by iwmc3200wifi.
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwmc3200wifi/Kconfig | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/iwmc3200wifi/Kconfig b/drivers/net/wireless/iwmc3200wifi/Kconfig
index ae84dda..41bd4b2 100644
--- a/drivers/net/wireless/iwmc3200wifi/Kconfig
+++ b/drivers/net/wireless/iwmc3200wifi/Kconfig
@@ -1,7 +1,8 @@
config IWM
tristate "Intel Wireless Multicomm 3200 WiFi driver"
depends on MMC && WLAN_80211 && EXPERIMENTAL
- select LIB80211
+ select WIRELESS_EXT
+ select CFG80211
select FW_LOADER
select RFKILL
--
1.6.0.4
Hi Yi,
> > > Firmware names can not be longer than 30 characters, and
> > > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > > Renaming it to iwmc3200wifi-calib-sdio.bin.
> >
> > hold on with this one. This limit should _NOT_ exist. Is this an issue
> > in the driver model?
>
> It should be FW_LOADER framework implementation limitation.
>
> request_firmware()
> -> fw_setup_device()
> -> fw_register_device()
>
> fw_register_device()
> {
> ...
> strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
> ...
> }
>
> FIRMWARE_NAME_MAX is defined 30 at this time.
I think we need to fix this. Especially since all the other name length
limits in the driver model are gone.
Regards
Marcel
On Tue, May 26, 2009 at 13:04, Samuel Ortiz <[email protected]> wrote:
> - strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX);
> + fw_priv->fw_id = kzalloc(strlen(fw_name) + 1, GFP_KERNEL);
kstrdup()?
Kay
From: Samuel Ortiz <[email protected]>
Firmware names can not be longer than 30 characters, and
iwmc3200wifi-lmac-calib-sdio.bin is too long.
Renaming it to iwmc3200wifi-calib-sdio.bin.
Signed-off-by: Samuel Ortiz <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwmc3200wifi/sdio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/iwmc3200wifi/sdio.c b/drivers/net/wireless/iwmc3200wifi/sdio.c
index edc0a00..b54da67 100644
--- a/drivers/net/wireless/iwmc3200wifi/sdio.c
+++ b/drivers/net/wireless/iwmc3200wifi/sdio.c
@@ -395,7 +395,7 @@ static struct iwm_if_ops if_sdio_ops = {
.debugfs_init = if_sdio_debugfs_init,
.debugfs_exit = if_sdio_debugfs_exit,
.umac_name = "iwmc3200wifi-umac-sdio.bin",
- .calib_lmac_name = "iwmc3200wifi-lmac-calib-sdio.bin",
+ .calib_lmac_name = "iwmc3200wifi-calib-sdio.bin",
.lmac_name = "iwmc3200wifi-lmac-sdio.bin",
};
--
1.6.0.4
Hi John,
On Mon, Jun 01, 2009 at 11:29:40AM -0700, John W. Linville wrote:
> On Tue, May 26, 2009 at 11:10:47AM +0800, Zhu Yi wrote:
> > From: Samuel Ortiz <[email protected]>
> >
> > Firmware names can not be longer than 30 characters, and
> > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > Renaming it to iwmc3200wifi-calib-sdio.bin.
> >
> > Signed-off-by: Samuel Ortiz <[email protected]>
> > Signed-off-by: Zhu Yi <[email protected]>
>
> Is this still wanted?
I havent heard anything from Greg regarding my firmware name length patch set.
I'll ping him and check if the patches are queued for rc1 inclusion.
Meanwhile I think you can leave this one aside for now, thanks.
Cheers,
Samuel.
> --
> John W. Linville Someday the world will need a hero, and you
> [email protected] might be all we have. Be ready.
--
Intel Open Source Technology Centre
http://oss.intel.com/
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On Mon, 2009-06-01 at 19:29 +0100, John W. Linville wrote:
> On Tue, May 26, 2009 at 11:10:47AM +0800, Zhu Yi wrote:
> > From: Samuel Ortiz <[email protected]>
> >
> > Firmware names can not be longer than 30 characters, and
> > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > Renaming it to iwmc3200wifi-calib-sdio.bin.
> >
> > Signed-off-by: Samuel Ortiz <[email protected]>
> > Signed-off-by: Zhu Yi <[email protected]>
>
> Is this still wanted?
Yes, it is still wanted, but probably with a better description:
From: Samuel Ortiz <[email protected]>
iwmc3200wifi: trim down calibration firmware name
The patch trims down iwmc3200wifi calibration firmware name from
iwmc3200wifi-lmac-calib-sdio.bin to iwmc3200wifi-calib-sdio.bin. We can
shorten the firmware name because all calibration is done by LMAC.
Signed-off-by: Samuel Ortiz <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
Thanks,
-yi
On Tue, Jun 02, 2009 at 01:02:03AM -0700, Marcel Holtmann wrote:
> Hi Samuel,
>
> > > > Firmware names can not be longer than 30 characters, and
> > > > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > > > Renaming it to iwmc3200wifi-calib-sdio.bin.
> > > >
> > > > Signed-off-by: Samuel Ortiz <[email protected]>
> > > > Signed-off-by: Zhu Yi <[email protected]>
> > >
> > > Is this still wanted?
> > I havent heard anything from Greg regarding my firmware name length patch set.
> > I'll ping him and check if the patches are queued for rc1 inclusion.
> > Meanwhile I think you can leave this one aside for now, thanks.
>
> actually we have to get the firmware straight before 2.6.31-rc1 hits the
> streets. So at some point we have to make up our mind ;)
Yep, we lacked some synchronization with Yi.
> Either way is fine btw. and fixing the firmware name length limitation
> is also the right thing to do.
Yes, that needed a fix, regardless of our local iwmc issues. I also agree with
Yi that our calibration firmware name is too long/verbose and needs to be
shortened. So both fixes should go in I think.
> Cutting the name because of a bogus
> legacy limitation was just the wrong fix here ;)
I agree, I was just too lazy to fix the firmware class stuff at first :/
Cheers,
Samuel.
> Regards
>
> Marcel
>
>
--
Intel Open Source Technology Centre
http://oss.intel.com/
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On Tue, Jun 02, 2009 at 12:52:20AM -0700, Zhu, Yi wrote:
> On Mon, 2009-06-01 at 19:29 +0100, John W. Linville wrote:
> > On Tue, May 26, 2009 at 11:10:47AM +0800, Zhu Yi wrote:
> > > From: Samuel Ortiz <[email protected]>
> > >
> > > Firmware names can not be longer than 30 characters, and
> > > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > > Renaming it to iwmc3200wifi-calib-sdio.bin.
> > >
> > > Signed-off-by: Samuel Ortiz <[email protected]>
> > > Signed-off-by: Zhu Yi <[email protected]>
> >
> > Is this still wanted?
>
> Yes, it is still wanted, but probably with a better description:
Sorry John, an obvious lack of communication on our part here :/
I agree with Yi about the fact that our calibration firmware name is
redundant, so in that regard it does make sense to push this patch now.
Sorry again for the confusion...
Cheers,
Samuel.
> From: Samuel Ortiz <[email protected]>
>
> iwmc3200wifi: trim down calibration firmware name
>
> The patch trims down iwmc3200wifi calibration firmware name from
> iwmc3200wifi-lmac-calib-sdio.bin to iwmc3200wifi-calib-sdio.bin. We can
> shorten the firmware name because all calibration is done by LMAC.
>
> Signed-off-by: Samuel Ortiz <[email protected]>
> Signed-off-by: Zhu Yi <[email protected]>
>
>
> Thanks,
> -yi
>
>
>
--
Intel Open Source Technology Centre
http://oss.intel.com/
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Hi Samuel,
> > > Firmware names can not be longer than 30 characters, and
> > > iwmc3200wifi-lmac-calib-sdio.bin is too long.
> > > Renaming it to iwmc3200wifi-calib-sdio.bin.
> > >
> > > Signed-off-by: Samuel Ortiz <[email protected]>
> > > Signed-off-by: Zhu Yi <[email protected]>
> >
> > Is this still wanted?
> I havent heard anything from Greg regarding my firmware name length patch set.
> I'll ping him and check if the patches are queued for rc1 inclusion.
> Meanwhile I think you can leave this one aside for now, thanks.
actually we have to get the firmware straight before 2.6.31-rc1 hits the
streets. So at some point we have to make up our mind ;)
Either way is fine btw. and fixing the firmware name length limitation
is also the right thing to do. Cutting the name because of a bogus
legacy limitation was just the wrong fix here ;)
Regards
Marcel
On Tue, May 26, 2009 at 11:10:47AM +0800, Zhu Yi wrote:
> From: Samuel Ortiz <[email protected]>
>
> Firmware names can not be longer than 30 characters, and
> iwmc3200wifi-lmac-calib-sdio.bin is too long.
> Renaming it to iwmc3200wifi-calib-sdio.bin.
>
> Signed-off-by: Samuel Ortiz <[email protected]>
> Signed-off-by: Zhu Yi <[email protected]>
Is this still wanted?
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.