Re: [PATCH v2 2/2] toshiba_acpi: Add WWAN RFKill support
Hi Darren,
2015-11-20 15:50 GMT-07:00 Darren Hart :
> On Thu, Nov 19, 2015 at 08:49:25AM -0700, Azael Avalos wrote:
>
> Hi Azael,
>
>> A previuos patch added WWAN support to the driver, allowing to query
>> and set the device status.
>>
>> This patch adds RFKill support for the recently introduced WWAN device,
>> making use of the WWAN and *wireless_status functions to query the
>> killswitch and (de)activate the device accordingly to its status.
>>
>> Signed-off-by: Fabian Koester
>
> So this is Fabian's code which he sent to you and you are submitting on his
> behalf?
Yes, he sent me the code for review, but made some changes to the
actual code he sent me (mostly patch 01), sent back to him the resulting
changes for testing, and I told him I was gonna submmit the changes :-)
>
>> Signed-off-by: Azael Avalos
>
>
> A couple minor nits below.
>
>> ---
>> drivers/platform/x86/toshiba_acpi.c | 79
>> +
>> 1 file changed, 79 insertions(+)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c
>> b/drivers/platform/x86/toshiba_acpi.c
>> index 60d1ad9..d1315c5 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -51,6 +51,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -174,6 +175,7 @@ struct toshiba_acpi_dev {
>> struct led_classdev kbd_led;
>> struct led_classdev eco_led;
>> struct miscdevice miscdev;
>> + struct rfkill *wwan_rfk;
>>
>> int force_fan;
>> int last_key_event;
>> @@ -2330,6 +2332,67 @@ static const struct file_operations toshiba_acpi_fops
>> = {
>> };
>>
>> /*
>> + * WWAN RFKill handlers
>> + */
>> +static int toshiba_acpi_wwan_set_block(void *data, bool blocked)
>> +{
>> + struct toshiba_acpi_dev *dev = data;
>> + int ret;
>> +
>> + ret = toshiba_wireless_status(dev);
>> + if (ret)
>> + return ret;
>> +
>> + if (!dev->killswitch)
>> + return 0;
>> +
>> + return toshiba_wwan_set(dev, blocked ? 0 : 1);
>
> You can avoid the ternary operation with binary output and just invert the
> bool.
>
> !blocked
OK, will do.
>
>
>> +}
>> +
>> +static void toshiba_acpi_wwan_poll(struct rfkill *rfkill, void *data)
>> +{
>> + struct toshiba_acpi_dev *dev = data;
>> +
>> + if (toshiba_wireless_status(dev))
>> + return;
>> +
>> + rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
>> +}
>> +
>> +static const struct rfkill_ops wwan_rfk_ops = {
>> + .set_block = toshiba_acpi_wwan_set_block,
>> + .poll = toshiba_acpi_wwan_poll,
>> +};
>> +
>> +static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev)
>> +{
>> + int ret = toshiba_wireless_status(dev);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + dev->wwan_rfk = rfkill_alloc("Toshiba WWAN",
>> + &dev->acpi_dev->dev,
>> + RFKILL_TYPE_WWAN,
>> + &wwan_rfk_ops,
>> + dev);
>> + if (!dev->wwan_rfk) {
>> + pr_err("Unable to allocate WWAN rfkill device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
>> +
>> + ret = rfkill_register(dev->wwan_rfk);
>> + if (ret) {
>> + pr_err("Unable to register WWAN rfkill device\n");
>> + rfkill_destroy(dev->wwan_rfk);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> * Hotkeys
>> */
>> static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
>> @@ -2688,6 +2751,11 @@ static int toshiba_acpi_remove(struct acpi_device
>> *acpi_dev)
>> if (dev->eco_led_registered)
>> led_classdev_unregister(&dev->eco_led);
>>
>> + if (dev->wwan_rfk) {
>> + rfkill_unregister(dev->wwan_rfk);
>> + rfkill_destroy(dev->wwan_rfk);
>> + }
>> +
>> if (toshiba_acpi)
>> toshiba_acpi = NULL;
>>
>> @@ -2827,6 +2895,8 @@ static int toshiba_acpi_add(struct acpi_device
>> *acpi_dev)
>> dev->fan_supported = !ret;
>>
>> toshiba_wwan_available(dev);
>> + if (dev->wwan_supported)
>> + toshiba_acpi_setup_wwan_rfkill(dev);
>>
>> print_supported_features(dev);
>>
>> @@ -2930,6 +3000,15 @@ static int toshiba_acpi_resume(struct device *device)
>> pr_info("Unable to re-enable hotkeys\n");
>> }
>>
>> + if (dev->wwan_rfk) {
>> + int error = toshiba_wireless_status(dev);
>> +
>> + if (error)
>> + return error;
>
> For consistency, please use ret. You can just initialize it to 0 outside the
> if
> block, use it inside, and return it outside as well. We don't buy much by
> declaring inside the if block on a resume function.
Here "error" was used on the previous if, that's why I chose the same name,
I will rename the variable to "ret" for co
Re: [PATCH v2 2/2] toshiba_acpi: Add WWAN RFKill support
On Thu, Nov 19, 2015 at 08:49:25AM -0700, Azael Avalos wrote:
Hi Azael,
> A previuos patch added WWAN support to the driver, allowing to query
> and set the device status.
>
> This patch adds RFKill support for the recently introduced WWAN device,
> making use of the WWAN and *wireless_status functions to query the
> killswitch and (de)activate the device accordingly to its status.
>
> Signed-off-by: Fabian Koester
So this is Fabian's code which he sent to you and you are submitting on his
behalf?
> Signed-off-by: Azael Avalos
A couple minor nits below.
> ---
> drivers/platform/x86/toshiba_acpi.c | 79
> +
> 1 file changed, 79 insertions(+)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c
> b/drivers/platform/x86/toshiba_acpi.c
> index 60d1ad9..d1315c5 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -51,6 +51,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -174,6 +175,7 @@ struct toshiba_acpi_dev {
> struct led_classdev kbd_led;
> struct led_classdev eco_led;
> struct miscdevice miscdev;
> + struct rfkill *wwan_rfk;
>
> int force_fan;
> int last_key_event;
> @@ -2330,6 +2332,67 @@ static const struct file_operations toshiba_acpi_fops
> = {
> };
>
> /*
> + * WWAN RFKill handlers
> + */
> +static int toshiba_acpi_wwan_set_block(void *data, bool blocked)
> +{
> + struct toshiba_acpi_dev *dev = data;
> + int ret;
> +
> + ret = toshiba_wireless_status(dev);
> + if (ret)
> + return ret;
> +
> + if (!dev->killswitch)
> + return 0;
> +
> + return toshiba_wwan_set(dev, blocked ? 0 : 1);
You can avoid the ternary operation with binary output and just invert the
bool.
!blocked
> +}
> +
> +static void toshiba_acpi_wwan_poll(struct rfkill *rfkill, void *data)
> +{
> + struct toshiba_acpi_dev *dev = data;
> +
> + if (toshiba_wireless_status(dev))
> + return;
> +
> + rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
> +}
> +
> +static const struct rfkill_ops wwan_rfk_ops = {
> + .set_block = toshiba_acpi_wwan_set_block,
> + .poll = toshiba_acpi_wwan_poll,
> +};
> +
> +static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev)
> +{
> + int ret = toshiba_wireless_status(dev);
> +
> + if (ret)
> + return ret;
> +
> + dev->wwan_rfk = rfkill_alloc("Toshiba WWAN",
> + &dev->acpi_dev->dev,
> + RFKILL_TYPE_WWAN,
> + &wwan_rfk_ops,
> + dev);
> + if (!dev->wwan_rfk) {
> + pr_err("Unable to allocate WWAN rfkill device\n");
> + return -ENOMEM;
> + }
> +
> + rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
> +
> + ret = rfkill_register(dev->wwan_rfk);
> + if (ret) {
> + pr_err("Unable to register WWAN rfkill device\n");
> + rfkill_destroy(dev->wwan_rfk);
> + }
> +
> + return ret;
> +}
> +
> +/*
> * Hotkeys
> */
> static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
> @@ -2688,6 +2751,11 @@ static int toshiba_acpi_remove(struct acpi_device
> *acpi_dev)
> if (dev->eco_led_registered)
> led_classdev_unregister(&dev->eco_led);
>
> + if (dev->wwan_rfk) {
> + rfkill_unregister(dev->wwan_rfk);
> + rfkill_destroy(dev->wwan_rfk);
> + }
> +
> if (toshiba_acpi)
> toshiba_acpi = NULL;
>
> @@ -2827,6 +2895,8 @@ static int toshiba_acpi_add(struct acpi_device
> *acpi_dev)
> dev->fan_supported = !ret;
>
> toshiba_wwan_available(dev);
> + if (dev->wwan_supported)
> + toshiba_acpi_setup_wwan_rfkill(dev);
>
> print_supported_features(dev);
>
> @@ -2930,6 +3000,15 @@ static int toshiba_acpi_resume(struct device *device)
> pr_info("Unable to re-enable hotkeys\n");
> }
>
> + if (dev->wwan_rfk) {
> + int error = toshiba_wireless_status(dev);
> +
> + if (error)
> + return error;
For consistency, please use ret. You can just initialize it to 0 outside the if
block, use it inside, and return it outside as well. We don't buy much by
declaring inside the if block on a resume function.
> +
> + rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
> + }
> +
> return 0;
> }
> #endif
> --
> 2.6.2
>
>
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] toshiba_acpi: Add WWAN RFKill support
A previuos patch added WWAN support to the driver, allowing to query
and set the device status.
This patch adds RFKill support for the recently introduced WWAN device,
making use of the WWAN and *wireless_status functions to query the
killswitch and (de)activate the device accordingly to its status.
Signed-off-by: Fabian Koester
Signed-off-by: Azael Avalos
---
drivers/platform/x86/toshiba_acpi.c | 79 +
1 file changed, 79 insertions(+)
diff --git a/drivers/platform/x86/toshiba_acpi.c
b/drivers/platform/x86/toshiba_acpi.c
index 60d1ad9..d1315c5 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -51,6 +51,7 @@
#include
#include
#include
+#include
#include
#include
@@ -174,6 +175,7 @@ struct toshiba_acpi_dev {
struct led_classdev kbd_led;
struct led_classdev eco_led;
struct miscdevice miscdev;
+ struct rfkill *wwan_rfk;
int force_fan;
int last_key_event;
@@ -2330,6 +2332,67 @@ static const struct file_operations toshiba_acpi_fops = {
};
/*
+ * WWAN RFKill handlers
+ */
+static int toshiba_acpi_wwan_set_block(void *data, bool blocked)
+{
+ struct toshiba_acpi_dev *dev = data;
+ int ret;
+
+ ret = toshiba_wireless_status(dev);
+ if (ret)
+ return ret;
+
+ if (!dev->killswitch)
+ return 0;
+
+ return toshiba_wwan_set(dev, blocked ? 0 : 1);
+}
+
+static void toshiba_acpi_wwan_poll(struct rfkill *rfkill, void *data)
+{
+ struct toshiba_acpi_dev *dev = data;
+
+ if (toshiba_wireless_status(dev))
+ return;
+
+ rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
+}
+
+static const struct rfkill_ops wwan_rfk_ops = {
+ .set_block = toshiba_acpi_wwan_set_block,
+ .poll = toshiba_acpi_wwan_poll,
+};
+
+static int toshiba_acpi_setup_wwan_rfkill(struct toshiba_acpi_dev *dev)
+{
+ int ret = toshiba_wireless_status(dev);
+
+ if (ret)
+ return ret;
+
+ dev->wwan_rfk = rfkill_alloc("Toshiba WWAN",
+&dev->acpi_dev->dev,
+RFKILL_TYPE_WWAN,
+&wwan_rfk_ops,
+dev);
+ if (!dev->wwan_rfk) {
+ pr_err("Unable to allocate WWAN rfkill device\n");
+ return -ENOMEM;
+ }
+
+ rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
+
+ ret = rfkill_register(dev->wwan_rfk);
+ if (ret) {
+ pr_err("Unable to register WWAN rfkill device\n");
+ rfkill_destroy(dev->wwan_rfk);
+ }
+
+ return ret;
+}
+
+/*
* Hotkeys
*/
static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
@@ -2688,6 +2751,11 @@ static int toshiba_acpi_remove(struct acpi_device
*acpi_dev)
if (dev->eco_led_registered)
led_classdev_unregister(&dev->eco_led);
+ if (dev->wwan_rfk) {
+ rfkill_unregister(dev->wwan_rfk);
+ rfkill_destroy(dev->wwan_rfk);
+ }
+
if (toshiba_acpi)
toshiba_acpi = NULL;
@@ -2827,6 +2895,8 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
dev->fan_supported = !ret;
toshiba_wwan_available(dev);
+ if (dev->wwan_supported)
+ toshiba_acpi_setup_wwan_rfkill(dev);
print_supported_features(dev);
@@ -2930,6 +3000,15 @@ static int toshiba_acpi_resume(struct device *device)
pr_info("Unable to re-enable hotkeys\n");
}
+ if (dev->wwan_rfk) {
+ int error = toshiba_wireless_status(dev);
+
+ if (error)
+ return error;
+
+ rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch);
+ }
+
return 0;
}
#endif
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
