[PATCH 1/4] Input: atmel_mxt_ts - Remove firmware version check

[PATCH 1/4] Input: atmel_mxt_ts - Remove firmware version check

[Joonyoung Shim]

Atmel touchscreen chips have different firmware version with each chip,
so we cannot distinguish attribute of chip by firmware version.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |   21 +++++----------------
 1 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 6264ba8..0986fa4 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -60,11 +60,11 @@
 #define MXT_PROCG_NOISE		22
 #define MXT_PROCI_ONETOUCH	24
 #define MXT_PROCI_TWOTOUCH	27
-#define MXT_SPT_COMMSCONFIG	18	/* firmware ver 21 over */
+#define MXT_SPT_COMMSCONFIG	18
 #define MXT_SPT_GPIOPWM		19
 #define MXT_SPT_SELFTEST	25
 #define MXT_SPT_CTECONFIG	28
-#define MXT_SPT_USERDATA	38	/* firmware ver 21 over */
+#define MXT_SPT_USERDATA	38
 
 /* MXT_GEN_COMMAND field */
 #define MXT_COMMAND_RESET	0
@@ -115,7 +115,7 @@
 #define MXT_TOUCH_XEDGEDIST	27
 #define MXT_TOUCH_YEDGECTRL	28
 #define MXT_TOUCH_YEDGEDIST	29
-#define MXT_TOUCH_JUMPLIMIT	30	/* firmware ver 22 over */
+#define MXT_TOUCH_JUMPLIMIT	30
 
 /* MXT_PROCI_GRIPFACE field */
 #define MXT_GRIPFACE_CTRL	0
@@ -157,7 +157,7 @@
 #define MXT_CTE_MODE		2
 #define MXT_CTE_IDLEGCAFDEPTH	3
 #define MXT_CTE_ACTVGCAFDEPTH	4
-#define MXT_CTE_VOLTAGE		5	/* firmware ver 21 over */
+#define MXT_CTE_VOLTAGE		5
 
 #define MXT_VOLTAGE_DEFAULT	2700000
 #define MXT_VOLTAGE_STEP	10000
@@ -686,7 +686,7 @@ static void mxt_handle_pdata(struct mxt_data *data)
 			MXT_TOUCH_YRANGE_MSB, (pdata->y_size - 1) >> 8);
 
 	/* Set touchscreen voltage */
