2009-08-13 18:01:07

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [RFC/RFT] b43: LP-PHY: Implement channel switching for rev2+/B2063 radio

(List re-CC-ed.)

2009/8/13 Michael Buesch <[email protected]>:
> On Thursday 13 August 2009 19:50:16 G?bor Stefanik wrote:
>> 2009/8/13 Michael Buesch <[email protected]>:
>> > On Thursday 13 August 2009 17:02:21 G?bor Stefanik wrote:
>> >> Rev.2+/B2063 will now hopefully show some signs of life, though
>> >> it won't work at full performance, as calibration is still missing.
>> >>
>> >> Signed-off-by: G?bor Stefanik <[email protected]>
>> >
>> >> ?static void lpphy_baseband_rev0_1_init(struct b43_wldev *dev)
>> >> @@ -1369,7 +1370,7 @@ static int b43_lpphy_op_init(struct b43_wldev *dev)
>> >> ? ? ? lpphy_baseband_init(dev);
>> >> ? ? ? lpphy_radio_init(dev);
>> >> ? ? ? lpphy_calibrate_rc(dev);
>> >> - ? ? //TODO set channel
>> >> + ? ? b43_switch_channel(dev, dev->wl->hw->conf.channel->hw_value);
>> >
>> > Does dev->wl->hw->conf.channel->hw_value already have a sane value here?
>>
>> I think it should - hw->conf.channel was handed to us by mac80211.
>> http://bcm-v4.sipsolutions.net/802.11/PHY/LP/Init says "Set channel
>> with current chanspec as argument", and hw->conf.channel is our
>> equivalent of chanspec.
>
> hw->conf.channel was not handed to us at all. You simply pick it
> from mac80211's conf structure and _assume_ that it is initialized.
> In the other PHY implementations we use phyop_default_channel (or mandatory
> hardcoded channel values) in the PHY init only. I think you should do the
> same here. Mac80211 will make sure to select the correct channel later.

OK, I will use the default channel (though that's not exactly what the
spec says - Larry, is using the default channel correct?)

>
>> > Also please call b43_lpphy_op_switch_channel() instead of b43_switch_channel().
>>
>> No, that would be wrong; the generic parts of b43_switch_channel also
>> need to be executed. See
>
> I don't see why. The generic parts are nothing that should be done at the PHY init.

Well, the spec says that the generic parts need to be run, so I
implemented it as such.

Larry, is this part of the spec correct?

>
> --
> Greetings, Michael.
>



--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)


2009-08-13 19:05:05

by Larry Finger

[permalink] [raw]
Subject: Re: [RFC/RFT] b43: LP-PHY: Implement channel switching for rev2+/B2063 radio

Michael Buesch wrote:
> On Thursday 13 August 2009 20:00:47 G?bor Stefanik wrote:
>> (List re-CC-ed.)
>>
>> 2009/8/13 Michael Buesch <[email protected]>:
>>> On Thursday 13 August 2009 19:50:16 G?bor Stefanik wrote:
>>>> 2009/8/13 Michael Buesch <[email protected]>:
>>>>> On Thursday 13 August 2009 17:02:21 G?bor Stefanik wrote:
>>>>>> Rev.2+/B2063 will now hopefully show some signs of life, though
>>>>>> it won't work at full performance, as calibration is still missing.
>>>>>>
>>>>>> Signed-off-by: G?bor Stefanik <[email protected]>
>>>>>> static void lpphy_baseband_rev0_1_init(struct b43_wldev *dev)
>>>>>> @@ -1369,7 +1370,7 @@ static int b43_lpphy_op_init(struct b43_wldev *dev)
>>>>>> lpphy_baseband_init(dev);
>>>>>> lpphy_radio_init(dev);
>>>>>> lpphy_calibrate_rc(dev);
>>>>>> - //TODO set channel
>>>>>> + b43_switch_channel(dev, dev->wl->hw->conf.channel->hw_value);
>>>>> Does dev->wl->hw->conf.channel->hw_value already have a sane value here?
>>>> I think it should - hw->conf.channel was handed to us by mac80211.
>>>> http://bcm-v4.sipsolutions.net/802.11/PHY/LP/Init says "Set channel
>>>> with current chanspec as argument", and hw->conf.channel is our
>>>> equivalent of chanspec.
>>> hw->conf.channel was not handed to us at all. You simply pick it
>>> from mac80211's conf structure and _assume_ that it is initialized.
>>> In the other PHY implementations we use phyop_default_channel (or mandatory
>>> hardcoded channel values) in the PHY init only. I think you should do the
>>> same here. Mac80211 will make sure to select the correct channel later.
>> OK, I will use the default channel (though that's not exactly what the
>> spec says - Larry, is using the default channel correct?)
>
> What on earth could be incorrect about it? It's an as arbitrary value as
> dev->wl->hw->conf.channel->hw_value would be.

