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ń <ker...@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()

Reply via email to