[rt2x00-users] [PATCH] Rt2x00 : Replace rfkill EV_SW by EV_KEY.

[Alban Browaeys]

Le mercredi 22 avril 2009 à 23:23 +0200, Ivo van Doorn a écrit :
> Hi,
> 
> >      /*
> > +     * Schedule a new allocation of rfkill upon resume.
> > +     */
> > +    clear_bit(RFKILL_STATE_ALLOCATED, &rt2x00dev->rfkill_state);
> 
> This is a problem, this means rt2x00 will leak memory during resume since
> it will not free the previous input device but rather allocate a new one and
> overwrite the old one.

I am looking after if this is the input field of rfkill_poll_dev that is
freed automagically upon suspend and which trigger the oops in resume
when input_register_poll_device is called. THough I get a null reference
at this call. (with no_console_suspend and pm_test set to "devices").


> > -    state = !!rt2x00dev->ops->lib->rfkill_poll(rt2x00dev);
> > +    state = !rt2x00dev->ops->lib->rfkill_poll(rt2x00dev);
> >      old_state = !!test_bit(RFKILL_STATE_BLOCKED, &rt2x00dev->rfkill_state);
> 
> No. This is completley incorrect. rfkill_poll returns 1 to indicate it is blocked.
> As you can see on the next line, the old state and the new state will be compared
> against eachother so this change will automatically block the radio when the
> user has put the switch the unblock.
> 

I agree . This part was supposed to be in a second set of request for
comments tagged as "ugly".
What I don't understand is that GPIO always return ff1f thus 1 for pin2
in rfkill_poll for rt2800pci . So the current behaviour is to always
disable the device.
Which is workaround because the mac layer calls phy1 after phy0 failed
with Device error -4 and this second instance still call the input
switch which restore it to on.
Thus the second one works but the first one always fails.


But if one prevent this first off switch it does not help. 
What really help in the EV_SW case would be not to set the SW_RFKILL_ALL
to off at allocation (that is removing the line
poll_dev->input->swbit[0] = BIT(SW_RFKILL_ALL);


I also really don't understand why there is no other SW bit than
SW_RFKILL_ALL for wlan. This one turns off all radio devices (including
bluetooth) when blocked. Seems one can only control the wlan
distinguished from bluetooth only by using EV_KEY. THat s the reason I
did the previous attempt though not knowing yet really what the previous
event type was in fact.


> > @@ -108,8 +108,8 @@ void rt2x00rfkill_allocate(struct rt2x00_dev *rt2x00dev)
> >      poll_dev->input->id.product = rt2x00dev->chip.rt;
> >      poll_dev->input->id.version = rt2x00dev->chip.rev;
> >      poll_dev->input->dev.parent = wiphy_dev(rt2x00dev->hw->wiphy);
> > -    poll_dev->input->evbit[0] = BIT(EV_SW);
> > -    poll_dev->input->swbit[0] = BIT(SW_RFKILL_ALL);
> > +    poll_dev->input->evbit[0] = BIT(EV_KEY);
> > +    __set_bit(KEY_WLAN, poll_dev->input->keybit);
> 
> The rfkill is a switch and not a key. rfkill_poll returns the state of the
> switch (blocked or unblocked). So it is not a key which is pressed once to
> toggle the state.

Is keeping the current implementation while removing the switch off all
radio at allocation/registration before poll  correct ?
Why is the state of the rt2800pci gpio pin 2 always 1 when unblocked ? I
guess it is not the case for other devices . One other ugly trick would
be to query bit 5 or 6 of gpio thus ff1f would return 0 ...

Patch is easy to do at this stage. Knowing what other devices returns is
harder :)


Best regards,
Alban