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

[Ivo van Doorn]

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").

Ok, that suggests that the input device must be destroyed by calling rt2x00rfkill_free,
that is the function which will also clear the RFKILL_STATE_ALLOCATED flag and
_properly_ cleanup rfkill during suspend.

> > > -    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.

Perhaps the meaning of the button has switched in which case rt2800pci_rfkill_poll
needs to flip the return value.

> 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 ?

If you know a better switch keycode which can be send. then it is find with me,
but at the moment only SW_RFKILL_ALL exist so perhaps a new code would be needed
although I don't know how many people will be happy with a new RFKILL code.

> 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 ...

Well perhaps we should check if we are polling the correct pin. ;)
If there is another pin which does display proper behavior, then the fix
would be simple. :)

Ivo