2017-12-02 13:46:54

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH 1/4] platform/x86: ideapad-laptop: Remove unnesscary else

To deal with checkpatch warning:
WARNING: else is not generally useful after a break or return

Signed-off-by: Jiaxun Yang <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 53ab4e0f8962..9f2a4bc58df0 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -124,10 +124,10 @@ static int read_method_int(acpi_handle handle, const char *method, int *val)
if (ACPI_FAILURE(status)) {
*val = -1;
return -1;
- } else {
- *val = result;
- return 0;
}
+ *val = result;
+ return 0;
+
}

static int method_gbmd(acpi_handle handle, unsigned long *ret)
@@ -164,10 +164,10 @@ static int method_vpcr(acpi_handle handle, int cmd, int *ret)
if (ACPI_FAILURE(status)) {
*ret = -1;
return -1;
- } else {
- *ret = result;
- return 0;
}
+ *ret = result;
+ return 0;
+
}

static int method_vpcw(acpi_handle handle, int cmd, int data)
--
2.14.1


2017-12-02 13:47:02

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH 2/4] platform/x86: ideapad-laptop: Use __func__ instead of write_ec_cmd in pr_err

To deal with checkpatch warning:
WARNING: Prefer using '"%s...", __func__' to using 'write_ec_cmd',
this function's name, in a string

Signed-off-by: Jiaxun Yang <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 9f2a4bc58df0..37a88938bbaa 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -231,7 +231,7 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
if (val == 0)
return 0;
}
- pr_err("timeout in write_ec_cmd\n");
+ pr_err("timeout in %s\n", __func__);
return -1;
}

--
2.14.1

2017-12-02 13:47:08

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH 3/4] platform/x86: ideapad-laptop: use kstrto instead of sscanf and do clean up

To deal with checkpatch warnings:
WARNING: Prefer kstrto<type> to single variable sscanf

WARNING: Missing a blank line after declarations

WARNING: Block comments use a trailing */ on a separate line

Signed-off-by: Jiaxun Yang <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 1072b24370ac..924b07f7db06 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -406,12 +406,14 @@ static ssize_t store_ideapad_cam(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- int ret, state;
+ int ret, state, rc;
struct ideapad_private *priv = dev_get_drvdata(dev);

+ rc = kstrtoint(buf, 0, &state);
+
if (!count)
return 0;
- if (sscanf(buf, "%i", &state) != 1)
+ if (rc != 0)
return -EINVAL;
ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
if (ret < 0)
@@ -437,12 +439,14 @@ static ssize_t store_ideapad_fan(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- int ret, state;
+ int ret, state, rc;
struct ideapad_private *priv = dev_get_drvdata(dev);

+ rc = kstrtoint(buf, 0, &state);
+
if (!count)
return 0;
- if (sscanf(buf, "%i", &state) != 1)
+ if (rc != 0)
return -EINVAL;
if (state < 0 || state > 4 || state == 3)
return -EINVAL;
@@ -541,6 +545,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
supported = test_bit(CFG_CAMERA_BIT, &(priv->cfg));
else if (attr == &dev_attr_fan_mode.attr) {
unsigned long value;
+
supported = !read_ec_data(priv->adev->handle, VPCCMD_R_FAN,
&value);
} else if (attr == &dev_attr_conservation_mode.attr) {
@@ -880,11 +885,14 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv)

/* Without reading from EC touchpad LED doesn't switch state */
if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value)) {
- /* Some IdeaPads don't really turn off touchpad - they only
+ /*
+ * Some IdeaPads don't really turn off touchpad - they only
* switch the LED state. We (de)activate KBC AUX port to turn
* touchpad off and on. We send KEY_TOUCHPAD_OFF and
- * KEY_TOUCHPAD_ON to not to get out of sync with LED */
+ * KEY_TOUCHPAD_ON to not to get out of sync with LED
+ */
unsigned char param;
+
i8042_command(&param, value ? I8042_CMD_AUX_ENABLE :
I8042_CMD_AUX_DISABLE);
ideapad_input_report(priv, value ? 67 : 66);
--
2.14.1

2017-12-02 13:47:15

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH 4/4] platform/x86: ideapad-laptop: add lenovo RESCUER R720-15IKBN to no_hw_rfkill_list

