From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751348AbdAQVWu (ORCPT ); Tue, 17 Jan 2017 16:22:50 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35749 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751000AbdAQVV1 (ORCPT ); Tue, 17 Jan 2017 16:21:27 -0500 Subject: Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues To: =?UTF-8?B?TWljaGHFgiBLxJlwaWXFhA==?= , Joe Perches References: <20170116132204.6421-1-kernel@kempniu.pl> <20170117071714.21594-1-kernel@kempniu.pl> <1484641283.9538.5.camel@perches.com> <20170117091925.GA1042@ozzy.nask.waw.pl> Cc: Richard Purdie , Pavel Machek , =?UTF-8?Q?Pali_Roh=c3=a1r?= , Darren Hart , Jaroslav Kysela , Takashi Iwai , Andy Shevchenko , Anthony Wong , linux-leds@vger.kernel.org, platform-driver-x86@vger.kernel.org, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org From: Jacek Anaszewski X-Enigmail-Draft-Status: N1110 Message-ID: Date: Tue, 17 Jan 2017 22:20:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170117091925.GA1042@ozzy.nask.waw.pl> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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ń >>> --- >>> 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