I have not found anything that says what should be used. If the
channel is valid for the selected band, it should be as good as any.
>
>>>>> Also please call b43_lpphy_op_switch_channel() instead of b43_switch_channel().
>>>> No, that would be wrong; the generic parts of b43_switch_channel also
>>>> need to be executed. See
>>> I don't see why. The generic parts are nothing that should be done at the PHY init.
>> Well, the spec says that the generic parts need to be run, so I
>> implemented it as such.
>
> We do understand what the generic stuff does and it is not required at this point.
>
>> Larry, is this part of the spec correct?
>
> I'm pretty sure it is. But that does not mean we have to implement the same
> bullshit broadcom does ;)

Amen. I have to put it on the web site just in case it is important,
but it must be done that way _ONLY_ if found to be needed for
operation or performance.

Larry

2009-08-13 18:05:39

by Michael Büsch

[permalink] [raw]
Subject: Re: [RFC/RFT] b43: LP-PHY: Implement channel switching for rev2+/B2063 radio

On Thursday 13 August 2009 20:00:47 G?bor Stefanik wrote:
> (List re-CC-ed.)
>
> 2009/8/13 Michael Buesch <[email protected]>:
> > On Thursday 13 August 2009 19:50:16 G?bor Stefanik wrote:
> >> 2009/8/13 Michael Buesch <[email protected]>:
> >> > On Thursday 13 August 2009 17:02:21 G?bor Stefanik wrote:
> >> >> Rev.2+/B2063 will now hopefully show some signs of life, though
> >> >> it won't work at full performance, as calibration is still missing.
> >> >>
> >> >> Signed-off-by: G?bor Stefanik <[email protected]>
> >> >
> >> >> ?static void lpphy_baseband_rev0_1_init(struct b43_wldev *dev)
> >> >> @@ -1369,7 +1370,7 @@ static int b43_lpphy_op_init(struct b43_wldev *dev)
> >> >> ? ? ? lpphy_baseband_init(dev);
> >> >> ? ? ? lpphy_radio_init(dev);
> >> >> ? ? ? lpphy_calibrate_rc(dev);
> >> >> - ? ? //TODO set channel
> >> >> + ? ? b43_switch_channel(dev, dev->wl->hw->conf.channel->hw_value);
> >> >
> >> > Does dev->wl->hw->conf.channel->hw_value already have a sane value here?
> >>
> >> I think it should - hw->conf.channel was handed to us by mac80211.
> >> http://bcm-v4.sipsolutions.net/802.11/PHY/LP/Init says "Set channel
> >> with current chanspec as argument", and hw->conf.channel is our
> >> equivalent of chanspec.
> >
> > hw->conf.channel was not handed to us at all. You simply pick it
> > from mac80211's conf structure and _assume_ that it is initialized.
> > In the other PHY implementations we use phyop_default_channel (or mandatory
> > hardcoded channel values) in the PHY init only. I think you should do the
> > same here. Mac80211 will make sure to select the correct channel later.
>
> OK, I will use the default channel (though that's not exactly what the
> spec says - Larry, is using the default channel correct?)

What on earth could be incorrect about it? It's an as arbitrary value as
dev->wl->hw->conf.channel->hw_value would be.

>
> >
> >> > Also please call b43_lpphy_op_switch_channel() instead of b43_switch_channel().
> >>
> >> No, that would be wrong; the generic parts of b43_switch_channel also
> >> need to be executed. See
> >
> > I don't see why. The generic parts are nothing that should be done at the PHY init.
>
> Well, the spec says that the generic parts need to be run, so I
> implemented it as such.

We do understand what the generic stuff does and it is not required at this point.

> Larry, is this part of the spec correct?

I'm pretty sure it is. But that does not mean we have to implement the same
bullshit broadcom does ;)

--
Greetings, Michael.