On Wednesday 14 October 2009 17:04:15 Larry Finger wrote:
> No, the logic in b43_rfkill_poll gets the device into a state that it
> works for newer hardware. There is something about the PPC G4 (at
> least) that is not in a state where the B43_MMIO_RADIO_HWENABLED_LO
> register can be read and the processor faults. I do not have access to
> PPC hardware, thus I proposed the above fix as one that would avoid
> the fault on PPC, yet not hurt my hardware.
>
> > I don't think this fix is correct at all.
> > The only way we can have status<STARTED here is through the rfkill_poll
> > function being called before the device is initialized. So it will enable
> > the ssb device and try to fetch the rfkill register. _However_, with this
> > check
> > added it will _always_ return False. So why do we do the polling anyway, if
> > it's
> > always guaranteed to be false? I think that's buggy.
> > To get the same effect, we could just do that:
> >
> > 44 void b43_rfkill_poll(struct ieee80211_hw *hw)
> > 45 {
> > 46 struct b43_wl *wl = hw_to_b43_wl(hw);
> > 47 struct b43_wldev *dev = wl->current_dev;
> > 48 struct ssb_bus *bus = dev->dev->bus;
> > 49 bool enabled;
> > 50 bool brought_up = false;
> > 51
> > 52 mutex_lock(&wl->mutex);
> > 53 if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
> >
> > wiphy_rfkill_set_hw_state(false);
> > unlock;
> > return;
> > // Remove the powerup logic...
> >
> > 54 if (ssb_bus_powerup(bus, 0)) {
> > 55 mutex_unlock(&wl->mutex);
> > 56 return;
> > 57 }
> > 58 ssb_device_enable(dev->dev, 0);
> > 59 brought_up = true;
> > 60 }
> >
> >
> > And no, it's not correct to force device state to STARTED or something else
> > here, because
> > that won't fix the issue and introduce other bugs.
>
> This code would probably fix the PPC fault, but it would fail for
> everything else. Once b43 reports that the RF switch is off, then
> mac80211 calls to b43_op_stop, which in turn calls b43_wireless_stop,
> and it sets the state to B43_STAT_INITIALIZED. Your proposed code
> would have the same failure as my previous patch to stop polling in
> b43_op_stop - there would be no way to detect that the switch was back on.
Well, so your patch trades one bug for another.
> If there is a way to detect that an MMIO read will fail on PPC
> architecture, then that is what we need. I do not have a clue on what
> that might be.
Ehm, no. You need to find out which part of initialization is missing to
get the MMIO access _not_ fault at all.
It does not fault, if the device is fully initialized, so you simply miss some
initialization. I don't know what's missing, however. I guess it probably
needs the PHY/Radio 4-wire bus initialized.
> I'm not sure that any devices that use b43 and have a PHY revision < 3
> actually have a radio kill switch. The case in bug 14181 does not.
> Would you prefer a patch that assumes that none do, and doesn't start
> polling unless phy.rev >= 3? Then we could kill the test in
> b43_is_hw_radio_enabled. The routine could even be inlined.
I don't have any preference, as I'm not the maintainer.
I just want to point out that the patch trades one bug for another. That
may be acceptable. I don't know. I have a PPC with a phyrev 2 card in it,
but I don't compile in RFKILL, as there's no switch. So I can't really test it
without an actual switch.
--
Greetings, Michael.
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
