linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] HID: hid-multitouch: support fine-grain orientation reporting
@ 2017-09-28  9:42 Wei-Ning Huang
  2017-10-05  9:34 ` Jiri Kosina
  0 siblings, 1 reply; 4+ messages in thread
From: Wei-Ning Huang @ 2017-09-28  9:42 UTC (permalink / raw)
  To: LKML, Linux Input; +Cc: Dmitry Torokhov, Jiri Kosina, Wei-Ning Huang

The current hid-multitouch driver only allow the report of two
orientations, vertical and horizontal. We use the Azimuth orientation
usage 0x3F under the Digitizer usage page to report orientation if the
device supports it.

Changelog:
  v1 -> v2:
   - Fix commit message.
   - Remove resolution reporting for ABS_MT_ORIENTATION.
  v2 -> v3:
   - Fix commit message.

Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
---
 drivers/hid/hid-multitouch.c | 35 +++++++++++++++++++++++++++++++++--
 include/linux/hid.h          |  1 +
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 440b999304a5..f9801d1b1eae 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
 #define MT_IO_FLAGS_PENDING_SLOTS	2
 
 struct mt_slot {
-	__s32 x, y, cx, cy, p, w, h;
+	__s32 x, y, cx, cy, p, w, h, a;
 	__s32 contactid;	/* the device ContactID assigned to this slot */
 	bool touch_state;	/* is the touch valid? */
 	bool inrange_state;	/* is the finger in proximity of the sensor? */
 	bool confidence_state;  /* is the touch made by a finger? */
+	bool has_azimuth;       /* the contact reports azimuth */
 };
 
 struct mt_class {
@@ -591,6 +592,20 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			td->cc_index = field->index;
 			td->cc_value_index = usage->usage_index;
 			return 1;
+		case HID_DG_AZIMUTH:
+			hid_map_usage(hi, usage, bit, max,
+				EV_ABS, ABS_MT_ORIENTATION);
+			/*
+			 * Azimuth has the range of [0, MAX) representing a full
+			 * revolution. Set ABS_MT_ORIENTATION to a quarter of
+			 * MAX according the definition of ABS_MT_ORIENTATION
+			 */
+			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
+				0, field->logical_maximum / 4,
+				cls->sn_move ?
+				field->logical_maximum / cls->sn_move : 0, 0);
+			mt_store_field(usage, td, hi);
+			return 1;
 		case HID_DG_CONTACTMAX:
 			/* we don't set td->last_slot_field as contactcount and
 			 * contact max are global to the report */
@@ -683,6 +698,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 			int wide = (s->w > s->h);
 			int major = max(s->w, s->h);
 			int minor = min(s->w, s->h);
+			int orientation = wide;
+
+			if (s->has_azimuth)
+				orientation = s->a;
 
 			/*
 			 * divided by two to match visual scale of touch
@@ -699,7 +718,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 			input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
 			input_event(input, EV_ABS, ABS_MT_DISTANCE,
 				!s->touch_state);
-			input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
+			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
+				orientation);
 			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
 			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
@@ -789,6 +809,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 			break;
 		case HID_DG_CONTACTCOUNT:
 			break;
+		case HID_DG_AZIMUTH:
+			/*
+			 * Azimuth is counter-clockwise and ranges from [0, MAX)
+			 * (a full revolution). Convert it to clockwise ranging
+			 * [-MAX/2, MAX/2].
+			 */
+			if (value > field->logical_maximum / 2)
+				value -= field->logical_maximum;
+			td->curdata.a = -value;
+			td->curdata.has_azimuth = true;
+			break;
 		case HID_DG_TOUCH:
 			/* do nothing */
 			break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index ab05a86269dc..d81b9b6fd83a 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -281,6 +281,7 @@ struct hid_item {
 
 #define HID_DG_DEVICECONFIG	0x000d000e
 #define HID_DG_DEVICESETTINGS	0x000d0023
+#define HID_DG_AZIMUTH		0x000d003f
 #define HID_DG_CONFIDENCE	0x000d0047
 #define HID_DG_WIDTH		0x000d0048
 #define HID_DG_HEIGHT		0x000d0049
-- 
2.12.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] HID: hid-multitouch: support fine-grain orientation reporting
  2017-09-28  9:42 [PATCH v3] HID: hid-multitouch: support fine-grain orientation reporting Wei-Ning Huang
@ 2017-10-05  9:34 ` Jiri Kosina
  2017-10-06  8:13   ` Benjamin Tissoires
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Kosina @ 2017-10-05  9:34 UTC (permalink / raw)
  To: Wei-Ning Huang
  Cc: LKML, Linux Input, Dmitry Torokhov, Wei-Ning Huang,
	Benjamin Tissoires


[ adding Benjamin to CC ]

On Thu, 28 Sep 2017, Wei-Ning Huang wrote:

> The current hid-multitouch driver only allow the report of two
> orientations, vertical and horizontal. We use the Azimuth orientation
> usage 0x3F under the Digitizer usage page to report orientation if the
> device supports it.
> 
> Changelog:
>   v1 -> v2:
>    - Fix commit message.
>    - Remove resolution reporting for ABS_MT_ORIENTATION.
>   v2 -> v3:
>    - Fix commit message.
> 
> Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
> Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
> ---
>  drivers/hid/hid-multitouch.c | 35 +++++++++++++++++++++++++++++++++--
>  include/linux/hid.h          |  1 +
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 440b999304a5..f9801d1b1eae 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
>  #define MT_IO_FLAGS_PENDING_SLOTS	2
>  
>  struct mt_slot {
> -	__s32 x, y, cx, cy, p, w, h;
> +	__s32 x, y, cx, cy, p, w, h, a;
>  	__s32 contactid;	/* the device ContactID assigned to this slot */
>  	bool touch_state;	/* is the touch valid? */
>  	bool inrange_state;	/* is the finger in proximity of the sensor? */
>  	bool confidence_state;  /* is the touch made by a finger? */
> +	bool has_azimuth;       /* the contact reports azimuth */
>  };
>  
>  struct mt_class {
> @@ -591,6 +592,20 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  			td->cc_index = field->index;
>  			td->cc_value_index = usage->usage_index;
>  			return 1;
> +		case HID_DG_AZIMUTH:
> +			hid_map_usage(hi, usage, bit, max,
> +				EV_ABS, ABS_MT_ORIENTATION);
> +			/*
> +			 * Azimuth has the range of [0, MAX) representing a full
> +			 * revolution. Set ABS_MT_ORIENTATION to a quarter of
> +			 * MAX according the definition of ABS_MT_ORIENTATION
> +			 */
> +			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
> +				0, field->logical_maximum / 4,
> +				cls->sn_move ?
> +				field->logical_maximum / cls->sn_move : 0, 0);
> +			mt_store_field(usage, td, hi);
> +			return 1;
>  		case HID_DG_CONTACTMAX:
>  			/* we don't set td->last_slot_field as contactcount and
>  			 * contact max are global to the report */
> @@ -683,6 +698,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>  			int wide = (s->w > s->h);
>  			int major = max(s->w, s->h);
>  			int minor = min(s->w, s->h);
> +			int orientation = wide;
> +
> +			if (s->has_azimuth)
> +				orientation = s->a;
>  
>  			/*
>  			 * divided by two to match visual scale of touch
> @@ -699,7 +718,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>  			input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
>  			input_event(input, EV_ABS, ABS_MT_DISTANCE,
>  				!s->touch_state);
> -			input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
> +			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
> +				orientation);
>  			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>  			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>  			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> @@ -789,6 +809,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
>  			break;
>  		case HID_DG_CONTACTCOUNT:
>  			break;
> +		case HID_DG_AZIMUTH:
> +			/*
> +			 * Azimuth is counter-clockwise and ranges from [0, MAX)
> +			 * (a full revolution). Convert it to clockwise ranging
> +			 * [-MAX/2, MAX/2].
> +			 */
> +			if (value > field->logical_maximum / 2)
> +				value -= field->logical_maximum;
> +			td->curdata.a = -value;
> +			td->curdata.has_azimuth = true;
> +			break;
>  		case HID_DG_TOUCH:
>  			/* do nothing */
>  			break;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index ab05a86269dc..d81b9b6fd83a 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -281,6 +281,7 @@ struct hid_item {
>  
>  #define HID_DG_DEVICECONFIG	0x000d000e
>  #define HID_DG_DEVICESETTINGS	0x000d0023
> +#define HID_DG_AZIMUTH		0x000d003f
>  #define HID_DG_CONFIDENCE	0x000d0047
>  #define HID_DG_WIDTH		0x000d0048
>  #define HID_DG_HEIGHT		0x000d0049
> -- 
> 2.12.2
> 

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] HID: hid-multitouch: support fine-grain orientation reporting
  2017-10-05  9:34 ` Jiri Kosina
