On Friday 10 August 2007 04:27:33 Ehud Gavron wrote:
> I have spent eight hours on this today and I can't find a way to do a
> subset of the patches. I haven't quite given up, but I'm reaching a
> point where I could use some insight. I didn't copy the list... but
> feel free to if you think it of public value.
>
> The only things that don't make obvious sense (TO ME) are
> 1. all the changes to bcm43xx_interrupt_handler <---- I don't
> understand what it NEEDS to be or why it WAS one way and IS another
> 2. @@ -2314,7 +2310,6 @@ static void bcm43xx_periodic_tasks_setup
> {
> struct delayed_work *work = &dev->periodic_work;
>
> - assert(bcm43xx_status(dev) ==
> BCM43xx_STAT_INITIALIZED); <------------ this is a
> removal with no insertion to replace it
It's an assertion. That doesn't generate code.
> 3. - if (likely(bcm43xx_status(dev) == BCM43xx_STAT_INITIALIZED)) {
> + if (likely(bcm43xx_status(dev) >= BCM43xx_STAT_STARTED))
> { <---------------------- shouldn't this be <
> BCM43xx_STAT_STARTED ???
(That's in bcm43xx_get_tx_stats() right?)
No, we are checking for "Are we at least started here?".
So we need >=STARTED. ==STARTED would do it, too, but that's
the same with the current code. <STARTED would clearly be wrong.
> I have other questions, but the code is blurring in front of my eyes.
Yeah, well. I think it does for everybody. ;)
I'm pretty sure that it's a bug in the toolchain (gcc or binutils
or maybe something else) that's triggered by this patch.
So the toolchain generating bad code and corrupting the data.
The weird thing, however, is that you said you were able to associate,
but not able to send that packet (it was a ping, or something like that?).
>From the driver point of view it doesn't matter if we TX/RX an
association packet or some other packet. The codepaths are _exactly_
the same.
That is all _very_ strange. I never had such a weird bug
in my whole life.
> Michael, I know you are busy. Could you please re-look at your patch,
> and note the cases where you change
> bcm43xx_status(dev) == BCM43xx_STAT_INITIALIZED
> I think in the other cases you change them to
> bcm43xx_status(dev) < BCM43xx_STAT_STARTED
> which makes sense to me... so I can't understand #3 above.
The patch looks absolutely correct to me, except the tiny tiny
part in the core-starting, where I already sent a patch to
you (you said it wouldn't help).
> Any input would be appreciated.
> PS No, I don't know this code. No I've never written a Linux driver.
> I'm a newbie. I just happen to have a laptop that doesn't like the new
> code and likes the old code for bcm43xx-mac80211. I'm not the only
> one.
Huh? Someone else able to reproduce it??
--
Greetings Michael.
_______________________________________________
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
