diff mbox series

[v2,19/24] platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent style

Message ID 20210113182016.166049-20-pobrn@protonmail.com (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control | expand

Commit Message

Barnabás Pőcze Jan. 13, 2021, 6:22 p.m. UTC
Fix (almost all) checkpatch warnings. Reorder variable definitions from
longest to shortest. Add more whitespaces for better readability. Rename
variables named `ret` to `err` where appropriate.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Comments

Andy Shevchenko Jan. 16, 2021, 8:04 p.m. UTC | #1
On Wed, Jan 13, 2021 at 8:25 PM Barnabás Pőcze <pobrn@protonmail.com> wrote:
>
> Fix (almost all) checkpatch warnings. Reorder variable definitions from
> longest to shortest. Add more whitespaces for better readability. Rename
> variables named `ret` to `err` where appropriate.

As I said in many cases you actually introduced (okay, left it as is
while can fix at the same time) such a problem in the same series.
Ike Panhc Jan. 25, 2021, 8:55 a.m. UTC | #2
On 1/14/21 2:22 AM, Barnabás Pőcze wrote:
> Fix (almost all) checkpatch warnings. Reorder variable definitions from
> longest to shortest. Add more whitespaces for better readability. Rename
> variables named `ret` to `err` where appropriate.
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> @@ -265,30 +280,40 @@ static int debugfs_status_show(struct seq_file *s, void *data)
>  
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
>  		seq_printf(s, "Backlight max:\t%lu\n", value);
> +
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
>  		seq_printf(s, "Backlight now:\t%lu\n", value);
> +
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
>  		seq_printf(s, "BL power value:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	seq_puts(s, "=====================\n");
>  
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
>  		seq_printf(s, "Radio status:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
>  		seq_printf(s, "Wifi status:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
>  		seq_printf(s, "BT status:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
>  		seq_printf(s, "3G status:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	seq_puts(s, "=====================\n");
>  
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
>  		seq_printf(s, "Touchpad status:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
>  		seq_printf(s, "Camera status:\t%s (%lu)\n", value ? "on" : "off", value);
> +
>  	seq_puts(s, "=====================\n");
>  
>  	if (!eval_gbmd(priv->adev->handle, &value))
>  		seq_printf(s, "GBMD: %#010lx\n", value);
> +
>  	if (!eval_hals(priv->adev->handle, &value))
>  		seq_printf(s, "HALS: %#010lx\n", value);
>  

checkpatch.pl suggests empty lines? I think they are doing the same thing. It's better
to put them tightly.

--
Ike
Barnabás Pőcze Jan. 25, 2021, 2:25 p.m. UTC | #3
Hi


2021. január 25., hétfő 9:55 keltezéssel, Ike Panhc írta:

> On 1/14/21 2:22 AM, Barnabás Pőcze wrote:
> > Fix (almost all) checkpatch warnings. Reorder variable definitions from
> > longest to shortest. Add more whitespaces for better readability. Rename
> > variables named `ret` to `err` where appropriate.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >
> > @@ -265,30 +280,40 @@ static int debugfs_status_show(struct seq_file *s, void *data)
> >
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
> >  		seq_printf(s, "Backlight max:\t%lu\n", value);
> > +
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
> >  		seq_printf(s, "Backlight now:\t%lu\n", value);
> > +
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
> >  		seq_printf(s, "BL power value:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	seq_puts(s, "=====================\n");
> >
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
> >  		seq_printf(s, "Radio status:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
> >  		seq_printf(s, "Wifi status:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
> >  		seq_printf(s, "BT status:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
> >  		seq_printf(s, "3G status:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	seq_puts(s, "=====================\n");
> >
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
> >  		seq_printf(s, "Touchpad status:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
> >  		seq_printf(s, "Camera status:\t%s (%lu)\n", value ? "on" : "off", value);
> > +
> >  	seq_puts(s, "=====================\n");
> >
> >  	if (!eval_gbmd(priv->adev->handle, &value))
> >  		seq_printf(s, "GBMD: %#010lx\n", value);
> > +
> >  	if (!eval_hals(priv->adev->handle, &value))
> >  		seq_printf(s, "HALS: %#010lx\n", value);
> >
>
> checkpatch.pl suggests empty lines? I think they are doing the same thing. It's better
> to put them tightly.
>
> --
> Ike

I added them at my own discretion, and I don't recall checkpatch suggesting
it, so if you want to, I can remove them, but I'd like to keep one empty
line before and after

  seq_puts(s, "=====================\n");

.

What do you think?


Regards,
Barnabás Pőcze
Ike Panhc Jan. 26, 2021, 4:05 a.m. UTC | #4
On 1/25/21 10:25 PM, Barnabás Pőcze wrote:
> Hi
> 
> 
> 2021. január 25., hétfő 9:55 keltezéssel, Ike Panhc írta:
> 
>> On 1/14/21 2:22 AM, Barnabás Pőcze wrote:
>>> Fix (almost all) checkpatch warnings. Reorder variable definitions from
>>> longest to shortest. Add more whitespaces for better readability. Rename
>>> variables named `ret` to `err` where appropriate.
>>>
>>> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
>>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>> @@ -265,30 +280,40 @@ static int debugfs_status_show(struct seq_file *s, void *data)
>>>
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
>>>  		seq_printf(s, "Backlight max:\t%lu\n", value);
>>> +
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
>>>  		seq_printf(s, "Backlight now:\t%lu\n", value);
>>> +
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
>>>  		seq_printf(s, "BL power value:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	seq_puts(s, "=====================\n");
>>>
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
>>>  		seq_printf(s, "Radio status:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
>>>  		seq_printf(s, "Wifi status:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
>>>  		seq_printf(s, "BT status:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
>>>  		seq_printf(s, "3G status:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	seq_puts(s, "=====================\n");
>>>
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
>>>  		seq_printf(s, "Touchpad status:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
>>>  		seq_printf(s, "Camera status:\t%s (%lu)\n", value ? "on" : "off", value);
>>> +
>>>  	seq_puts(s, "=====================\n");
>>>
>>>  	if (!eval_gbmd(priv->adev->handle, &value))
>>>  		seq_printf(s, "GBMD: %#010lx\n", value);
>>> +
>>>  	if (!eval_hals(priv->adev->handle, &value))
>>>  		seq_printf(s, "HALS: %#010lx\n", value);
>>>
>>
>> checkpatch.pl suggests empty lines? I think they are doing the same thing. It's better
>> to put them tightly.
>>
>> --
>> Ike
> 
> I added them at my own discretion, and I don't recall checkpatch suggesting
> it, so if you want to, I can remove them, but I'd like to keep one empty
> line before and after
> 
>   seq_puts(s, "=====================\n");
> 
> .
> 
> What do you think?
> 
> 
> Regards,
> Barnabás Pőcze
> 

That looks great. Many thanks.

--
Ike
diff mbox series

Patch

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 83ad82b21b59..0e50969bd194 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -129,13 +129,15 @@  MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
 
 static int eval_int(acpi_handle handle, const char *method, unsigned long *val)
 {
-	acpi_status acpi_err;
 	unsigned long long result;
+	acpi_status acpi_err;
 
 	acpi_err = acpi_evaluate_integer(handle, (char *)method, NULL, &result);
 	if (ACPI_FAILURE(acpi_err))
 		return -EIO;
+
 	*val = result;
+
 	return 0;
 }
 
@@ -167,10 +169,10 @@  static int eval_sals(acpi_handle handle, unsigned long arg)
 
 static int eval_vpcr(acpi_handle handle, unsigned long cmd, unsigned long *val)
 {
-	acpi_status acpi_err;
-	unsigned long long result;
 	struct acpi_object_list params;
+	unsigned long long result;
 	union acpi_object in_obj;
+	acpi_status acpi_err;
 
 	params.count = 1;
 	params.pointer = &in_obj;
@@ -178,12 +180,12 @@  static int eval_vpcr(acpi_handle handle, unsigned long cmd, unsigned long *val)
 	in_obj.integer.value = cmd;
 
 	acpi_err = acpi_evaluate_integer(handle, "VPCR", &params, &result);
-
 	if (ACPI_FAILURE(acpi_err))
 		return -EIO;
+
 	*val = result;
-	return 0;
 
+	return 0;
 }
 
 static int eval_vpcw(acpi_handle handle, unsigned long cmd, unsigned long data)
@@ -202,13 +204,14 @@  static int eval_vpcw(acpi_handle handle, unsigned long cmd, unsigned long data)
 	acpi_err = acpi_evaluate_object(handle, "VPCW", &params, NULL);
 	if (ACPI_FAILURE(acpi_err))
 		return -EIO;
+
 	return 0;
 }
 
 static int read_ec_data(acpi_handle handle, unsigned long cmd, unsigned long *data)
 {
+	unsigned long end_jiffies, val;
 	int err;
-	unsigned long int end_jiffies, val;
 
 	err = eval_vpcw(handle, 1, cmd);
 	if (err)
@@ -216,39 +219,51 @@  static int read_ec_data(acpi_handle handle, unsigned long cmd, unsigned long *da
 
 	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
+
 		schedule();
+
 		err = eval_vpcr(handle, 1, &val);
+
 		if (err)
 			return err;
+
 		if (val == 0)
 			return eval_vpcr(handle, 0, data);
 	}
+
 	acpi_handle_err(handle, "timeout in %s\n", __func__);
+
 	return -ETIMEDOUT;
 }
 
 static int write_ec_cmd(acpi_handle handle, unsigned long cmd, unsigned long data)
 {
-	int err;
 	unsigned long end_jiffies, val;
+	int err;
 
 	err = eval_vpcw(handle, 0, data);
 	if (err)
 		return err;
+
 	err = eval_vpcw(handle, 1, cmd);
 	if (err)
 		return err;
 
 	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
+
 		schedule();
+
 		err = eval_vpcr(handle, 1, &val);
 		if (err)
 			return err;
+
 		if (val == 0)
 			return 0;
 	}
+
 	acpi_handle_err(handle, "timeout in %s\n", __func__);
+
 	return -ETIMEDOUT;
 }
 
@@ -265,30 +280,40 @@  static int debugfs_status_show(struct seq_file *s, void *data)
 
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
 		seq_printf(s, "Backlight max:\t%lu\n", value);
+
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
 		seq_printf(s, "Backlight now:\t%lu\n", value);
+
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
 		seq_printf(s, "BL power value:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	seq_puts(s, "=====================\n");
 
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
 		seq_printf(s, "Radio status:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
 		seq_printf(s, "Wifi status:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
 		seq_printf(s, "BT status:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
 		seq_printf(s, "3G status:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	seq_puts(s, "=====================\n");
 
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
 		seq_printf(s, "Touchpad status:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
 		seq_printf(s, "Camera status:\t%s (%lu)\n", value ? "on" : "off", value);
+
 	seq_puts(s, "=====================\n");
 
 	if (!eval_gbmd(priv->adev->handle, &value))
 		seq_printf(s, "GBMD: %#010lx\n", value);
+
 	if (!eval_hals(priv->adev->handle, &value))
 		seq_printf(s, "HALS: %#010lx\n", value);
 
@@ -349,8 +374,8 @@  static void ideapad_debugfs_init(struct ideapad_private *priv)
 	dir = debugfs_create_dir("ideapad", NULL);
 	priv->debug = dir;
 
-	debugfs_create_file("cfg", S_IRUGO, dir, priv, &debugfs_cfg_fops);
-	debugfs_create_file("status", S_IRUGO, dir, priv, &debugfs_status_fops);
+	debugfs_create_file("cfg", 0444, dir, priv, &debugfs_cfg_fops);
+	debugfs_create_file("status", 0444, dir, priv, &debugfs_status_fops);
 }
 
 static void ideapad_debugfs_exit(struct ideapad_private *priv)
@@ -362,73 +387,80 @@  static void ideapad_debugfs_exit(struct ideapad_private *priv)
 /*
  * sysfs
  */
-static ssize_t show_ideapad_cam(struct device *dev,
-				struct device_attribute *attr,
-				char *buf)
+static ssize_t camera_power_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
 {
-	unsigned long result;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
+	unsigned long result;
 	int err;
 
 	err = read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &result);
 	if (err)
 		return err;
+
 	return sysfs_emit(buf, "%lu\n", result);
 }
 
-static ssize_t store_ideapad_cam(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf, size_t count)
+static ssize_t camera_power_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
 {
-	int ret;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	unsigned int state;
+	int err;
+
+	err = kstrtouint(buf, 0, &state);
+	if (err)
+		return err;
+
+	err = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
+	if (err)
+		return err;
 
-	ret = kstrtouint(buf, 0, &state);
-	if (ret)
-		return ret;
-	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
-	if (ret)
-		return ret;
 	return count;
 }
 
-static DEVICE_ATTR(camera_power, 0644, show_ideapad_cam, store_ideapad_cam);
+static DEVICE_ATTR_RW(camera_power);
 
-static ssize_t show_ideapad_fan(struct device *dev,
-				struct device_attribute *attr,
-				char *buf)
+static ssize_t fan_mode_show(struct device *dev,
+			     struct device_attribute *attr,
+			     char *buf)
 {
-	unsigned long result;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
+	unsigned long result;
 	int err;
 
 	err = read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result);
 	if (err)
 		return err;
+
 	return sysfs_emit(buf, "%lu\n", result);
 }
 
-static ssize_t store_ideapad_fan(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf, size_t count)
+static ssize_t fan_mode_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
 {
-	int ret;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	unsigned int state;
+	int err;
+
+	err = kstrtouint(buf, 0, &state);
+	if (err)
+		return err;
 
-	ret = kstrtouint(buf, 0, &state);
-	if (ret)
-		return ret;
 	if (state > 4 || state == 3)
 		return -EINVAL;
-	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_FAN, state);
-	if (ret)
-		return ret;
+
+	err = write_ec_cmd(priv->adev->handle, VPCCMD_W_FAN, state);
+	if (err)
+		return err;
+
 	return count;
 }
 
-static DEVICE_ATTR(fan_mode, 0644, show_ideapad_fan, store_ideapad_fan);
+static DEVICE_ATTR_RW(fan_mode);
 
 static ssize_t touchpad_show(struct device *dev,
 			     struct device_attribute *attr,
@@ -441,6 +473,7 @@  static ssize_t touchpad_show(struct device *dev,
 	err = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result);
 	if (err)
 		return err;
+
 	return sysfs_emit(buf, "%lu\n", result);
 }
 
@@ -450,23 +483,24 @@  static ssize_t touchpad_store(struct device *dev,
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	bool state;
-	int ret;
+	int err;
 
-	ret = kstrtobool(buf, &state);
-	if (ret)
-		return ret;
+	err = kstrtobool(buf, &state);
+	if (err)
+		return err;
+
+	err = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
+	if (err)
+		return err;
 
-	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
-	if (ret)
-		return ret;
 	return count;
 }
 
 static DEVICE_ATTR_RW(touchpad);
 
 static ssize_t conservation_mode_show(struct device *dev,
-				struct device_attribute *attr,
-				char *buf)
+				      struct device_attribute *attr,
+				      char *buf)
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	unsigned long result;
@@ -475,25 +509,27 @@  static ssize_t conservation_mode_show(struct device *dev,
 	err = eval_gbmd(priv->adev->handle, &result);
 	if (err)
 		return err;
+
 	return sysfs_emit(buf, "%u\n", test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
 }
 
 static ssize_t conservation_mode_store(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf, size_t count)
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	bool state;
-	int ret;
+	int err;
 
-	ret = kstrtobool(buf, &state);
-	if (ret)
-		return ret;
+	err = kstrtobool(buf, &state);
+	if (err)
+		return err;
 
-	ret = eval_smbc(priv->adev->handle,
+	err = eval_smbc(priv->adev->handle,
 			state ? SMBC_CONSERVATION_ON : SMBC_CONSERVATION_OFF);
-	if (ret)
-		return ret;
+	if (err)
+		return err;
+
 	return count;
 }
 
@@ -505,10 +541,11 @@  static ssize_t fn_lock_show(struct device *dev,
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	unsigned long hals;
-	int fail = eval_hals(priv->adev->handle, &hals);
+	int err;
 
-	if (fail)
-		return fail;
+	err = eval_hals(priv->adev->handle, &hals);
+	if (err)
+		return err;
 
 	return sysfs_emit(buf, "%u\n", test_bit(HALS_FNLOCK_STATE_BIT, &hals));
 }
@@ -519,22 +556,22 @@  static ssize_t fn_lock_store(struct device *dev,
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	bool state;
-	int ret;
+	int err;
 
-	ret = kstrtobool(buf, &state);
-	if (ret)
-		return ret;
+	err = kstrtobool(buf, &state);
+	if (err)
+		return err;
 
-	ret = eval_sals(priv->adev->handle,
+	err = eval_sals(priv->adev->handle,
 			state ? SALS_FNLOCK_ON : SALS_FNLOCK_OFF);
-	if (ret)
-		return ret;
+	if (err)
+		return err;
+
 	return count;
 }
 
 static DEVICE_ATTR_RW(fn_lock);
 
-
 static struct attribute *ideapad_attributes[] = {
 	&dev_attr_camera_power.attr,
 	&dev_attr_fan_mode.attr,
@@ -618,16 +655,16 @@  static void ideapad_sync_rfk_state(struct ideapad_private *priv)
 
 static int ideapad_register_rfkill(struct ideapad_private *priv, int dev)
 {
-	int ret;
-	unsigned long sw_blocked;
+	unsigned long rf_enabled;
+	int err;
 
-	if (no_bt_rfkill &&
-	    (ideapad_rfk_data[dev].type == RFKILL_TYPE_BLUETOOTH)) {
+	if (no_bt_rfkill && ideapad_rfk_data[dev].type == RFKILL_TYPE_BLUETOOTH) {
 		/* Force to enable bluetooth when no_bt_rfkill=1 */
 		write_ec_cmd(priv->adev->handle,
 			     ideapad_rfk_data[dev].opcode, 1);
 		return 0;
 	}
+
 	priv->rfk_priv[dev].dev = dev;
 	priv->rfk_priv[dev].priv = priv;
 
@@ -639,19 +676,19 @@  static int ideapad_register_rfkill(struct ideapad_private *priv, int dev)
 	if (!priv->rfk[dev])
 		return -ENOMEM;
 
-	if (read_ec_data(priv->adev->handle, ideapad_rfk_data[dev].opcode-1,
-			 &sw_blocked)) {
-		rfkill_init_sw_state(priv->rfk[dev], 0);
-	} else {
-		sw_blocked = !sw_blocked;
-		rfkill_init_sw_state(priv->rfk[dev], sw_blocked);
-	}
+	err = read_ec_data(priv->adev->handle, ideapad_rfk_data[dev].opcode - 1,
+			   &rf_enabled);
+	if (err)
+		rf_enabled = 1;
+
+	rfkill_init_sw_state(priv->rfk[dev], !rf_enabled);
 
-	ret = rfkill_register(priv->rfk[dev]);
-	if (ret) {
+	err = rfkill_register(priv->rfk[dev]);
+	if (err) {
 		rfkill_destroy(priv->rfk[dev]);
-		return ret;
+		return err;
 	}
+
 	return 0;
 }
 
@@ -670,7 +707,7 @@  static void ideapad_unregister_rfkill(struct ideapad_private *priv, int dev)
 static int ideapad_sysfs_init(struct ideapad_private *priv)
 {
 	return sysfs_create_group(&priv->platform_device->dev.kobj,
-				    &ideapad_attribute_group);
+				  &ideapad_attribute_group);
 }
 
 static void ideapad_sysfs_exit(struct ideapad_private *priv)
@@ -696,13 +733,13 @@  static const struct key_entry ideapad_keymap[] = {
 	{ KE_KEY, 67, { KEY_TOUCHPAD_ON } },
 	{ KE_KEY, 128, { KEY_ESC } },
 
-	{ KE_END, 0 },
+	{ KE_END },
 };
 
 static int ideapad_input_init(struct ideapad_private *priv)
 {
 	struct input_dev *inputdev;
-	int error;
+	int err;
 
 	inputdev = input_allocate_device();
 	if (!inputdev)
@@ -713,15 +750,15 @@  static int ideapad_input_init(struct ideapad_private *priv)
 	inputdev->id.bustype = BUS_HOST;
 	inputdev->dev.parent = &priv->platform_device->dev;
 
-	error = sparse_keymap_setup(inputdev, ideapad_keymap, NULL);
-	if (error) {
+	err = sparse_keymap_setup(inputdev, ideapad_keymap, NULL);
+	if (err) {
 		dev_err(&priv->platform_device->dev,
 			"Unable to setup input device keymap\n");
 		goto err_free_dev;
 	}
 
-	error = input_register_device(inputdev);
-	if (error) {
+	err = input_register_device(inputdev);
+	if (err) {
 		dev_err(&priv->platform_device->dev,
 			"Unable to register input device\n");
 		goto err_free_dev;
@@ -732,7 +769,7 @@  static int ideapad_input_init(struct ideapad_private *priv)
 
 err_free_dev:
 	input_free_device(inputdev);
-	return error;
+	return err;
 }
 
 static void ideapad_input_exit(struct ideapad_private *priv)
@@ -753,6 +790,7 @@  static void ideapad_input_novokey(struct ideapad_private *priv)
 
 	if (read_ec_data(priv->adev->handle, VPCCMD_R_NOVO, &long_pressed))
 		return;
+
 	if (long_pressed)
 		ideapad_input_report(priv, 17);
 	else
@@ -815,6 +853,7 @@  static int ideapad_backlight_update_status(struct backlight_device *blightdev)
 			   blightdev->props.brightness);
 	if (err)
 		return err;
+
 	err = write_ec_cmd(priv->adev->handle, VPCCMD_W_BL_POWER,
 			   blightdev->props.power != FB_BLANK_POWERDOWN);
 	if (err)
@@ -875,13 +914,15 @@  static void ideapad_backlight_exit(struct ideapad_private *priv)
 
 static void ideapad_backlight_notify_power(struct ideapad_private *priv)
 {
-	unsigned long power;
 	struct backlight_device *blightdev = priv->blightdev;
+	unsigned long power;
 
 	if (!blightdev)
 		return;
+
 	if (read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &power))
 		return;
+
 	blightdev->props.power = power ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
 }
 
@@ -890,12 +931,10 @@  static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
 	unsigned long now;
 
 	/* if we control brightness via acpi video driver */
-	if (priv->blightdev == NULL) {
+	if (!priv->blightdev)
 		read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now);
-		return;
-	}
-
-	backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
+	else
+		backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
 }
 
 /*
@@ -910,13 +949,15 @@  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
+		unsigned char param;
+		/*
+		 * 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 */
-		unsigned char param;
-		i8042_command(&param, value ? I8042_CMD_AUX_ENABLE :
-			      I8042_CMD_AUX_DISABLE);
+		 * KEY_TOUCHPAD_ON to not to get out of sync with LED
+		 */
+		i8042_command(&param,
+			      value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
 		ideapad_input_report(priv, value ? 67 : 66);
 	}
 }
@@ -960,7 +1001,8 @@  static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 			ideapad_check_special_buttons(priv);
 			break;
 		case 1:
-			/* Some IdeaPads report event 1 every ~20
+			/*
+			 * Some IdeaPads report event 1 every ~20
 			 * seconds while on battery power; some
 			 * report this when changing to/from tablet
 			 * mode. Squelch this event.
@@ -1033,14 +1075,14 @@  static void ideapad_check_features(struct ideapad_private *priv)
 
 static int ideapad_acpi_add(struct platform_device *pdev)
 {
-	int ret, i;
-	unsigned long cfg;
 	struct ideapad_private *priv;
 	struct acpi_device *adev;
 	acpi_status acpi_err;
+	unsigned long cfg;
+	int err, i;
 
-	ret = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
-	if (ret)
+	err = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
+	if (err)
 		return -ENODEV;
 
 	if (eval_int(adev->handle, "_CFG", &cfg))
@@ -1057,14 +1099,14 @@  static int ideapad_acpi_add(struct platform_device *pdev)
 
 	ideapad_check_features(priv);
 
-	ret = ideapad_sysfs_init(priv);
-	if (ret)
-		return ret;
+	err = ideapad_sysfs_init(priv);
+	if (err)
+		return err;
 
 	ideapad_debugfs_init(priv);
 
-	ret = ideapad_input_init(priv);
-	if (ret)
+	err = ideapad_input_init(priv);
+	if (err)
 		goto input_failed;
 
 	/*
@@ -1086,14 +1128,16 @@  static int ideapad_acpi_add(struct platform_device *pdev)
 	ideapad_sync_touchpad_state(priv);
 
 	if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
-		ret = ideapad_backlight_init(priv);
-		if (ret && ret != -ENODEV)
+		err = ideapad_backlight_init(priv);
+		if (err && err != -ENODEV)
 			goto backlight_failed;
 	}
+
 	acpi_err = acpi_install_notify_handler(adev->handle,
-			ACPI_DEVICE_NOTIFY, ideapad_acpi_notify, priv);
+					       ACPI_DEVICE_NOTIFY,
+					       ideapad_acpi_notify, priv);
 	if (ACPI_FAILURE(acpi_err)) {
-		ret = -EIO;
+		err = -EIO;
 		goto notification_failed;
 	}
 
@@ -1106,28 +1150,36 @@  static int ideapad_acpi_add(struct platform_device *pdev)
 			break;
 		}
 	}
+
 	if (ACPI_FAILURE(acpi_err) && acpi_err != AE_NOT_EXIST) {
-		ret = -EIO;
+		err = -EIO;
 		goto notification_failed_wmi;
 	}
 #endif
 
 	return 0;
+
 #if IS_ENABLED(CONFIG_ACPI_WMI)
 notification_failed_wmi:
 	acpi_remove_notify_handler(priv->adev->handle,
-		ACPI_DEVICE_NOTIFY, ideapad_acpi_notify);
+				   ACPI_DEVICE_NOTIFY,
+				   ideapad_acpi_notify);
 #endif
+
 notification_failed:
 	ideapad_backlight_exit(priv);
+
 backlight_failed:
 	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		ideapad_unregister_rfkill(priv, i);
+
 	ideapad_input_exit(priv);
+
 input_failed:
 	ideapad_debugfs_exit(priv);
 	ideapad_sysfs_exit(priv);
-	return ret;
+
+	return err;
 }
 
 static int ideapad_acpi_remove(struct platform_device *pdev)
@@ -1139,11 +1191,15 @@  static int ideapad_acpi_remove(struct platform_device *pdev)
 	if (priv->fnesc_guid)
 		wmi_remove_notify_handler(priv->fnesc_guid);
 #endif
+
 	acpi_remove_notify_handler(priv->adev->handle,
-		ACPI_DEVICE_NOTIFY, ideapad_acpi_notify);
+				   ACPI_DEVICE_NOTIFY,
+				   ideapad_acpi_notify);
 	ideapad_backlight_exit(priv);
+
 	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		ideapad_unregister_rfkill(priv, i);
+
 	ideapad_input_exit(priv);
 	ideapad_debugfs_exit(priv);
 	ideapad_sysfs_exit(priv);
@@ -1158,18 +1214,20 @@  static int ideapad_acpi_resume(struct device *device)
 
 	if (!device)
 		return -EINVAL;
+
 	priv = dev_get_drvdata(device);
 
 	ideapad_sync_rfk_state(priv);
 	ideapad_sync_touchpad_state(priv);
+
 	return 0;
 }
 #endif
 static SIMPLE_DEV_PM_OPS(ideapad_pm, NULL, ideapad_acpi_resume);
 
 static const struct acpi_device_id ideapad_device_ids[] = {
-	{ "VPC2004", 0},
-	{ "", 0},
+	{"VPC2004", 0},
+	{"", 0},
 };
 MODULE_DEVICE_TABLE(acpi, ideapad_device_ids);