[PATCH 2/2] rfkill: Create rfkill-none LED trigger

2018-05-22 Thread João Paulo Rechi Vita
Creates a new trigger rfkill-none, as a complement to rfkill-any, which
drives LEDs when any radio is enabled. The new trigger is meant to turn
a LED ON whenever all radios are OFF, and turn it OFF otherwise.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 6d64d14f4b0a..07235520b00f 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -178,6 +178,7 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
 }
 
 static struct led_trigger rfkill_any_led_trigger;
+static struct led_trigger rfkill_none_led_trigger;
 static struct work_struct rfkill_global_led_trigger_work;
 
 static void rfkill_global_led_trigger_worker(struct work_struct *work)
@@ -195,6 +196,8 @@ static void rfkill_global_led_trigger_worker(struct 
work_struct *work)
mutex_unlock(_global_mutex);
 
led_trigger_event(_any_led_trigger, brightness);
+   led_trigger_event(_none_led_trigger, brightness == LED_OFF ?
+   LED_FULL : LED_OFF);
 }
 
 static void rfkill_global_led_trigger_event(void)
@@ -202,22 +205,32 @@ static void rfkill_global_led_trigger_event(void)
schedule_work(_global_led_trigger_work);
 }
 
-static void rfkill_global_led_trigger_activate(struct led_classdev *led_cdev)
-{
-   rfkill_global_led_trigger_event();
-}
-
 static int rfkill_global_led_trigger_register(void)
 {
+   int ret;
+
INIT_WORK(_global_led_trigger_work,
rfkill_global_led_trigger_worker);
+
rfkill_any_led_trigger.name = "rfkill-any";
-   rfkill_any_led_trigger.activate = rfkill_global_led_trigger_activate;
-   return led_trigger_register(_any_led_trigger);
+   ret = led_trigger_register(_any_led_trigger);
+   if (ret)
+   return ret;
+
+   rfkill_none_led_trigger.name = "rfkill-none";
+   ret = led_trigger_register(_none_led_trigger);
+   if (ret)
+   led_trigger_unregister(_any_led_trigger);
+   else
+   /* Delay activation until all global triggers are registered */
+   rfkill_global_led_trigger_event();
+
+   return ret;
 }
 
 static void rfkill_global_led_trigger_unregister(void)
 {
+   led_trigger_unregister(_none_led_trigger);
led_trigger_unregister(_any_led_trigger);
cancel_work_sync(_global_led_trigger_work);
 }
-- 
2.17.0



[PATCH 1/2] rfkill: Rename rfkill_any_led_trigger* functions

2018-05-22 Thread João Paulo Rechi Vita
Rename these functions to rfkill_global_led_trigger*, as they are going
to be extended to handle another global rfkill led trigger.

This commit does not change any functionality.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 47 ---
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 59d0eb960275..6d64d14f4b0a 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -178,9 +178,9 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
 }
 
 static struct led_trigger rfkill_any_led_trigger;
-static struct work_struct rfkill_any_work;
+static struct work_struct rfkill_global_led_trigger_work;
 
-static void rfkill_any_led_trigger_worker(struct work_struct *work)
+static void rfkill_global_led_trigger_worker(struct work_struct *work)
 {
enum led_brightness brightness = LED_OFF;
struct rfkill *rfkill;
@@ -197,28 +197,29 @@ static void rfkill_any_led_trigger_worker(struct 
work_struct *work)
led_trigger_event(_any_led_trigger, brightness);
 }
 
-static void rfkill_any_led_trigger_event(void)
+static void rfkill_global_led_trigger_event(void)
 {
-   schedule_work(_any_work);
+   schedule_work(_global_led_trigger_work);
 }
 
-static void rfkill_any_led_trigger_activate(struct led_classdev *led_cdev)
+static void rfkill_global_led_trigger_activate(struct led_classdev *led_cdev)
 {
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
 }
 
-static int rfkill_any_led_trigger_register(void)
+static int rfkill_global_led_trigger_register(void)
 {
-   INIT_WORK(_any_work, rfkill_any_led_trigger_worker);
+   INIT_WORK(_global_led_trigger_work,
+   rfkill_global_led_trigger_worker);
rfkill_any_led_trigger.name = "rfkill-any";
-   rfkill_any_led_trigger.activate = rfkill_any_led_trigger_activate;
+   rfkill_any_led_trigger.activate = rfkill_global_led_trigger_activate;
return led_trigger_register(_any_led_trigger);
 }
 
-static void rfkill_any_led_trigger_unregister(void)
+static void rfkill_global_led_trigger_unregister(void)
 {
led_trigger_unregister(_any_led_trigger);
-   cancel_work_sync(_any_work);
+   cancel_work_sync(_global_led_trigger_work);
 }
 #else
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
@@ -234,16 +235,16 @@ static inline void rfkill_led_trigger_unregister(struct 
rfkill *rfkill)
 {
 }
 
-static void rfkill_any_led_trigger_event(void)
+static void rfkill_global_led_trigger_event(void)
 {
 }
 
-static int rfkill_any_led_trigger_register(void)
+static int rfkill_global_led_trigger_register(void)
 {
return 0;
 }
 
-static void rfkill_any_led_trigger_unregister(void)
+static void rfkill_global_led_trigger_unregister(void)
 {
 }
 #endif /* CONFIG_RFKILL_LEDS */
@@ -354,7 +355,7 @@ static void rfkill_set_block(struct rfkill *rfkill, bool 
blocked)
spin_unlock_irqrestore(>lock, flags);
 
rfkill_led_trigger_event(rfkill);
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
 
if (prev != curr)
rfkill_event(rfkill);
@@ -535,7 +536,7 @@ bool rfkill_set_hw_state(struct rfkill *rfkill, bool 
blocked)
spin_unlock_irqrestore(>lock, flags);
 
rfkill_led_trigger_event(rfkill);
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
 
if (rfkill->registered && prev != blocked)
schedule_work(>uevent_work);
@@ -579,7 +580,7 @@ bool rfkill_set_sw_state(struct rfkill *rfkill, bool 
blocked)
schedule_work(>uevent_work);
 
rfkill_led_trigger_event(rfkill);
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
 
return blocked;
 }
@@ -629,7 +630,7 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool 
hw)
schedule_work(>uevent_work);
 
rfkill_led_trigger_event(rfkill);
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
}
 }
 EXPORT_SYMBOL(rfkill_set_states);
@@ -1046,7 +1047,7 @@ int __must_check rfkill_register(struct rfkill *rfkill)
 #endif
}
 
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
rfkill_send_events(rfkill, RFKILL_OP_ADD);
 
mutex_unlock(_global_mutex);
@@ -1079,7 +1080,7 @@ void rfkill_unregister(struct rfkill *rfkill)
mutex_lock(_global_mutex);
rfkill_send_events(rfkill, RFKILL_OP_DEL);
list_del_init(>node);
-   rfkill_any_led_trigger_event();
+   rfkill_global_led_trigger_event();
mutex_unlock(_global_mutex);
 
rfkill_led_trigger_unregister(rfkill);
@@ -1332,7 +1333,7 @@ static int __init rfkill_init(void)
if (error)
goto error_misc;
 
- 

Re: RTL8723BE performance regression

2018-05-09 Thread João Paulo Rechi Vita
On Tue, May 8, 2018 at 1:37 AM, Pkshih <pks...@realtek.com> wrote:
> On Mon, 2018-05-07 at 14:49 -0700, João Paulo Rechi Vita wrote:
>> On Tue, May 1, 2018 at 10:58 PM, Pkshih <pks...@realtek.com> wrote:
>> > On Wed, 2018-05-02 at 05:44 +, Pkshih wrote:
>> >>
>> >> > -Original Message-
>> >> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com]
>> >> > Sent: Wednesday, May 02, 2018 6:41 AM
>> >> > To: Larry Finger
>> >> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; 
>> >> > Chaoming_Li; Kalle Valo;
>> >> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo 
>> >> > Rechi Vita; linux@endless
>> m.c
>> >> om
>> >> > Subject: Re: RTL8723BE performance regression
>> >> >
>> >> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger 
>> >> > <larry.fin...@lwfinger.net> wrote:
>> >> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote:
>> >> > >>
>> >> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger 
>> >> > >> <larry.fin...@lwfinger.net>
>> >> > >> wrote:
>> >> > >>
>> >> > >> (...)
>> >> > >>
>> >> > >>> As the antenna selection code changes affected your first 
>> >> > >>> bisection, do
>> >> > >>> you
>> >> > >>> have one of those HP laptops with only one antenna and the incorrect
>> >> > >>> coding
>> >> > >>> in the FUSE?
>> >> > >>
>> >> > >>
>> >> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this
>> >> > >> was needed to achieve a good performance in the past, before this
>> >> > >> regression. I've also opened the laptop chassis and confirmed the
>> >> > >> antenna cable is plugged to the connector labeled with "1" on the
>> >> > >> card.
>> >> > >>
>> >> > >>> If so, please make sure that you still have the same signal
>> >> > >>> strength for good and bad cases. I have tried to keep the driver 
>> >> > >>> and the
>> >> > >>> btcoex code in sync, but there may be some combinations of antenna
>> >> > >>> configuration and FUSE contents that cause the code to fail.
>> >> > >>>
>> >> > >>
>> >> > >> What is the recommended way to monitor the signal strength?
>> >> > >
>> >> > >
>> >> > > The btcoex code is developed for multiple platforms by a different 
>> >> > > group
>> >> > > than the Linux driver. I think they made a change that caused ant_sel 
>> >> > > to
>> >> > > switch from 1 to 2. At least numerous comments at
>> >> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that 
>> >> > > change.
>> >> > >
>> >> > > Mhy recommended method is to verify the wifi device name with "iw 
>> >> > > dev". Then
>> >> > > using that device
>> >> > >
>> >> > > sudo iw dev  scan | egrep "SSID|signal"
>> >> > >
>> >> >
>> >> > I have confirmed that the performance regression is indeed tied to
>> >> > signal strength: on the good cases signal was between -16 and -8 dBm,
>> >> > whereas in bad cases signal was always between -50 to - 40 dBm. I've
>> >> > also switched to testing bandwidth in controlled LAN environment using
>> >> > iperf3, as suggested by Steve deRosier, with the DUT being the only
>> >> > machine connected to the 2.4 GHz radio and the machine running the
>> >> > iperf3 server connected via ethernet.
>> >> >
>> >>
>> >> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: 
>> >> cleanup
>> >> 8723be ant_sel definition"). You can use the above commit and do the same
>> >> experiments (with ant_sel=0, 1 and 2) in your side, and then share your 
>> >> results.
>> >> Since performance is tied to signal strength, you can only share signal 
>> >> streng

Re: RTL8723BE performance regression

