Hi,
I've N-PHY card that doesn't report any scan results, so after
checking N-PHY code (it looks fine so far) I decided to dig into ssb.
Hopefully this will lead to some fixes and maybe working card.
I've dumped MMIO operations performed by wl and ssb/b43, compared
them, documented with parts from specs and noted differences.
As you should know, we have to enable wireless core twice: first time
to get basic information about hardware, second to configure it in
correct (like PHY specific) way.
ssb_device_enable_1 is for first reset
ssb_device_enable_2 is for second reset
Notes from ssb_device_enable_1:
1) There are few undocumented flushes performed by both drivers. Could
be nice to document it in specs
2) wl does not reset PHY (no B43_TMSLOW_PHYRESET and no taking it out
of reset after enabling wireless core)
3) After reset (last step in ssb_device_enable) wl doesn't leave core
specific flags
Notes from ssb_device_enable_2:
1) ssb does not implement fast disable in case clocks are not enabled
2) ssb uses different Reject bit than wl
3) ssb doesn't check for "If the Core ID Low register has the
"Initiator" bit set"
4) wl checks for "Busy" bit in IM STATE, not TM as documented in specs
5) when writing "Force Gated Clocks On", "Clock Enable", and "Reset"
both drivers keep Reject set, however specs don't mention that
6) wl checks "If the "Initiator" bit in TM State Low is set" using ff8
register which is Core ID Low, not TMSLOW
7) rest differences are duplicated from ssb_device_enable_1
Larry: could you care for specs notes I collected?
Michael: was there any reasons why we didn't implement some parts of
core-disabling code?
Michael: should we care about the way wl sets core specific flags? I
didn't dig into that moment in MMIO dumps, but as ssb_device_enable
implementation ignores flags at the end, it has to set flags somehow
differently on it's own.
--
Rafał
W dniu 14 lutego 2011 23:01 użytkownik Michael Büsch <[email protected]> napisał:
> On Mon, 2011-02-14 at 20:51 +0100, Rafał Miłecki wrote:
>> W dniu 9 lutego 2011 21:17 użytkownik Michael Büsch <[email protected]> napisał:
>> > On Wed, 2011-02-09 at 21:00 +0100, Rafał Miłecki wrote:
>> >> Michael: was there any reasons why we didn't implement some parts of
>> >> core-disabling code?
>> >
>> > The function are complete as of latest reverse engineering efforts.
>> > Broadcom added stuff, if they do more stuff in latest code.
>>
>> Nothing has changed in specs since 2006:
>> http://bcm-v4.sipsolutions.net/Backplane?action=info
>> For some reason routines that were present even in 2006 was not implemented.
>
> Well, so the function was implemented prior to 2006. Which doesn't
> surprise me. It was one of the first ones.
>
>> I've just written missing parts,
>
> May I see them?
Sure. It's not for submission, so you have to expect magic values.
>> tested and it still does not work :|
>> The only advantage discovered so far is that ssb detects sth is wrong
>> with IM state:
>> [ 2661.449647] ssb: Timeout waiting for bitmask 01800000 on register
>> 0F90 to clear.
>>
>> I can see wl experiencing the same problems after loading b43. It
>> reads 0xf90 dozen of times in a row.
>
> And that's the thing why I always avoid touching that function.
> There's so much magic going on, so that completely weird things happen
> all the time. :D
Hm, that looks pretty easy and not really complicated. Of course, if
you figure out how it's working.
Personally I don't like for example description at
http://bcm-v4.sipsolutions.net/802.11/PHY . It's really messed in "Put
PHY Into Reset".
Section "Put PHY Into Reset" is really "How to reset PHY" and it
duplicates code of takine PHY out of reset.
We also have a lot of magic in ssb_device_enable. We reset SB and PHY
at the same time. I guess it introduces some optimization but makes it
harder to understand, especially if you try to understand implemented
code and look as specs at the same time.
--
Rafał
On Mon, 2011-02-14 at 23:32 +0100, Rafał Miłecki wrote:
> Sure. It's not for submission, so you have to expect magic values.
That patch doesn't look too bad.
> Personally I don't like for example description at
> http://bcm-v4.sipsolutions.net/802.11/PHY . It's really messed in "Put
> PHY Into Reset".
> Section "Put PHY Into Reset" is really "How to reset PHY" and it
> duplicates code of takine PHY out of reset.
Yeah, well. Maybe I repeat myself for the hundredth time:
Do not duplicate the specifications exactly. Apply common sense before
implementing the code.
If we can do better on certain things, do it.
> We also have a lot of magic in ssb_device_enable. We reset SB and PHY
> at the same time. I guess it introduces some optimization but makes it
> harder to understand, especially if you try to understand implemented
> code and look as specs at the same time.
Well, it's core specific flags. I don't see anything
wrong here. I think the PHY reset might even require an actual core
reset to work correctly. So you could never untie both.
But that's exactly those magic things we're never going to find out
unless we have hardware documentation.
--
Greetings Michael.
On Mon, 2011-02-14 at 20:51 +0100, Rafał Miłecki wrote:
> W dniu 9 lutego 2011 21:17 użytkownik Michael Büsch <[email protected]> napisał:
> > On Wed, 2011-02-09 at 21:00 +0100, Rafał Miłecki wrote:
> >> Michael: was there any reasons why we didn't implement some parts of
> >> core-disabling code?
> >
> > The function are complete as of latest reverse engineering efforts.
> > Broadcom added stuff, if they do more stuff in latest code.
>
> Nothing has changed in specs since 2006:
> http://bcm-v4.sipsolutions.net/Backplane?action=info
> For some reason routines that were present even in 2006 was not implemented.
Well, so the function was implemented prior to 2006. Which doesn't
surprise me. It was one of the first ones.
> I've just written missing parts,
May I see them?
> tested and it still does not work :|
> The only advantage discovered so far is that ssb detects sth is wrong
> with IM state:
> [ 2661.449647] ssb: Timeout waiting for bitmask 01800000 on register
> 0F90 to clear.
>
> I can see wl experiencing the same problems after loading b43. It
> reads 0xf90 dozen of times in a row.
And that's the thing why I always avoid touching that function.
There's so much magic going on, so that completely weird things happen
all the time. :D
--
Greetings Michael.
W dniu 14 lutego 2011 23:52 użytkownik Michael Büsch <[email protected]> napisał:
> On Mon, 2011-02-14 at 23:32 +0100, Rafał Miłecki wrote:
>> Sure. It's not for submission, so you have to expect magic values.
>
> That patch doesn't look too bad.
Should we commit such a patch? Even if we didn't notice it to improve anything?
--
Rafał
On Tue, 2011-02-15 at 00:02 +0100, Rafał Miłecki wrote:
> W dniu 14 lutego 2011 23:52 użytkownik Michael Büsch <[email protected]> napisał:
> > On Mon, 2011-02-14 at 23:32 +0100, Rafał Miłecki wrote:
> >> Sure. It's not for submission, so you have to expect magic values.
> >
> > That patch doesn't look too bad.
>
> Should we commit such a patch? Even if we didn't notice it to improve anything?
I think it should wait until you got the device working.
But once it works, it should be committed.
--
Greetings Michael.
On Wed, 2011-02-09 at 21:00 +0100, Rafał Miłecki wrote:
> Michael: was there any reasons why we didn't implement some parts of
> core-disabling code?
The function are complete as of latest reverse engineering efforts.
Broadcom added stuff, if they do more stuff in latest code.
> Michael: should we care about the way wl sets core specific flags? I
> didn't dig into that moment in MMIO dumps, but as ssb_device_enable
> implementation ignores flags at the end, it has to set flags somehow
> differently on it's own.
I have no idea. ssb_device_enable is very hairy and I'm not going
to touch it without good reason and regression testing.
You didn't tell us the important part: Does changing ssb_device_enable
make it work?
--
Greetings Michael.
W dniu 9 lutego 2011 21:17 użytkownik Michael Büsch <[email protected]> napisał:
> On Wed, 2011-02-09 at 21:00 +0100, Rafał Miłecki wrote:
>> Michael: was there any reasons why we didn't implement some parts of
>> core-disabling code?
>
> The function are complete as of latest reverse engineering efforts.
> Broadcom added stuff, if they do more stuff in latest code.
Nothing has changed in specs since 2006:
http://bcm-v4.sipsolutions.net/Backplane?action=info
For some reason routines that were present even in 2006 was not implemented.
>> Michael: should we care about the way wl sets core specific flags? I
>> didn't dig into that moment in MMIO dumps, but as ssb_device_enable
>> implementation ignores flags at the end, it has to set flags somehow
>> differently on it's own.
>
> I have no idea. ssb_device_enable is very hairy and I'm not going
> to touch it without good reason and regression testing.
>
> You didn't tell us the important part: Does changing ssb_device_enable
> make it work?
I've just written missing parts, tested and it still does not work :|
The only advantage discovered so far is that ssb detects sth is wrong
with IM state:
[ 2661.449647] ssb: Timeout waiting for bitmask 01800000 on register
0F90 to clear.
I can see wl experiencing the same problems after loading b43. It
reads 0xf90 dozen of times in a row.
--
Rafał