Michael Buesch wrote:
> 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??
>
>
Yes I am able to reproduce it. I have done upgraded and downgraded my
enitre toolchain. exact same problem is present on my system when I try
my 4306 and 4318.
-Jory
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
