Re: [PATCH] Input: appletouch - driver refactoring

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]

On Sun, 2008-10-12 at 16:44 +0200, Stelian Pop wrote:
> The appletouch driver has grown up from supporting only a couple of
> touchpads into supporting many touchpads, which can have different
> number of sensors, different aspect ratios etc.
> 
> This patch cleans up the current driver code and makes it easy to
> support the features of each different touchpad.
> 
> As a side effect, this patch also modifies the 'Y' multiplication factor
> of the 'geyser3' and 'geyser4' touchpads (found on Core Duo and Core2
> Duo MacBook and MacBook Pro laptops) in order to make the touchpad
> output match the aspect ratio of the touchpad (Y factor changed from 43
> to 64).

Looks nice, will give it a test later, just a small comment.


> +static struct atp_info fountain_info = {
> +	.type		= ATP_FOUNTAIN,

> +static struct atp_info geyser1_info = {
> +	.type		= ATP_GEYSER1,

> +static struct atp_info geyser2_info = {
> +	.type		= ATP_GEYSER2,

> +static struct atp_info geyser3_info = {
> +	.type		= ATP_GEYSER3,

> +static struct atp_info geyser4_info = {
> +	.type		= ATP_GEYSER4,

Those have all different types.

>  	/* reorder the sensors values */
> -	if (dev->type == ATP_GEYSER2) {
> +	if (dev->info->type == ATP_GEYSER2) {

> +				if (dev->info->type == ATP_GEYSER2)
> +					dev->info->xsensors = 20;

> -	if (dev->type != ATP_FOUNTAIN) {
> +	if (dev->info->type != ATP_FOUNTAIN) {

And there are only three uses. Do we expect to add new uses of the
types, especially different structs that share the type? Otherwise I'd
suggest to use

if (dev->info == &geyser2_info) and &fountain_info respectively and just
remove the type thing completely.

johannes

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