This is informative information to provide about which actual firmware
file is being used.
Signed-off-by: Jarl Friis <[email protected]>
---
drivers/net/wireless/b43/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index a140165..202a0eb 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -2131,6 +2131,7 @@ int b43_do_request_fw(struct b43_request_fw_context *ctx,
B43_WARN_ON(1);
return -ENOSYS;
}
+ b43info(ctx->dev->wl, "Requesting firmware file '%s'\n", ctx->fwname);
err = request_firmware(&blob, ctx->fwname, ctx->dev->dev->dev);
if (err == -ENOENT) {
snprintf(ctx->errors[ctx->req_type],
--
1.7.9.5
On 09/19/2012 08:54 PM, Jarl Friis wrote:
>> Is the addition of your copyright to the driver
>> >warranted by this change?
> As far as I understand the copyright law: Yes, but I'm not an expert.
> Neither am I 100% sure what you mean.
You add your name to the copy notice of the file so you claim it for the
entire file.
>> >For example, I have made much larger contributions
>> >to b43 over the years before I started doing reverse-engineering on this
>> >driver, but I never added my copyright.
> I suggest you do.
Typically, the names you see there are and/or were b43 maintainers.
>> >Your "Signed-off-by" implies
>> >copyright for the patch.
> The fact that I authored the patch implies copyright (even without
> Signed-off-by)
>
> Jarl
You can indeed claim it for your patch, but I think not beyond that. But
I am no lawyer either so .....pffff.
Gr. AvS
2012/9/19 Jarl Friis <[email protected]>:
> This is using the LP specific firmware to better take advantage of the
> Low-Power capabilities.
Gosh... I just realized that the code I introduced is completely
untested. My hardware does not reach these pieces of code...
Sorry...
However the code seems natural (due to the firmware file name pattern)
for a PHY-LP hardware, that is a bcm4313 chip (pciid 14e4:4315).
So if there is anybody with such hardware... you might want to try it... Sorry.
Jarl
On 19.09.2012 15:18, Jarl Friis wrote:
> + b43info(ctx->dev->wl, "Requesting firmware file '%s'\n", ctx->fwname);
> err = request_firmware(&blob, ctx->fwname, ctx->dev->dev->dev);
Hmm. I wonder if this should be printed in request_firmware()
itself instead of in all callers?
/mjt
Never mind the copyright notice I was just trying to follow procedures...
And apparently that was not one of them... :-)
Jarl
2012/9/19 Arend van Spriel <[email protected]>:
> On 09/19/2012 08:54 PM, Jarl Friis wrote:
>>>
>>> Is the addition of your copyright to the driver
>>> >warranted by this change?
>>
>> As far as I understand the copyright law: Yes, but I'm not an expert.
>> Neither am I 100% sure what you mean.
>
>
> You add your name to the copy notice of the file so you claim it for the
> entire file.
>
>
>>> >For example, I have made much larger contributions
>>> >to b43 over the years before I started doing reverse-engineering on this
>>> >driver, but I never added my copyright.
>>
>> I suggest you do.
>
>
> Typically, the names you see there are and/or were b43 maintainers.
>
>
>>> >Your "Signed-off-by" implies
>>> >copyright for the patch.
>>
>> The fact that I authored the patch implies copyright (even without
>> Signed-off-by)
>>
>> Jarl
>
>
> You can indeed claim it for your patch, but I think not beyond that. But I
> am no lawyer either so .....pffff.
>
> Gr. AvS
>
--
Jarl Friis
Softace ApS
Rådhustorvet 7, 2.
3520 Farum
LinkedIn: http://dk.linkedin.com/in/jarlfriis
Thanks for feed back.
2012/9/19 Larry Finger <[email protected]>:
>
>
> I have some questions about this patch. Where did you get the information
> needed to make these changes?
To be completely honest: I didn't get any information, it is based
purely on practical experiments: I experienced some inconsistency wrt.
performance on my hp6735b. I had some time to look at the driver (it's
15 years since I patched the kernel last time). Anyway I saw the
filename pattern for firmware and saw that `b43-fwcutter
broadcom-wl-5.100.138/linux/wl_apsta.o -w` contained similar files
ending on "16". So I simple tried it out (for version 16), and so far
it seems to work better. It also seems to wake up faster after a
sleep.
> Did it come from reverse engineering some
> Broadcom code
No!
> or did you look at their actual code?
No!
> There is a great deal
> of difference relative to our "clean-room" status. Anyone that has seen
> non-GPL Broadcom material cannot contribute code to b43.
I have not seen any Broadcom code at all (apart from the stuff that is
already in the linux source tree)
>
> Have you tested this code on devices with rev>=16?
Yes on my HP6735b having this chip integrated:
[ 1577.549270] b43-phy1: Broadcom 4322 WLAN found (core revision 16)
[ 1577.592117] b43-phy1 debug: Found PHY: Analog 8, Type 4, Revision 4
[ 1577.592158] b43-phy1 debug: Found Radio: Manuf 0x17F, Version
0x2056, Revision 3
I guess the part of the patch for PHY_LP has not been reached. I will
submit a new series of patches that separates things
>
> Now for some comments: This patch also needs the "b43:" added to the
> subject.
Sorry. It's long ago I have submitted patches to the kernel.
> In addition, you appear to have at least one white-space error in
> the MODULE_FIRMWARE line.
I am not sure what you mean here. Is this a mail issue... (I wrote it
just like the other ones around it)
> Is the addition of your copyright to the driver
> warranted by this change?
As far as I understand the copyright law: Yes, but I'm not an expert.
Neither am I 100% sure what you mean.
> For example, I have made much larger contributions
> to b43 over the years before I started doing reverse-engineering on this
> driver, but I never added my copyright.
I suggest you do.
> Your "Signed-off-by" implies
> copyright for the patch.
The fact that I authored the patch implies copyright (even without
Signed-off-by)
Jarl
2012/9/19 Jarl Friis <[email protected]>:
> 2012/9/19 Jarl Friis <[email protected]>:
>> This is using the LP specific firmware to better take advantage of the
>> Low-Power capabilities.
>
> Gosh... I just realized that the code I introduced is completely
> untested. My hardware does not reach these pieces of code...
>
> Sorry...
>
> However the code seems natural (due to the firmware file name pattern)
> for a PHY-LP hardware, that is a bcm4313 chip (pciid 14e4:4315).
Well, your BCM4322 is not LP-PHY card... does it really work better
after that change, and wakes up faster? ;)
BCM4313 is LCN-PHY, so your change won't affect it, not to mention it
is not supported by b43.
14e4:4315 is really BCM4312 (LP-PHY) and maybe could be affected by
this patch... but just in case of core 16. AFAIK our 14e4:4315 are
usually devices with core rev 15.
--
Rafał
2012/9/20 Rafał Miłecki <[email protected]>:
> Well, your BCM4322 is not LP-PHY card... does it really work better
> after that change, and wakes up faster? ;)
That was my un-scientific impression, yes (probably biased by
expectations, right? :-) ). But it could also be that I was using a
more upstream kernel than the stock ubuntu kernel. I don't know :-)
> BCM4313 is LCN-PHY, so your change won't affect it, not to mention it
> is not supported by b43.
>
> 14e4:4315 is really BCM4312 (LP-PHY) and maybe could be affected by
> this patch... but just in case of core 16. AFAIK our 14e4:4315 are
> usually devices with core rev 15.
Thanks for clearing that out...
Jarl
Hi Jarl,
You should really mention which driver these are for in the subject line, say:
[PATCH] b43: Added information about which firmware file is being requested
On Wed, Sep 19, 2012 at 9:18 PM, Jarl Friis <[email protected]> wrote:
> This is informative information to provide about which actual firmware
> file is being used.
Also, this patch is so small and obvious that you could arguably get
away with something like:
Subject: [PATCH] b43: Log firmware filename
Log the name of the firmware file requested.
Or something along those lines.
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
2012/9/19 Jarl Friis <[email protected]>:
> 2012/9/19 Michael Tokarev <[email protected]>:
>> On 19.09.2012 15:18, Jarl Friis wrote:
>>
>>> + b43info(ctx->dev->wl, "Requesting firmware file '%s'\n", ctx->fwname);
>>> err = request_firmware(&blob, ctx->fwname, ctx->dev->dev->dev);
>>
>> Hmm. I wonder if this should be printed in request_firmware()
>> itself instead of in all callers?
>
> Now that you mention it, I also think that is a much better idea.
> However that would be a much more central place to do the change, so I
> would gladly see somebody else do that patch (in replacement of mine)
I agree, please submit patch modifying request_firmware if you believe
it's important.
--
Rafał
2012/9/19 Michael Tokarev <[email protected]>:
> On 19.09.2012 15:18, Jarl Friis wrote:
>
>> + b43info(ctx->dev->wl, "Requesting firmware file '%s'\n", ctx->fwname);
>> err = request_firmware(&blob, ctx->fwname, ctx->dev->dev->dev);
>
> Hmm. I wonder if this should be printed in request_firmware()
> itself instead of in all callers?
Now that you mention it, I also think that is a much better idea.
However that would be a much more central place to do the change, so I
would gladly see somebody else do that patch (in replacement of mine)
Jarl
On 09/19/2012 06:18 AM, Jarl Friis wrote:
> This is using the LP specific firmware to better take advantage of the
> Low-Power capabilities.
>
> Signed-off-by: Jarl Friis <[email protected]>
> ---
> drivers/net/wireless/b43/main.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> index 202a0eb..9ee6030 100644
> --- a/drivers/net/wireless/b43/main.c
> +++ b/drivers/net/wireless/b43/main.c
> @@ -8,6 +8,7 @@
> Copyright (c) 2005 Danny van Dyk <[email protected]>
> Copyright (c) 2005 Andreas Jaggi <[email protected]>
> Copyright (c) 2010-2011 Rafał Miłecki <[email protected]>
> + Copyright (c) 2012 Jarl Friis <[email protected]>
>
> SDIO support
> Copyright (c) 2009 Albert Herranz <[email protected]>
> @@ -72,6 +73,7 @@ MODULE_FIRMWARE("b43/ucode11.fw");
> MODULE_FIRMWARE("b43/ucode13.fw");
> MODULE_FIRMWARE("b43/ucode14.fw");
> MODULE_FIRMWARE("b43/ucode15.fw");
> +MODULE_FIRMWARE("b43/ucode16_lp.fw");
> MODULE_FIRMWARE("b43/ucode16_mimo.fw");
> MODULE_FIRMWARE("b43/ucode5.fw");
> MODULE_FIRMWARE("b43/ucode9.fw");
> @@ -2208,6 +2210,12 @@ static int b43_try_request_fw(struct b43_request_fw_context *ctx)
> else
> goto err_no_ucode;
> break;
> + case B43_PHYTYPE_LP:
> + if (rev >= 16)
> + filename = "ucode16_lp";
> + else
> + goto err_no_ucode;
> + break;
> case B43_PHYTYPE_HT:
> if (rev == 29)
> filename = "ucode29_mimo";
> @@ -2277,8 +2285,10 @@ static int b43_try_request_fw(struct b43_request_fw_context *ctx)
> filename = "lp0initvals13";
> else if (rev == 14)
> filename = "lp0initvals14";
> - else if (rev >= 15)
> + else if (rev == 15)
> filename = "lp0initvals15";
> + else if (rev >= 16)
> + filename = "lp0initvals16";
> else
> goto err_no_initvals;
> break;
> @@ -2336,8 +2346,10 @@ static int b43_try_request_fw(struct b43_request_fw_context *ctx)
> filename = "lp0bsinitvals13";
> else if (rev == 14)
> filename = "lp0bsinitvals14";
> - else if (rev >= 15)
> + else if (rev == 15)
> filename = "lp0bsinitvals15";
> + else if (rev >= 16)
> + filename = "lp0bsinitvals16";
> else
> goto err_no_initvals;
> break;
I have some questions about this patch. Where did you get the information needed
to make these changes? Did it come from reverse engineering some Broadcom code,
or did you look at their actual code? There is a great deal of difference
relative to our "clean-room" status. Anyone that has seen non-GPL Broadcom
material cannot contribute code to b43.
Have you tested this code on devices with rev>=16?
Now for some comments: This patch also needs the "b43:" added to the subject. In
addition, you appear to have at least one white-space error in the
MODULE_FIRMWARE line. Is the addition of your copyright to the driver warranted
by this change? For example, I have made much larger contributions to b43 over
the years before I started doing reverse-engineering on this driver, but I never
added my copyright. Your "Signed-off-by" implies copyright for the patch.
Larry
This is using the LP specific firmware to better take advantage of the
Low-Power capabilities.
Signed-off-by: Jarl Friis <[email protected]>
---
drivers/net/wireless/b43/main.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index 202a0eb..9ee6030 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -8,6 +8,7 @@
Copyright (c) 2005 Danny van Dyk <[email protected]>
Copyright (c) 2005 Andreas Jaggi <[email protected]>
Copyright (c) 2010-2011 Rafał Miłecki <[email protected]>
+ Copyright (c) 2012 Jarl Friis <[email protected]>
SDIO support
Copyright (c) 2009 Albert Herranz <[email protected]>
@@ -72,6 +73,7 @@ MODULE_FIRMWARE("b43/ucode11.fw");
MODULE_FIRMWARE("b43/ucode13.fw");
MODULE_FIRMWARE("b43/ucode14.fw");
MODULE_FIRMWARE("b43/ucode15.fw");
+MODULE_FIRMWARE("b43/ucode16_lp.fw");
MODULE_FIRMWARE("b43/ucode16_mimo.fw");
MODULE_FIRMWARE("b43/ucode5.fw");
MODULE_FIRMWARE("b43/ucode9.fw");
@@ -2208,6 +2210,12 @@ static int b43_try_request_fw(struct b43_request_fw_context *ctx)
else
goto err_no_ucode;
break;
+ case B43_PHYTYPE_LP:
+ if (rev >= 16)
+ filename = "ucode16_lp";
+ else
+ goto err_no_ucode;
+ break;
case B43_PHYTYPE_HT:
if (rev == 29)
filename = "ucode29_mimo";
@@ -2277,8 +2285,10 @@ static int b43_try_request_fw(struct b43_request_fw_context *ctx)
filename = "lp0initvals13";
else if (rev == 14)
filename = "lp0initvals14";
- else if (rev >= 15)
+ else if (rev == 15)
filename = "lp0initvals15";
+ else if (rev >= 16)
+ filename = "lp0initvals16";
else
goto err_no_initvals;
break;
@@ -2336,8 +2346,10 @@ static int b43_try_request_fw(struct b43_request_fw_context *ctx)
filename = "lp0bsinitvals13";
else if (rev == 14)
filename = "lp0bsinitvals14";
- else if (rev >= 15)
+ else if (rev == 15)
filename = "lp0bsinitvals15";
+ else if (rev >= 16)
+ filename = "lp0bsinitvals16";
else
goto err_no_initvals;
break;
--
1.7.9.5