-	if (data->info.version >= MXT_VER_21 && pdata->voltage) {
+	if (pdata->voltage) {
 		if (pdata->voltage < MXT_VOLTAGE_DEFAULT) {
 			voltage = (MXT_VOLTAGE_DEFAULT - pdata->voltage) /
 				MXT_VOLTAGE_STEP;
@@ -951,19 +951,8 @@ static ssize_t mxt_update_fw_store(struct device *dev,
 					const char *buf, size_t count)
 {
 	struct mxt_data *data = dev_get_drvdata(dev);
-	unsigned int version;
 	int error;
 
-	if (sscanf(buf, "%u", &version) != 1) {
-		dev_err(dev, "Invalid values\n");
-		return -EINVAL;
-	}
-
-	if (data->info.version < MXT_VER_21 || version < MXT_VER_21) {
-		dev_err(dev, "FW update supported starting with version 21\n");
-		return -EINVAL;
-	}
-
 	disable_irq(data->irq);
 
 	error = mxt_load_fw(dev, MXT_FW_NAME);
-- 
1.7.0.4

[PATCH 2/4] Input: atmel_mxt_ts - Support 12bit resolution

[Joonyoung Shim]

Atmel touchscreen chip can support 12bit resolution and this patch
modifies to get maximum x and y size from platform data.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |   50 +++++++++++++++++++++--------
 1 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 0986fa4..0ed0b1f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -192,9 +192,12 @@
 #define MXT_PRESS		(1 << 6)
 #define MXT_DETECT		(1 << 7)
 
+/* Touch orient bits */
+#define MXT_XY_SWITCH		(1 << 0)
+#define MXT_X_INVERT		(1 << 1)
+#define MXT_Y_INVERT		(1 << 2)
+
 /* Touchscreen absolute values */
-#define MXT_MAX_XC		0x3ff
-#define MXT_MAX_YC		0x3ff
 #define MXT_MAX_AREA		0xff
 
 #define MXT_MAX_FINGER		10
@@ -242,6 +245,8 @@ struct mxt_data {
 	struct mxt_info info;
 	struct mxt_finger finger[MXT_MAX_FINGER];
 	unsigned int irq;
+	unsigned int x_size;
+	unsigned int y_size;
 };
 
 static bool mxt_object_readable(unsigned int type)
@@ -541,8 +546,13 @@ static void mxt_input_touchevent(struct mxt_data *data,
 	if (!(status & (MXT_PRESS | MXT_MOVE)))
 		return;
 
-	x = (message->message[1] << 2) | ((message->message[3] & ~0x3f) >> 6);
-	y = (message->message[2] << 2) | ((message->message[3] & ~0xf3) >> 2);
+	x = (message->message[1] << 4) | ((message->message[3] >> 4) & 0xf);
+	y = (message->message[2] << 4) | ((message->message[3] & 0xf));
+	if (data->x_size - 1 < 1024)
+		x = x >> 2;
+	if (data->y_size - 1 < 1024)
+		y = y >> 2;
+
 	area = message->message[4];
 
 	dev_dbg(dev, "[%d] %s x: %d, y: %d, area: %d\n", id,
@@ -837,6 +847,17 @@ static int mxt_initialize(struct mxt_data *data)
 	return 0;
 }
 
+static void mxt_calc_resolution(struct mxt_data *data)
+{
+	if (data->pdata->orient & MXT_XY_SWITCH) {
+		data->x_size = data->pdata->y_size;
+		data->y_size = data->pdata->x_size;
+	} else {
+		data->x_size = data->pdata->x_size;
+		data->y_size = data->pdata->y_size;
+	}
+}
+
 static ssize_t mxt_object_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -1044,31 +1065,32 @@ static int __devinit mxt_probe(struct i2c_client *client,
 	input_dev->open = mxt_input_open;
 	input_dev->close = mxt_input_close;
 
+	data->client = client;
+	data->input_dev = input_dev;
+	data->pdata = pdata;
+	data->irq = client->irq;
+
+	mxt_calc_resolution(data);
+
 	__set_bit(EV_ABS, input_dev->evbit);
 	__set_bit(EV_KEY, input_dev->evbit);
 	__set_bit(BTN_TOUCH, input_dev->keybit);
 
 	/* For single touch */
 	input_set_abs_params(input_dev, ABS_X,
-			     0, MXT_MAX_XC, 0, 0);
+			     0, data->x_size, 0, 0);
 	input_set_abs_params(input_dev, ABS_Y,
-			     0, MXT_MAX_YC, 0, 0);
+			     0, data->y_size, 0, 0);
 
 	/* For multi touch */
 	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
 			     0, MXT_MAX_AREA, 0, 0);
 	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
-			     0, MXT_MAX_XC, 0, 0);
+			     0, data->x_size, 0, 0);
 	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
-			     0, MXT_MAX_YC, 0, 0);
+			     0, data->y_size, 0, 0);
 
 	input_set_drvdata(input_dev, data);
-
-	data->client = client;
-	data->input_dev = input_dev;
-	data->pdata = pdata;
-	data->irq = client->irq;
-
 	i2c_set_clientdata(client, data);
 
 	error = mxt_initialize(data);
-- 
1.7.0.4

[PATCH 3/4] Input: atmel_mxt_ts - Add objects of mXT1386 chip

[Joonyoung Shim]

Atmel mXT1386 chip is operated by atmel_mxt_ts driver and it has some
different objects.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 0ed0b1f..fd1fda5 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -60,11 +60,15 @@
 #define MXT_PROCG_NOISE		22
 #define MXT_PROCI_ONETOUCH	24
 #define MXT_PROCI_TWOTOUCH	27
+#define MXT_PROCI_GRIP		40
+#define MXT_PROCI_PALM		41
 #define MXT_SPT_COMMSCONFIG	18
 #define MXT_SPT_GPIOPWM		19
 #define MXT_SPT_SELFTEST	25
 #define MXT_SPT_CTECONFIG	28
 #define MXT_SPT_USERDATA	38
+#define MXT_SPT_DIGITIZER	43
+#define MXT_SPT_MESSAGECOUNT	44
 
 /* MXT_GEN_COMMAND field */
 #define MXT_COMMAND_RESET	0
