hal, rfkill and compat-wireless (Re: [RFC/RFT] rtl8187: Implement rfkill support)

[Hin-Tak Leung-4]

(added list hal to To:, since it has become relevant; previous
exchanges of the thread on linux-wireless)

On Wed, Aug 26, 2009 at 5:29 PM, Hin-Tak Leung<hintak.leung@...> wrote:

> On Wed, Aug 26, 2009 at 3:34 PM, Johannes Berg<johannes@...> wrote:
>> On Wed, 2009-08-26 at 13:33 +0000, Hin-Tak Leung wrote:
>>
>>> > Or wait ... are you using compat-wireless?
>>>
>>> Yes, I am. I mentioned this and did wonder if the _backport/ part
>>> in /sys/class is important.
>>
>> Sorry, didn't see.
>>
>> Anyway, that's pretty clearly the reason -- Luis added NETDEV_PRE_UP to
>> some compat*.h but obviously the kernel won't ever call that notifier,
>> so cfg80211 doesn't get a chance to reject the IFUP. No idea how to
>> handle that -- it'll be working fine in a regular tree.
>>
>> Luis, the only way to handle that would be to manually call the PRE_UP
>> notifier from mac80211's subif_open() and if that returns an error
>> (warning: the calling convention is weird) return the error... that's
>> weird but would work.
>>
>> johannes
>>
>
> Hmm, got a bit side-tracked. But hal doesn't know the device having a
> killswitch is still wrong somewhere?
> (i.e. am wondering where we should advertise that ability, or how hal
> works that out)
>
> Hin-Tak
>

I looked into hal and I can now say that it is certainly not
compat-wireless "rfkill_backport"-aware; apparently all it does is
monitoring entries under /sys/class that it knows about. I made a
quick hack:
-----------
diff --git a/hald/linux/device.c b/hald/linux/device.c
index 2eca1ef..61e94b7 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -4621,7 +4621,7 @@ static DevHandler dev_handler_power_supply =

 static DevHandler dev_handler_rfkill =
 {
-       .subsystem    = "rfkill",
+       .subsystem    = "rfkill_backport",
        .add          = rfkill_add,
        .compute_udi  = rfkill_compute_udi,
        .refresh      = rfkill_refresh,
-----------------
so that it looks for /sys/class/rfkill_backport instead of rfkill ,
and restarting hald, and lshal finally shows the killswitch. And
NetworkManager also becomes aware of the rfkill state and no longer
re-enable the device:

---------------
Aug 26 22:08:36 localhost NetworkManager: <info>  HAL disappeared
.
Aug 26 22:08:43 localhost NetworkManager: <info>  HAL re-appeared
Aug 26 22:08:43 localhost NetworkManager: <info>  Found radio
killswitch /org/freedesktop/Hal/devices/usb_device_bda_8197_00e04c000001_if0_rfkill_phy0_wlan
Aug 26 22:09:16 localhost kernel: rtl8187: wireless radio switch turned off
Aug 26 22:09:20 localhost NetworkManager: <info>  Wireless now
disabled by radio killswitch
.
Aug 26 22:13:21 localhost kernel: rtl8187: wireless radio switch turned on
Aug 26 22:13:27 localhost NetworkManager: <info>  Wireless now enabled
by radio killswitch
-------------

So, to summarise, from 2.6.31 onwards, cfg80211 should enforce the
rfkill state from within the kernel no matter what hal/NM does; on
non-bleeding edge systems, hal works out rfkill states from
'/sys/class/rfkill' and let NM know. if you have a non-bleeding edge
system but want to try compat-wireless anyway, hal simply doesn't know
about rfkill_backport and NM re-enables the device when one plays with
the rfkill switch.

I guess there are two ways to go about this:
(1) make compat-wireless unload and replace the as-shipped rfkill
module completely and populate /sys/class/rfkill .
(2) make hal monitor and treat /sys/class/rfkill_backport the same way
and in addition to /sys/class/rfkill .

So I went about (2) and just duplicated the rfkill entry like this,
and it seems to work - anybody from hal wants to take this in?
--------------
diff --git a/hald/linux/device.c b/hald/linux/device.c
index 2eca1ef..feb198e 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -4628,6 +4628,15 @@ static DevHandler dev_handler_rfkill =
        .remove       = dev_remove
 };

