Re: [PATCH] Fix V8 device information issue

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 1252 bytes --]

Hi! I have just two notes:

On Friday 10 March 2017 07:41:47 Masaki Ota wrote:
...
> +		x_phys = x_pitch * (num_x_electrode - 1); /* In 0.1 mm units */
> +		y_phys = y_pitch * (num_y_electrode - 1); /* In 0.1 mm units */
...
> +		priv->x_res = priv->x_max * 10 / x_phys; /* units / mm */
> +		priv->y_res = priv->y_max * 10 / y_phys; /* units / mm */
> +
> +	} else {
...
> +		x_phys = x_pitch * (num_x_electrode - 1); /* In 0.1 mm units */
> +		y_phys = y_pitch * (num_y_electrode - 1); /* In 0.1 mm units */
> +
> +		priv->x_res = priv->x_max * 10 / x_phys; /* units / mm */
> +		priv->y_res = priv->y_max * 10 / y_phys; /* units / mm */

Looks like above 4 lines are same in both if { } and else { } blocks.
So it can be moved outside of if blocks.

> +	}
>  	return 0;
>  }
>  
> @@ -2490,7 +2515,10 @@ static int alps_update_btn_info_ss4_v2(unsigned char otp[][4],
>  {
>  	unsigned char is_btnless;
>  
> -	is_btnless = (otp[1][1] >> 3) & 0x01;
> +	if (priv->dev_id[2] == 0x28)

This check "dev_id[2] == 0x28" is used on more places. What about
introducing some flag or some boolean macro? Check "dev_id[2] == 0x28"
is magical does not say anything what it is doing...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]