@@ -263,6 +267,8 @@ static bool mxt_object_readable(unsigned int type)
 	case MXT_PROCG_NOISE:
 	case MXT_PROCI_ONETOUCH:
 	case MXT_PROCI_TWOTOUCH:
+	case MXT_PROCI_GRIP:
+	case MXT_PROCI_PALM:
 	case MXT_SPT_COMMSCONFIG:
 	case MXT_SPT_GPIOPWM:
 	case MXT_SPT_SELFTEST:
@@ -287,6 +293,8 @@ static bool mxt_object_writable(unsigned int type)
 	case MXT_PROCG_NOISE:
 	case MXT_PROCI_ONETOUCH:
 	case MXT_PROCI_TWOTOUCH:
+	case MXT_PROCI_GRIP:
+	case MXT_PROCI_PALM:
 	case MXT_SPT_GPIOPWM:
 	case MXT_SPT_SELFTEST:
 	case MXT_SPT_CTECONFIG:
-- 
1.7.0.4

Re: [PATCH 2/4] Input: atmel_mxt_ts - Support 12bit resolution

[Iiro Valkonen]

Hi Joonyoung,

my comments below. They a bit late, but looks like the patch is not applied yet.

On 03/07/2011 10:42 AM, Joonyoung Shim wrote:
> Atmel touchscreen chip can support 12bit resolution and this patch
> modifies to get maximum x and y size from platform data.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c |   50 +++++++++++++++++++++--------
>  1 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 0986fa4..0ed0b1f 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -192,9 +192,12 @@
>  #define MXT_PRESS              (1 << 6)
>  #define MXT_DETECT             (1 << 7)
> 
> +/* Touch orient bits */
> +#define MXT_XY_SWITCH          (1 << 0)
> +#define MXT_X_INVERT           (1 << 1)
> +#define MXT_Y_INVERT           (1 << 2)
> +
>  /* Touchscreen absolute values */
> -#define MXT_MAX_XC             0x3ff
> -#define MXT_MAX_YC             0x3ff
>  #define MXT_MAX_AREA           0xff
> 
>  #define MXT_MAX_FINGER         10
> @@ -242,6 +245,8 @@ struct mxt_data {
>         struct mxt_info info;
>         struct mxt_finger finger[MXT_MAX_FINGER];
>         unsigned int irq;
> +       unsigned int x_size;
> +       unsigned int y_size;
>  };
> 
>  static bool mxt_object_readable(unsigned int type)
> @@ -541,8 +546,13 @@ static void mxt_input_touchevent(struct mxt_data *data,
>         if (!(status & (MXT_PRESS | MXT_MOVE)))
>                 return;
> 
> -       x = (message->message[1] << 2) | ((message->message[3] & ~0x3f) >> 6);
> -       y = (message->message[2] << 2) | ((message->message[3] & ~0xf3) >> 2);
> +       x = (message->message[1] << 4) | ((message->message[3] >> 4) & 0xf);
> +       y = (message->message[2] << 4) | ((message->message[3] & 0xf));
> +       if (data->x_size - 1 < 1024)
> +               x = x >> 2;
> +       if (data->y_size - 1 < 1024)
> +               y = y >> 2;
> +

Not a big deal, but I would just store x_max_value instead of x_size to avoid
the subtraction. Same for y.

>         area = message->message[4];
> 
>         dev_dbg(dev, "[%d] %s x: %d, y: %d, area: %d\n", id,
> @@ -837,6 +847,17 @@ static int mxt_initialize(struct mxt_data *data)
>         return 0;
>  }
> 
> +static void mxt_calc_resolution(struct mxt_data *data)
> +{
> +       if (data->pdata->orient & MXT_XY_SWITCH) {
> +               data->x_size = data->pdata->y_size;
> +               data->y_size = data->pdata->x_size;
> +       } else {
> +               data->x_size = data->pdata->x_size;
> +               data->y_size = data->pdata->y_size;
> +       }
> +}
> +

What's the reason for this? If we have set the x/y switch in the config, then 
we probably want to swap the axes. Or is this axis swap something that should be
done on upper layers? Even so, then we shouldn't have the MXT_XY_SWITCH bit set
in the config, and we could just say "data->x_max_value = data->pdata->xsize - 1"
(and same for y) in the probe function. We wouldn't need Touch orient bit defines
either.

>  static ssize_t mxt_object_show(struct device *dev,
>                                     struct device_attribute *attr, char *buf)
>  {
> @@ -1044,31 +1065,32 @@ static int __devinit mxt_probe(struct i2c_client *client,
>         input_dev->open = mxt_input_open;
>         input_dev->close = mxt_input_close;
> 
> +       data->client = client;
> +       data->input_dev = input_dev;
> +       data->pdata = pdata;
> +       data->irq = client->irq;
> +
> +       mxt_calc_resolution(data);
> +
>         __set_bit(EV_ABS, input_dev->evbit);
>         __set_bit(EV_KEY, input_dev->evbit);
>         __set_bit(BTN_TOUCH, input_dev->keybit);
> 
>         /* For single touch */
>         input_set_abs_params(input_dev, ABS_X,
> -                            0, MXT_MAX_XC, 0, 0);
> +                            0, data->x_size, 0, 0);
>         input_set_abs_params(input_dev, ABS_Y,
> -                            0, MXT_MAX_YC, 0, 0);
> +                            0, data->y_size, 0, 0);
> 
>         /* For multi touch */
>         input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
>                              0, MXT_MAX_AREA, 0, 0);
>         input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> -                            0, MXT_MAX_XC, 0, 0);
> +                            0, data->x_size, 0, 0);
>         input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> -                            0, MXT_MAX_YC, 0, 0);
> +                            0, data->y_size, 0, 0);
> 

