linux-kernel - Re: [PATCH] lis3lv02d: support both one- and two-byte sensors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <499218C4.4080808@tremplin-utc.net>
Date:	Wed, 11 Feb 2009 01:16:04 +0100
From:	Éric Piel <Eric.Piel@...mplin-utc.net>
To:	Giuseppe Bilotta <giuseppe.bilotta@...il.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Pavel Machek <pavel@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] lis3lv02d: support both one- and two-byte sensors

Giuseppe Bilotta schreef:

> As pointed out by Eric, there's no need to jump through hoops to obtain
> a signed 16-bit number from a signed 8-bit number.
> 
> This patch is good for -mm
> 
> Note also that this patch is only about 8 vs 16 bit data, so the axis
> orientation for HP notebooks still has to be fixed in a separate patch,
> but at least now I should be able to check it out properly.
> 
Whoops, sorry for not catching it before, this part is not nice:

> @@ -121,6 +125,21 @@ void lis3lv02d_poweron(acpi_handle handle)
>  {
>  	adev.is_on = 1;
>  	adev.init(handle);
> +	adev.read(handle, WHO_AM_I, &adev.whoami);
> +	/* Tell apart LISxLV02Dy from LISx02Dy family by checking the LSB
> +	 * (0x3A vs 0x3B). TODO More sophisticated checks on other registers
> +	 * could be implemented, for example to see if we have 2 or 3 axes, and
> +	 * configure the joystick accordingly.
> +	 */
> +	if (adev.whoami & 1) {
> +		printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
> +		adev.lis3lv02d_read = lis3lv02d_read_8;
> +		adev.mdps_max_val = 128;
> +	} else {
> +		printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
> +		adev.lis3lv02d_read = lis3lv02d_read_16;
> +		adev.mdps_max_val = 2048;
> +	}
>  }
poweron() is called each time the joystick is opened, no need to check 
(and do a printk) so often! You need to move all this check to 
lis3lv02d_add() in hp_accel.c . Moreover, don't do tricky things like 
(adev.whoami & 1): just do a:
switch (adev.whoami) {
LIS302DL_ID:
	printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found.\n");
	adev.lis3lv02d_read = lis3lv02d_read_8;
	adev.mdps_max_val = 128;
	break;
LIS3LV02DL_ID:
	printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found.\n");
	adev.lis3lv02d_read = lis3lv02d_read_16;
	adev.mdps_max_val = 2048;
	break;
default:
	printk(KERN_ERR DRIVER_NAME ": unknown sensor found of type %d.\n", 
adev.whoami);
	return -EINVAL;
}


Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/