Only the HW rfkill state is toggled on laptops with quirks->ec_read_only
(so far only MSI Wind U90/U100). There are, however, a few issues with
the implementation:
1. The initial HW state is always unblocked, regardless of the actual
state on boot, because msi_init_rfkill only sets the SW state,
regardless of ec_read_only.
2. The initial SW state corresponds to the actual state on boot, but it
can't be changed afterwards, because set_device_state returns
-EOPNOTSUPP. It confuses the userspace, making Wi-Fi and/or Bluetooth
unusable if it was blocked on boot, and breaking the airplane mode if
the rfkill was unblocked on boot.
Address the above issues by properly initializing the HW state on
ec_read_only laptops and by allowing the userspace to toggle the SW
state. Don't set the SW state ourselves and let the userspace fully
control it. Toggling the SW state is a no-op, however, it allows the
userspace to properly toggle the airplane mode. The actual SW radio
disablement is handled by the corresponding rtl818x_pci and btusb
drivers that have their own rfkills.
Tested on MSI Wind U100 Plus, BIOS ver 1.0G, EC ver 130.
Fixes: 0816392b97d4 ("msi-laptop: merge quirk tables to one")
Fixes: 0de6575ad0a8 ("msi-laptop: Add MSI Wind U90/U100 support")
Signed-off-by: Maxim Mikityanskiy <[email protected]>
---
drivers/platform/x86/msi-laptop.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 6b18ec543ac3..f4c6c36e05a5 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -208,7 +208,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
return -EINVAL;
if (quirks->ec_read_only)
- return -EOPNOTSUPP;
+ return 0;
/* read current device state */
result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
@@ -838,15 +838,15 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
static void msi_init_rfkill(struct work_struct *ignored)
{
if (rfk_wlan) {
- rfkill_set_sw_state(rfk_wlan, !wlan_s);
+ msi_rfkill_set_state(rfk_wlan, !wlan_s);
rfkill_wlan_set(NULL, !wlan_s);
}
if (rfk_bluetooth) {
- rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
+ msi_rfkill_set_state(rfk_bluetooth, !bluetooth_s);
rfkill_bluetooth_set(NULL, !bluetooth_s);
}
if (rfk_threeg) {
- rfkill_set_sw_state(rfk_threeg, !threeg_s);
+ msi_rfkill_set_state(rfk_threeg, !threeg_s);
rfkill_threeg_set(NULL, !threeg_s);
}
}
--
2.41.0
Hi,
On 7/21/23 16:54, Maxim Mikityanskiy wrote:
> Only the HW rfkill state is toggled on laptops with quirks->ec_read_only
> (so far only MSI Wind U90/U100). There are, however, a few issues with
> the implementation:
>
> 1. The initial HW state is always unblocked, regardless of the actual
> state on boot, because msi_init_rfkill only sets the SW state,
> regardless of ec_read_only.
>
> 2. The initial SW state corresponds to the actual state on boot, but it
> can't be changed afterwards, because set_device_state returns
> -EOPNOTSUPP. It confuses the userspace, making Wi-Fi and/or Bluetooth
> unusable if it was blocked on boot, and breaking the airplane mode if
> the rfkill was unblocked on boot.
>
> Address the above issues by properly initializing the HW state on
> ec_read_only laptops and by allowing the userspace to toggle the SW
> state. Don't set the SW state ourselves and let the userspace fully
> control it. Toggling the SW state is a no-op, however, it allows the
> userspace to properly toggle the airplane mode. The actual SW radio
> disablement is handled by the corresponding rtl818x_pci and btusb
> drivers that have their own rfkills.
>
> Tested on MSI Wind U100 Plus, BIOS ver 1.0G, EC ver 130.
>
> Fixes: 0816392b97d4 ("msi-laptop: merge quirk tables to one")
> Fixes: 0de6575ad0a8 ("msi-laptop: Add MSI Wind U90/U100 support")
> Signed-off-by: Maxim Mikityanskiy <[email protected]>
Thank you for your patch, I've applied this patch to my fixes
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes
Note it will show up in my fixes branch once I've pushed my
local branch there, which might take a while.
I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.
Regards,
Hans
> ---
> drivers/platform/x86/msi-laptop.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 6b18ec543ac3..f4c6c36e05a5 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -208,7 +208,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
> return -EINVAL;
>
> if (quirks->ec_read_only)
> - return -EOPNOTSUPP;
> + return 0;
>
> /* read current device state */
> result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
> @@ -838,15 +838,15 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
> static void msi_init_rfkill(struct work_struct *ignored)
> {
> if (rfk_wlan) {
> - rfkill_set_sw_state(rfk_wlan, !wlan_s);
> + msi_rfkill_set_state(rfk_wlan, !wlan_s);
> rfkill_wlan_set(NULL, !wlan_s);
> }
> if (rfk_bluetooth) {
> - rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
> + msi_rfkill_set_state(rfk_bluetooth, !bluetooth_s);
> rfkill_bluetooth_set(NULL, !bluetooth_s);
> }
> if (rfk_threeg) {
> - rfkill_set_sw_state(rfk_threeg, !threeg_s);
> + msi_rfkill_set_state(rfk_threeg, !threeg_s);
> rfkill_threeg_set(NULL, !threeg_s);
> }
> }