@ 2017-10-06  8:13   ` Benjamin Tissoires
  2017-10-10  4:22     ` Wei-Ning Huang
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Tissoires @ 2017-10-06  8:13 UTC (permalink / raw)
  To: Jiri Kosina, Wei-Ning Huang, Wei-Ning Huang
  Cc: LKML, Linux Input, Dmitry Torokhov

Hi,

On Oct 05 2017 or thereabouts, Jiri Kosina wrote:
> 
> [ adding Benjamin to CC ]
> 
> On Thu, 28 Sep 2017, Wei-Ning Huang wrote:
> 
> > The current hid-multitouch driver only allow the report of two
> > orientations, vertical and horizontal. We use the Azimuth orientation
> > usage 0x3F under the Digitizer usage page to report orientation if the
> > device supports it.
> > 
> > Changelog:
> >   v1 -> v2:
> >    - Fix commit message.
> >    - Remove resolution reporting for ABS_MT_ORIENTATION.
> >   v2 -> v3:
> >    - Fix commit message.
> > 
> > Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
> > Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
> > ---
> >  drivers/hid/hid-multitouch.c | 35 +++++++++++++++++++++++++++++++++--
> >  include/linux/hid.h          |  1 +
> >  2 files changed, 34 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index 440b999304a5..f9801d1b1eae 100644
> > --- a/drivers/hid/hid-multitouch.c
> > +++ b/drivers/hid/hid-multitouch.c
> > @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
> >  #define MT_IO_FLAGS_PENDING_SLOTS	2
> >  
> >  struct mt_slot {
> > -	__s32 x, y, cx, cy, p, w, h;
> > +	__s32 x, y, cx, cy, p, w, h, a;
> >  	__s32 contactid;	/* the device ContactID assigned to this slot */
> >  	bool touch_state;	/* is the touch valid? */
> >  	bool inrange_state;	/* is the finger in proximity of the sensor? */
> >  	bool confidence_state;  /* is the touch made by a finger? */
> > +	bool has_azimuth;       /* the contact reports azimuth */
> >  };
> >  
> >  struct mt_class {
> > @@ -591,6 +592,20 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> >  			td->cc_index = field->index;
> >  			td->cc_value_index = usage->usage_index;
> >  			return 1;
> > +		case HID_DG_AZIMUTH:
> > +			hid_map_usage(hi, usage, bit, max,
> > +				EV_ABS, ABS_MT_ORIENTATION);
> > +			/*
> > +			 * Azimuth has the range of [0, MAX) representing a full
> > +			 * revolution. Set ABS_MT_ORIENTATION to a quarter of
> > +			 * MAX according the definition of ABS_MT_ORIENTATION
> > +			 */
> > +			input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
> > +				0, field->logical_maximum / 4,

Shouldn't this be "-field->logical_maximum / 4, field->logical_maximum / 4,"?

Also, if HID_DG_HEIGHT is seen after HID_DG_AZIMUTH, it'll change the
ABS_MT_ORIENTATION you just set here.

> > +				cls->sn_move ?
> > +				field->logical_maximum / cls->sn_move : 0, 0);
> > +			mt_store_field(usage, td, hi);
> > +			return 1;
> >  		case HID_DG_CONTACTMAX:
> >  			/* we don't set td->last_slot_field as contactcount and
> >  			 * contact max are global to the report */
> > @@ -683,6 +698,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> >  			int wide = (s->w > s->h);
> >  			int major = max(s->w, s->h);
> >  			int minor = min(s->w, s->h);
> > +			int orientation = wide;
> > +
> > +			if (s->has_azimuth)
> > +				orientation = s->a;
> >  
> >  			/*
> >  			 * divided by two to match visual scale of touch
> > @@ -699,7 +718,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> >  			input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
> >  			input_event(input, EV_ABS, ABS_MT_DISTANCE,
> >  				!s->touch_state);
> > -			input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
> > +			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
> > +				orientation);
> >  			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
> >  			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
> >  			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> > @@ -789,6 +809,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
> >  			break;
> >  		case HID_DG_CONTACTCOUNT:
> >  			break;
> > +		case HID_DG_AZIMUTH:
> > +			/*
> > +			 * Azimuth is counter-clockwise and ranges from [0, MAX)
> > +			 * (a full revolution). Convert it to clockwise ranging
> > +			 * [-MAX/2, MAX/2].

It took me a while to get this right. The problem is the definition of
ABS_MT_ORIENTATION says that we report [-MAX/4, MAX/4], *but* we can go
out of range to [-MAX/2, MAX/2] to report upside down ellipsis.

So this is correct, but I'd like to have some mention of [-MAX/4, MAX/4]
in this comment, to ease further rereading of the code.

Cheers,
Benjamin

> > +			 */
> > +			if (value > field->logical_maximum / 2)
> > +				value -= field->logical_maximum;
> > +			td->curdata.a = -value;
> > +			td->curdata.has_azimuth = true;
> > +			break;
> >  		case HID_DG_TOUCH:
> >  			/* do nothing */
> >  			break;
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index ab05a86269dc..d81b9b6fd83a 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -281,6 +281,7 @@ struct hid_item {
> >  
> >  #define HID_DG_DEVICECONFIG	0x000d000e
> >  #define HID_DG_DEVICESETTINGS	0x000d0023
> > +#define HID_DG_AZIMUTH		0x000d003f
> >  #define HID_DG_CONFIDENCE	0x000d0047
> >  #define HID_DG_WIDTH		0x000d0048
> >  #define HID_DG_HEIGHT		0x000d0049
> > -- 
> > 2.12.2
> > 
> 
> -- 
> Jiri Kosina
> SUSE Labs
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] HID: hid-multitouch: support fine-grain orientation reporting
  2017-10-06  8:13   ` Benjamin Tissoires
@ 2017-10-10  4:22     ` Wei-Ning Huang
  0 siblings, 0 replies; 4+ messages in thread
