Report on bcma with 14e4:4353 (BCM43224)

Report on bcma with 14e4:4353 (BCM43224)

[Larry Finger]

Rafa?,

I'm using a recent pull from wireless-testing - 'git describe' results in 
master-2011-07-26-150-g4ea94a9.

The driver is working well with reasonable performance. Using tcpperf, I get 
8-10 Mb/s upload on an 802.11g connection. The connection has been up for 17 
hours with no disconnects.

A minor annoyance is that the driver does not autoload on boot and had to be 
manually modprobed. AFAIK, I don't have any blacklisting or other configuration 
parameters that would cause this. Furthermore, I don't see anything in the 
driver code that would cause this. Does autoload work on your system?

A more serious problem is that the driver does not work on my 802.11n AP that is 
set for "up to 270 Mbps at 2.4 GHz". It will authenticate and associate, but the 
throughput is minimal. It also gets the "PHY transmission error" messages. None 
occur with 802.11g. I would guess some kind of error in the HT40 settings.

Thanks for your hard work on this device.

Larry

Report on bcma with 14e4:4353 (BCM43224)

[Rafał Miłecki]

W dniu 30 lipca 2011 17:19 u?ytkownik Larry Finger
<Larry.Finger@lwfinger.net> napisa?:
> I'm using a recent pull from wireless-testing - 'git describe' results in
> master-2011-07-26-150-g4ea94a9.
>
> The driver is working well with reasonable performance. Using tcpperf, I get
> 8-10 Mb/s upload on an 802.11g connection. The connection has been up for 17
> hours with no disconnects.

Thanks for your testing, I appreciate it! I was busy recently with
other stuff, so didn't have much time to reply and test everything
myself.


> A minor annoyance is that the driver does not autoload on boot and had to be
> manually modprobed. AFAIK, I don't have any blacklisting or other
> configuration parameters that would cause this. Furthermore, I don't see
> anything in the driver code that would cause this. Does autoload work on
> your system?

I've everything blacklisted (ssb, bcma, b43, wl, brcmsmac), so I
didn't even notice that. However I got some reports (2 of them) that
b43 doesn't auto load. I've to take a look at it, however I've no idea
yet on what may be causing it.


> A more serious problem is that the driver does not work on my 802.11n AP
> that is set for "up to 270 Mbps at 2.4 GHz". It will authenticate and
> associate, but the throughput is minimal. It also gets the "PHY transmission
> error" messages. None occur with 802.11g. I would guess some kind of error
> in the HT40 settings.

My network (AP) is set up to work in 802.11g mode and I've performed
first tests today. Not good:
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0-124.5 sec  10.8 MBytes    724 Kbits/sec
I've noticed that connection dies when I force network rate to 54M.
Same for 48M and 9M. Connection works fine for 1M, 2M, 11M.
So this is again OFDM vs. CCK problem.

I didn't see any "PHY transmission error" messages however (maybe it's
matter of firmware? 666.2 here).


Could you take a look if anything has changed in phy_ctl1 in recent
drivers? Do you have any other suspicions?

-- 
Rafa?

[PATCH] Add uevent to bcma bus, to autoload drivers.

[David Woodhouse]

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
--
On Wed, 2011-08-17 at 20:27 +0200, Rafa? Mi?ecki wrote:
> > A minor annoyance is that the driver does not autoload on boot and had to be
> > manually modprobed. AFAIK, I don't have any blacklisting or other
> > configuration parameters that would cause this. Furthermore, I don't see
> > anything in the driver code that would cause this. Does autoload work on
> > your system?
>
> I've everything blacklisted (ssb, bcma, b43, wl, brcmsmac), so I
> didn't even notice that. However I got some reports (2 of them) that
> b43 doesn't auto load. I've to take a look at it, however I've no idea
> yet on what may be causing it.

The lack of uevent causes it. While looking, I note that suspend/resume
methods are also lacking from bcma.

diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index 7072216..8c09c3e 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -15,6 +15,7 @@ MODULE_LICENSE("GPL");
 static int bcma_bus_match(struct device *dev, struct device_driver *drv);
 static int bcma_device_probe(struct device *dev);
 static int bcma_device_remove(struct device *dev);
+static int bcma_device_uevent(struct device *dev, struct kobj_uevent_env *env);
 
 static ssize_t manuf_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -49,6 +50,7 @@ static struct bus_type bcma_bus_type = {
 	.match		= bcma_bus_match,
 	.probe		= bcma_device_probe,
 	.remove		= bcma_device_remove,
+	.uevent		= bcma_device_uevent,
 	.dev_attrs	= bcma_device_attrs,
 };
 
@@ -295,6 +297,16 @@ static int bcma_device_remove(struct device *dev)
 	return 0;
 }
 
