Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1559730 > unrolled thread

[PATCH v2 0/6] Move dell-led to drivers/platform/x86

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień <kernel@kempniu.pl> - 2017-01-16 14:30 +0100
    [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set() Michał Kępień <kernel@kempniu.pl> - 2017-01-16 14:30 +0100
    [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień <kernel@kempniu.pl> - 2017-01-17 08:20 +0100
      Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style  issues Joe Perches <joe@perches.com> - 2017-01-17 09:30 +0100
        Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style  issues Michał Kępień <kernel@kempniu.pl> - 2017-01-17 10:30 +0100
          Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style  issues Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2017-01-17 22:30 +0100
      Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style  issues Pavel Machek <pavel@ucw.cz> - 2017-01-17 12:10 +0100
      Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-18 20:10 +0100
    Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2017-01-17 22:30 +0100

#1559730 — [PATCH v2 0/6] Move dell-led to drivers/platform/x86

This patch series moves the dell-led driver from the LED subsystem to
the x86 platform driver subsystem.

The original motivation behind this effort was to move all code using
the dell-smbios module to the x86 platform driver subsystem.  While I
was investigating the possibilities to do that, it quickly emerged that
dell-led can and in fact should be moved to the x86 platform driver
subsystem in its entirety.

dell-led consists of two major parts:

  - the part exposing a microphone mute LED interface, introduced in
    db6d8cc00773 ("dell-led: add mic mute led interface"); this
    interface is used by sound/pci/hda/dell_wmi_helper.c; while the
    original implementation used a WMI interface, it was changed to use
    dell-smbios in cf0d7ea33596 ("dell-led: use dell_smbios_find_token()
    for finding mic DMI tokens") and 0c41a08e131d ("dell-led: use
    dell_smbios_send_request() for performing SMBIOS calls"),

  - the part handling an activity LED present in Dell Latitude 2100
    netbooks, introduced in 72dcd8d08aca ("leds: Add Dell Business Class
    Netbook LED driver"); it binds to a specific WMI GUID and then
    registers a LED device which is controlled using WMI (i.e. it is
    essentially a WMI driver).

Patches 1 and 2 clean up the microphone mute LED interface to minimize
the amount of code moved around.

Patch 3 updates a variable name in sound/pci/hda/dell_wmi_helper.c so
that it better matches that variable's role.

Patch 4 moves the microphone mute LED interface to
drivers/platform/x86/dell-laptop.c, effectively causing
sound/pci/hda/dell_wmi_helper.c to depend on CONFIG_DELL_LAPTOP instead
of CONFIG_LEDS_DELL_NETBOOKS.

Patch 5 reverts dell-led to the state it was in after its initial commit
72dcd8d08aca ("leds: Add Dell Business Class Netbook LED driver") by
removing all remnants of the microphone mute LED handling code.

Patch 6 moves all that is left of dell-led (i.e. the activity LED part,
as originally implemented), to a new module which is placed in
drivers/platform/x86/dell-wmi-led.c.

As all patches except patch 3 in this series affect the LED subsystem,
the series is based on linux-leds/for-4.11.

Anthony, I would be grateful if you could test this patch series on the
Dell machines you have access to that were previously supported by
dell-led as Jacek needs a Tested-by from someone to sign off on these
changes.  Please note the Kconfig option rename done by the last patch.
Thanks!

Changes from v1:

  - Squash patches 2-4 from v1 into a single patch (#2 in v2).

  - Add patch 3.

  - Fix subject pattern in patch 4.

  - Slight commit message adjustments, including fixing a typo
    ("COFIG_LEDS_DELL_NETBOOKS") in patch 6.

  - Remove the name of the module's source file from the header comment
    in drivers/platform/x86/dell-wmi-led.c to avoid the need to update
    it in the future.

 drivers/leds/Kconfig                               |  9 ---
 drivers/leds/Makefile                              |  1 -
 drivers/platform/x86/Kconfig                       |  8 +++
 drivers/platform/x86/Makefile                      |  1 +
 drivers/platform/x86/dell-laptop.c                 | 28 ++++++++
 .../dell-led.c => platform/x86/dell-wmi-led.c}     | 75 ++--------------------
 include/linux/dell-led.h                           |  6 +-
 sound/pci/hda/dell_wmi_helper.c                    | 30 ++++-----
 8 files changed, 60 insertions(+), 98 deletions(-)
 rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (73%)

-- 
2.11.0

[toc] | [next] | [standalone]

#1559732 — [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set()

As dell_micmute_led_set() no longer uses the dell_wmi_perform_query()
method, which was removed in commit 0c41a08e131d ("dell-led: use
dell_smbios_send_request() for performing SMBIOS calls"), the
DELL_APP_GUID check is redundant and thus can be safely removed.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/leds/dell-led.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index b3d6e9c15cf9..e8e8f67224c1 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -51,9 +51,6 @@ static int dell_micmute_led_set(int state)
 	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 
-	if (!wmi_has_guid(DELL_APP_GUID))
-		return -ENODEV;
-
 	if (state == 0)
 		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
 	else if (state == 1)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1560294 — [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues

Fix coding style issues in dell-wmi-led which checkpatch complains about
to make sure the module gets a clean start in the x86 platform driver
subsystem.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
This is an extra patch that Jacek asked for [1].

[1] https://lkml.org/lkml/2017/1/16/631

 drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
index d0232c7f1909..8753c4fc36b8 100644
--- a/drivers/platform/x86/dell-wmi-led.c
+++ b/drivers/platform/x86/dell-wmi-led.c
@@ -46,21 +46,16 @@ struct bios_args {
 	unsigned char off_time;
 };
 
-static int dell_led_perform_fn(u8 length,
-		u8 result_code,
-		u8 device_id,
-		u8 command,
-		u8 on_time,
-		u8 off_time)
+static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
+			       u8 command, u8 on_time, u8 off_time)
 {
-	struct bios_args *bios_return;
-	u8 return_code;
-	union acpi_object *obj;
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct bios_args *bios_return, args;
 	struct acpi_buffer input;
+	union acpi_object *obj;
 	acpi_status status;
+	u8 return_code;
 
-	struct bios_args args;
 	args.length = length;
 	args.result_code = result_code;
 	args.device_id = device_id;
@@ -71,11 +66,7 @@ static int dell_led_perform_fn(u8 length,
 	input.length = sizeof(struct bios_args);
 	input.pointer = &args;
 
-	status = wmi_evaluate_method(DELL_LED_BIOS_GUID,
-		1,
-		1,
-		&input,
-		&output);
+	status = wmi_evaluate_method(DELL_LED_BIOS_GUID, 1, 1, &input, &output);
 
 	if (ACPI_FAILURE(status))
 		return status;
@@ -84,7 +75,7 @@ static int dell_led_perform_fn(u8 length,
 
 	if (!obj)
 		return -EINVAL;
-	else if (obj->type != ACPI_TYPE_BUFFER) {
+	if (obj->type != ACPI_TYPE_BUFFER) {
 		kfree(obj);
 		return -EINVAL;
 	}
@@ -117,8 +108,7 @@ static int led_off(void)
 		0);			/* not used */
 }
 
-static int led_blink(unsigned char on_eighths,
-		unsigned char off_eighths)
+static int led_blink(unsigned char on_eighths, unsigned char off_eighths)
 {
 	return dell_led_perform_fn(5,	/* Length of command */
 		INTERFACE_ERROR,	/* Init to  INTERFACE_ERROR */
@@ -129,7 +119,7 @@ static int led_blink(unsigned char on_eighths,
 }
 
 static void dell_led_set(struct led_classdev *led_cdev,
-		enum led_brightness value)
+			 enum led_brightness value)
 {
 	if (value == LED_OFF)
 		led_off();
@@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
 }
 
 static int dell_led_blink(struct led_classdev *led_cdev,
-		unsigned long *delay_on,
-		unsigned long *delay_off)
+			  unsigned long *delay_on, unsigned long *delay_off)
 {
 	unsigned long on_eighths;
 	unsigned long off_eighths;
 
-	/* The Dell LED delay is based on 125ms intervals.
-	   Need to round up to next interval. */
+	/*
+	 * The Dell LED delay is based on 125ms intervals.
+	 * Need to round up to next interval.
+	 */
 
 	on_eighths = (*delay_on + 124) / 125;
-	if (0 == on_eighths)
+	if (on_eighths == 0)
 		on_eighths = 1;
 	if (on_eighths > 255)
 		on_eighths = 255;
 	*delay_on = on_eighths * 125;
 
 	off_eighths = (*delay_off + 124) / 125;
-	if (0 == off_eighths)
+	if (off_eighths == 0)
 		off_eighths = 1;
 	if (off_eighths > 255)
 		off_eighths = 255;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1560344 — Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues

On Tue, 2017-01-17 at 08:17 +0100, Michał Kępień wrote:
> Fix coding style issues in dell-wmi-led which checkpatch complains about
> to make sure the module gets a clean start in the x86 platform driver
> subsystem.

trivia:

> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
> This is an extra patch that Jacek asked for [1].
> 
> [1] https://lkml.org/lkml/2017/1/16/631
> 
>  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
[]
> @@ -46,21 +46,16 @@ struct bios_args {
>  	unsigned char off_time;
>  };
>  
> -static int dell_led_perform_fn(u8 length,
> -		u8 result_code,
> -		u8 device_id,
> -		u8 command,
> -		u8 on_time,
> -		u8 off_time)
> +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
> +			       u8 command, u8 on_time, u8 off_time)
>  {
> -	struct bios_args *bios_return;
> -	u8 return_code;
> -	union acpi_object *obj;
>  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct bios_args *bios_return, args;
>  	struct acpi_buffer input;
> +	union acpi_object *obj;
>  	acpi_status status;
> +	u8 return_code;
>  
> -	struct bios_args args;
>  	args.length = length;
>  	args.result_code = result_code;
>  	args.device_id = device_id;

This declaration might be nicer using

	struct bios_args args = {
		.length = length,
		.result_code = result_code,
		.device_id = device_id,
		[...]
	};

[]

> @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
>  }
>  
>  static int dell_led_blink(struct led_classdev *led_cdev,
> -		unsigned long *delay_on,
> -		unsigned long *delay_off)
> +			  unsigned long *delay_on, unsigned long *delay_off)
>  {
>  	unsigned long on_eighths;
>  	unsigned long off_eighths;
>  
> -	/* The Dell LED delay is based on 125ms intervals.
> -	   Need to round up to next interval. */
> +	/*
> +	 * The Dell LED delay is based on 125ms intervals.
> +	 * Need to round up to next interval.
> +	 */
>  
>  	on_eighths = (*delay_on + 124) / 125;
> -	if (0 == on_eighths)
> +	if (on_eighths == 0)
>  		on_eighths = 1;
>  	if (on_eighths > 255)
>  		on_eighths = 255;
>  	*delay_on = on_eighths * 125;
>  
>  	off_eighths = (*delay_off + 124) / 125;
> -	if (0 == off_eighths)
> +	if (off_eighths == 0)
>  		off_eighths = 1;
>  	if (off_eighths > 255)
>  		off_eighths = 255;

These could use DIV_ROUND_UP and clamp()

[toc] | [prev] | [next] | [standalone]

#1560372 — Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues

> On Tue, 2017-01-17 at 08:17 +0100, Michał Kępień wrote:
> > Fix coding style issues in dell-wmi-led which checkpatch complains about
> > to make sure the module gets a clean start in the x86 platform driver
> > subsystem.
> 
> trivia:
> 
> > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > ---
> > This is an extra patch that Jacek asked for [1].
> > 
> > [1] https://lkml.org/lkml/2017/1/16/631
> > 
> >  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
> >  1 file changed, 16 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
> []
> > @@ -46,21 +46,16 @@ struct bios_args {
> >  	unsigned char off_time;
> >  };
> >  
> > -static int dell_led_perform_fn(u8 length,
> > -		u8 result_code,
> > -		u8 device_id,
> > -		u8 command,
> > -		u8 on_time,
> > -		u8 off_time)
> > +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
> > +			       u8 command, u8 on_time, u8 off_time)
> >  {
> > -	struct bios_args *bios_return;
> > -	u8 return_code;
> > -	union acpi_object *obj;
> >  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> > +	struct bios_args *bios_return, args;
> >  	struct acpi_buffer input;
> > +	union acpi_object *obj;
> >  	acpi_status status;
> > +	u8 return_code;
> >  
> > -	struct bios_args args;
> >  	args.length = length;
> >  	args.result_code = result_code;
> >  	args.device_id = device_id;
> 
> This declaration might be nicer using
> 
> 	struct bios_args args = {
> 		.length = length,
> 		.result_code = result_code,
> 		.device_id = device_id,
> 		[...]
> 	};
> 
> []
> 
> > @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
> >  }
> >  
> >  static int dell_led_blink(struct led_classdev *led_cdev,
> > -		unsigned long *delay_on,
> > -		unsigned long *delay_off)
> > +			  unsigned long *delay_on, unsigned long *delay_off)
> >  {
> >  	unsigned long on_eighths;
> >  	unsigned long off_eighths;
> >  
> > -	/* The Dell LED delay is based on 125ms intervals.
> > -	   Need to round up to next interval. */
> > +	/*
> > +	 * The Dell LED delay is based on 125ms intervals.
> > +	 * Need to round up to next interval.
> > +	 */
> >  
> >  	on_eighths = (*delay_on + 124) / 125;
> > -	if (0 == on_eighths)
> > +	if (on_eighths == 0)
> >  		on_eighths = 1;
> >  	if (on_eighths > 255)
> >  		on_eighths = 255;
> >  	*delay_on = on_eighths * 125;
> >  
> >  	off_eighths = (*delay_off + 124) / 125;
> > -	if (0 == off_eighths)
> > +	if (off_eighths == 0)
> >  		off_eighths = 1;
> >  	if (off_eighths > 255)
> >  		off_eighths = 255;
> 
> These could use DIV_ROUND_UP and clamp()

Thanks for taking a look, Joe, I can certainly fix these.

Jacek, as resending an updated version of this patch with Joe's
suggestions taken into account would be even more confusing than the
"PATCH v2 6+/6" subject I already resorted to, I suggest the following:
if this series goes to v3, I will include an updated version of this
patch in v3, but in case the remaining patches get acked in their
current shape by all maintainers, I will send an updated version of this
extra patch separately, after the rest of the series gets applied.  Does
this sound reasonable?

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]

#1561032 — Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues

Hi Michał,

On 01/17/2017 10:19 AM, Michał Kępień wrote:
>> On Tue, 2017-01-17 at 08:17 +0100, Michał Kępień wrote:
>>> Fix coding style issues in dell-wmi-led which checkpatch complains about
>>> to make sure the module gets a clean start in the x86 platform driver
>>> subsystem.
>>
>> trivia:
>>
>>> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
>>> ---
>>> This is an extra patch that Jacek asked for [1].
>>>
>>> [1] https://lkml.org/lkml/2017/1/16/631
>>>
>>>  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
>>>  1 file changed, 16 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
>> []
>>> @@ -46,21 +46,16 @@ struct bios_args {
>>>  	unsigned char off_time;
>>>  };
>>>  
>>> -static int dell_led_perform_fn(u8 length,
>>> -		u8 result_code,
>>> -		u8 device_id,
>>> -		u8 command,
>>> -		u8 on_time,
>>> -		u8 off_time)
>>> +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
>>> +			       u8 command, u8 on_time, u8 off_time)
>>>  {
>>> -	struct bios_args *bios_return;
>>> -	u8 return_code;
>>> -	union acpi_object *obj;
>>>  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +	struct bios_args *bios_return, args;
>>>  	struct acpi_buffer input;
>>> +	union acpi_object *obj;
>>>  	acpi_status status;
>>> +	u8 return_code;
>>>  
>>> -	struct bios_args args;
>>>  	args.length = length;
>>>  	args.result_code = result_code;
>>>  	args.device_id = device_id;
>>
>> This declaration might be nicer using
>>
>> 	struct bios_args args = {
>> 		.length = length,
>> 		.result_code = result_code,
>> 		.device_id = device_id,
>> 		[...]
>> 	};
>>
>> []
>>
>>> @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
>>>  }
>>>  
>>>  static int dell_led_blink(struct led_classdev *led_cdev,
>>> -		unsigned long *delay_on,
>>> -		unsigned long *delay_off)
>>> +			  unsigned long *delay_on, unsigned long *delay_off)
>>>  {
>>>  	unsigned long on_eighths;
>>>  	unsigned long off_eighths;
>>>  
>>> -	/* The Dell LED delay is based on 125ms intervals.
>>> -	   Need to round up to next interval. */
>>> +	/*
>>> +	 * The Dell LED delay is based on 125ms intervals.
>>> +	 * Need to round up to next interval.
>>> +	 */
>>>  
>>>  	on_eighths = (*delay_on + 124) / 125;
>>> -	if (0 == on_eighths)
>>> +	if (on_eighths == 0)
>>>  		on_eighths = 1;
>>>  	if (on_eighths > 255)
>>>  		on_eighths = 255;
>>>  	*delay_on = on_eighths * 125;
>>>  
>>>  	off_eighths = (*delay_off + 124) / 125;
>>> -	if (0 == off_eighths)
>>> +	if (off_eighths == 0)
>>>  		off_eighths = 1;
>>>  	if (off_eighths > 255)
>>>  		off_eighths = 255;
>>
>> These could use DIV_ROUND_UP and clamp()
> 
> Thanks for taking a look, Joe, I can certainly fix these.
> 
> Jacek, as resending an updated version of this patch with Joe's
> suggestions taken into account would be even more confusing than the
> "PATCH v2 6+/6" subject I already resorted to, I suggest the following:
> if this series goes to v3, I will include an updated version of this
> patch in v3, but in case the remaining patches get acked in their
> current shape by all maintainers, I will send an updated version of this
> extra patch separately, after the rest of the series gets applied.  Does
> this sound reasonable?

Sure. I'll merge whole patch set after getting acks from sound
and x86 platform drivers maintainers.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1560502 — Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues

On Tue 2017-01-17 08:17:14, Michał Kępień wrote:
> Fix coding style issues in dell-wmi-led which checkpatch complains about
> to make sure the module gets a clean start in the x86 platform driver
> subsystem.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>

Acked-by: Pavel Machek <pavel@ucw.cz>

> ---
> This is an extra patch that Jacek asked for [1].
> 
> [1] https://lkml.org/lkml/2017/1/16/631
> 
>  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
> index d0232c7f1909..8753c4fc36b8 100644
> --- a/drivers/platform/x86/dell-wmi-led.c
> +++ b/drivers/platform/x86/dell-wmi-led.c
> @@ -46,21 +46,16 @@ struct bios_args {
>  	unsigned char off_time;
>  };
>  
> -static int dell_led_perform_fn(u8 length,
> -		u8 result_code,
> -		u8 device_id,
> -		u8 command,
> -		u8 on_time,
> -		u8 off_time)
> +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
> +			       u8 command, u8 on_time, u8 off_time)
>  {
> -	struct bios_args *bios_return;
> -	u8 return_code;
> -	union acpi_object *obj;
>  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct bios_args *bios_return, args;
>  	struct acpi_buffer input;
> +	union acpi_object *obj;
>  	acpi_status status;
> +	u8 return_code;
>  
> -	struct bios_args args;
>  	args.length = length;
>  	args.result_code = result_code;
>  	args.device_id = device_id;
> @@ -71,11 +66,7 @@ static int dell_led_perform_fn(u8 length,
>  	input.length = sizeof(struct bios_args);
>  	input.pointer = &args;
>  
> -	status = wmi_evaluate_method(DELL_LED_BIOS_GUID,
> -		1,
> -		1,
> -		&input,
> -		&output);
> +	status = wmi_evaluate_method(DELL_LED_BIOS_GUID, 1, 1, &input, &output);
>  
>  	if (ACPI_FAILURE(status))
>  		return status;
> @@ -84,7 +75,7 @@ static int dell_led_perform_fn(u8 length,
>  
>  	if (!obj)
>  		return -EINVAL;
> -	else if (obj->type != ACPI_TYPE_BUFFER) {
> +	if (obj->type != ACPI_TYPE_BUFFER) {
>  		kfree(obj);
>  		return -EINVAL;
>  	}
> @@ -117,8 +108,7 @@ static int led_off(void)
>  		0);			/* not used */
>  }
>  
> -static int led_blink(unsigned char on_eighths,
> -		unsigned char off_eighths)
> +static int led_blink(unsigned char on_eighths, unsigned char off_eighths)
>  {
>  	return dell_led_perform_fn(5,	/* Length of command */
>  		INTERFACE_ERROR,	/* Init to  INTERFACE_ERROR */
> @@ -129,7 +119,7 @@ static int led_blink(unsigned char on_eighths,
>  }
>  
>  static void dell_led_set(struct led_classdev *led_cdev,
> -		enum led_brightness value)
> +			 enum led_brightness value)
>  {
>  	if (value == LED_OFF)
>  		led_off();
> @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
>  }
>  
>  static int dell_led_blink(struct led_classdev *led_cdev,
> -		unsigned long *delay_on,
> -		unsigned long *delay_off)
> +			  unsigned long *delay_on, unsigned long *delay_off)
>  {
>  	unsigned long on_eighths;
>  	unsigned long off_eighths;
>  
> -	/* The Dell LED delay is based on 125ms intervals.
> -	   Need to round up to next interval. */
> +	/*
> +	 * The Dell LED delay is based on 125ms intervals.
> +	 * Need to round up to next interval.
> +	 */
>  
>  	on_eighths = (*delay_on + 124) / 125;
> -	if (0 == on_eighths)
> +	if (on_eighths == 0)
>  		on_eighths = 1;
>  	if (on_eighths > 255)
>  		on_eighths = 255;
>  	*delay_on = on_eighths * 125;
>  
>  	off_eighths = (*delay_off + 124) / 125;
> -	if (0 == off_eighths)
> +	if (off_eighths == 0)
>  		off_eighths = 1;
>  	if (off_eighths > 255)
>  		off_eighths = 255;
> -- 
> 2.11.0

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1562076 — Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues

On Tue, Jan 17, 2017 at 9:17 AM, Michał Kępień <kernel@kempniu.pl> wrote:
> Fix coding style issues in dell-wmi-led which checkpatch complains about
> to make sure the module gets a clean start in the x86 platform driver
> subsystem.

> +       status = wmi_evaluate_method(DELL_LED_BIOS_GUID, 1, 1, &input, &output);
>

You may remove extra line.

>         if (ACPI_FAILURE(status))
>                 return status;

> -       if (0 == on_eighths)
> +       if (on_eighths == 0)
>                 on_eighths = 1;
>         if (on_eighths > 255)
>                 on_eighths = 255;

> -       if (0 == off_eighths)
> +       if (off_eighths == 0)
>                 off_eighths = 1;
>         if (off_eighths > 255)
>                 off_eighths = 255;

Obviously they both are custom clamp{_t}().

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1561037

Hi Darren,

Are you OK with the patches 4/6 and 6/6? If so, could you give
your ack so that I could apply the whole patch set to the LED tree?

Best regards,
Jacek Anaszewski

On 01/16/2017 02:21 PM, Michał Kępień wrote:
> This patch series moves the dell-led driver from the LED subsystem to
> the x86 platform driver subsystem.
> 
> The original motivation behind this effort was to move all code using
> the dell-smbios module to the x86 platform driver subsystem.  While I
> was investigating the possibilities to do that, it quickly emerged that
> dell-led can and in fact should be moved to the x86 platform driver
> subsystem in its entirety.
> 
> dell-led consists of two major parts:
> 
>   - the part exposing a microphone mute LED interface, introduced in
>     db6d8cc00773 ("dell-led: add mic mute led interface"); this
>     interface is used by sound/pci/hda/dell_wmi_helper.c; while the
>     original implementation used a WMI interface, it was changed to use
>     dell-smbios in cf0d7ea33596 ("dell-led: use dell_smbios_find_token()
>     for finding mic DMI tokens") and 0c41a08e131d ("dell-led: use
>     dell_smbios_send_request() for performing SMBIOS calls"),
> 
>   - the part handling an activity LED present in Dell Latitude 2100
>     netbooks, introduced in 72dcd8d08aca ("leds: Add Dell Business Class
>     Netbook LED driver"); it binds to a specific WMI GUID and then
>     registers a LED device which is controlled using WMI (i.e. it is
>     essentially a WMI driver).
> 
> Patches 1 and 2 clean up the microphone mute LED interface to minimize
> the amount of code moved around.
> 
> Patch 3 updates a variable name in sound/pci/hda/dell_wmi_helper.c so
> that it better matches that variable's role.
> 
> Patch 4 moves the microphone mute LED interface to
> drivers/platform/x86/dell-laptop.c, effectively causing
> sound/pci/hda/dell_wmi_helper.c to depend on CONFIG_DELL_LAPTOP instead
> of CONFIG_LEDS_DELL_NETBOOKS.
> 
> Patch 5 reverts dell-led to the state it was in after its initial commit
> 72dcd8d08aca ("leds: Add Dell Business Class Netbook LED driver") by
> removing all remnants of the microphone mute LED handling code.
> 
> Patch 6 moves all that is left of dell-led (i.e. the activity LED part,
> as originally implemented), to a new module which is placed in
> drivers/platform/x86/dell-wmi-led.c.
> 
> As all patches except patch 3 in this series affect the LED subsystem,
> the series is based on linux-leds/for-4.11.
> 
> Anthony, I would be grateful if you could test this patch series on the
> Dell machines you have access to that were previously supported by
> dell-led as Jacek needs a Tested-by from someone to sign off on these
> changes.  Please note the Kconfig option rename done by the last patch.
> Thanks!
> 
> Changes from v1:
> 
>   - Squash patches 2-4 from v1 into a single patch (#2 in v2).
> 
>   - Add patch 3.
> 
>   - Fix subject pattern in patch 4.
> 
>   - Slight commit message adjustments, including fixing a typo
>     ("COFIG_LEDS_DELL_NETBOOKS") in patch 6.
> 
>   - Remove the name of the module's source file from the header comment
>     in drivers/platform/x86/dell-wmi-led.c to avoid the need to update
>     it in the future.
> 
>  drivers/leds/Kconfig                               |  9 ---
>  drivers/leds/Makefile                              |  1 -
>  drivers/platform/x86/Kconfig                       |  8 +++
>  drivers/platform/x86/Makefile                      |  1 +
>  drivers/platform/x86/dell-laptop.c                 | 28 ++++++++
>  .../dell-led.c => platform/x86/dell-wmi-led.c}     | 75 ++--------------------
>  include/linux/dell-led.h                           |  6 +-
>  sound/pci/hda/dell_wmi_helper.c                    | 30 ++++-----
>  8 files changed, 60 insertions(+), 98 deletions(-)
>  rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (73%)
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web