From: Wei-Ning Huang @ 2017-10-10  4:22 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, LKML, Linux Input, Dmitry Torokhov

Hi Benjamin,

Thanks for the review. I've fixed according to your comments and sent
the v4 patch:
https://patchwork.kernel.org/patch/9995003/

Wei-Ning

On Fri, Oct 6, 2017 at 4:13 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Hi,
>
> On Oct 05 2017 or thereabouts, Jiri Kosina wrote:
>>
>> [ adding Benjamin to CC ]
>>
>> On Thu, 28 Sep 2017, Wei-Ning Huang wrote:
>>
>> > The current hid-multitouch driver only allow the report of two
>> > orientations, vertical and horizontal. We use the Azimuth orientation
>> > usage 0x3F under the Digitizer usage page to report orientation if the
>> > device supports it.
>> >
>> > Changelog:
>> >   v1 -> v2:
>> >    - Fix commit message.
>> >    - Remove resolution reporting for ABS_MT_ORIENTATION.
>> >   v2 -> v3:
>> >    - Fix commit message.
>> >
>> > Signed-off-by: Wei-Ning Huang <wnhuang@google.com>
>> > Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
>> > ---
>> >  drivers/hid/hid-multitouch.c | 35 +++++++++++++++++++++++++++++++++--
>> >  include/linux/hid.h          |  1 +
>> >  2 files changed, 34 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> > index 440b999304a5..f9801d1b1eae 100644
>> > --- a/drivers/hid/hid-multitouch.c
>> > +++ b/drivers/hid/hid-multitouch.c
>> > @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
>> >  #define MT_IO_FLAGS_PENDING_SLOTS  2
>> >
>> >  struct mt_slot {
>> > -   __s32 x, y, cx, cy, p, w, h;
>> > +   __s32 x, y, cx, cy, p, w, h, a;
>> >     __s32 contactid;        /* the device ContactID assigned to this slot */
>> >     bool touch_state;       /* is the touch valid? */
>> >     bool inrange_state;     /* is the finger in proximity of the sensor? */
>> >     bool confidence_state;  /* is the touch made by a finger? */
>> > +   bool has_azimuth;       /* the contact reports azimuth */
>> >  };
>> >
>> >  struct mt_class {
>> > @@ -591,6 +592,20 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> >                     td->cc_index = field->index;
>> >                     td->cc_value_index = usage->usage_index;
>> >                     return 1;
>> > +           case HID_DG_AZIMUTH:
>> > +                   hid_map_usage(hi, usage, bit, max,
>> > +                           EV_ABS, ABS_MT_ORIENTATION);
>> > +                   /*
>> > +                    * Azimuth has the range of [0, MAX) representing a full
>> > +                    * revolution. Set ABS_MT_ORIENTATION to a quarter of
>> > +                    * MAX according the definition of ABS_MT_ORIENTATION
>> > +                    */
>> > +                   input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
>> > +                           0, field->logical_maximum / 4,
>
> Shouldn't this be "-field->logical_maximum / 4, field->logical_maximum / 4,"?
>
> Also, if HID_DG_HEIGHT is seen after HID_DG_AZIMUTH, it'll change the
> ABS_MT_ORIENTATION you just set here.
>
>> > +                           cls->sn_move ?
>> > +                           field->logical_maximum / cls->sn_move : 0, 0);
>> > +                   mt_store_field(usage, td, hi);
>> > +                   return 1;
>> >             case HID_DG_CONTACTMAX:
>> >                     /* we don't set td->last_slot_field as contactcount and
>> >                      * contact max are global to the report */
>> > @@ -683,6 +698,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> >                     int wide = (s->w > s->h);
>> >                     int major = max(s->w, s->h);
>> >                     int minor = min(s->w, s->h);
>> > +                   int orientation = wide;
>> > +
>> > +                   if (s->has_azimuth)
>> > +                           orientation = s->a;
>> >
>> >                     /*
>> >                      * divided by two to match visual scale of touch
>> > @@ -699,7 +718,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> >                     input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
>> >                     input_event(input, EV_ABS, ABS_MT_DISTANCE,
>> >                             !s->touch_state);
>> > -                   input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
>> > +                   input_event(input, EV_ABS, ABS_MT_ORIENTATION,
>> > +                           orientation);
>> >                     input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>> >                     input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>> >                     input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
>> > @@ -789,6 +809,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
>> >                     break;
>> >             case HID_DG_CONTACTCOUNT:
>> >                     break;
>> > +           case HID_DG_AZIMUTH:
>> > +                   /*
>> > +                    * Azimuth is counter-clockwise and ranges from [0, MAX)
>> > +                    * (a full revolution). Convert it to clockwise ranging
>> > +                    * [-MAX/2, MAX/2].
>
> It took me a while to get this right. The problem is the definition of
> ABS_MT_ORIENTATION says that we report [-MAX/4, MAX/4], *but* we can go
> out of range to [-MAX/2, MAX/2] to report upside down ellipsis.
>
> So this is correct, but I'd like to have some mention of [-MAX/4, MAX/4]
> in this comment, to ease further rereading of the code.
>
> Cheers,
> Benjamin
>
>> > +                    */
>> > +                   if (value > field->logical_maximum / 2)
>> > +                           value -= field->logical_maximum;
>> > +                   td->curdata.a = -value;
>> > +                   td->curdata.has_azimuth = true;
>> > +                   break;
>> >             case HID_DG_TOUCH:
>> >                     /* do nothing */
>> >                     break;
>> > diff --git a/include/linux/hid.h b/include/linux/hid.h
>> > index ab05a86269dc..d81b9b6fd83a 100644
>> > --- a/include/linux/hid.h
>> > +++ b/include/linux/hid.h
>> > @@ -281,6 +281,7 @@ struct hid_item {
>> >
>> >  #define HID_DG_DEVICECONFIG        0x000d000e
>> >  #define HID_DG_DEVICESETTINGS      0x000d0023
>> > +#define HID_DG_AZIMUTH             0x000d003f
>> >  #define HID_DG_CONFIDENCE  0x000d0047
>> >  #define HID_DG_WIDTH               0x000d0048
>> >  #define HID_DG_HEIGHT              0x000d0049
>> > --
>> > 2.12.2
>> >
>>
>> --
>> Jiri Kosina
>> SUSE Labs
>>



-- 
Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
wnhuang@google.com | Cell: +886 910-380678

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-10  4:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-28  9:42 [PATCH v3] HID: hid-multitouch: support fine-grain orientation reporting Wei-Ning Huang
2017-10-05  9:34 ` Jiri Kosina
2017-10-06  8:13   ` Benjamin Tissoires
2017-10-10  4:22     ` Wei-Ning Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).