+static int bcma_device_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct bcma_device *core = container_of(dev, struct bcma_device, dev);
+
+	return add_uevent_var(env,
+			      "MODALIAS=bcma:m%04Xid%04Xrev%02Xcl%02X",
+			      core->id.manuf, core->id.id,
+			      core->id.rev, core->id.class);
+}
+
 static int __init bcma_modinit(void)
 {
 	int err;

-- 
dwmw2

[PATCH] Add uevent to bcma bus, to autoload drivers.

[Rafał Miłecki]

W dniu 17 sierpnia 2011 23:43 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> --
> On Wed, 2011-08-17 at 20:27 +0200, Rafa? Mi?ecki wrote:
>> > A minor annoyance is that the driver does not autoload on boot and had to be
>> > manually modprobed. AFAIK, I don't have any blacklisting or other
>> > configuration parameters that would cause this. Furthermore, I don't see
>> > anything in the driver code that would cause this. Does autoload work on
>> > your system?
>>
>> I've everything blacklisted (ssb, bcma, b43, wl, brcmsmac), so I
>> didn't even notice that. However I got some reports (2 of them) that
>> b43 doesn't auto load. I've to take a look at it, however I've no idea
>> yet on what may be causing it.
>
> The lack of uevent causes it. While looking, I note that suspend/resume
> methods are also lacking from bcma.

Tested on my BCM43224, thanks a lot! :)

Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>

You didn't send this directly to John, not sure if he will pick it up.

Do you think this should go for 3.1? With this patch ppl will directly
see interface is present and will just use it or see error about
firmware in dmesg. Without the patch, person just have to know b43 is
supposed to support his card.


Maybe you just could use "bcma: " prefix :)

-- 
Rafa?

[PATCH] Add uevent to bcma bus, to autoload drivers.

[David Woodhouse]

On Thu, 2011-08-18 at 08:46 +0200, Rafa? Mi?ecki wrote:
> 
> Tested on my BCM43224, thanks a lot! :)
> 
> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
> 
> You didn't send this directly to John, not sure if he will pick it up.
> 
> Do you think this should go for 3.1? With this patch ppl will directly
> see interface is present and will just use it or see error about
> firmware in dmesg. Without the patch, person just have to know b43 is
> supposed to support his card. 

Yes, it should go in for 3.1.

I wasn't really expecting John to pick it up. You are the maintainer of
drivers/bcma, so I'd expect to send it to *you*. Many maintainers would
be unhappy at patches being sent upstream and bypassing them. Normally
I'd expect you to pick up the patch and add it to your tree/queue with
your own signed-off-by, and for it to go upstream that way.

Of course, I'm happy to send it on directly if that's what you prefer.

-- 
dwmw2

[PATCH] Add uevent to bcma bus, to autoload drivers.

[Rafał Miłecki]

W dniu 18 sierpnia 2011 09:51 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Thu, 2011-08-18 at 08:46 +0200, Rafa? Mi?ecki wrote:
>>
>> Tested on my BCM43224, thanks a lot! :)
>>
>> Acked-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>
>> You didn't send this directly to John, not sure if he will pick it up.
>>
>> Do you think this should go for 3.1? With this patch ppl will directly
>> see interface is present and will just use it or see error about
>> firmware in dmesg. Without the patch, person just have to know b43 is
>> supposed to support his card.
>
> Yes, it should go in for 3.1.
>
> I wasn't really expecting John to pick it up. You are the maintainer of
> drivers/bcma, so I'd expect to send it to *you*. Many maintainers would
> be unhappy at patches being sent upstream and bypassing them. Normally
> I'd expect you to pick up the patch and add it to your tree/queue with
> your own signed-off-by, and for it to go upstream that way.

I can accept sending patch directly to John, if I'm included in CC and
linux-wireless is included as well.

However, I can work the way you proposed as well :) Thanks for the
patch, just sent it to John.

-- 
Rafa?

[PATCH] Add uevent to bcma bus, to autoload drivers.

[David Woodhouse]

On Fri, 2011-08-19 at 22:13 +0200, Rafa? Mi?ecki wrote:
> I can accept sending patch directly to John, if I'm included in CC and
> linux-wireless is included as well.

But if b43 isn't ready for *some* bcma card, so you don't want it
autoloading, then perhaps we might have wanted to remove a MODULE_ALIAS
from b43 to prevent it being autoloaded for just *that* device. Or
something like that. So even for this "obviously correct" patch I'd
rather run it by you first.

-- 
dwmw2

[PATCH] Add uevent to bcma bus, to autoload drivers.

[Rafał Miłecki]

W dniu 19 sierpnia 2011 22:52 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Fri, 2011-08-19 at 22:13 +0200, Rafa? Mi?ecki wrote:
>> I can accept sending patch directly to John, if I'm included in CC and
>> linux-wireless is included as well.
>
> But if b43 isn't ready for *some* bcma card, so you don't want it
> autoloading, then perhaps we might have wanted to remove a MODULE_ALIAS
> from b43 to prevent it being autoloaded for just *that* device. Or
> something like that. So even for this "obviously correct" patch I'd
> rather run it by you first.

There is nothing wrong in auto-loading b43 for unsupported card. b43
will detect the PHY type is not supported and will return false from
probing function.

-- 
Rafa?