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

[Joe Perches <joe@perches.com>]

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()