Since Roger Jargoyhen <[email protected]> reported that this device doesn't have
Hardware rfkill switch, this patch add it to no_hw_rfkill_list to prevent radio always
be blocked

Signed-off-by: Jiaxun Yang <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 924b07f7db06..4d3c7a6990d4 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -971,6 +971,13 @@ static void ideapad_wmi_notify(u32 value, void *context)
* report all radios as hardware-blocked.
*/
static const struct dmi_system_id no_hw_rfkill_list[] = {
+ {
+ .ident = "Lenovo RESCUER R720-15IKBN",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_BOARD_NAME, "80WW"),
+ },
+ },
{
.ident = "Lenovo G40-30",
.matches = {
--
2.14.1

2017-12-08 23:39:55

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH 3/4] platform/x86: ideapad-laptop: use kstrto instead of sscanf and do clean up

On Sat, Dec 02, 2017 at 09:45:33PM +0800, Jiaxun Yang wrote:
> To deal with checkpatch warnings:
> WARNING: Prefer kstrto<type> to single variable sscanf
>
> WARNING: Missing a blank line after declarations
>
> WARNING: Block comments use a trailing */ on a separate line
>
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> drivers/platform/x86/ideapad-laptop.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 1072b24370ac..924b07f7db06 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -406,12 +406,14 @@ static ssize_t store_ideapad_cam(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - int ret, state;
> + int ret, state, rc;
> struct ideapad_private *priv = dev_get_drvdata(dev);
>
> + rc = kstrtoint(buf, 0, &state);
> +
> if (!count)
> return 0;

Better to keep the string to int here after the check for !count. No reason I
can see to move it above.

> - if (sscanf(buf, "%i", &state) != 1)
> + if (rc != 0)
> return -EINVAL;
> ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
> if (ret < 0)
> @@ -437,12 +439,14 @@ static ssize_t store_ideapad_fan(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - int ret, state;
> + int ret, state, rc;
> struct ideapad_private *priv = dev_get_drvdata(dev);
>
> + rc = kstrtoint(buf, 0, &state);
> +

Same here

> if (!count)
> return 0;
> - if (sscanf(buf, "%i", &state) != 1)
> + if (rc != 0)

And in both cases, kstrtoint can be used within the if condition, avoiding the
need for a new rc variable.

> return -EINVAL;
> if (state < 0 || state > 4 || state == 3)
> return -EINVAL;
> @@ -541,6 +545,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
> supported = test_bit(CFG_CAMERA_BIT, &(priv->cfg));
> else if (attr == &dev_attr_fan_mode.attr) {
> unsigned long value;
> +
> supported = !read_ec_data(priv->adev->handle, VPCCMD_R_FAN,
> &value);
> } else if (attr == &dev_attr_conservation_mode.attr) {
> @@ -880,11 +885,14 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv)
>
> /* Without reading from EC touchpad LED doesn't switch state */
> if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value)) {
> - /* Some IdeaPads don't really turn off touchpad - they only
> + /*
> + * Some IdeaPads don't really turn off touchpad - they only
> * switch the LED state. We (de)activate KBC AUX port to turn
> * touchpad off and on. We send KEY_TOUCHPAD_OFF and
> - * KEY_TOUCHPAD_ON to not to get out of sync with LED */
> + * KEY_TOUCHPAD_ON to not to get out of sync with LED
> + */
> unsigned char param;
> +
> i8042_command(&param, value ? I8042_CMD_AUX_ENABLE :
> I8042_CMD_AUX_DISABLE);
> ideapad_input_report(priv, value ? 67 : 66);
> --
> 2.14.1
>
>

--
Darren Hart
VMware Open Source Technology Center

2017-12-08 23:41:42

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH 1/4] platform/x86: ideapad-laptop: Remove unnesscary else

On Sat, Dec 02, 2017 at 09:45:31PM +0800, Jiaxun Yang wrote:
> To deal with checkpatch warning:
> WARNING: else is not generally useful after a break or return
>

Patches 1,2, and 4 queued for testing. Thanks. See comments in 3 of 4.

--
Darren Hart
VMware Open Source Technology Center