+static DevHandler dev_handler_rfkill_backport =
+{
+       .subsystem    = "rfkill_backport",
+       .add          = rfkill_add,
+       .compute_udi  = rfkill_compute_udi,
+       .refresh      = rfkill_refresh,
+       .remove       = dev_remove
+};
+
 static DevHandler dev_handler_scsi = {
  .subsystem   = "scsi",
  .add         = scsi_add,
@@ -4817,6 +4826,7 @@ static DevHandler *dev_handlers[] = {
  &dev_handler_ps3_system_bus,
  &dev_handler_pseudo,
  &dev_handler_rfkill,
+ &dev_handler_rfkill_backport,
  &dev_handler_scsi,
  &dev_handler_scsi_generic,
  &dev_handler_scsi_host,
------------------------------
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Luis R. Rodriguez]

On Wed, Aug 26, 2009 at 3:11 PM, Hin-Tak Leung<hintak.leung@...> wrote:

> (added list hal to To:, since it has become relevant; previous
> exchanges of the thread on linux-wireless)
>
> On Wed, Aug 26, 2009 at 5:29 PM, Hin-Tak Leung<hintak.leung@...> wrote:
>> On Wed, Aug 26, 2009 at 3:34 PM, Johannes Berg<johannes@...> wrote:
>>> On Wed, 2009-08-26 at 13:33 +0000, Hin-Tak Leung wrote:
>>>
>>>> > Or wait ... are you using compat-wireless?
>>>>
>>>> Yes, I am. I mentioned this and did wonder if the _backport/ part
>>>> in /sys/class is important.
>>>
>>> Sorry, didn't see.
>>>
>>> Anyway, that's pretty clearly the reason -- Luis added NETDEV_PRE_UP to
>>> some compat*.h but obviously the kernel won't ever call that notifier,
>>> so cfg80211 doesn't get a chance to reject the IFUP. No idea how to
>>> handle that -- it'll be working fine in a regular tree.
>>>
>>> Luis, the only way to handle that would be to manually call the PRE_UP
>>> notifier from mac80211's subif_open() and if that returns an error
>>> (warning: the calling convention is weird) return the error... that's
>>> weird but would work.
>>>
>>> johannes
>>>
>>
>> Hmm, got a bit side-tracked. But hal doesn't know the device having a
>> killswitch is still wrong somewhere?
>> (i.e. am wondering where we should advertise that ability, or how hal
>> works that out)
>>
>> Hin-Tak
>>
>
> I looked into hal and I can now say that it is certainly not
> compat-wireless "rfkill_backport"-aware; apparently all it does is
> monitoring entries under /sys/class that it knows about. I made a
> quick hack:

This is wrong, we just do not need to use rfkill_backport for sysfs
stuff, consider sending me patch that removes that hunk for
compat-wireless instead and test it.

  Luis
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Hin-Tak Leung-4]

On Wed, Aug 26, 2009 at 11:28 PM, Luis R. Rodriguez<mcgrof@...> wrote:

> On Wed, Aug 26, 2009 at 3:11 PM, Hin-Tak Leung<hintak.leung@...> wrote:
>> (added list hal to To:, since it has become relevant; previous
>> exchanges of the thread on linux-wireless)
>>
>> On Wed, Aug 26, 2009 at 5:29 PM, Hin-Tak Leung<hintak.leung@...> wrote:
>>> On Wed, Aug 26, 2009 at 3:34 PM, Johannes Berg<johannes@...> wrote:
>>>> On Wed, 2009-08-26 at 13:33 +0000, Hin-Tak Leung wrote:
>>>>
>>>>> > Or wait ... are you using compat-wireless?
>>>>>
>>>>> Yes, I am. I mentioned this and did wonder if the _backport/ part
>>>>> in /sys/class is important.
>>>>
>>>> Sorry, didn't see.
>>>>
>>>> Anyway, that's pretty clearly the reason -- Luis added NETDEV_PRE_UP to
>>>> some compat*.h but obviously the kernel won't ever call that notifier,
>>>> so cfg80211 doesn't get a chance to reject the IFUP. No idea how to
>>>> handle that -- it'll be working fine in a regular tree.
>>>>
>>>> Luis, the only way to handle that would be to manually call the PRE_UP
>>>> notifier from mac80211's subif_open() and if that returns an error
>>>> (warning: the calling convention is weird) return the error... that's
>>>> weird but would work.
>>>>
>>>> johannes
>>>>
>>>
>>> Hmm, got a bit side-tracked. But hal doesn't know the device having a
>>> killswitch is still wrong somewhere?
>>> (i.e. am wondering where we should advertise that ability, or how hal
>>> works that out)
>>>
>>> Hin-Tak
>>>
>>
>> I looked into hal and I can now say that it is certainly not
>> compat-wireless "rfkill_backport"-aware; apparently all it does is
>> monitoring entries under /sys/class that it knows about. I made a
>> quick hack:
>
> This is wrong, we just do not need to use rfkill_backport for sysfs
> stuff, consider sending me patch that removes that hunk for
> compat-wireless instead and test it.
>
>  Luis
>

