Re: [PATCH 2/4] platform/x86: fujitsu-laptop: move keycode processing to separate functions

[Jonathan Woithe]

On Wed, Jan 11, 2017 at 09:59:31AM +0100, Micha?? K??pie?? wrote:
> acpi_fujitsu_hotkey_notify() is pretty deeply nested, which hurts
> readability.  Move the keycode processing part to two separate functions
> to make the code easier to understand and save a few line breaks.
> Rename variable keycode_r to keycode as there is no longer any need to
> differentiate between the two.  Tweak indentations to make checkpatch
> happy.
> 
> Signed-off-by: Micha?? K??pie?? <kernel@xxxxxxxxxx>

Acked-by: Jonathan Woithe <jwoithe@xxxxxxxxxx>

> ---
>  drivers/platform/x86/fujitsu-laptop.c | 76 ++++++++++++++++++++---------------
>  1 file changed, 43 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index c2022f8af51b..e57d3724d2ce 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -1030,12 +1030,48 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device)
>  	return 0;
>  }
>  
> +static void acpi_fujitsu_hotkey_press(int keycode)
> +{
> +	struct input_dev *input = fujitsu_hotkey->input;
> +	int status;
> +
> +	vdbg_printk(FUJLAPTOP_DBG_TRACE,
> +		    "Push keycode into ringbuffer [%d]\n", keycode);
> +	status = kfifo_in_locked(&fujitsu_hotkey->fifo,
> +				 (unsigned char *)&keycode, sizeof(keycode),
> +				 &fujitsu_hotkey->fifo_lock);
> +	if (status != sizeof(keycode)) {
> +		vdbg_printk(FUJLAPTOP_DBG_WARN,
> +			    "Could not push keycode [0x%x]\n", keycode);
> +	} else {
> +		input_report_key(input, keycode, 1);
> +		input_sync(input);
> +	}
> +}
> +
> +static void acpi_fujitsu_hotkey_release(void)
> +{
> +	struct input_dev *input = fujitsu_hotkey->input;
> +	int keycode, status;
> +
> +	while ((status = kfifo_out_locked(&fujitsu_hotkey->fifo,
> +					  (unsigned char *)&keycode,
> +					  sizeof(keycode),
> +					  &fujitsu_hotkey->fifo_lock))
> +					  == sizeof(keycode)) {
> +		input_report_key(input, keycode, 0);
> +		input_sync(input);
> +		vdbg_printk(FUJLAPTOP_DBG_TRACE,
> +			    "Pop keycode from ringbuffer [%d]\n", keycode);
> +	}
> +}
> +
>  static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
>  {
>  	struct input_dev *input;
> -	int keycode, keycode_r;
> +	int keycode;
>  	unsigned int irb = 1;
> -	int i, status;
> +	int i;
>  
>  	input = fujitsu_hotkey->input;
>  
> @@ -1083,37 +1119,11 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
>  			keycode = -1;
>  			break;
>  		}
> -		if (keycode > 0) {
> -			vdbg_printk(FUJLAPTOP_DBG_TRACE,
> -				"Push keycode into ringbuffer [%d]\n",
> -				keycode);
> -			status = kfifo_in_locked(&fujitsu_hotkey->fifo,
> -					   (unsigned char *)&keycode,
> -					   sizeof(keycode),
> -					   &fujitsu_hotkey->fifo_lock);
> -			if (status != sizeof(keycode)) {
> -				vdbg_printk(FUJLAPTOP_DBG_WARN,
> -				    "Could not push keycode [0x%x]\n",
> -				    keycode);
> -			} else {
> -				input_report_key(input, keycode, 1);
> -				input_sync(input);
> -			}
> -		} else if (keycode == 0) {
> -			while ((status =
> -				kfifo_out_locked(
> -				 &fujitsu_hotkey->fifo,
> -				 (unsigned char *) &keycode_r,
> -				 sizeof(keycode_r),
> -				 &fujitsu_hotkey->fifo_lock))
> -				 == sizeof(keycode_r)) {
> -				input_report_key(input, keycode_r, 0);
> -				input_sync(input);
> -				vdbg_printk(FUJLAPTOP_DBG_TRACE,
> -				  "Pop keycode from ringbuffer [%d]\n",
> -				  keycode_r);
> -			}
> -		}
> +
> +		if (keycode > 0)
> +			acpi_fujitsu_hotkey_press(keycode);
> +		else if (keycode == 0)
> +			acpi_fujitsu_hotkey_release();
>  	}
>  
>  	/* On some models (first seen on the Skylake-based Lifebook
> -- 
> 2.11.0

--