Should these be "data->x_size - 1"? Or if we store max_x_value like suggested
above, this too will be fixed.

>         input_set_drvdata(input_dev, data);
> -
> -       data->client = client;
> -       data->input_dev = input_dev;
> -       data->pdata = pdata;
> -       data->irq = client->irq;
> -
>         i2c_set_clientdata(client, data);
> 
>         error = mxt_initialize(data);
> --
> 1.7.0.4
> 

Thanks,

-- 
Iiro

Re: [PATCH 2/4] Input: atmel_mxt_ts - Support 12bit resolution

[Joonyoung Shim]

Hi,

On 2011-03-31 오후 10:43, Iiro Valkonen wrote:
> Hi Joonyoung,
>
> my comments below. They a bit late, but looks like the patch is not applied yet.
>
> On 03/07/2011 10:42 AM, Joonyoung Shim wrote:
>> Atmel touchscreen chip can support 12bit resolution and this patch
>> modifies to get maximum x and y size from platform data.
>>
>> Signed-off-by: Joonyoung Shim<jy0922.shim@samsung.com>
>> ---
>>   drivers/input/touchscreen/atmel_mxt_ts.c |   50 +++++++++++++++++++++--------
>>   1 files changed, 36 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
>> index 0986fa4..0ed0b1f 100644
>> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
>> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
>> @@ -192,9 +192,12 @@
>>   #define MXT_PRESS              (1<<  6)
>>   #define MXT_DETECT             (1<<  7)
>>
>> +/* Touch orient bits */
>> +#define MXT_XY_SWITCH          (1<<  0)
>> +#define MXT_X_INVERT           (1<<  1)
>> +#define MXT_Y_INVERT           (1<<  2)
>> +
>>   /* Touchscreen absolute values */
>> -#define MXT_MAX_XC             0x3ff
>> -#define MXT_MAX_YC             0x3ff
>>   #define MXT_MAX_AREA           0xff
>>
>>   #define MXT_MAX_FINGER         10
>> @@ -242,6 +245,8 @@ struct mxt_data {
>>          struct mxt_info info;
>>          struct mxt_finger finger[MXT_MAX_FINGER];
>>          unsigned int irq;
>> +       unsigned int x_size;
>> +       unsigned int y_size;
>>   };
>>
>>   static bool mxt_object_readable(unsigned int type)
>> @@ -541,8 +546,13 @@ static void mxt_input_touchevent(struct mxt_data *data,
>>          if (!(status&  (MXT_PRESS | MXT_MOVE)))
>>                  return;
>>
>> -       x = (message->message[1]<<  2) | ((message->message[3]&  ~0x3f)>>  6);
>> -       y = (message->message[2]<<  2) | ((message->message[3]&  ~0xf3)>>  2);
>> +       x = (message->message[1]<<  4) | ((message->message[3]>>  4)&  0xf);
>> +       y = (message->message[2]<<  4) | ((message->message[3]&  0xf));
>> +       if (data->x_size - 1<  1024)
>> +               x = x>>  2;
>> +       if (data->y_size - 1<  1024)
>> +               y = y>>  2;
>> +
>
> Not a big deal, but I would just store x_max_value instead of x_size to avoid
> the subtraction. Same for y.
>

OK.

>>          area = message->message[4];
>>
>>          dev_dbg(dev, "[%d] %s x: %d, y: %d, area: %d\n", id,
>> @@ -837,6 +847,17 @@ static int mxt_initialize(struct mxt_data *data)
>>          return 0;
>>   }
>>
>> +static void mxt_calc_resolution(struct mxt_data *data)
>> +{
>> +       if (data->pdata->orient&  MXT_XY_SWITCH) {
>> +               data->x_size = data->pdata->y_size;
>> +               data->y_size = data->pdata->x_size;
>> +       } else {
>> +               data->x_size = data->pdata->x_size;
>> +               data->y_size = data->pdata->y_size;
>> +       }
>> +}
>> +
>
> What's the reason for this? If we have set the x/y switch in the config, then
> we probably want to swap the axes. Or is this axis swap something that should be
> done on upper layers? Even so, then we shouldn't have the MXT_XY_SWITCH bit set
> in the config, and we could just say "data->x_max_value = data->pdata->xsize - 1"
> (and same for y) in the probe function. We wouldn't need Touch orient bit defines
> either.
>

If we set XY_SWITCH for special purpose then the axis is swapped and
driver will report also coordinates out of max value.

>>   static ssize_t mxt_object_show(struct device *dev,
>>                                      struct device_attribute *attr, char *buf)
>>   {
>> @@ -1044,31 +1065,32 @@ static int __devinit mxt_probe(struct i2c_client *client,
>>          input_dev->open = mxt_input_open;
>>          input_dev->close = mxt_input_close;
>>
>> +       data->client = client;
>> +       data->input_dev = input_dev;
>> +       data->pdata = pdata;
>> +       data->irq = client->irq;
>> +
>> +       mxt_calc_resolution(data);
>> +
>>          __set_bit(EV_ABS, input_dev->evbit);
>>          __set_bit(EV_KEY, input_dev->evbit);
>>          __set_bit(BTN_TOUCH, input_dev->keybit);
>>
>>          /* For single touch */
>>          input_set_abs_params(input_dev, ABS_X,
>> -                            0, MXT_MAX_XC, 0, 0);
>> +                            0, data->x_size, 0, 0);
>>          input_set_abs_params(input_dev, ABS_Y,
>> -                            0, MXT_MAX_YC, 0, 0);
>> +                            0, data->y_size, 0, 0);
>>
>>          /* For multi touch */
>>          input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
>>                               0, MXT_MAX_AREA, 0, 0);
>>          input_set_abs_params(input_dev, ABS_MT_POSITION_X,
>> -                            0, MXT_MAX_XC, 0, 0);
>> +                            0, data->x_size, 0, 0);
>>          input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
>> -                            0, MXT_MAX_YC, 0, 0);
>> +                            0, data->y_size, 0, 0);
>>
>
> Should these be "data->x_size - 1"? Or if we store max_x_value like suggested
> above, this too will be fixed.
>