Hmm, I did mention the other option - make rfkill_backport exposes its
data structure as '/sys/class/rfkill' instead of
'/sys/class/rfkill_backport'. Is there any reason why
compat-wireless's rfkill_backport  does not called itself 'rfkill' and
unload and replace the old rfkill? That would be much neater, and
userland tools like hal won't need to know anything about
compat-wireless.

Hin-Tak
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Luis R. Rodriguez]

On Wed, Aug 26, 2009 at 3:55 PM, Hin-Tak Leung<hintak.leung@...> wrote:

> On Wed, Aug 26, 2009 at 11:28 PM, Luis R. Rodriguez<mcgrof@...> wrote:
>> On Wed, Aug 26, 2009 at 3:11 PM, Hin-Tak Leung<hintak.leung@...> wrote:
>>> (added list hal to To:, since it has become relevant; previous
>>> exchanges of the thread on linux-wireless)
>>>
>>> On Wed, Aug 26, 2009 at 5:29 PM, Hin-Tak Leung<hintak.leung@...> wrote:
>>>> On Wed, Aug 26, 2009 at 3:34 PM, Johannes Berg<johannes@...> wrote:
>>>>> On Wed, 2009-08-26 at 13:33 +0000, Hin-Tak Leung wrote:
>>>>>
>>>>>> > Or wait ... are you using compat-wireless?
>>>>>>
>>>>>> Yes, I am. I mentioned this and did wonder if the _backport/ part
>>>>>> in /sys/class is important.
>>>>>
>>>>> Sorry, didn't see.
>>>>>
>>>>> Anyway, that's pretty clearly the reason -- Luis added NETDEV_PRE_UP to
>>>>> some compat*.h but obviously the kernel won't ever call that notifier,
>>>>> so cfg80211 doesn't get a chance to reject the IFUP. No idea how to
>>>>> handle that -- it'll be working fine in a regular tree.
>>>>>
>>>>> Luis, the only way to handle that would be to manually call the PRE_UP
>>>>> notifier from mac80211's subif_open() and if that returns an error
>>>>> (warning: the calling convention is weird) return the error... that's
>>>>> weird but would work.
>>>>>
>>>>> johannes
>>>>>
>>>>
>>>> Hmm, got a bit side-tracked. But hal doesn't know the device having a
>>>> killswitch is still wrong somewhere?
>>>> (i.e. am wondering where we should advertise that ability, or how hal
>>>> works that out)
>>>>
>>>> Hin-Tak
>>>>
>>>
>>> I looked into hal and I can now say that it is certainly not
>>> compat-wireless "rfkill_backport"-aware; apparently all it does is
>>> monitoring entries under /sys/class that it knows about. I made a
>>> quick hack:
>>
>> This is wrong, we just do not need to use rfkill_backport for sysfs
>> stuff, consider sending me patch that removes that hunk for
>> compat-wireless instead and test it.
>>
>>  Luis
>>
>
> Hmm, I did mention the other option - make rfkill_backport exposes its
> data structure as '/sys/class/rfkill' instead of
> '/sys/class/rfkill_backport'. Is there any reason why
> compat-wireless's rfkill_backport  does not called itself 'rfkill' and
> unload and replace the old rfkill? That would be much neater, and
> userland tools like hal won't need to know anything about
> compat-wireless.

Johannes, did kernels < 2.6.31 have /sys/class/rfkill ?

  Luis
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Hin-Tak Leung-4]

