diff mbox

[05/10] platform/x86: fujitsu-laptop: distinguish current uses of device-specific data

Message ID 20170424133334.7064-6-kernel@kempniu.pl (mailing list archive)
State Changes Requested, archived
Delegated to: Darren Hart
Headers show

Commit Message

Michał Kępień April 24, 2017, 1:33 p.m. UTC 
In portions of the driver which use device-specific data, rename local
variables from fujitsu_bl and fujitsu_laptop to priv in order to clearly
distinguish these parts from code that uses module-wide data.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/fujitsu-laptop.c | 48 +++++++++++++++++------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

Comments

Jonathan Woithe May 1, 2017, 1:40 p.m. UTC | #1 
On Mon, Apr 24, 2017 at 03:33:29PM +0200, Micha?? K??pie?? wrote:
> In portions of the driver which use device-specific data, rename local
> variables from fujitsu_bl and fujitsu_laptop to priv in order to clearly
> distinguish these parts from code that uses module-wide data.
> 
> Signed-off-by: Micha?? K??pie?? <kernel@kempniu.pl>
> ---
>  drivers/platform/x86/fujitsu-laptop.c | 48 +++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 5f6b34a97348..536b601c7067 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -369,26 +369,26 @@ static const struct key_entry keymap_backlight[] = {
>  
>  static int acpi_fujitsu_bl_input_setup(struct acpi_device *device)
>  {
> -	struct fujitsu_bl *fujitsu_bl = acpi_driver_data(device);
> +	struct fujitsu_bl *priv = acpi_driver_data(device);

[cut]

>  static int fujitsu_backlight_register(struct acpi_device *device)
> @@ -566,27 +566,27 @@ static const struct dmi_system_id fujitsu_laptop_dmi_table[] = {
>  
>  static int acpi_fujitsu_laptop_input_setup(struct acpi_device *device)
>  {
> -	struct fujitsu_laptop *fujitsu_laptop = acpi_driver_data(device);
> +	struct fujitsu_laptop *priv = acpi_driver_data(device);
>  	int ret;

Distinguishing between local and global use like this makes sense, but I
feel we should stick with a slightly more descriptive name than "priv". 
Without any qualification, "priv" could refer to private device-specific
data from either the fujitsu_bl or fujitsu_laptop drivers.  From the source
it is far from obvious which is being accessed in a given function.  If we
implemented only a single ACPI device driver then this would be largely a
moot point, but as there are two within the one module the loss of the
description could make it harder to follow the code later on.

Could we use "bl_priv" and "laptop_priv" for example, so as to provide a
clue within the source code as to what exactly is being referenced? 
Obviously it doesn't provide any compile time type checking, but it's better
than nothing.

Regards
  jonathan
diff mbox

Patch

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 5f6b34a97348..536b601c7067 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -369,26 +369,26 @@  static const struct key_entry keymap_backlight[] = {
 
 static int acpi_fujitsu_bl_input_setup(struct acpi_device *device)
 {
-	struct fujitsu_bl *fujitsu_bl = acpi_driver_data(device);
+	struct fujitsu_bl *priv = acpi_driver_data(device);
 	int ret;
 
-	fujitsu_bl->input = devm_input_allocate_device(&device->dev);
-	if (!fujitsu_bl->input)
+	priv->input = devm_input_allocate_device(&device->dev);
+	if (!priv->input)
 		return -ENOMEM;
 
-	snprintf(fujitsu_bl->phys, sizeof(fujitsu_bl->phys),
-		 "%s/video/input0", acpi_device_hid(device));
+	snprintf(priv->phys, sizeof(priv->phys), "%s/video/input0",
+		 acpi_device_hid(device));
 
-	fujitsu_bl->input->name = acpi_device_name(device);
-	fujitsu_bl->input->phys = fujitsu_bl->phys;
-	fujitsu_bl->input->id.bustype = BUS_HOST;
-	fujitsu_bl->input->id.product = 0x06;
+	priv->input->name = acpi_device_name(device);
+	priv->input->phys = priv->phys;
+	priv->input->id.bustype = BUS_HOST;
+	priv->input->id.product = 0x06;
 
-	ret = sparse_keymap_setup(fujitsu_bl->input, keymap_backlight, NULL);
+	ret = sparse_keymap_setup(priv->input, keymap_backlight, NULL);
 	if (ret)
 		return ret;
 
-	return input_register_device(fujitsu_bl->input);
+	return input_register_device(priv->input);
 }
 
 static int fujitsu_backlight_register(struct acpi_device *device)
@@ -566,27 +566,27 @@  static const struct dmi_system_id fujitsu_laptop_dmi_table[] = {
 
 static int acpi_fujitsu_laptop_input_setup(struct acpi_device *device)
 {
-	struct fujitsu_laptop *fujitsu_laptop = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = acpi_driver_data(device);
 	int ret;
 
-	fujitsu_laptop->input = devm_input_allocate_device(&device->dev);
-	if (!fujitsu_laptop->input)
+	priv->input = devm_input_allocate_device(&device->dev);
+	if (!priv->input)
 		return -ENOMEM;
 
-	snprintf(fujitsu_laptop->phys, sizeof(fujitsu_laptop->phys),
-		 "%s/video/input0", acpi_device_hid(device));
+	snprintf(priv->phys, sizeof(priv->phys), "%s/video/input0",
+		 acpi_device_hid(device));
 
-	fujitsu_laptop->input->name = acpi_device_name(device);
-	fujitsu_laptop->input->phys = fujitsu_laptop->phys;
-	fujitsu_laptop->input->id.bustype = BUS_HOST;
-	fujitsu_laptop->input->id.product = 0x06;
+	priv->input->name = acpi_device_name(device);
+	priv->input->phys = priv->phys;
+	priv->input->id.bustype = BUS_HOST;
+	priv->input->id.product = 0x06;
 
 	dmi_check_system(fujitsu_laptop_dmi_table);
-	ret = sparse_keymap_setup(fujitsu_laptop->input, keymap, NULL);
+	ret = sparse_keymap_setup(priv->input, keymap, NULL);
 	if (ret)
 		return ret;
 
-	return input_register_device(fujitsu_laptop->input);
+	return input_register_device(priv->input);
 }
 
 static int fujitsu_laptop_platform_add(void)
@@ -885,11 +885,11 @@  static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 
 static int acpi_fujitsu_laptop_remove(struct acpi_device *device)
 {
-	struct fujitsu_laptop *fujitsu_laptop = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = acpi_driver_data(device);
 
 	fujitsu_laptop_platform_remove();
 
-	kfifo_free(&fujitsu_laptop->fifo);
+	kfifo_free(&priv->fifo);
 
 	return 0;
 }