Right, i also think better to use you suggest.

>>          input_set_drvdata(input_dev, data);
>> -
>> -       data->client = client;
>> -       data->input_dev = input_dev;
>> -       data->pdata = pdata;
>> -       data->irq = client->irq;
>> -
>>          i2c_set_clientdata(client, data);
>>
>>          error = mxt_initialize(data);
>> --
>> 1.7.0.4
>>
>
> Thanks,
>

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] Input: atmel_mxt_ts - Support 12bit resolution

[Iiro Valkonen]

Hi,

On 04/01/2011 09:10 AM, Joonyoung Shim wrote:
>>> @@ -837,6 +847,17 @@ static int mxt_initialize(struct mxt_data *data)
>>>          return 0;
>>>   }
>>>
>>> +static void mxt_calc_resolution(struct mxt_data *data)
>>> +{
>>> +       if (data->pdata->orient&  MXT_XY_SWITCH) {
>>> +               data->x_size = data->pdata->y_size;
>>> +               data->y_size = data->pdata->x_size;
>>> +       } else {
>>> +               data->x_size = data->pdata->x_size;
>>> +               data->y_size = data->pdata->y_size;
>>> +       }
>>> +}
>>> +
>>
>> What's the reason for this? If we have set the x/y switch in the config, then
>> we probably want to swap the axes. Or is this axis swap something that should be
>> done on upper layers? Even so, then we shouldn't have the MXT_XY_SWITCH bit set
>> in the config, and we could just say "data->x_max_value = data->pdata->xsize - 1"
>> (and same for y) in the probe function. We wouldn't need Touch orient bit defines
>> either.
>>
> 
> If we set XY_SWITCH for special purpose then the axis is swapped and
> driver will report also coordinates out of max value.
> 