On Thu, Aug 27, 2009 at 12:45 AM, Luis R. Rodriguez<mcgrof@...> wrote:
> Johannes, did kernels < 2.6.31 have /sys/class/rfkill ?

2.6.30 has /sys/class/rfkill (it is exposed by the vendor as-shipped
rfkill.ko). But I just tried something like this, and it seems to work
- the patch is simple enough and just 3 hunks - what it does:
1) 'make unload' should unload rfkill also,
2) remove the compat-wireless class renaming hunk (so it advertise
itself as 'rfkill' rather than 'rfkill_backport')
3) remove the compat-wireless input hander renaming hunk.

I am not sure what the input handle->name (3) is for, somebody care to
explain? Is there any reason why I should do something like outlined
here in the patch?

Hin-Tak

[compat-wireless_remove_backport_naming.diff]

---

diff --git a/compat/patches/03-rfkill.patch b/compat/patches/03-rfkill.patch
index 61eb762..368767f 100644
--- a/compat/patches/03-rfkill.patch
+++ b/compat/patches/03-rfkill.patch
@@ -34,15 +34,6 @@ just keep /dev/rfkill and not /dev/rfkill_backport.
  #include <linux/sched.h>
 
  #include "rfkill.h"
-@@ -229,7 +233,7 @@ static int rfkill_connect(struct input_h
-
- handle->dev = dev;
- handle->handler = handler;
-- handle->name = "rfkill";
-+ handle->name = "rfkill_backport";
-
- /* causes rfkill_start() to be called */
- error = input_register_handle(handle);
 --- a/net/rfkill/core.c 2009-08-20 13:48:36.083267397 -0700
 +++ b/net/rfkill/core.c 2009-08-20 13:48:37.051267098 -0700
 @@ -26,7 +26,7 @@
@@ -81,15 +72,6 @@ just keep /dev/rfkill and not /dev/rfkill_backport.
  static atomic_t rfkill_input_disabled = ATOMIC_INIT(0);
 
  /**
-@@ -776,7 +776,7 @@
- }
-
- static struct class rfkill_class = {
-- .name = "rfkill",
-+ .name = "rfkill_backport",
- .dev_release = rfkill_release,
- .dev_attrs = rfkill_dev_attrs,
- .dev_uevent = rfkill_dev_uevent,
 @@ -922,7 +922,7 @@
  if (!rfkill->persistent || rfkill_epo_lock_active) {
  schedule_work(&rfkill->sync_work);
diff --git a/scripts/unload.sh b/scripts/unload.sh
index 75bbfc9..0537056 100755
--- a/scripts/unload.sh
+++ b/scripts/unload.sh
@@ -25,6 +25,7 @@ MODULES="$MODULES rndis_wlan rndis_host cdc_ether usbnet"
 # eeprom_93cx6 is used by rt2x00 (rt61pci, rt2500pci, rt2400pci)
 # and Realtek drivers ( rtl8187, rtl8180)
 MODULES="$MODULES eeprom_93cx6"
+MODULES="$MODULES rfkill"
 MODULES="$MODULES lib80211_crypt_ccmp lib80211_crypt_tkip lib80211_crypt_wep"
 MODULES="$MODULES mac80211 cfg80211 lib80211"
 MADWIFI_MODULES="ath_pci ath_rate_sample wlan_scan_sta wlan ath_hal"

---

_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Johannes Berg]

On Wed, 2009-08-26 at 16:45 -0700, Luis R. Rodriguez wrote:

> Johannes, did kernels < 2.6.31 have /sys/class/rfkill ?

Yes, but I never understood why that matters. If you've got
compat-wireless "rfkill" loaded then the original rfkill can't be loaded
anyway. And their symbols would probably clash anyway if the original is
built in, in which case you can't use compat.

You haven't renamed cfg80211's sysfs either, so why rfkill?

johannes


_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Hin-Tak Leung-4]

On Thu, Aug 27, 2009 at 8:14 AM, Johannes Berg<johannes@...> wrote:

> On Wed, 2009-08-26 at 16:45 -0700, Luis R. Rodriguez wrote:
>
>> Johannes, did kernels < 2.6.31 have /sys/class/rfkill ?
>
> Yes, but I never understood why that matters. If you've got
> compat-wireless "rfkill" loaded then the original rfkill can't be loaded
> anyway. And their symbols would probably clash anyway if the original is
> built in, in which case you can't use compat.
>
> You haven't renamed cfg80211's sysfs either, so why rfkill?
>
> johannes
>

There are a couple of wimax drivers which depends on rfkill... or
bluetooth drivers? I think one likely reason is that there are valid
situations or hardware combinations which requires having both of them
loaded. (but that's a whole can of worms )
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Hin-Tak Leung-4]

On Thu, Aug 27, 2009 at 10:43 AM, Hin-Tak Leung<hintak.leung@...> wrote:

> On Thu, Aug 27, 2009 at 8:14 AM, Johannes Berg<johannes@...> wrote:
>> On Wed, 2009-08-26 at 16:45 -0700, Luis R. Rodriguez wrote:
>>
>>> Johannes, did kernels < 2.6.31 have /sys/class/rfkill ?
>>
>> Yes, but I never understood why that matters. If you've got
>> compat-wireless "rfkill" loaded then the original rfkill can't be loaded
>> anyway. And their symbols would probably clash anyway if the original is
>> built in, in which case you can't use compat.
>>
>> You haven't renamed cfg80211's sysfs either, so why rfkill?
>>
>> johannes
>>
>
> There are a couple of wimax drivers which depends on rfkill... or
> bluetooth drivers? I think one likely reason is that there are valid
> situations or hardware combinations which requires having both of them
> loaded. (but that's a whole can of worms )

Argh, I just have a stacktrace after reboot - toshiba_acpi.ko (on a
toshiba laptop) requires the stock-kernel rfkill-* symbols, and rfkill
won't load because of my modified compat-wireless rfkill_backport.ko
which takes up /sys/class/rfkill rather than
/sys/class/rfkill_backport . There's your reason from sysfs renaming.
I wonder why/how I could unload rfkill.ko earlier, since the *_acpi.ko
needs it - maybe I had acpi unloaded as a side-effect also.

I guess we'll need hal to treat /sys/class/rfkill_backport like
/sys/class/rfkill  then - will file a bug somewhere on freedesktop,
although I hear hal is on its way out....
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal

[Luis R. Rodriguez]

On Mon, Aug 31, 2009 at 11:43 AM, Hin-Tak Leung<hintak.leung@...> wrote:

> On Thu, Aug 27, 2009 at 10:43 AM, Hin-Tak Leung<hintak.leung@...> wrote:
>> On Thu, Aug 27, 2009 at 8:14 AM, Johannes Berg<johannes@...> wrote:
>>> On Wed, 2009-08-26 at 16:45 -0700, Luis R. Rodriguez wrote:
>>>
>>>> Johannes, did kernels < 2.6.31 have /sys/class/rfkill ?
>>>
>>> Yes, but I never understood why that matters. If you've got
>>> compat-wireless "rfkill" loaded then the original rfkill can't be loaded
>>> anyway. And their symbols would probably clash anyway if the original is
>>> built in, in which case you can't use compat.
>>>
>>> You haven't renamed cfg80211's sysfs either, so why rfkill?
>>>
>>> johannes
>>>
>>
>> There are a couple of wimax drivers which depends on rfkill... or
>> bluetooth drivers? I think one likely reason is that there are valid
>> situations or hardware combinations which requires having both of them
>> loaded. (but that's a whole can of worms )
>
> Argh, I just have a stacktrace after reboot - toshiba_acpi.ko (on a
> toshiba laptop) requires the stock-kernel rfkill-* symbols, and rfkill
> won't load because of my modified compat-wireless rfkill_backport.ko
> which takes up /sys/class/rfkill rather than
> /sys/class/rfkill_backport . There's your reason from sysfs renaming.
> I wonder why/how I could unload rfkill.ko earlier, since the *_acpi.ko
> needs it - maybe I had acpi unloaded as a side-effect also.
>
> I guess we'll need hal to treat /sys/class/rfkill_backport like
> /sys/class/rfkill  then - will file a bug somewhere on freedesktop,
> although I hear hal is on its way out....

How many modules depend on rfkill, as with ssb maybe we should just
add them all ? I'm not volunteering though, but just pointing out that
if its not that many as with ssb perhaps its best to just add the
other drivers.

  Luis
_______________________________________________
hal mailing list
hal@...
http://lists.freedesktop.org/mailman/listinfo/hal