2011-07-30 15:19:08

by Larry Finger

[permalink] [raw]
Subject: Report on bcma with 14e4:4353 (BCM43224)

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


2011-08-17 18:27:24

by Rafał Miłecki

[permalink] [raw]
Subject: Re: Report on bcma with 14e4:4353 (BCM43224)

W dniu 30 lipca 2011 17:19 użytkownik Larry Finger
<[email protected]> 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ł

2011-08-18 06:46:09

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH] Add uevent to bcma bus, to autoload drivers.

W dniu 17 sierpnia 2011 23:43 użytkownik David Woodhouse
<[email protected]> napisał:
> Signed-off-by: David Woodhouse <[email protected]>
> --
> 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 <[email protected]>

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ł

2011-08-19 20:52:26

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Add uevent to bcma bus, to autoload drivers.

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


2011-08-17 21:43:43

by David Woodhouse

[permalink] [raw]
Subject: [PATCH] Add uevent to bcma bus, to autoload drivers.

Signed-off-by: David Woodhouse <[email protected]>
--
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


2011-08-19 21:18:35

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH] Add uevent to bcma bus, to autoload drivers.

W dniu 19 sierpnia 2011 22:52 użytkownik David Woodhouse
<[email protected]> 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ł

2011-08-18 07:52:01

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Add uevent to bcma bus, to autoload drivers.

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


2011-08-19 20:13:10

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH] Add uevent to bcma bus, to autoload drivers.

W dniu 18 sierpnia 2011 09:51 użytkownik David Woodhouse
<[email protected]> 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 <[email protected]>
>>
>> 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ł