Right, of course.

Regards,

-- 
Iiro

Re: [PATCH 2/4] Input: atmel_mxt_ts - Support 12bit resolution

[Joonyoung Shim]

On 2011-04-01 오후 6:58, Iiro Valkonen wrote:
> Hi,
>
> On 04/01/2011 09:10 AM, Joonyoung Shim wrote:
>>>> @@ -837,6 +847,17 @@ static int mxt_initialize(struct mxt_data *data)
>>>>           return 0;
>>>>    }
>>>>
>>>> +static void mxt_calc_resolution(struct mxt_data *data)
>>>> +{
>>>> +       if (data->pdata->orient&   MXT_XY_SWITCH) {
>>>> +               data->x_size = data->pdata->y_size;
>>>> +               data->y_size = data->pdata->x_size;
>>>> +       } else {
>>>> +               data->x_size = data->pdata->x_size;
>>>> +               data->y_size = data->pdata->y_size;
>>>> +       }
>>>> +}
>>>> +
>>>
>>> What's the reason for this? If we have set the x/y switch in the config, then
>>> we probably want to swap the axes. Or is this axis swap something that should be
>>> done on upper layers? Even so, then we shouldn't have the MXT_XY_SWITCH bit set
>>> in the config, and we could just say "data->x_max_value = data->pdata->xsize - 1"
>>> (and same for y) in the probe function. We wouldn't need Touch orient bit defines
>>> either.
>>>
>>
>> If we set XY_SWITCH for special purpose then the axis is swapped and
>> driver will report also coordinates out of max value.
>>
>
> Right, of course.
>

I mean it is the reason to add above codes, i think driver should
consider a case MXT_XY_SWITCH is setted.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] Input: atmel_mxt_ts - Support 12bit resolution

[Iiro Valkonen]

On 04/01/2011 01:25 PM, Joonyoung Shim wrote:
> On 2011-04-01 오후 6:58, Iiro Valkonen wrote:
>> Hi,
>>
>> On 04/01/2011 09:10 AM, Joonyoung Shim wrote:
>>>>> @@ -837,6 +847,17 @@ static int mxt_initialize(struct mxt_data *data)
>>>>>           return 0;
>>>>>    }
>>>>>
>>>>> +static void mxt_calc_resolution(struct mxt_data *data)
>>>>> +{
>>>>> +       if (data->pdata->orient&   MXT_XY_SWITCH) {
>>>>> +               data->x_size = data->pdata->y_size;
>>>>> +               data->y_size = data->pdata->x_size;
>>>>> +       } else {
>>>>> +               data->x_size = data->pdata->x_size;
>>>>> +               data->y_size = data->pdata->y_size;
>>>>> +       }
>>>>> +}
>>>>> +
>>>>
>>>> What's the reason for this? If we have set the x/y switch in the config, then
>>>> we probably want to swap the axes. Or is this axis swap something that should be
>>>> done on upper layers? Even so, then we shouldn't have the MXT_XY_SWITCH bit set
>>>> in the config, and we could just say "data->x_max_value = data->pdata->xsize - 1"
>>>> (and same for y) in the probe function. We wouldn't need Touch orient bit defines
>>>> either.
>>>>
>>>
>>> If we set XY_SWITCH for special purpose then the axis is swapped and
>>> driver will report also coordinates out of max value.
>>>
>>
>> Right, of course.
>>
> 
> I mean it is the reason to add above codes, i think driver should
> consider a case MXT_XY_SWITCH is setted.

Yes, I agree, the above code is fine. I was thinking that the x & y resolution
would also get swapped. But that's not the case, and this mxt_calc_resolution
function is correct the way you originally wrote it.

BR,

-- 
Iiro
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html