2018-05-07 Thread João Paulo Rechi Vita
On Tue, May 1, 2018 at 10:58 PM, Pkshih <pks...@realtek.com> wrote:
> On Wed, 2018-05-02 at 05:44 +, Pkshih wrote:
>>
>> > -Original Message-----
>> > From: João Paulo Rechi Vita [mailto:jprv...@gmail.com]
>> > Sent: Wednesday, May 02, 2018 6:41 AM
>> > To: Larry Finger
>> > Cc: Steve deRosier; 莊彥宣; Pkshih; Birming Chiu; Shaofu; Steven Ting; 
>> > Chaoming_Li; Kalle Valo;
>> > linux-wireless; Network Development; LKML; Daniel Drake; João Paulo Rechi 
>> > Vita; linux@endlessm.c
>> om
>> > Subject: Re: RTL8723BE performance regression
>> >
>> > On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger <larry.fin...@lwfinger.net> 
>> > wrote:
>> > > On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote:
>> > >>
>> > >> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger <larry.fin...@lwfinger.net>
>> > >> wrote:
>> > >>
>> > >> (...)
>> > >>
>> > >>> As the antenna selection code changes affected your first bisection, do
>> > >>> you
>> > >>> have one of those HP laptops with only one antenna and the incorrect
>> > >>> coding
>> > >>> in the FUSE?
>> > >>
>> > >>
>> > >> Yes, that is why I've been passing ant_sel=1 during my tests -- this
>> > >> was needed to achieve a good performance in the past, before this
>> > >> regression. I've also opened the laptop chassis and confirmed the
>> > >> antenna cable is plugged to the connector labeled with "1" on the
>> > >> card.
>> > >>
>> > >>> If so, please make sure that you still have the same signal
>> > >>> strength for good and bad cases. I have tried to keep the driver and 
>> > >>> the
>> > >>> btcoex code in sync, but there may be some combinations of antenna
>> > >>> configuration and FUSE contents that cause the code to fail.
>> > >>>
>> > >>
>> > >> What is the recommended way to monitor the signal strength?
>> > >
>> > >
>> > > The btcoex code is developed for multiple platforms by a different group
>> > > than the Linux driver. I think they made a change that caused ant_sel to
>> > > switch from 1 to 2. At least numerous comments at
>> > > github.com/lwfinger/rtlwifi_new claimed they needed to make that change.
>> > >
>> > > Mhy recommended method is to verify the wifi device name with "iw dev". 
>> > > Then
>> > > using that device
>> > >
>> > > sudo iw dev  scan | egrep "SSID|signal"
>> > >
>> >
>> > I have confirmed that the performance regression is indeed tied to
>> > signal strength: on the good cases signal was between -16 and -8 dBm,
>> > whereas in bad cases signal was always between -50 to - 40 dBm. I've
>> > also switched to testing bandwidth in controlled LAN environment using
>> > iperf3, as suggested by Steve deRosier, with the DUT being the only
>> > machine connected to the 2.4 GHz radio and the machine running the
>> > iperf3 server connected via ethernet.
>> >
>>
>> We have new experimental results in commit af8a41cccf8f46 ("rtlwifi: cleanup
>> 8723be ant_sel definition"). You can use the above commit and do the same
>> experiments (with ant_sel=0, 1 and 2) in your side, and then share your 
>> results.
>> Since performance is tied to signal strength, you can only share signal 
>> strength.
>>
>
> Please pay attention to cold reboot once ant_sel is changed.
>

I've tested the commit mentioned above and it fixes the problem on top
of v4.16 (in addition to the latest wireless-drivers-next also been
fixed as it already contains such commit). On v4.15, we also need the
following commits before "af8a41cccf8f rtlwifi: cleanup 8723be ant_sel
definition" to have a good performance again:

  874e837d67d0 rtlwifi: fill FW version and subversion
  a44709bba70f rtlwifi: btcoex: Add power_on_setting routine
  40d9dd4f1c5d rtlwifi: btcoex: Remove global variables from btcoex

Surprisingly, it seems forcing ant_sel=1 is not needed anymore on
these machines, as the shown by the numbers bellow (ant_sel=0 means
that actually no parameter was passed to the module). I have powered
off the machine and done a cold boot for every test. It seems
something have changed in the antenna auto-selection code since v4.11,
the latest point where I could confirm we definitely need to force
ant_sel=1. I've been trying to understand what causes this difference,
but haven't made progress on that so far, so any suggestions are
appreciated (we are trying to decide if we can confidently drop the
downstream DMI quirks for these specific machines).

  w-d-n ant_sel=0: -14.00 dBm,  69.5 Mbps -> good
  w-d-n ant_sel=1: -10.00 dBm,  41.1 Mbps -> good
  w-d-n ant_sel=2: -44.00 dBm,   607 kbps -> bad

  v4.16 ant_sel=0: -12.00 dBm,  63.0 Mbps -> good
  v4.16 ant_sel=1: - 8.00 dBm,  69.0 Mbps -> good
  v4.16 ant_sel=2: -50.00 dBm,   224 kbps -> bad

  v4.15 ant_sel=0: - 8.00 dBm,  33.0 Mbps -> good
  v4.15 ant_sel=1: -10.00 dBm,  38.1 Mbps -> good
  v4.15 ant_sel=2: -48.00 dBm,   206 kbps -> bad

--
João Paulo Rechi Vita
http://about.me/jprvita


[RFC PATCH 2/3] Revert "rtlwifi: fill FW version and subversion"

2018-05-01 Thread João Paulo Rechi Vita
This reverts commit 874e837d67d0db179c9771f38fd21df07c703e93.

This drastically affects the upload performance and signal strenght on
the HP 240 G4 laptop, as shown by the results bellow:

Without this change:

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -42.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec   735 KBytes  6.02 Mbits/sec1   1.41 KBytes
 [  4]   1.00-2.00   sec   274 KBytes  2.25 Mbits/sec1   1.41 KBytes
 [  4]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec1   28.3 KBytes
 [  4]   5.00-6.00   sec   423 KBytes  3.47 Mbits/sec3   41.0 KBytes
 [  4]   6.00-7.00   sec   840 KBytes  6.88 Mbits/sec0   58.0 KBytes
 [  4]   7.00-8.00   sec   830 KBytes  6.79 Mbits/sec1   1.41 KBytes
 [  4]   8.00-9.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   9.00-10.00  sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 - - - - - - - - - - - - - - - - - - - - - - - - -
 [ ID] Interval   Transfer Bandwidth   Retr
 [  4]   0.00-10.00  sec  3.03 MBytes  2.54 Mbits/sec7 sender
 [  4]   0.00-10.00  sec  2.88 MBytes  2.41 Mbits/sec  receiver

 iperf Done.

With this change:

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -14.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec  4.63 MBytes  38.8 Mbits/sec0194 KBytes
 [  4]   1.00-2.00   sec  4.58 MBytes  38.4 Mbits/sec0273 KBytes
 [  4]   2.00-3.00   sec  5.05 MBytes  42.3 Mbits/sec0332 KBytes
 [  4]   3.00-4.00   sec  4.98 MBytes  41.8 Mbits/sec0393 KBytes
 [  4]   4.00-5.00   sec  4.76 MBytes  39.9 Mbits/sec0434 KBytes
 [  4]   5.00-6.00   sec  4.85 MBytes  40.7 Mbits/sec0434 KBytes
 [  4]   6.00-7.00   sec  3.96 MBytes  33.2 Mbits/sec0464 KBytes
 [  4]   7.00-8.00   sec  4.74 MBytes  39.8 Mbits/sec0481 KBytes
 [  4]   8.00-9.00   sec  4.22 MBytes  35.4 Mbits/sec0508 KBytes
 [  4]   9.00-10.00  sec  4.09 MBytes  34.3 Mbits/sec0564 KBytes
 - - - - - - - - - - - - - - - - - - - - - - - - -
 [ ID] Interval   Transfer Bandwidth   Retr
 [  4]   0.00-10.00  sec  45.9 MBytes  38.5 Mbits/sec0 sender
 [  4]   0.00-10.00  sec  45.0 MBytes  37.7 Mbits/sec  receiver

 iperf Done.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c | 2 --
 drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c 
b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c
index 63874512598b..a2eca669873b 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/fw.c
@@ -141,8 +141,6 @@ int rtl88e_download_fw(struct ieee80211_hw *hw,
return 1;
 
pfwheader = (struct rtlwifi_firmware_header *)rtlhal->pfirmware;
-   rtlhal->fw_version = le16_to_cpu(pfwheader->version);
-   rtlhal->fw_subversion = pfwheader->subversion;
pfwdata = rtlhal->pfirmware;
fwsize = rtlhal->fwsize;
RT_TRACE(rtlpriv, COMP_FW, DBG_DMESG,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c 
b/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c
index 521039c5d4ce..0d1ebc861720 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723com/fw_common.c
@@ -200,8 +200,6 @@ int rtl8723_download_fw(struct ieee80211_hw *hw,
return 1;
 
pfwheader = (struct rtlwifi_firmware_header *)rtlhal->pfirmware;
-   rtlhal->fw_version = le16_to_cpu(pfwheader->version);
-   rtlhal->fw_subversion = pfwheader->subversion;
pfwdata = rtlhal->pfirmware;
fwsize = rtlhal->fwsize;
 
-- 
2.17.0



[RFC PATCH 3/3] rtlwifi: btcoex: Always use 2ant-functions for RTL8723BE

2018-05-01 Thread João Paulo Rechi Vita
This partially reverts commit 7937f02d1953952a6eaf626b175ea9db5541e699,
which not only hooked external functions for newer ICs, as described in
its commit message, but also changed the behavior for RTL8723BE
depending on the value of board_info.btdm_ant_num.

When board_info.btdm_ant_num == 1, 7937f02d19 changes the codepath to
use a whole different set of functions ex_btc8723b1ant_*, instead of the
ex_btc8723b2ant_* that were used before it.

This drastically affects the upload performance and signal strenght on
the HP 240 G4 laptop, as shown by the results bellow:

Without this change:

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -42.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec   735 KBytes  6.02 Mbits/sec1   1.41 KBytes
 [  4]   1.00-2.00   sec   274 KBytes  2.25 Mbits/sec1   1.41 KBytes
 [  4]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec1   28.3 KBytes
 [  4]   5.00-6.00   sec   423 KBytes  3.47 Mbits/sec3   41.0 KBytes
 [  4]   6.00-7.00   sec   840 KBytes  6.88 Mbits/sec0   58.0 KBytes
 [  4]   7.00-8.00   sec   830 KBytes  6.79 Mbits/sec1   1.41 KBytes
 [  4]   8.00-9.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   9.00-10.00  sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 - - - - - - - - - - - - - - - - - - - - - - - - -
 [ ID] Interval   Transfer Bandwidth   Retr
 [  4]   0.00-10.00  sec  3.03 MBytes  2.54 Mbits/sec7 sender
 [  4]   0.00-10.00  sec  2.88 MBytes  2.41 Mbits/sec  receiver

 iperf Done.

With this change:

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -14.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec  4.63 MBytes  38.8 Mbits/sec0194 KBytes
 [  4]   1.00-2.00   sec  4.58 MBytes  38.4 Mbits/sec0273 KBytes
 [  4]   2.00-3.00   sec  5.05 MBytes  42.3 Mbits/sec0332 KBytes
 [  4]   3.00-4.00   sec  4.98 MBytes  41.8 Mbits/sec0393 KBytes
 [  4]   4.00-5.00   sec  4.76 MBytes  39.9 Mbits/sec0434 KBytes
 [  4]   5.00-6.00   sec  4.85 MBytes  40.7 Mbits/sec0434 KBytes
 [  4]   6.00-7.00   sec  3.96 MBytes  33.2 Mbits/sec0464 KBytes
 [  4]   7.00-8.00   sec  4.74 MBytes  39.8 Mbits/sec0481 KBytes
 [  4]   8.00-9.00   sec  4.22 MBytes  35.4 Mbits/sec0508 KBytes
 [  4]   9.00-10.00  sec  4.09 MBytes  34.3 Mbits/sec0564 KBytes
 - - - - - - - - - - - - - - - - - - - - - - - - -
 [ ID] Interval   Transfer Bandwidth   Retr
 [  4]   0.00-10.00  sec  45.9 MBytes  38.5 Mbits/sec0 sender
 [  4]   0.00-10.00  sec  45.0 MBytes  37.7 Mbits/sec  receiver

 iperf Done.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c  | 62 ---
 1 file changed, 12 insertions(+), 50 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 86182b917c92..a862b5efdf55 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -1452,10 +1452,7 @@ void exhalbtc_init_hw_config(struct btc_coexist 
*btcoexist, bool wifi_only)
else if (btcoexist->board_info.btdm_ant_num == 1)
ex_btc8821a1ant_init_hwconfig(btcoexist, wifi_only);
} else if (IS_HARDWARE_TYPE_8723B(btcoexist->adapter)) {
-   if (btcoexist->board_info.btdm_ant_num == 2)
-   ex_btc8723b2ant_init_hwconfig(btcoexist);
-   else if (btcoexist->board_info.btdm_ant_num == 1)
-   ex_btc8723b1ant_init_hwconfig(btcoexist, wifi_only);
+   ex_btc8723b2ant_init_hwconfig(btcoexist);
} else if (IS_HARDWARE_TYPE_8723A(btcoexist->adapter)) {
/* 8723A has no this function */
} else if (IS_HARDWARE_TYPE_8192E(btcoexist->adapter)) {
@@ -1481,10 +1478,7 @@ void exhalbtc_init_coex_dm(struct btc_coexist *btcoexist)
else if (btcoexist->board_info.btdm_ant_num == 1)
ex_btc8821a1ant_init_coex_dm(btcoexist);
} else if (IS_HARDWARE_TYPE_8723B(btcoexist->adapter)) {
-   if (btcoexist->board_info.btdm_ant_num == 2)
-   ex_btc8723b2ant_init_coex_dm(btcoexist);
-

[RFC PATCH 1/3] rtlwifi: btcoex: Don't init antenna position to main port

2018-05-01 Thread João Paulo Rechi Vita
This partially reverts commit 40d9dd4f1c5dcd0d4a2a1f0efcd225c9c4b36d6f,
which does not only remove global variables from btcoex, as described on
its commit message, but also does a few other things -- in particular,
setting the default antenna position to BTC_ANTENNA_AT_MAIN_PORT.

This drastically affects the upload performance and signal strenght on
the HP 240 G4 laptop, as shown by the results bellow:

Without this change:

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -42.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec   735 KBytes  6.02 Mbits/sec1   1.41 KBytes
 [  4]   1.00-2.00   sec   274 KBytes  2.25 Mbits/sec1   1.41 KBytes
 [  4]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec1   28.3 KBytes
 [  4]   5.00-6.00   sec   423 KBytes  3.47 Mbits/sec3   41.0 KBytes
 [  4]   6.00-7.00   sec   840 KBytes  6.88 Mbits/sec0   58.0 KBytes
 [  4]   7.00-8.00   sec   830 KBytes  6.79 Mbits/sec1   1.41 KBytes
 [  4]   8.00-9.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   9.00-10.00  sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 - - - - - - - - - - - - - - - - - - - - - - - - -
 [ ID] Interval   Transfer Bandwidth   Retr
 [  4]   0.00-10.00  sec  3.03 MBytes  2.54 Mbits/sec7 sender
 [  4]   0.00-10.00  sec  2.88 MBytes  2.41 Mbits/sec  receiver

 iperf Done.

With this change:

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -14.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 59380 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec  4.63 MBytes  38.8 Mbits/sec0194 KBytes
 [  4]   1.00-2.00   sec  4.58 MBytes  38.4 Mbits/sec0273 KBytes
 [  4]   2.00-3.00   sec  5.05 MBytes  42.3 Mbits/sec0332 KBytes
 [  4]   3.00-4.00   sec  4.98 MBytes  41.8 Mbits/sec0393 KBytes
 [  4]   4.00-5.00   sec  4.76 MBytes  39.9 Mbits/sec0434 KBytes
 [  4]   5.00-6.00   sec  4.85 MBytes  40.7 Mbits/sec0434 KBytes
 [  4]   6.00-7.00   sec  3.96 MBytes  33.2 Mbits/sec0464 KBytes
 [  4]   7.00-8.00   sec  4.74 MBytes  39.8 Mbits/sec0481 KBytes
 [  4]   8.00-9.00   sec  4.22 MBytes  35.4 Mbits/sec0508 KBytes
 [  4]   9.00-10.00  sec  4.09 MBytes  34.3 Mbits/sec0564 KBytes
 - - - - - - - - - - - - - - - - - - - - - - - - -
 [ ID] Interval   Transfer Bandwidth   Retr
 [  4]   0.00-10.00  sec  45.9 MBytes  38.5 Mbits/sec0 sender
 [  4]   0.00-10.00  sec  45.0 MBytes  37.7 Mbits/sec  receiver

 iperf Done.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index b026e80940a4..86182b917c92 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -1388,9 +1388,6 @@ bool exhalbtc_bind_bt_coex_withadapter(void *adapter)
ant_num = rtl_get_hwpg_ant_num(rtlpriv);
exhalbtc_set_ant_num(rtlpriv, BT_COEX_ANT_TYPE_PG, ant_num);
 
-   /* set default antenna position to main  port */
-   btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT;
-
single_ant_path = rtl_get_hwpg_single_ant_path(rtlpriv);
exhalbtc_set_single_ant_path(btcoexist, single_ant_path);
 
-- 
2.17.0



Re: RTL8723BE performance regression

2018-05-01 Thread João Paulo Rechi Vita
On Tue, Apr 3, 2018 at 7:51 PM, Larry Finger <larry.fin...@lwfinger.net> wrote:
> On 04/03/2018 09:37 PM, João Paulo Rechi Vita wrote:
>>
>> On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger <larry.fin...@lwfinger.net>
>> wrote:
>>
>> (...)
>>
>>> As the antenna selection code changes affected your first bisection, do
>>> you
>>> have one of those HP laptops with only one antenna and the incorrect
>>> coding
>>> in the FUSE?
>>
>>
>> Yes, that is why I've been passing ant_sel=1 during my tests -- this
>> was needed to achieve a good performance in the past, before this
>> regression. I've also opened the laptop chassis and confirmed the
>> antenna cable is plugged to the connector labeled with "1" on the
>> card.
>>
>>> If so, please make sure that you still have the same signal
>>> strength for good and bad cases. I have tried to keep the driver and the
>>> btcoex code in sync, but there may be some combinations of antenna
>>> configuration and FUSE contents that cause the code to fail.
>>>
>>
>> What is the recommended way to monitor the signal strength?
>
>
> The btcoex code is developed for multiple platforms by a different group
> than the Linux driver. I think they made a change that caused ant_sel to
> switch from 1 to 2. At least numerous comments at
> github.com/lwfinger/rtlwifi_new claimed they needed to make that change.
>
> Mhy recommended method is to verify the wifi device name with "iw dev". Then
> using that device
>
> sudo iw dev  scan | egrep "SSID|signal"
>

I have confirmed that the performance regression is indeed tied to
signal strength: on the good cases signal was between -16 and -8 dBm,
whereas in bad cases signal was always between -50 to - 40 dBm. I've
also switched to testing bandwidth in controlled LAN environment using
iperf3, as suggested by Steve deRosier, with the DUT being the only
machine connected to the 2.4 GHz radio and the machine running the
iperf3 server connected via ethernet.

Using those two tests (iperf3 and signal strength) I've dug deeper
into the culprit I had found previously, commit 7937f02d1953,
reverting it partially and testing the resulting driver, to isolate
which change was causing the problem. Besides "hooking up external
functions for newer ICs", as described by the commit message, that
commit also added code to decided whether ex_btc8723b1ant_*() or
ex_btc8723b2ant_*() functions should be used in halbtcoutsrc.c,
depending on the value of board_info.btdm_ant_num, whereas before that
commit ex_btc8723b2ant_*() were always used. Reverting to always using
ex_btc8723b2ant_*() functions fixes the regression on v4.15.

I've also tried to bisect between v4.15..v4.16 to find what else was
causing problems there, as the changes mentioned above on top of v4.16
did not solve the problem. The bisect pointed to "874e837d67d0
rtlwifi: fill FW version and subversion", only but reverting it plus
the changes mentioned above also didn't yield good results. That's
when I decided to get a bit creative: starting on v4.16 I first
applied the changes to have ex_btc8723b2ant_*() always being used, as
mentioned above, then reverted every commit between v4.15..v4.16
affecting drivers/net/wireless/realtek/rtlwifi/, and verified the
resulting kernel had a good performance. Then I started trimming down
the history and testing along the way, to reduce to the minimum set of
changes that had to be reverted in order to restore the good
performance. In addition to the ex_btc8723b2ant_*() changes and
reverting "874e837d67d0 rtlwifi: fill FW version and subversion", I've
also had to remove the following lines from
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c, which
were introduced by "40d9dd4f1c5d rtlwifi: btcoex: Remove global
variables from btcoex", in order to restore the upload performance and
signal strength.

/* set default antenna position to main  port */
btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT;

These are the results I've got on v4.16 (similarly on
wireless-drivers-next-for-davem-2018-03-29 or v4.15):

 $ sudo iw dev wlp2s0 scan | grep -B3 JJ | grep signal
signal: -42.00 dBm
 $ iperf3 -c 192.168.1.254
 Connecting to host 192.168.1.254, port 5201
 [  4] local 192.168.1.253 port 39678 connected to 192.168.1.254 port 5201
 [ ID] Interval   Transfer Bandwidth   Retr  Cwnd
 [  4]   0.00-1.00   sec   735 KBytes  6.02 Mbits/sec1   1.41 KBytes
 [  4]   1.00-2.00   sec   274 KBytes  2.25 Mbits/sec1   1.41 KBytes
 [  4]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 [  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec0   1.41 KBytes
 

Re: RTL8723BE performance regression

2018-04-03 Thread João Paulo Rechi Vita
On Tue, Apr 3, 2018 at 7:28 PM, Larry Finger <larry.fin...@lwfinger.net> wrote:

(...)

> As the antenna selection code changes affected your first bisection, do you
> have one of those HP laptops with only one antenna and the incorrect coding
> in the FUSE?

Yes, that is why I've been passing ant_sel=1 during my tests -- this
was needed to achieve a good performance in the past, before this
regression. I've also opened the laptop chassis and confirmed the
antenna cable is plugged to the connector labeled with "1" on the
card.

> If so, please make sure that you still have the same signal
> strength for good and bad cases. I have tried to keep the driver and the
> btcoex code in sync, but there may be some combinations of antenna
> configuration and FUSE contents that cause the code to fail.
>

What is the recommended way to monitor the signal strength?

Thanks for such a quick reply,

--
João Paulo Rechi Vita
http://about.me/jprvita


RTL8723BE performance regression

2018-04-03 Thread João Paulo Rechi Vita
Hello,

I've been trying to track a performance regression on the RTL8723BE
WiFi adapter, which mainly affects the upload bandwidth (although we
can see a decreased download performance as well, the effect on upload
is more drastic). This was first reported by users after upgrading
from our 4.11-based kernel to our 4.13-based kernel, but also
confirmed to affect our development branch (4.15-based kernel) and
wireless-drivers-next at the
wireless-drivers-next-for-davem-2018-03-29 tag. This is happening on
an HP laptop that needs rtl8723be.ant_sel=1 (and all the following
tests have been made with that param).

My first bisect attempt pointed me to the following commit:

bcd37f4a0831 rtlwifi: btcoex: 23b 2ant: let bt transmit when hw
initialisation done

Which I later found to be already fixed by

a33fcba6ec01 rtlwifi: btcoexist: Fix breakage of ant_sel for rtl8723be.

That fix is already included in v4.15 though (and our dev branch as
well), so I did a second bisect, now cherry-picking a33fcba6ec01 at
every step, and it pointed me to the following commit:

7937f02d1953 rtlwifi: btcoex: hook external functions for newer chips

Reverting that commit on top of our development branch fixes the
problem, but on top of v4.15 I get mixed results: a few times getting
a good upload performance (~5-6Mbps) but most of the time just getting
~1-1.5Mpbs (which is still better than the 0.0 then test failure I've
gotten on most bad points of the bisect).

Bisecting the downstream patches we carry on top of v4.15 (we base our
kernel on Ubuntu's, so there are quite a few downstream changes) did
not bring any clarity, as at all bisect points (plus reverting
7937f02d1953) the performance was good, so probably there was some
other difference in the resulting kernels from my initial revert of
that patch on top of v4.15 and each step during the bisect. I've
experimented a bit with fwlps=0, but it did not bring any conclusive
results either. I'll try to look at other things that may have changed
(configuration perhaps?), but I don't have a clear plan yet.

Have you seen anything similar, or have any other ideas or suggestions
to track this problem? Even without crystal clear results, it looks
like 7937f02d1953 is having a negative impact on the RTL8723BE
performance, so perhaps it is worth reverting it and reworking it a
later point?

This are the results (testing with speedtest.net) I got at some key points:

VersionCommitPingDownUp

v4.11a351e9b1225.445.99
v4.11a351e9b131  17.025.89

v4.13569dbb8174  14.080.00
v4.13569dbb8261  8.41  0.00

v4.15+revert d8a5b801923.861.41
v4.15+revert d8a5b80189  18.691.39

Best regards,

--
João Paulo Rechi Vita
http://about.me/jprvita


[PATCH v3] iwlwifi: Demote messages about fw flags size to info

2017-08-03 Thread João Paulo Rechi Vita
These messages are not reporting a real error, just the fact that the
firmware knows about more flags than the driver.

Currently these messages are presented to the user during boot if there
is no bootsplash covering the console, even when booting the kernel with
"quiet".

Demoting it to the warn level helps having a clean boot process.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---

v2 changes:
 - Set to warn level instead of info

v3 changes:
 - Fix commit message typo

 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index be466a074c1d..c98a254bbd4d 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -461,9 +461,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, 
const u8 *data,
int i;
 
if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) {
-   IWL_ERR(drv,
-   "api flags index %d larger than supported by driver\n",
-   api_index);
+   IWL_WARN(drv,
+"api flags index %d larger than supported by driver\n",
+api_index);
/* don't return an error so we can load FW that has more bits */
return 0;
}
@@ -485,9 +485,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, 
const u8 *data,
int i;
 
if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) {
-   IWL_ERR(drv,
-   "capa flags index %d larger than supported by driver\n",
-   api_index);
+   IWL_WARN(drv,
+"capa flags index %d larger than supported by 
driver\n",
+api_index);
/* don't return an error so we can load FW that has more bits */
return 0;
}
-- 
2.13.2



[PATCH v2] iwlwifi: Demote messages about fw flags size to info

2017-08-03 Thread João Paulo Rechi Vita
These messages are not reporting a real error, just the fact that the
firmware knows about more flags then the driver.

Currently these messages are presented to the user during boot if there
is no bootsplash covering the console, even when booting the kernel with
"quiet".

Demoting it to the warn level helps having a clean boot process.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---

v2 changes:
 - Set to warn level instead of info

 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index be466a074c1d..c98a254bbd4d 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -461,9 +461,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, 
const u8 *data,
int i;
 
if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) {
-   IWL_ERR(drv,
-   "api flags index %d larger than supported by driver\n",
-   api_index);
+   IWL_WARN(drv,
+"api flags index %d larger than supported by driver\n",
+api_index);
/* don't return an error so we can load FW that has more bits */
return 0;
}
@@ -485,9 +485,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, 
const u8 *data,
int i;
 
if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) {
-   IWL_ERR(drv,
-   "capa flags index %d larger than supported by driver\n",
-   api_index);
+   IWL_WARN(drv,
+"capa flags index %d larger than supported by 
driver\n",
+api_index);
/* don't return an error so we can load FW that has more bits */
return 0;
}
-- 
2.13.2



Re: [PATCH] iwlwifi: Demote messages about fw flags size to info

2017-08-01 Thread João Paulo Rechi Vita
Hello Luca,

On Mon, Jul 24, 2017 at 4:01 AM, Coelho, Luciano
<luciano.coe...@intel.com> wrote:
> On Fri, 2017-07-21 at 07:51 -0700, João Paulo Rechi Vita wrote:

(...)

>> Currently these messages are presented to the user during boot if there
>> is no bootsplash covering the console, sometimes even if the boot splash
>> is enabled but has not started yet by the time this message is shown.
>>

I should have provided another piece of information here: all of this
happens even when booting with the 'quiet' kernel parameter.

> This specific case is harmless, but I'd rather keep this message as an
> error, because in other situations it could lead to unexpected
> behavioir, so I prefer to keep it very visible.
>
>

I see your point, and I understand the purpose of these messages. I'm
wondering if perhaps having them at the warning level would give them
enough visibility, while still keeping a clean boot process to the end
user. If so, I can send an updated patch.

Thanks for your reply and for pointing to the fix for the root cause
of that message.

Cheers,

..........
João Paulo Rechi Vita  |  +1.415.851.5778  |  Endless


[PATCH] iwlwifi: Demote messages about fw flags size to info

2017-07-21 Thread João Paulo Rechi Vita
These messages are not reporting a real error, just the fact that the
firmware knows about more flags then the driver.

Currently these messages are presented to the user during boot if there
is no bootsplash covering the console, sometimes even if the boot splash
is enabled but has not started yet by the time this message is shown.

Demoting it to the info level helps having a clean boot process.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 6fdb5921e17f..557acd43d705 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -487,9 +487,9 @@ static int iwl_set_ucode_api_flags(struct iwl_drv *drv, 
const u8 *data,
int i;
 
if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_API, 32)) {
-   IWL_ERR(drv,
-   "api flags index %d larger than supported by driver\n",
-   api_index);
+   IWL_INFO(drv,
+"api flags index %d larger than supported by driver\n",
+api_index);
/* don't return an error so we can load FW that has more bits */
return 0;
}
@@ -511,9 +511,9 @@ static int iwl_set_ucode_capabilities(struct iwl_drv *drv, 
const u8 *data,
int i;
 
if (api_index >= DIV_ROUND_UP(NUM_IWL_UCODE_TLV_CAPA, 32)) {
-   IWL_ERR(drv,
-   "capa flags index %d larger than supported by driver\n",
-   api_index);
+   IWL_INFO(drv,
+"capa flags index %d larger than supported by 
driver\n",
+api_index);
/* don't return an error so we can load FW that has more bits */
return 0;
}
-- 
2.13.2



Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

2016-06-13 Thread João Paulo Rechi Vita
On 13 June 2016 at 17:01, Pavel Machek <pa...@ucw.cz> wrote:
> On Mon 2016-06-13 15:59:35, João Paulo Rechi Vita wrote:
>> On 13 June 2016 at 15:00, Pavel Machek <pa...@ucw.cz> wrote:
>> > Hi!
>> >
>> >> > João, that means you should send a patch to add the ::rfkill suffix.
>> >> >
>> >>
>> >> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
>> >> reflects the label on the machine's chassis. I'll name it
>> >> "asus-wireless::airplane" and send this through platform-drivers-x86,
>> >> as this is now contained in the platform-drivers-x86 subsystem. Thanks
>> >> Johannes for your patience and help designing and reviewing the rfkill
>> >> changes, even if not all of them made it through in the end. And
>> >> thanks everyone else involved for the feedback.
>> >
>> > Actually, I'd do '::rfkill', for consistency with other places in
>> > /sys.
>> >
>> > /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
>> > /sys/class/rfkill
>> > /sys/module/rfkill
>> >
>>
>> If we use "rfkill" as a suffix, how do you expect userspace to be able
>> to differentiate between a LED that indicates airplane-mode (LED ON
>> when all radios are OFF) and a LED that indicates the state of a
>> specific radio like WiFi or Bluetooth (LED ON when that specific radio
>> is ON)? If we're going this route we should provide meaningful
>> information here.
>
> '::airplane' has same problem, no?
>

No, because in this case we would not use "airplane" as a suffix for a
LED associated with an individual radio.

> If you want to distinguish that, maybe you can do '::rfkill' for
> everything vs '::rfkill-wifi' for wifi-only and '::rfkill-bt' for
> bluetooth...
>

The problem here is that the "rfkill" name is already associated with
individual rfkill switches under /sys/class/rfkill,
/sys/devices/platform/*/rfkill etc, so I think we're better off
distinguishing "airplane" vs "wifi" vs "bluetooth" etc, to avoid
confusion.

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

2016-06-13 Thread João Paulo Rechi Vita
On 13 June 2016 at 15:00, Pavel Machek <pa...@ucw.cz> wrote:
> Hi!
>
>> > João, that means you should send a patch to add the ::rfkill suffix.
>> >
>>
>> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
>> reflects the label on the machine's chassis. I'll name it
>> "asus-wireless::airplane" and send this through platform-drivers-x86,
>> as this is now contained in the platform-drivers-x86 subsystem. Thanks
>> Johannes for your patience and help designing and reviewing the rfkill
>> changes, even if not all of them made it through in the end. And
>> thanks everyone else involved for the feedback.
>
> Actually, I'd do '::rfkill', for consistency with other places in
> /sys.
>
> /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
> /sys/class/rfkill
> /sys/module/rfkill
>

If we use "rfkill" as a suffix, how do you expect userspace to be able
to differentiate between a LED that indicates airplane-mode (LED ON
when all radios are OFF) and a LED that indicates the state of a
specific radio like WiFi or Bluetooth (LED ON when that specific radio
is ON)? If we're going this route we should provide meaningful
information here.

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

2016-06-13 Thread João Paulo Rechi Vita
On 9 June 2016 at 08:43, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Thu, 2016-05-19 at 09:16 +0200, Pavel Machek wrote:
>

(...)

>> LED
>> subsystem seems to use suffix of LED name to do that. So if we
>> standartize, lets say "::rfkill" suffix for this, it should work and
>> follow existing practice.
> [...]
>> There is one -- suffix in the LED name.
>
> I don't really think that's a good way, and it doesn't seem to be used
> universally, but I suppose it's good enough.
>

The main practical drawback of this approach IMO is that we can't
guarantee that userspace processes will not step on each other's toes
trying to control the LED concurrently. But I guess that is something
that userspace will have to solve for now, I rather get this moving
without the trigger than not moving at all.

> João, that means you should send a patch to add the ::rfkill suffix.
>

IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
reflects the label on the machine's chassis. I'll name it
"asus-wireless::airplane" and send this through platform-drivers-x86,
as this is now contained in the platform-drivers-x86 subsystem. Thanks
Johannes for your patience and help designing and reviewing the rfkill
changes, even if not all of them made it through in the end. And
thanks everyone else involved for the feedback.

Best regards,

--
João Paulo Rechi Vita
http://about.me/jprvita


[RESEND PATCH 2/3] rfkill: Userspace control for airplane mode

2016-05-02 Thread João Paulo Rechi Vita
From: João Paulo Rechi Vita <jprv...@gmail.com>

Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE and keep the fd open for the
whole time it wants to be in control of the indicator. Closing the fd
restores the default policy.

To change state of the indicator, the
RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE operation is used, passing the
value on "struct rfkill_event.soft". If the caller has not acquired the
airplane-mode control beforehand, the operation fails.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt| 10 ++
 include/uapi/linux/rfkill.h |  6 ++
 net/rfkill/core.c   | 40 ++--
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..9dbe3fc 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to 
drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE, and is only available for one
+userspace application at a time. Closing the fd reverts the airplane-mode
+indicator back to the default kernel policy and makes it available for other
+applications to take control. Changes to the airplane-mode indicator state can
+be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
+in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..36e0770 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -61,12 +61,18 @@ enum rfkill_type {
  * @RFKILL_OP_CHANGE_ALL: userspace changes all devices (of a type, or all)
  * into a state, also updating the default state used for devices that
  * are hot-plugged later.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
+ * the airplane-mode indicator.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
+ * airplane-mode indicator state.
  */
 enum rfkill_operation {
RFKILL_OP_ADD = 0,
RFKILL_OP_DEL,
RFKILL_OP_CHANGE,
RFKILL_OP_CHANGE_ALL,
+   RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE,
+   RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 9adf95e..95824b3 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
struct mutexmtx;
wait_queue_head_t   read_wait;
boolinput_handler;
+   boolis_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
-   rfkill_apm_led_trigger_event(blocked);
+   if (!rfkill_apm_owned)
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1174,9 +1176,23 @@ static ssize_t rfkill_fop_read(struct file *file, char 
__user *buf,
return ret;
 }
 
+static int rfkill_airplane_mode_release(struct rfkill_data *data)
+{
+   bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+   if (rfkill_apm_owned && data->is_apm_owner) {
+   rfkill_apm_owned = false;
+   data->is_apm_owner = false;
+   rfkill_apm_led_trigger_event(state);
+   return 0;
+   }
+   return -EACCES;
+}
+
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
 {
+   struct rfkill_data *data = file->private_data;
struct rfkill *rfkill;
struct rfkill_event ev;
int ret;
@@ -1216,6 +1232,25 @@ st

[RESEND PATCH 0/3] RFKill airplane-mode indicator

2016-05-02 Thread João Paulo Rechi Vita
This series implements an airplane-mode indicator LED trigger, which can be
used by platform drivers. By default the trigger fires on RFKILL_OP_CHANGE_ALL,
but this policy can be overwritten by userspace using the new operations
_AIRPLANE_MODE_INDICATOR_ACQUIRE and _AIRPLANE_MODE_INDICATOR_CHANGE. When the
airplane-mode indicator state changes, userspace gets notifications through the
RFKill control misc device (/dev/rfkill). I also have patches to the rfkill
userspace tool that makes use of this interface, so it can serve as API usage
example and to do quick checks on a running system, which I'll send in a
separate series.

After some hiatus I found the time to get back to this, and this time I've used
Jouni's hwsim suite to test the patches on top of mac80211-next/master. Lines
containing rfkill are pasted bellow:

START wext_rfkill 14/1880
PASS wext_rfkill 4.12 2016-04-29 12:30:23.792682
START rfkill_wpas 571/1880
PASS rfkill_wpas 1.245307 2016-04-29 12:48:51.804344
START rfkill_autogo 572/1880
PASS rfkill_autogo 1.154174 2016-04-29 12:48:52.959605
START rfkill_p2p_discovery 573/1880
PASS rfkill_p2p_discovery 0.534903 2016-04-29 12:48:53.495547
START rfkill_open 574/1880
PASS rfkill_open 0.34073 2016-04-29 12:48:53.836963
START rfkill_p2p_discovery_p2p_dev 575/1880
PASS rfkill_p2p_discovery_p2p_dev 1.159446 2016-04-29 12:48:54.997555
START rfkill_hostapd 576/1880
PASS rfkill_hostapd 3.686868 2016-04-29 12:48:58.685162
START rfkill_wpa2_psk 577/1880
PASS rfkill_wpa2_psk 0.330014 2016-04-29 12:48:59.016711

João Paulo Rechi Vita (3):
  rfkill: Create "rfkill-airplane-mode" LED trigger
  rfkill: Userspace control for airplane mode
  rfkill: Notify userspace of airplane-mode state changes

 Documentation/rfkill.txt|  15 +++
 include/uapi/linux/rfkill.h |   6 +++
 net/rfkill/core.c   | 100 +++-
 3 files changed, 119 insertions(+), 2 deletions(-)

-- 
2.5.0



[RESEND PATCH 3/3] rfkill: Notify userspace of airplane-mode state changes

2016-05-02 Thread João Paulo Rechi Vita
From: João Paulo Rechi Vita <jprv...@gmail.com>

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt|  3 +++
 include/uapi/linux/rfkill.h |  4 ++--
 net/rfkill/core.c   | 13 +
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 9dbe3fc..588b4bf 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -133,5 +133,8 @@ applications to take control. Changes to the airplane-mode 
indicator state can
 be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
 in the 'soft' field of 'struct rfkill_event'.
 
+This same API is also used to provide userspace with notifications of changes
+to airplane-mode indicator state.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 36e0770..2ccb02f 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -63,8 +63,8 @@ enum rfkill_type {
  * are hot-plugged later.
  * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
  * the airplane-mode indicator.
- * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
- * airplane-mode indicator state.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: the airplane-mode indicator state
+ * changed -- userspace changes the airplane-mode indicator state.
  */
 enum rfkill_operation {
RFKILL_OP_ADD = 0,
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 95824b3..c4bbd19 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -131,7 +131,20 @@ static struct led_trigger rfkill_apm_led_trigger;
 
 static void rfkill_apm_led_trigger_event(bool state)
 {
+   struct rfkill_data *data;
+   struct rfkill_int_event *ev;
+
led_trigger_event(_apm_led_trigger, state ? LED_FULL : LED_OFF);
+
+   list_for_each_entry(data, _fds, list) {
+   ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+   if (!ev)
+   continue;
+   ev->ev.op = RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE;
+   ev->ev.soft = state;
+   list_add_tail(>list, >events);
+   wake_up_interruptible(>read_wait);
+   }
 }
 
 static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
-- 
2.5.0



[RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

2016-05-02 Thread João Paulo Rechi Vita
From: João Paulo Rechi Vita <jprv...@gmail.com>

This creates a new LED trigger to be used by platform drivers as a
default trigger for airplane-mode indicator LEDs.

By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
for all types (RFKILL_TYPE_ALL), setting the LED brightness to LED_FULL
when the changing the state to blocked, and to LED_OFF when the changing
the state to unblocked. In the future there will be a mechanism for
userspace to override the default policy, so it can implement its own.

This trigger will be used by the asus-wireless x86 platform driver.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt |  2 ++
 net/rfkill/core.c| 49 +++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 1f0c270..b13025a 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -85,6 +85,8 @@ device). Don't do this unless you cannot get the event in any 
other way.
 
 RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
+An airplane-mode indicator LED trigger is also available, which triggers
+LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
 
 
 5. Userspace support
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 884027f..9adf95e 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -126,6 +126,30 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static struct led_trigger rfkill_apm_led_trigger;
+
+static void rfkill_apm_led_trigger_event(bool state)
+{
+   led_trigger_event(_apm_led_trigger, state ? LED_FULL : LED_OFF);
+}
+
+static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
+{
+   rfkill_apm_led_trigger_event(!rfkill_default_state);
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+   rfkill_apm_led_trigger.name = "rfkill-airplane-mode";
+   rfkill_apm_led_trigger.activate = rfkill_apm_led_trigger_activate;
+   return led_trigger_register(_apm_led_trigger);
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+   led_trigger_unregister(_apm_led_trigger);
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
struct led_trigger *trigger;
@@ -177,6 +201,19 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
led_trigger_unregister(>led_trigger);
 }
 #else
+static void rfkill_apm_led_trigger_event(bool state)
+{
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+   return 0;
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -313,6 +350,7 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1262,15 +1300,22 @@ static int __init rfkill_init(void)
 {
int error;
 
+   error = rfkill_apm_led_trigger_register();
+   if (error)
+   goto out;
+
rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
 
error = class_register(_class);
-   if (error)
+   if (error) {
+   rfkill_apm_led_trigger_unregister();
goto out;
+   }
 
error = misc_register(_miscdev);
if (error) {
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
goto out;
}
 
@@ -1279,6 +1324,7 @@ static int __init rfkill_init(void)
if (error) {
misc_deregister(_miscdev);
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
goto out;
}
 #endif
@@ -1295,5 +1341,6 @@ static void __exit rfkill_exit(void)
 #endif
misc_deregister(_miscdev);
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
 }
 module_exit(rfkill_exit);
-- 
2.5.0



Re: [PATCHv2 08/10] rfkill: Use switch to demux userspace operations

2016-03-08 Thread João Paulo Rechi Vita
Hello Johannes,

On 1 March 2016 at 11:15, João Paulo Rechi Vita <jprv...@gmail.com> wrote:
> On 1 March 2016 at 08:43, Johannes Berg <johan...@sipsolutions.net> wrote:
>>
>> I'm fine with Jouni's change, preserving the original behaviour of
>> requiring TYPE_ALL or the correct type, but I'm tempted to simply
>> remove the type check entirely.
>>
>> Thoughts?
>>
>
> I think this patch should keep the original logic, as this is supposed
> to be a refactor only. If we decide to remove the check, we should to
> it in a separate patch, to make it clear for someone looking at the
> history later.
>
> I'm fine with removing the type check (in a separate patch), but I
> don't see much gain in doing so.
>

I just saw you picked this patch with Jouni's fix, thanks!

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCHv2 08/10] rfkill: Use switch to demux userspace operations

2016-03-01 Thread João Paulo Rechi Vita
On 1 March 2016 at 08:43, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Tue, 2016-03-01 at 00:39 +0200, Jouni Malinen wrote:
>
>> > I agree there is a difference in the logic here,
>
> Gah. I thought I'd reviewed the logic and made sure there's no
> difference ... :)
>
>> >  thanks for taking the
>> > time to point it out so clearly, and sorry for missing this. But AFAIU
>> > userspace should not call RFKILL_OP_CHANGE with ev.type ==
>> > RFKILL_TYPE_ALL, as RFKILL_OP_CHANGE is intended to be used to
>> > block/unblock one RFKill switch, and it is not possible to create a
>> > RFKill switch with type == RFKILL_TYPE_ALL (rfkill_alloc() would
>> > return NULL).
>
>> Interesting. Maybe Johannes can comment on that part since I think he
>> wrote the code that interacts with kernel for the rfkill test cases.
>
> So first of all, it seems that this argument is invalid since we can't break 
> the ABI/API here; although perhaps if it's only a test case ...
>

Yep, that's an important point (not breaking the API/ABI).

> Oh. It took me a while, but I see now. The original intent (I think)
> was that with RFKILL_OP_CHANGE, the type would be ignored entirely. It
> seems that the (my) original intent wouldn't have been to force
> userspace to specify *both* the index and the type, but instead do
>
> OP_CHANGE_ALL -> use type (possibly TYPE_ALL, ignoring idx)
> OP_CHANGE -> use idx (ignoring type)
>
>
> The original code implemented it as follows:
>
> if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
> continue;
>
> -> check the idx only for OP_CHANGE
>
> if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL)
> continue;
>
> -> check the type, allowing _ALL
>
> Now, all userspace that I found sets the ev.type field to TYPE_ALL all
> the time; and it had to given these checks.
>
> e.g. from rfkill.py:
>
> # idx, type, op, soft, hard
> _event_struct = '@I'
>
> [...]
>
> def block(self):
> rfk = open('/dev/rfkill', 'w')
> s = struct.pack(_event_struct, self.idx, TYPE_ALL, _OP_CHANGE, 1, 0)
> rfk.write(s)
> rfk.close()
>
>
> This check, originally, probably should've been
>
> if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL &&
> ev.op != RFKILL_OP_CHANGE)
> continue;
>
> to ignore the type entirely.
>
> I'm fine with Jouni's change, preserving the original behaviour of
> requiring TYPE_ALL or the correct type, but I'm tempted to simply
> remove the type check entirely.
>
> Thoughts?
>

I think this patch should keep the original logic, as this is supposed
to be a refactor only. If we decide to remove the check, we should to
it in a separate patch, to make it clear for someone looking at the
history later.

I'm fine with removing the type check (in a separate patch), but I
don't see much gain in doing so.

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCHv2 08/10] rfkill: Use switch to demux userspace operations

2016-02-29 Thread João Paulo Rechi Vita
Hello Jouni,

On 26 February 2016 at 12:59, Jouni Malinen <j...@w1.fi> wrote:
> On Mon, Feb 22, 2016 at 11:36:39AM -0500, João Paulo Rechi Vita wrote:
>> Using a switch to handle different ev.op values in rfkill_fop_write()
>> makes the code easier to extend, as out-of-range values can always be
>> handled by the default case.
>
> This breaks rfkill.. There are automated test scripts for testing this
> area (and most of Wi-Fi for that matter. It would be nice if these were
> used for changes before they get contributed upstream..
>
> http://buildbot.w1.fi/hwsim/
>

Thanks for pointing that out, I haven't heard of this tool before.
I'll give it a try before my next submission.

> This specific commit broke all the rfkill_* test cases because of
> following:
>
>> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
>> @@ -1199,29 +1200,32 @@ static ssize_t rfkill_fop_write(struct file *file, 
>> const char __user *buf,
>> - list_for_each_entry(rfkill, _list, node) {
>> - if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
>> - continue;
>> -
>> - if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL)
>> - continue;
>
> Note that RFKILL_TYPE_ALL here..
>
>> + list_for_each_entry(rfkill, _list, node)
>> + if (rfkill->type == ev.type ||
>> + ev.type == RFKILL_TYPE_ALL)
>> + rfkill_set_block(rfkill, ev.soft);
>
> It was included for RFKILL_OP_CHANGE_ALL.
>
>> + case RFKILL_OP_CHANGE:
>> + list_for_each_entry(rfkill, _list, node)
>> + if (rfkill->idx == ev.idx && rfkill->type == ev.type)
>> + rfkill_set_block(rfkill, ev.soft);
>
> but not for RFKILL_OP_CHANGE..
>
> This needs following to work:
>
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index 59ff92d..c4bbd19 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -1239,7 +1239,9 @@ static ssize_t rfkill_fop_write(struct file *file, 
> const char __user *buf,
> break;
> case RFKILL_OP_CHANGE:
> list_for_each_entry(rfkill, _list, node)
> -   if (rfkill->idx == ev.idx && rfkill->type == ev.type)
> +   if (rfkill->idx == ev.idx &&
> +   (rfkill->type == ev.type ||
> +ev.type == RFKILL_TYPE_ALL))
> rfkill_set_block(rfkill, ev.soft);
> ret = 0;
> break;
>

I agree there is a difference in the logic here, thanks for taking the
time to point it out so clearly, and sorry for missing this. But AFAIU
userspace should not call RFKILL_OP_CHANGE with ev.type ==
RFKILL_TYPE_ALL, as RFKILL_OP_CHANGE is intended to be used to
block/unblock one RFKill switch, and it is not possible to create a
RFKill switch with type == RFKILL_TYPE_ALL (rfkill_alloc() would
return NULL).

I tried to look into the source code of the test suite you pointed,
but couldn't easily figure out how it ends up with that combination.
Could you please explain (or point me in the code) how is that a valid
operation? If I'm not missing anything, we should probably return
EINVAL in this case.

Regards,

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCHv2 00/10] RFKill airplane-mode indicator

2016-02-22 Thread João Paulo Rechi Vita
On 22 February 2016 at 12:00, Dan Williams <d...@redhat.com> wrote:
> On Mon, 2016-02-22 at 11:36 -0500, João Paulo Rechi Vita wrote:
>> This series implements an airplane-mode indicator LED trigger, which
>> can be
>> used by platform drivers. The default policy have have airplane-mode
>> set when
>> all the radios known by RFKill are OFF, and unset otherwise. This
>> policy can be
>> overwritten by one single userspace application at a time using the
>> operations
>> _AIRPLANE_MODE_INDICATOR_ACQUIRE and _AIRPLANE_MODE_INDICATOR_CHANGE.
>>
> Double-check your commit messages on some of these patches; they didn't
> get updated to add INDICATOR.
>

Thanks for catching this, Dan. I've sent an updated version fixing
this problem in reply to the patch where this slept through (9/10).

--
João Paulo Rechi Vita
http://about.me/jprvita


[PATCHv3] rfkill: Userspace control for airplane mode

2016-02-22 Thread João Paulo Rechi Vita
Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE and keep the fd open for the
whole time it wants to be in control of the indicator. Closing the fd
restores the default policy.

To change state of the indicator, the
RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE operation is used, passing the
value on "struct rfkill_event.soft". If the caller has not acquired the
airplane-mode control beforehand, the operation fails.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt| 10 ++
 include/uapi/linux/rfkill.h |  6 ++
 net/rfkill/core.c   | 35 +--
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..9dbe3fc 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to 
drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE, and is only available for one
+userspace application at a time. Closing the fd reverts the airplane-mode
+indicator back to the default kernel policy and makes it available for other
+applications to take control. Changes to the airplane-mode indicator state can
+be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
+in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..36e0770 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -61,12 +61,18 @@ enum rfkill_type {
  * @RFKILL_OP_CHANGE_ALL: userspace changes all devices (of a type, or all)
  * into a state, also updating the default state used for devices that
  * are hot-plugged later.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
+ * the airplane-mode indicator.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
+ * airplane-mode indicator state.
  */
 enum rfkill_operation {
RFKILL_OP_ADD = 0,
RFKILL_OP_DEL,
RFKILL_OP_CHANGE,
RFKILL_OP_CHANGE_ALL,
+   RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE,
+   RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 04d7fa1..8ea8b73 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
struct mutexmtx;
wait_queue_head_t   read_wait;
boolinput_handler;
+   boolis_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
-   rfkill_apm_led_trigger_event(blocked);
+   if (!rfkill_apm_owned)
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1180,9 +1182,23 @@ static ssize_t rfkill_fop_read(struct file *file, char 
__user *buf,
return ret;
 }
 
+static int rfkill_airplane_mode_release(struct rfkill_data *data)
+{
+   bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+   if (rfkill_apm_owned && data->is_apm_owner) {
+   rfkill_apm_owned = false;
+   data->is_apm_owner = false;
+   rfkill_apm_led_trigger_event(state);
+   return 0;
+   }
+   return -EACCES;
+}
+
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
 {
+   struct rfkill_data *data = file->private_data;
struct rfkill *rfkill;
struct rfkill_event ev;
int ret = 0;
@@ -1218,6 +1234,20 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
  

[PATCHv2 09/10] rfkill: Userspace control for airplane mode

2016-02-22 Thread João Paulo Rechi Vita
Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
it wants to be in control of the indicator. Closing the fd or using
RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
operation is used, passing the value on "struct rfkill_event.soft". If
the caller has not acquired the airplane-mode control beforehand, the
operation fails.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt| 10 ++
 include/uapi/linux/rfkill.h |  6 ++
 net/rfkill/core.c   | 35 +--
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..9dbe3fc 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to 
drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE, and is only available for one
+userspace application at a time. Closing the fd reverts the airplane-mode
+indicator back to the default kernel policy and makes it available for other
+applications to take control. Changes to the airplane-mode indicator state can
+be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
+in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..36e0770 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -61,12 +61,18 @@ enum rfkill_type {
  * @RFKILL_OP_CHANGE_ALL: userspace changes all devices (of a type, or all)
  * into a state, also updating the default state used for devices that
  * are hot-plugged later.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
+ * the airplane-mode indicator.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
+ * airplane-mode indicator state.
  */
 enum rfkill_operation {
RFKILL_OP_ADD = 0,
RFKILL_OP_DEL,
RFKILL_OP_CHANGE,
RFKILL_OP_CHANGE_ALL,
+   RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE,
+   RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 04d7fa1..8ea8b73 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
struct mutexmtx;
wait_queue_head_t   read_wait;
boolinput_handler;
+   boolis_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
-   rfkill_apm_led_trigger_event(blocked);
+   if (!rfkill_apm_owned)
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1180,9 +1182,23 @@ static ssize_t rfkill_fop_read(struct file *file, char 
__user *buf,
return ret;
 }
 
+static int rfkill_airplane_mode_release(struct rfkill_data *data)
+{
+   bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+   if (rfkill_apm_owned && data->is_apm_owner) {
+   rfkill_apm_owned = false;
+   data->is_apm_owner = false;
+   rfkill_apm_led_trigger_event(state);
+   return 0;
+   }
+   return -EACCES;
+}
+
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
 {
+   struct rfkill_data *data = file->private_data;
struct rfkill *rfkill;
struct rfkill_event ev;
int ret = 0;
@@ -1218,6 +1234,20 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,

[PATCHv2 00/10] RFKill airplane-mode indicator

2016-02-22 Thread João Paulo Rechi Vita
This series implements an airplane-mode indicator LED trigger, which can be
used by platform drivers. The default policy have have airplane-mode set when
all the radios known by RFKill are OFF, and unset otherwise. This policy can be
overwritten by one single userspace application at a time using the operations
_AIRPLANE_MODE_INDICATOR_ACQUIRE and _AIRPLANE_MODE_INDICATOR_CHANGE. When the
airplane-mode indicator state changes, userspace gets notifications through the
RFKill control misc device (/dev/rfkill).

The series also contains a few general fixes and improvements to the subsystem.

Additionally, I have a couple of patches to have this feature supported by the
userspace tool 'rfkill' [1]. Should I use a different subject prefix to help
separate those from kernel patches in linux-wireless?

[1] https://wireless.wiki.kernel.org/en/users/documentation/rfkill

João Paulo Rechi Vita (10):
  rfkill: Improve documentation language
  rfkill: Remove extra blank line
  rfkill: Point to the correct deprecated doc location
  rfkill: Move "state" sysfs file back to stable
  rfkill: Factor rfkill_global_states[].cur assignments
  rfkill: Add documentation about LED triggers
  rfkill: Create "rfkill-airplane-mode" LED trigger
  rfkill: Use switch to demux userspace operations
  rfkill: Userspace control for airplane mode
  rfkill: Notify userspace of airplane-mode state changes

 Documentation/ABI/obsolete/sysfs-class-rfkill |  20 
 Documentation/ABI/stable/sysfs-class-rfkill   |  27 -
 Documentation/rfkill.txt  |  17 +++
 include/uapi/linux/rfkill.h   |   6 +
 net/rfkill/core.c | 162 --
 5 files changed, 173 insertions(+), 59 deletions(-)
 delete mode 100644 Documentation/ABI/obsolete/sysfs-class-rfkill

-- 
2.5.0



[PATCHv2 05/10] rfkill: Factor rfkill_global_states[].cur assignments

2016-02-22 Thread João Paulo Rechi Vita
Factor all assignments to rfkill_global_states[].cur into a single
function rfkill_update_global_state().

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 56d79cb..8b96869 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -302,6 +302,19 @@ static void rfkill_set_block(struct rfkill *rfkill, bool 
blocked)
rfkill_event(rfkill);
 }
 
+static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
+{
+   int i;
+
+   if (type != RFKILL_TYPE_ALL) {
+   rfkill_global_states[type].cur = blocked;
+   return;
+   }
+
+   for (i = 0; i < NUM_RFKILL_TYPES; i++)
+   rfkill_global_states[i].cur = blocked;
+}
+
 #ifdef CONFIG_RFKILL_INPUT
 static atomic_t rfkill_input_disabled = ATOMIC_INIT(0);
 
@@ -319,15 +332,7 @@ static void __rfkill_switch_all(const enum rfkill_type 
type, bool blocked)
 {
struct rfkill *rfkill;
 
-   if (type == RFKILL_TYPE_ALL) {
-   int i;
-
-   for (i = 0; i < NUM_RFKILL_TYPES; i++)
-   rfkill_global_states[i].cur = blocked;
-   } else {
-   rfkill_global_states[type].cur = blocked;
-   }
-
+   rfkill_update_global_state(type, blocked);
list_for_each_entry(rfkill, _list, node) {
if (rfkill->type != type && type != RFKILL_TYPE_ALL)
continue;
@@ -1164,15 +1169,8 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
 
mutex_lock(_global_mutex);
 
-   if (ev.op == RFKILL_OP_CHANGE_ALL) {
-   if (ev.type == RFKILL_TYPE_ALL) {
-   enum rfkill_type i;
-   for (i = 0; i < NUM_RFKILL_TYPES; i++)
-   rfkill_global_states[i].cur = ev.soft;
-   } else {
-   rfkill_global_states[ev.type].cur = ev.soft;
-   }
-   }
+   if (ev.op == RFKILL_OP_CHANGE_ALL)
+   rfkill_update_global_state(ev.type, ev.soft);
 
list_for_each_entry(rfkill, _list, node) {
if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
@@ -1261,10 +1259,8 @@ static struct miscdevice rfkill_miscdev = {
 static int __init rfkill_init(void)
 {
int error;
-   int i;
 
-   for (i = 0; i < NUM_RFKILL_TYPES; i++)
-   rfkill_global_states[i].cur = !rfkill_default_state;
+   rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
 
error = class_register(_class);
if (error)
-- 
2.5.0



[PATCHv2 06/10] rfkill: Add documentation about LED triggers

2016-02-22 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 2ee6ef9..1f0c270 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -83,6 +83,8 @@ rfkill drivers that control devices that can be hard-blocked 
unless they also
 assign the poll_hw_block() callback (then the rfkill core will poll the
 device). Don't do this unless you cannot get the event in any other way.
 
+RFKill provides per-switch LED triggers, which can be used to drive LEDs
+according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 
 
 5. Userspace support
-- 
2.5.0



[PATCHv2 04/10] rfkill: Move "state" sysfs file back to stable

2016-02-22 Thread João Paulo Rechi Vita
There is still quite a bit of code using this interface, so we can't
just remove it. Hopefully it will be possible in the future, but since
its scheduled removal date is past 2 years already, we are better having
the documentation reflecting the current state of things.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/ABI/obsolete/sysfs-class-rfkill | 20 
 Documentation/ABI/stable/sysfs-class-rfkill   | 25 ++---
 2 files changed, 22 insertions(+), 23 deletions(-)
 delete mode 100644 Documentation/ABI/obsolete/sysfs-class-rfkill

diff --git a/Documentation/ABI/obsolete/sysfs-class-rfkill 
b/Documentation/ABI/obsolete/sysfs-class-rfkill
deleted file mode 100644
index e736d14..000
--- a/Documentation/ABI/obsolete/sysfs-class-rfkill
+++ /dev/null
@@ -1,20 +0,0 @@
-rfkill - radio frequency (RF) connector kill switch support
-
-For details to this subsystem look at Documentation/rfkill.txt.
-
-What:  /sys/class/rfkill/rfkill[0-9]+/state
-Date:  09-Jul-2007
-KernelVersion  v2.6.22
-Contact:   linux-wirel...@vger.kernel.org
-Description:   Current state of the transmitter.
-   This file is deprecated and scheduled to be removed in 2014,
-   because its not possible to express the 'soft and hard block'
-   state of the rfkill driver.
-Values:A numeric value.
-   0: RFKILL_STATE_SOFT_BLOCKED
-   transmitter is turned off by software
-   1: RFKILL_STATE_UNBLOCKED
-   transmitter is (potentially) active
-   2: RFKILL_STATE_HARD_BLOCKED
-   transmitter is forced off by something outside of
-   the driver's control.
diff --git a/Documentation/ABI/stable/sysfs-class-rfkill 
b/Documentation/ABI/stable/sysfs-class-rfkill
index e51571e..e1ba4a1 100644
--- a/Documentation/ABI/stable/sysfs-class-rfkill
+++ b/Documentation/ABI/stable/sysfs-class-rfkill
@@ -5,9 +5,6 @@ For details to this subsystem look at Documentation/rfkill.txt.
 For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
 Documentation/ABI/removed/sysfs-class-rfkill.
 
-For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
-Documentation/ABI/obsolete/sysfs-class-rfkill.
-
 What:  /sys/class/rfkill
 Date:  09-Jul-2007
 KernelVersion: v2.6.22
@@ -44,6 +41,28 @@ Values:  A numeric value.
1: true
 
 
+What:  /sys/class/rfkill/rfkill[0-9]+/state
+Date:  09-Jul-2007
+KernelVersion  v2.6.22
+Contact:   linux-wirel...@vger.kernel.org
+Description:   Current state of the transmitter.
+   This file was scheduled to be removed in 2014, but due to its
+   large number of users it will be sticking around for a bit
+   longer. Despite it being marked as stabe, the newer "hard" and
+   "soft" interfaces should be preffered, since it is not possible
+   to express the 'soft and hard block' state of the rfkill driver
+   through this interface. There will likely be another attempt to
+   remove it in the future.
+Values:A numeric value.
+   0: RFKILL_STATE_SOFT_BLOCKED
+   transmitter is turned off by software
+   1: RFKILL_STATE_UNBLOCKED
+   transmitter is (potentially) active
+   2: RFKILL_STATE_HARD_BLOCKED
+   transmitter is forced off by something outside of
+   the driver's control.
+
+
 What:  /sys/class/rfkill/rfkill[0-9]+/hard
 Date:  12-March-2010
 KernelVersion  v2.6.34
-- 
2.5.0



[PATCHv2 08/10] rfkill: Use switch to demux userspace operations

2016-02-22 Thread João Paulo Rechi Vita
Using a switch to handle different ev.op values in rfkill_fop_write()
makes the code easier to extend, as out-of-range values can always be
handled by the default case.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 50b538b..04d7fa1 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1185,6 +1185,7 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
 {
struct rfkill *rfkill;
struct rfkill_event ev;
+   int ret = 0;
 
/* we don't need the 'hard' variable but accept it */
if (count < RFKILL_EVENT_SIZE_V1 - 1)
@@ -1199,29 +1200,32 @@ static ssize_t rfkill_fop_write(struct file *file, 
const char __user *buf,
if (copy_from_user(, buf, count))
return -EFAULT;
 
-   if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL)
-   return -EINVAL;
-
if (ev.type >= NUM_RFKILL_TYPES)
return -EINVAL;
 
mutex_lock(_global_mutex);
 
-   if (ev.op == RFKILL_OP_CHANGE_ALL)
+   switch (ev.op) {
+   case RFKILL_OP_CHANGE_ALL:
rfkill_update_global_state(ev.type, ev.soft);
-
-   list_for_each_entry(rfkill, _list, node) {
-   if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
-   continue;
-
-   if (rfkill->type != ev.type && ev.type != RFKILL_TYPE_ALL)
-   continue;
-
-   rfkill_set_block(rfkill, ev.soft);
+   list_for_each_entry(rfkill, _list, node)
+   if (rfkill->type == ev.type ||
+   ev.type == RFKILL_TYPE_ALL)
+   rfkill_set_block(rfkill, ev.soft);
+   break;
+   case RFKILL_OP_CHANGE:
+   list_for_each_entry(rfkill, _list, node)
+   if (rfkill->idx == ev.idx && rfkill->type == ev.type)
+   rfkill_set_block(rfkill, ev.soft);
+   break;
+   default:
+   ret = -EINVAL;
+   break;
}
+
mutex_unlock(_global_mutex);
 
-   return count;
+   return ret ? ret : count;
 }
 
 static int rfkill_fop_release(struct inode *inode, struct file *file)
-- 
2.5.0



[PATCHv2 07/10] rfkill: Create "rfkill-airplane-mode" LED trigger

2016-02-22 Thread João Paulo Rechi Vita
This creates a new LED trigger to be used by platform drivers as a
default trigger for airplane-mode indicator LEDs.

By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
for all types (RFKILL_TYPE_ALL), setting the LED brightness to LED_FULL
when the changing the state to blocked, and to LED_OFF when the changing
the state to unblocked. In the future there will be a mechanism for
userspace to override the default policy, so it can implement its own.

This trigger will be used by the asus-wireless x86 platform driver.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt |  2 ++
 net/rfkill/core.c| 49 +++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 1f0c270..b13025a 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -85,6 +85,8 @@ device). Don't do this unless you cannot get the event in any 
other way.
 
 RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
+An airplane-mode indicator LED trigger is also available, which triggers
+LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
 
 
 5. Userspace support
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 8b96869..50b538b 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -126,6 +126,30 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static struct led_trigger rfkill_apm_led_trigger;
+
+static void rfkill_apm_led_trigger_event(bool state)
+{
+   led_trigger_event(_apm_led_trigger, state ? LED_FULL : LED_OFF);
+}
+
+static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
+{
+   rfkill_apm_led_trigger_event(!rfkill_default_state);
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+   rfkill_apm_led_trigger.name = "rfkill-airplane-mode";
+   rfkill_apm_led_trigger.activate = rfkill_apm_led_trigger_activate;
+   return led_trigger_register(_apm_led_trigger);
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+   led_trigger_unregister(_apm_led_trigger);
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
struct led_trigger *trigger;
@@ -177,6 +201,19 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
led_trigger_unregister(>led_trigger);
 }
 #else
+static void rfkill_apm_led_trigger_event(bool state)
+{
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+   return 0;
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -313,6 +350,7 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1260,15 +1298,22 @@ static int __init rfkill_init(void)
 {
int error;
 
+   error = rfkill_apm_led_trigger_register();
+   if (error)
+   goto out;
+
rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
 
error = class_register(_class);
-   if (error)
+   if (error) {
+   rfkill_apm_led_trigger_unregister();
goto out;
+   }
 
error = misc_register(_miscdev);
if (error) {
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
goto out;
}
 
@@ -1277,6 +1322,7 @@ static int __init rfkill_init(void)
if (error) {
misc_deregister(_miscdev);
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
goto out;
}
 #endif
@@ -1293,5 +1339,6 @@ static void __exit rfkill_exit(void)
 #endif
misc_deregister(_miscdev);
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
 }
 module_exit(rfkill_exit);
-- 
2.5.0



[PATCHv2 01/10] rfkill: Improve documentation language

2016-02-22 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index a805831..ffbc375 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -282,8 +282,8 @@ static void rfkill_set_block(struct rfkill *rfkill, bool 
blocked)
spin_lock_irqsave(>lock, flags);
if (err) {
/*
-* Failed -- reset status to _prev, this may be different
-* from what set set _PREV to earlier in this function
+* Failed -- reset status to _PREV, which may be different
+* from what we have set _PREV to earlier in this function
 * if rfkill_set_sw_state was invoked.
 */
if (rfkill->state & RFKILL_BLOCK_SW_PREV)
-- 
2.5.0



[PATCHv2 03/10] rfkill: Point to the correct deprecated doc location

2016-02-22 Thread João Paulo Rechi Vita
The "claim" sysfs interface has been removed, so its documentation now
lives in the "removed" folder.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/ABI/stable/sysfs-class-rfkill | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-class-rfkill 
b/Documentation/ABI/stable/sysfs-class-rfkill
index 097f522..e51571e 100644
--- a/Documentation/ABI/stable/sysfs-class-rfkill
+++ b/Documentation/ABI/stable/sysfs-class-rfkill
@@ -2,8 +2,10 @@ rfkill - radio frequency (RF) connector kill switch support
 
 For details to this subsystem look at Documentation/rfkill.txt.
 
-For the deprecated /sys/class/rfkill/*/state and
-/sys/class/rfkill/*/claim knobs of this interface look in
+For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
+Documentation/ABI/removed/sysfs-class-rfkill.
+
+For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
 Documentation/ABI/obsolete/sysfs-class-rfkill.
 
 What:  /sys/class/rfkill
-- 
2.5.0



[PATCHv2 10/10] rfkill: Notify userspace of airplane-mode state changes

2016-02-22 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt|  3 +++
 include/uapi/linux/rfkill.h |  4 ++--
 net/rfkill/core.c   | 13 +
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 9dbe3fc..588b4bf 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -133,5 +133,8 @@ applications to take control. Changes to the airplane-mode 
indicator state can
 be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE, passing the new value
 in the 'soft' field of 'struct rfkill_event'.
 
+This same API is also used to provide userspace with notifications of changes
+to airplane-mode indicator state.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 36e0770..2ccb02f 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -63,8 +63,8 @@ enum rfkill_type {
  * are hot-plugged later.
  * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
  * the airplane-mode indicator.
- * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
- * airplane-mode indicator state.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: the airplane-mode indicator state
+ * changed -- userspace changes the airplane-mode indicator state.
  */
 enum rfkill_operation {
RFKILL_OP_ADD = 0,
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 8ea8b73..c59fd1d 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -131,7 +131,20 @@ static struct led_trigger rfkill_apm_led_trigger;
 
 static void rfkill_apm_led_trigger_event(bool state)
 {
+   struct rfkill_data *data;
+   struct rfkill_int_event *ev;
+
led_trigger_event(_apm_led_trigger, state ? LED_FULL : LED_OFF);
+
+   list_for_each_entry(data, _fds, list) {
+   ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+   if (!ev)
+   continue;
+   ev->ev.op = RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE;
+   ev->ev.soft = state;
+   list_add_tail(>list, >events);
+   wake_up_interruptible(>read_wait);
+   }
 }
 
 static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
-- 
2.5.0



[PATCHv2 02/10] rfkill: Remove extra blank line

2016-02-22 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index ffbc375..56d79cb 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -455,7 +455,6 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)
 }
 #endif
 
-
 bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
 {
unsigned long flags;
-- 
2.5.0



Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-22 Thread João Paulo Rechi Vita
On 18 February 2016 at 15:12, Johannes Berg <johan...@sipsolutions.net> wrote:
> Hi,
>
> Sorry for the delay reviewing this.
>

No problems!

>
> On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
>> Provide an interface for the airplane-mode indicator be controlled
>> from
>> userspace. User has to first acquire the control through
>> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
>> time
>> it wants to be in control of the indicator. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>
> I've come to the conclusion that the new ops are probably the best
> thing to do here.
>

Nice.

>> +Userspace can also override the default airplane-mode indicator
>> policy through
>> +/dev/rfkill. Control of the airplane mode indicator has to be
>> acquired first,
>> +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one
>> userspace
>> +application at a time. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE
>> +reverts the airplane-mode indicator back to the default kernel
>> policy and makes
>> +it available for other applications to take control. Changes to the
>> +airplane-mode indicator state can be made using
>> RFKILL_OP_AIRPLANE_MODE_CHANGE,
>> +passing the new value in the 'soft' field of 'struct rfkill_event'.
>
> I don't really see any value in _RELEASE, since an application can just
> close the fd? I'd prefer not having the duplicate functionality
> and force us to exercise the single code path every time.
>

I actually added this op only for completion, I couldn't think of a
use-case where simply closing the fd wouldn't be enough. I'll remove
it for the next revision.

>>  For further details consult Documentation/ABI/stable/sysfs-class-
>> rfkill.
>> diff --git a/include/uapi/linux/rfkill.h
>> b/include/uapi/linux/rfkill.h
>> index 2e00dce..9cb999b 100644
>> --- a/include/uapi/linux/rfkill.h
>> +++ b/include/uapi/linux/rfkill.h
>> @@ -67,6 +67,9 @@ enum rfkill_operation {
>>   RFKILL_OP_DEL,
>>   RFKILL_OP_CHANGE,
>>   RFKILL_OP_CHANGE_ALL,
>> + RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
>> + RFKILL_OP_AIRPLANE_MODE_RELEASE,
>> + RFKILL_OP_AIRPLANE_MODE_CHANGE,
>>  };
>
>> @@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file
>> *file, const char __user *buf,
>>   if (copy_from_user(, buf, count))
>>   return -EFAULT;
>>
>> - if (ev.op != RFKILL_OP_CHANGE && ev.op !=
>> RFKILL_OP_CHANGE_ALL)
>> + if (ev.op < RFKILL_OP_CHANGE)
>>   return -EINVAL;
>
> You need to also reject invalid high values, like 27.
>

Yes, sorry for missing this.

>>   mutex_lock(_global_mutex);
>>
>> + if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
>> + if (rfkill_apm_owned && !data->is_apm_owner) {
>> + count = -EACCES;
>> + } else {
>> + rfkill_apm_owned = true;
>> + data->is_apm_owner = true;
>> +     }
>> + }
>> +
>> + if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
>
> It would probably be better to simply use "switch (ev.op)" and make the
> default case do a reject.
>

Sounds better indeed.

>>   if (ev.op == RFKILL_OP_CHANGE_ALL)
>>   rfkill_update_global_state(ev.type, ev.soft);
>
> Also moving the existing code inside the switch, of course.
>

Sure.

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger

2016-02-22 Thread João Paulo Rechi Vita
On 18 February 2016 at 15:08, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
>> This creates a new LED trigger to be used by platform drivers as a
>> default trigger for airplane-mode indicator LEDs.
>>
>> By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
>> for all types (RFKILL_TYPE_ALL), setting the LED brightness to
>> LED_FULL
>> when the changing the state to blocked, and to LED_OFF when the
>> changing
>> the state to unblocked. In the future there will be a mechanism for
>> userspace to override the default policy, so it can implement its
>> own.
>>
>> This trigger will be used by the asus-wireless x86 platform driver.
>
> Just one comment - I think you should be consistent with the _ vs - and
> just use "rfkill-airplane-mode" as the name.
>

Ok, no problem. I'll change for the next version.

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-16 Thread João Paulo Rechi Vita
On 10 February 2016 at 12:12, Johannes Berg <johan...@sipsolutions.net> wrote:
> On 2016-02-10 17:53, Dan Williams wrote:
>>
>> Yeah, I get that now.  It's just that to me, something called
>> "AIRPLANE_MODE_CHANGE" seems like it should actually change airplane
>> mode on/off, which implies killing radios.  I wouldn't have had the
>> problem if it was named AIRPLANE_MODE_INDICATOR_CHANGE, which makes it
>> clear this is simply an indicator and has no actual effect on anything
>> other than a LED.
>

I also agree the AIRPLANE_MODE_INDICATOR_* prefix makes things more
clear here. Thanks for the feedback, Dan!

>
> Yeah, I agree. I'm not even sure that it's a good idea to subsume this into
> the regular operations in the API, although that's obviously the easiest
> extension. I'll need to think about that a bit more with the code at hand
> though.
>

Initially I have thought about creating and additional RFKill switch
for that, but I think it would be a bit hackish since we would have to
treat it specially in sysfs attributes and probably other places, and
userspace would also need a special case when going through the list
of RFKill switches in the system. The proposed solution has equivalent
semantics to an RFKill switch, is backward-compatible (users would
only ignore the unknown operations and evens -- although gsd has a
"default:" case to abort execution on an unknown event) and does not
extend the RFKill event struct.

One alternative would be to move the control point to a separate
device, like /dev/rfkill-airplane-mode, but I'm not sure this is a
very elegant approach. Anyway, I'm open to work on changes to the API,
but it would be great if you could at least pick or reject the
non-polemical patches of the series, so I don't need to carry them
anymore.

--
João Paulo Rechi Vita
http://about.me/jprvita


[PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-09 Thread João Paulo Rechi Vita
Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
it wants to be in control of the indicator. Closing the fd or using
RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
operation is used, passing the value on "struct rfkill_event.soft". If
the caller has not acquired the airplane-mode control beforehand, the
operation fails.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt| 10 ++
 include/uapi/linux/rfkill.h |  3 +++
 net/rfkill/core.c   | 45 +
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..aa6e014 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to 
drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one userspace
+application at a time. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE
+reverts the airplane-mode indicator back to the default kernel policy and makes
+it available for other applications to take control. Changes to the
+airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
+passing the new value in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..9cb999b 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -67,6 +67,9 @@ enum rfkill_operation {
RFKILL_OP_DEL,
RFKILL_OP_CHANGE,
RFKILL_OP_CHANGE_ALL,
+   RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
+   RFKILL_OP_AIRPLANE_MODE_RELEASE,
+   RFKILL_OP_AIRPLANE_MODE_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index fb11547..c0da716 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
struct mutexmtx;
wait_queue_head_t   read_wait;
boolinput_handler;
+   boolis_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
-   rfkill_apm_led_trigger_event(blocked);
+   if (!rfkill_apm_owned)
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1180,11 +1182,26 @@ static ssize_t rfkill_fop_read(struct file *file, char 
__user *buf,
return ret;
 }
 
+static int rfkill_airplane_mode_release(struct rfkill_data *data)
+{
+   bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+   if (rfkill_apm_owned && data->is_apm_owner) {
+   rfkill_apm_owned = false;
+   data->is_apm_owner = false;
+   rfkill_apm_led_trigger_event(state);
+   return 0;
+   }
+   return -EACCES;
+}
+
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
 {
+   struct rfkill_data *data = file->private_data;
struct rfkill *rfkill;
struct rfkill_event ev;
+   int ret = 0;
 
/* we don't need the 'hard' variable but accept it */
if (count < RFKILL_EVENT_SIZE_V1 - 1)
@@ -1199,7 +1216,7 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
if (copy_from_user(, buf, count))
return -EFAULT;
 
-   if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL)
+   if (ev.op < RFKILL_OP_CHANGE)
return -EINVAL;
 
if (ev.type >= NUM_RFKILL_TYPES)
@@ -1207,6 +12

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-09 Thread João Paulo Rechi Vita
On 8 February 2016 at 17:53, Julian Calaby <julian.cal...@gmail.com> wrote:

>> +   if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
>> +   if (rfkill_apm_owned && !data->is_apm_owner) {
>
> Are you sure this is correct?
>
> In the case that airplane mode isn't owned, the
> rfkill_apm_led_trigger_event() call will be a noop, so we should
> arguably not be calling it.
>

Ok, I'm changing the check to be consistent with _CHANGE, so the call
only succeeds if (rfkill_apm_owned && data->is_apm_owner), and return
an error otherwise.

> Also, should we just fail silently if we're not the owner? I.e. what
> does userspace learn from this op failing and is that useful?
>

I think it is better to return an error every time userspace is trying
to call an operation that it was not supposed to call at a certain
state (without acquiring control of the airplane-mode indicator). If a
program has a logic error that makes it call _RELEASE without calling
_ACQUIRE first, it's easier for the programmer to spot the problem if
we return an error here.

>> +   count = -EACCES;
>> +   } else {
>> +   bool state = 
>> rfkill_global_states[RFKILL_TYPE_ALL].cur;
>> +
>> +   rfkill_apm_owned = false;
>> +   data->is_apm_owner = false;
>> +   rfkill_apm_led_trigger_event(state);
>> +   }
>> +   }
>> +
>> +   if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
>> +   if (rfkill_apm_owned && data->is_apm_owner)
>> +   rfkill_apm_led_trigger_event(ev.soft);
>> +   else
>> +   count = -EACCES;
>> +   }
>> +
>> if (ev.op == RFKILL_OP_CHANGE_ALL)
>> rfkill_update_global_state(ev.type, ev.soft);
>>
>> @@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, 
>> struct file *file)
>> struct rfkill_int_event *ev, *tmp;
>>
>> mutex_lock(_global_mutex);
>> +
>> +   if (data->is_apm_owner) {
>> +   bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
>> +
>> +   rfkill_apm_owned = false;
>> +   data->is_apm_owner = false;
>> +   rfkill_apm_led_trigger_event(state);
>
> Also, this code is duplicated from the _RELEASE op above. Would it
> make sense to factor it out into a separate function?
>

Yes, makes sense. This also made me notice I was assigning a negative
value to a size_t variable (count).

>> +   }
>> +
>> list_del(>list);
>> +
>
> (extra line)
>

After factoring out the _RELEASE code it looks better without this
additional line.

>> mutex_unlock(_global_mutex);
>>
>> mutex_destroy(>mtx);
>
> Thanks,
>

Thanks for the review, Julian. I'm sending an updated version.

--
João Paulo Rechi Vita
http://about.me/jprvita


[PATCH 0/9] RFKill airplane-mode indicator

2016-02-08 Thread João Paulo Rechi Vita
This series implements an airplane-mode indicator LED trigger, which can be
used by platform drivers. The default policy have have airplane-mode set when
all the radios known by RFKill are OFF, and unset otherwise. This policy can be
overwritten by userspace using the new operations _AIRPLANE_MODE_ACQUIRE,
_AIRPLANE_MODE_RELEASE, and _AIRPLANE_MODE_CHANGE. When the airplane-mode
indicator state changes, userspace gets notifications through the RFKill
control misc device (/dev/rfkill).

The series also contains a few general fixes and improvements to the subsystem.

João Paulo Rechi Vita (9):
  rfkill: Improve documentation language
  rfkill: Remove extra blank line
  rfkill: Point to the correct deprecated doc location
  rfkill: Move "state" sysfs file back to stable
  rfkill: Factor rfkill_global_states[].cur assignments
  rfkill: Add documentation about LED triggers
  rfkill: Create "rfkill-airplane_mode" LED trigger
  rfkill: Userspace control for airplane mode
  rfkill: Notify userspace of airplane-mode state changes

 Documentation/ABI/obsolete/sysfs-class-rfkill |  20 
 Documentation/ABI/stable/sysfs-class-rfkill   |  27 -
 Documentation/rfkill.txt  |  17 +++
 include/uapi/linux/rfkill.h   |   3 +
 net/rfkill/core.c | 144 +-
 5 files changed, 164 insertions(+), 47 deletions(-)
 delete mode 100644 Documentation/ABI/obsolete/sysfs-class-rfkill

-- 
2.5.0



[PATCH 5/9] rfkill: Factor rfkill_global_states[].cur assignments

2016-02-08 Thread João Paulo Rechi Vita
Factor all assignments to rfkill_global_states[].cur into a single
function rfkill_update_global_state().

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 56d79cb..8b96869 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -302,6 +302,19 @@ static void rfkill_set_block(struct rfkill *rfkill, bool 
blocked)
rfkill_event(rfkill);
 }
 
+static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
+{
+   int i;
+
+   if (type != RFKILL_TYPE_ALL) {
+   rfkill_global_states[type].cur = blocked;
+   return;
+   }
+
+   for (i = 0; i < NUM_RFKILL_TYPES; i++)
+   rfkill_global_states[i].cur = blocked;
+}
+
 #ifdef CONFIG_RFKILL_INPUT
 static atomic_t rfkill_input_disabled = ATOMIC_INIT(0);
 
@@ -319,15 +332,7 @@ static void __rfkill_switch_all(const enum rfkill_type 
type, bool blocked)
 {
struct rfkill *rfkill;
 
-   if (type == RFKILL_TYPE_ALL) {
-   int i;
-
-   for (i = 0; i < NUM_RFKILL_TYPES; i++)
-   rfkill_global_states[i].cur = blocked;
-   } else {
-   rfkill_global_states[type].cur = blocked;
-   }
-
+   rfkill_update_global_state(type, blocked);
list_for_each_entry(rfkill, _list, node) {
if (rfkill->type != type && type != RFKILL_TYPE_ALL)
continue;
@@ -1164,15 +1169,8 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
 
mutex_lock(_global_mutex);
 
-   if (ev.op == RFKILL_OP_CHANGE_ALL) {
-   if (ev.type == RFKILL_TYPE_ALL) {
-   enum rfkill_type i;
-   for (i = 0; i < NUM_RFKILL_TYPES; i++)
-   rfkill_global_states[i].cur = ev.soft;
-   } else {
-   rfkill_global_states[ev.type].cur = ev.soft;
-   }
-   }
+   if (ev.op == RFKILL_OP_CHANGE_ALL)
+   rfkill_update_global_state(ev.type, ev.soft);
 
list_for_each_entry(rfkill, _list, node) {
if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
@@ -1261,10 +1259,8 @@ static struct miscdevice rfkill_miscdev = {
 static int __init rfkill_init(void)
 {
int error;
-   int i;
 
-   for (i = 0; i < NUM_RFKILL_TYPES; i++)
-   rfkill_global_states[i].cur = !rfkill_default_state;
+   rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
 
error = class_register(_class);
if (error)
-- 
2.5.0



[PATCH 4/9] rfkill: Move "state" sysfs file back to stable

2016-02-08 Thread João Paulo Rechi Vita
There is still quite a bit of code using this interface, so we can't
just remove it. Hopefully it will be possible in the future, but since
its scheduled removal date is past 2 years already, we are better having
the documentation reflecting the current state of things.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/ABI/obsolete/sysfs-class-rfkill | 20 
 Documentation/ABI/stable/sysfs-class-rfkill   | 25 ++---
 2 files changed, 22 insertions(+), 23 deletions(-)
 delete mode 100644 Documentation/ABI/obsolete/sysfs-class-rfkill

diff --git a/Documentation/ABI/obsolete/sysfs-class-rfkill 
b/Documentation/ABI/obsolete/sysfs-class-rfkill
deleted file mode 100644
index e736d14..000
--- a/Documentation/ABI/obsolete/sysfs-class-rfkill
+++ /dev/null
@@ -1,20 +0,0 @@
-rfkill - radio frequency (RF) connector kill switch support
-
-For details to this subsystem look at Documentation/rfkill.txt.
-
-What:  /sys/class/rfkill/rfkill[0-9]+/state
-Date:  09-Jul-2007
-KernelVersion  v2.6.22
-Contact:   linux-wirel...@vger.kernel.org
-Description:   Current state of the transmitter.
-   This file is deprecated and scheduled to be removed in 2014,
-   because its not possible to express the 'soft and hard block'
-   state of the rfkill driver.
-Values:A numeric value.
-   0: RFKILL_STATE_SOFT_BLOCKED
-   transmitter is turned off by software
-   1: RFKILL_STATE_UNBLOCKED
-   transmitter is (potentially) active
-   2: RFKILL_STATE_HARD_BLOCKED
-   transmitter is forced off by something outside of
-   the driver's control.
diff --git a/Documentation/ABI/stable/sysfs-class-rfkill 
b/Documentation/ABI/stable/sysfs-class-rfkill
index e51571e..e1ba4a1 100644
--- a/Documentation/ABI/stable/sysfs-class-rfkill
+++ b/Documentation/ABI/stable/sysfs-class-rfkill
@@ -5,9 +5,6 @@ For details to this subsystem look at Documentation/rfkill.txt.
 For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
 Documentation/ABI/removed/sysfs-class-rfkill.
 
-For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
-Documentation/ABI/obsolete/sysfs-class-rfkill.
-
 What:  /sys/class/rfkill
 Date:  09-Jul-2007
 KernelVersion: v2.6.22
@@ -44,6 +41,28 @@ Values:  A numeric value.
1: true
 
 
+What:  /sys/class/rfkill/rfkill[0-9]+/state
+Date:  09-Jul-2007
+KernelVersion  v2.6.22
+Contact:   linux-wirel...@vger.kernel.org
+Description:   Current state of the transmitter.
+   This file was scheduled to be removed in 2014, but due to its
+   large number of users it will be sticking around for a bit
+   longer. Despite it being marked as stabe, the newer "hard" and
+   "soft" interfaces should be preffered, since it is not possible
+   to express the 'soft and hard block' state of the rfkill driver
+   through this interface. There will likely be another attempt to
+   remove it in the future.
+Values:A numeric value.
+   0: RFKILL_STATE_SOFT_BLOCKED
+   transmitter is turned off by software
+   1: RFKILL_STATE_UNBLOCKED
+   transmitter is (potentially) active
+   2: RFKILL_STATE_HARD_BLOCKED
+   transmitter is forced off by something outside of
+   the driver's control.
+
+
 What:  /sys/class/rfkill/rfkill[0-9]+/hard
 Date:  12-March-2010
 KernelVersion  v2.6.34
-- 
2.5.0



[PATCH 9/9] rfkill: Notify userspace of airplane-mode state changes

2016-02-08 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt |  3 +++
 net/rfkill/core.c| 13 +
 2 files changed, 16 insertions(+)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index aa6e014..5248812 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -133,5 +133,8 @@ it available for other applications to take control. 
Changes to the
 airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
 passing the new value in the 'soft' field of 'struct rfkill_event'.
 
+This same API is also used to provide userspace with notifications of changes
+to airplane-mode indicator state.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 8067701..abbb8f7 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -131,7 +131,20 @@ static struct led_trigger rfkill_apm_led_trigger;
 
 static void rfkill_apm_led_trigger_event(bool state)
 {
+   struct rfkill_data *data;
+   struct rfkill_int_event *ev;
+
led_trigger_event(_apm_led_trigger, state ? LED_FULL : LED_OFF);
+
+   list_for_each_entry(data, _fds, list) {
+   ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+   if (!ev)
+   continue;
+   ev->ev.op = RFKILL_OP_AIRPLANE_MODE_CHANGE;
+   ev->ev.soft = state;
+   list_add_tail(>list, >events);
+   wake_up_interruptible(>read_wait);
+   }
 }
 
 static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
-- 
2.5.0



[PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger

2016-02-08 Thread João Paulo Rechi Vita
This creates a new LED trigger to be used by platform drivers as a
default trigger for airplane-mode indicator LEDs.

By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
for all types (RFKILL_TYPE_ALL), setting the LED brightness to LED_FULL
when the changing the state to blocked, and to LED_OFF when the changing
the state to unblocked. In the future there will be a mechanism for
userspace to override the default policy, so it can implement its own.

This trigger will be used by the asus-wireless x86 platform driver.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt |  2 ++
 net/rfkill/core.c| 49 +++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 1f0c270..b13025a 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -85,6 +85,8 @@ device). Don't do this unless you cannot get the event in any 
other way.
 
 RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
+An airplane-mode indicator LED trigger is also available, which triggers
+LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
 
 
 5. Userspace support
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 8b96869..fb11547 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -126,6 +126,30 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static struct led_trigger rfkill_apm_led_trigger;
+
+static void rfkill_apm_led_trigger_event(bool state)
+{
+   led_trigger_event(_apm_led_trigger, state ? LED_FULL : LED_OFF);
+}
+
+static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
+{
+   rfkill_apm_led_trigger_event(!rfkill_default_state);
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+   rfkill_apm_led_trigger.name = "rfkill-airplane_mode";
+   rfkill_apm_led_trigger.activate = rfkill_apm_led_trigger_activate;
+   return led_trigger_register(_apm_led_trigger);
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+   led_trigger_unregister(_apm_led_trigger);
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
struct led_trigger *trigger;
@@ -177,6 +201,19 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
led_trigger_unregister(>led_trigger);
 }
 #else
+static void rfkill_apm_led_trigger_event(bool state)
+{
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+   return 0;
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -313,6 +350,7 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1260,15 +1298,22 @@ static int __init rfkill_init(void)
 {
int error;
 
+   error = rfkill_apm_led_trigger_register();
+   if (error)
+   goto out;
+
rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
 
error = class_register(_class);
-   if (error)
+   if (error) {
+   rfkill_apm_led_trigger_unregister();
goto out;
+   }
 
error = misc_register(_miscdev);
if (error) {
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
goto out;
}
 
@@ -1277,6 +1322,7 @@ static int __init rfkill_init(void)
if (error) {
misc_deregister(_miscdev);
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
goto out;
}
 #endif
@@ -1293,5 +1339,6 @@ static void __exit rfkill_exit(void)
 #endif
misc_deregister(_miscdev);
class_unregister(_class);
+   rfkill_apm_led_trigger_unregister();
 }
 module_exit(rfkill_exit);
-- 
2.5.0



[PATCH 3/9] rfkill: Point to the correct deprecated doc location

2016-02-08 Thread João Paulo Rechi Vita
The "claim" sysfs interface has been removed, so its documentation now
lives in the "removed" folder.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/ABI/stable/sysfs-class-rfkill | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-class-rfkill 
b/Documentation/ABI/stable/sysfs-class-rfkill
index 097f522..e51571e 100644
--- a/Documentation/ABI/stable/sysfs-class-rfkill
+++ b/Documentation/ABI/stable/sysfs-class-rfkill
@@ -2,8 +2,10 @@ rfkill - radio frequency (RF) connector kill switch support
 
 For details to this subsystem look at Documentation/rfkill.txt.
 
-For the deprecated /sys/class/rfkill/*/state and
-/sys/class/rfkill/*/claim knobs of this interface look in
+For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
+Documentation/ABI/removed/sysfs-class-rfkill.
+
+For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
 Documentation/ABI/obsolete/sysfs-class-rfkill.
 
 What:  /sys/class/rfkill
-- 
2.5.0



[PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-08 Thread João Paulo Rechi Vita
Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
it wants to be in control of the indicator. Closing the fd or using
RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
operation is used, passing the value on "struct rfkill_event.soft". If
the caller has not acquired the airplane-mode control beforehand, the
operation fails.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt| 10 ++
 include/uapi/linux/rfkill.h |  3 +++
 net/rfkill/core.c   | 47 ++---
 3 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..aa6e014 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to 
drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one userspace
+application at a time. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE
+reverts the airplane-mode indicator back to the default kernel policy and makes
+it available for other applications to take control. Changes to the
+airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
+passing the new value in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..9cb999b 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -67,6 +67,9 @@ enum rfkill_operation {
RFKILL_OP_DEL,
RFKILL_OP_CHANGE,
RFKILL_OP_CHANGE_ALL,
+   RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
+   RFKILL_OP_AIRPLANE_MODE_RELEASE,
+   RFKILL_OP_AIRPLANE_MODE_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index fb11547..8067701 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
struct mutexmtx;
wait_queue_head_t   read_wait;
boolinput_handler;
+   boolis_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
-   rfkill_apm_led_trigger_event(blocked);
+   if (!rfkill_apm_owned)
+   rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1183,6 +1185,7 @@ static ssize_t rfkill_fop_read(struct file *file, char 
__user *buf,
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
 {
+   struct rfkill_data *data = file->private_data;
struct rfkill *rfkill;
struct rfkill_event ev;
 
@@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
if (copy_from_user(, buf, count))
return -EFAULT;
 
-   if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL)
+   if (ev.op < RFKILL_OP_CHANGE)
return -EINVAL;
 
if (ev.type >= NUM_RFKILL_TYPES)
@@ -1207,6 +1210,34 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
 
mutex_lock(_global_mutex);
 
+   if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
+   if (rfkill_apm_owned && !data->is_apm_owner) {
+   count = -EACCES;
+   } else {
+   rfkill_apm_owned = true;
+   data->is_apm_owner = true;
+   }
+   }
+
+   if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
+   if (rfkill_apm_owned &&

[PATCH 1/9] rfkill: Improve documentation language

2016-02-08 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index a805831..ffbc375 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -282,8 +282,8 @@ static void rfkill_set_block(struct rfkill *rfkill, bool 
blocked)
spin_lock_irqsave(>lock, flags);
if (err) {
/*
-* Failed -- reset status to _prev, this may be different
-* from what set set _PREV to earlier in this function
+* Failed -- reset status to _PREV, which may be different
+* from what we have set _PREV to earlier in this function
 * if rfkill_set_sw_state was invoked.
 */
if (rfkill->state & RFKILL_BLOCK_SW_PREV)
-- 
2.5.0



[PATCH 2/9] rfkill: Remove extra blank line

2016-02-08 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index ffbc375..56d79cb 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -455,7 +455,6 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)
 }
 #endif
 
-
 bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
 {
unsigned long flags;
-- 
2.5.0



[PATCH 6/9] rfkill: Add documentation about LED triggers

2016-02-08 Thread João Paulo Rechi Vita
Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 Documentation/rfkill.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 2ee6ef9..1f0c270 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -83,6 +83,8 @@ rfkill drivers that control devices that can be hard-blocked 
unless they also
 assign the poll_hw_block() callback (then the rfkill core will poll the
 device). Don't do this unless you cannot get the event in any other way.
 
+RFKill provides per-switch LED triggers, which can be used to drive LEDs
+according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 
 
 5. Userspace support
-- 
2.5.0



Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-08 Thread João Paulo Rechi Vita
Hello Marcel,

On 8 February 2016 at 11:20, Marcel Holtmann <mar...@holtmann.org> wrote:
> Hi Joa Paulo,
>
>> Provide an interface for the airplane-mode indicator be controlled from
>> userspace. User has to first acquire the control through
>> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
>> it wants to be in control of the indicator. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>>
>> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
>> operation is used, passing the value on "struct rfkill_event.soft". If
>> the caller has not acquired the airplane-mode control beforehand, the
>> operation fails.
>
> as explained in an earlier response, the concept Airplane Mode seems to be 
> imposing policy into the kernel. Do we really want have this as a kernel 
> exposed API.
>

On that very same thread we decided to keep using the "airplane mode"
name both for when having the default policy of "set it when all
radios are off" or when allowing userspace to override the default.
Please see the following message from Johannes
http://www.spinics.net/lists/linux-wireless/msg146069.html and let me
know if that covers your concern.

Thanks!

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

2016-02-08 Thread João Paulo Rechi Vita
On 8 February 2016 at 11:11, Dan Williams <d...@redhat.com> wrote:
> On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
>> Provide an interface for the airplane-mode indicator be controlled
>> from
>> userspace. User has to first acquire the control through
>> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
>> time
>> it wants to be in control of the indicator. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>>
>> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
>> operation is used, passing the value on "struct rfkill_event.soft".
>> If
>> the caller has not acquired the airplane-mode control beforehand, the
>> operation fails.
>
> I'd like to clarify a bit, so tell me if I'm correct or not.  Using
> RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
> state. It's just an indicator with no relationship to any of the
> registered rfkill switches, right?
>

Yes, that's correct, and it is the intended behavior.

> I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't also
> softblock all switches, otherwise you can set airplane mode all day
> long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually enable
> airplane mode at all?
>

The idea behind it is to just provide the mechanism for userspace to
decide what exactly being in airplane mode means, so it would
set/unset the indicator in the right moment.

--
João Paulo Rechi Vita
http://about.me/jprvita


Re: [PATCH] net/rfkill: Create "airplane mode" LED trigger

2016-01-05 Thread João Paulo Rechi Vita
Hello Johannes,

On 26 December 2015 at 10:05, João Paulo Rechi Vita <jprv...@gmail.com> wrote:
> For platform drivers to be able to correctly drive the "Airplane Mode"
> indicative LED there needs to be a RFKill LED trigger tied to the global
> state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
> works in an inverted manner of regular RFKill LED triggers, that is, the
> LED is ON when the state is blocked, and OFF otherwise.
>
> This commit implements such a trigger, which will be used by the
> asus-wireless x86 platform driver.
>

The initial patches of the asus-wireless driver have been queue on the
platform-drivers-x86 tree. Do you have any comments on this one? It
would be great to have it in for 4.5, since the airplane mode LED
management in asus-wireless' pending patches depend on this one.

> Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
> ---
>  net/rfkill/core.c | 30 ++
>  1 file changed, 30 insertions(+)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index b41e9ea..3effc29 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -124,6 +124,26 @@ static bool rfkill_epo_lock_active;
>
>
>  #ifdef CONFIG_RFKILL_LEDS
> +static void airplane_mode_led_trigger_activate(struct led_classdev *led);
> +
> +static struct led_trigger airplane_mode_led_trigger = {
> +   .name = "rfkill-airplane-mode",
> +   .activate = airplane_mode_led_trigger_activate,
> +};
> +
> +static void airplane_mode_led_trigger_event(void)
> +{
> +   if (rfkill_global_states[RFKILL_TYPE_ALL].cur & RFKILL_BLOCK_ANY)
> +   led_trigger_event(_mode_led_trigger, LED_FULL);
> +   else
> +   led_trigger_event(_mode_led_trigger, LED_OFF);
> +}
> +
> +static void airplane_mode_led_trigger_activate(struct led_classdev *led)
> +{
> +   airplane_mode_led_trigger_event();
> +}
> +
>  static void rfkill_led_trigger_event(struct rfkill *rfkill)
>  {
> struct led_trigger *trigger;
> @@ -175,6 +195,10 @@ static void rfkill_led_trigger_unregister(struct rfkill 
> *rfkill)
> led_trigger_unregister(>led_trigger);
>  }
>  #else
> +static void airplane_mode_led_trigger_event(void)
> +{
> +}
> +
>  static void rfkill_led_trigger_event(struct rfkill *rfkill)
>  {
>  }
> @@ -346,6 +370,7 @@ static void __rfkill_switch_all(const enum rfkill_type 
> type, bool blocked)
>
> for (i = 0; i < NUM_RFKILL_TYPES; i++)
> rfkill_global_states[i].cur = blocked;
> +   airplane_mode_led_trigger_event();
> } else {
> rfkill_global_states[type].cur = blocked;
> }
> @@ -1177,6 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, 
> const char __user *buf,
> enum rfkill_type i;
> for (i = 0; i < NUM_RFKILL_TYPES; i++)
> rfkill_global_states[i].cur = ev.soft;
> +   airplane_mode_led_trigger_event();
> } else {
> rfkill_global_states[ev.type].cur = ev.soft;
>     }
> @@ -1293,6 +1319,10 @@ static int __init rfkill_init(void)
> }
>  #endif
>
> +#ifdef CONFIG_RFKILL_LEDS
> +   led_trigger_register(_mode_led_trigger);
> +#endif
> +
>   out:
> return error;
>  }
> --
> 2.5.0
>

Thanks and best regards,

--
João Paulo Rechi Vita
http://about.me/jprvita
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] RFKill airplane mode LED trigger

2015-12-26 Thread João Paulo Rechi Vita
This patch adds a LED trigger to drive the airplane mode LED present in some
modern laptops. It will be used by the asus-wireless platform driver, which is
currently under review on platform-driver-...@vger.kernel.org (the idea was
mainly approved, but I'm addressing some comments by the maintainers).

João Paulo Rechi Vita (1):
  net/rfkill: Create "airplane mode" LED trigger

 net/rfkill/core.c | 30 ++
 1 file changed, 30 insertions(+)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net/rfkill: Create "airplane mode" LED trigger

2015-12-26 Thread João Paulo Rechi Vita
For platform drivers to be able to correctly drive the "Airplane Mode"
indicative LED there needs to be a RFKill LED trigger tied to the global
state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
works in an inverted manner of regular RFKill LED triggers, that is, the
LED is ON when the state is blocked, and OFF otherwise.

This commit implements such a trigger, which will be used by the
asus-wireless x86 platform driver.

Signed-off-by: João Paulo Rechi Vita <jprv...@endlessm.com>
---
 net/rfkill/core.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index b41e9ea..3effc29 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -124,6 +124,26 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static void airplane_mode_led_trigger_activate(struct led_classdev *led);
+
+static struct led_trigger airplane_mode_led_trigger = {
+   .name = "rfkill-airplane-mode",
+   .activate = airplane_mode_led_trigger_activate,
+};
+
+static void airplane_mode_led_trigger_event(void)
+{
+   if (rfkill_global_states[RFKILL_TYPE_ALL].cur & RFKILL_BLOCK_ANY)
+   led_trigger_event(_mode_led_trigger, LED_FULL);
+   else
+   led_trigger_event(_mode_led_trigger, LED_OFF);
+}
+
+static void airplane_mode_led_trigger_activate(struct led_classdev *led)
+{
+   airplane_mode_led_trigger_event();
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
struct led_trigger *trigger;
@@ -175,6 +195,10 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
led_trigger_unregister(>led_trigger);
 }
 #else
+static void airplane_mode_led_trigger_event(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -346,6 +370,7 @@ static void __rfkill_switch_all(const enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
+   airplane_mode_led_trigger_event();
} else {
rfkill_global_states[type].cur = blocked;
}
@@ -1177,6 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
enum rfkill_type i;
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = ev.soft;
+   airplane_mode_led_trigger_event();
} else {
rfkill_global_states[ev.type].cur = ev.soft;
}
@@ -1293,6 +1319,10 @@ static int __init rfkill_init(void)
}
 #endif
 
+#ifdef CONFIG_RFKILL_LEDS
+   led_trigger_register(_mode_led_trigger);
+#endif
+
  out:
return error;
 }
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] net/rfkill: Create "airplane mode" LED trigger

2015-12-18 Thread João Paulo Rechi Vita
On 18 December 2015 at 19:22, Darren Hart <dvh...@infradead.org> wrote:
> On Tue, Dec 15, 2015 at 10:30:41AM -0500, João Paulo Rechi Vita wrote:
>> For platform drivers to be able to correctly drive the "Airplane Mode"
>> indicative LED there needs to be a RFKill LED trigger tied to the global
>> state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
>> works in an inverted manner of regular RFKill LED triggers, that is, the
>> LED is ON when the state is blocked, and OFF otherwise.
>>
>> This commit implements such a trigger, which will be used by the
>> asus-wrc x86 platform driver.
>
> So this will need to go through Johannes and David per get_maintainer.pl 
> before
> we can use it in platform drivers.
>

Yes, I am aware of that. I just wanted to get feedback and validate
the new platform driver that makes use of this new RFKill LED trigger
before proposing it, since I expect that will be their first
questioning.

For those who were not in the original message, this is patch is part
of a series implementing a new platform driver for the airplane mode
hotkey and LED present in some Asus laptops, which is a separate ACPI
device (not part of WMI) with ACPI _HID "ASHS" and named "Wireless
Radio Control" by Asus.

Thanks for your feedback!

--
João Paulo Rechi Vita
http://about.me/jprvita
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html