[PATCH 01/11] HID: playstation: Make use of bitfield macros

Cristian Ciocaltea posted 11 patches 6 months, 3 weeks ago
There is a newer version of this series
[PATCH 01/11] HID: playstation: Make use of bitfield macros
Posted by Cristian Ciocaltea 6 months, 3 weeks ago
Improve code readability by replacing open coded bit operations with the
equivalent bitfield macros.

While at it, vertically align all DS_OUTPUT_* bit constants.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/hid/hid-playstation.c | 68 ++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 1468fb11e39dffc883181663a4ad44252e0a7ebb..538194ce8902fe1383b57ac59743f32838dcb0df 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -5,6 +5,7 @@
  *  Copyright (c) 2020-2022 Sony Interactive Entertainment
  */
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/crc32.h>
 #include <linux/device.h>
@@ -116,29 +117,41 @@ struct ps_led_info {
 #define DS_STATUS_CHARGING_SHIFT	4
 
 /* Feature version from DualSense Firmware Info report. */
-#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
-
+#define DS_FEATURE_VERSION_MINOR		GENMASK(7, 0)
+#define DS_FEATURE_VERSION_MAJOR		GENMASK(15, 8)
+#define DS_FEATURE_VERSION(major, minor)	(FIELD_PREP(DS_FEATURE_VERSION_MAJOR, major) | \
+						 FIELD_PREP(DS_FEATURE_VERSION_MINOR, minor))
 /*
  * Status of a DualSense touch point contact.
  * Contact IDs, with highest bit set are 'inactive'
  * and any associated data is then invalid.
  */
-#define DS_TOUCH_POINT_INACTIVE BIT(7)
+#define DS_TOUCH_POINT_INACTIVE			BIT(7)
+#define DS_TOUCH_POINT_X_LO			GENMASK(7, 0)
+#define DS_TOUCH_POINT_X_HI			GENMASK(11, 8)
+#define DS_TOUCH_POINT_X(hi, lo)		(FIELD_PREP(DS_TOUCH_POINT_X_HI, hi) | \
+						 FIELD_PREP(DS_TOUCH_POINT_X_LO, lo))
+#define DS_TOUCH_POINT_Y_LO			GENMASK(3, 0)
+#define DS_TOUCH_POINT_Y_HI			GENMASK(11, 4)
+#define DS_TOUCH_POINT_Y(hi, lo)		(FIELD_PREP(DS_TOUCH_POINT_Y_HI, hi) | \
+						 FIELD_PREP(DS_TOUCH_POINT_Y_LO, lo))
 
  /* Magic value required in tag field of Bluetooth output report. */
-#define DS_OUTPUT_TAG 0x10
+#define DS_OUTPUT_TAG				0x10
+#define DS_OUTPUT_SEQ_TAG			GENMASK(3, 0)
+#define DS_OUTPUT_SEQ_NO			GENMASK(7, 4)
 /* Flags for DualSense output report. */
-#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
-#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
-#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
-#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
-#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
-#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
-#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
-#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
-#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
-#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
-#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
+#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION		BIT(0)
+#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT			BIT(1)
+#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE	BIT(0)
+#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE		BIT(1)
+#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE		BIT(2)
+#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS			BIT(3)
+#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE	BIT(4)
+#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE	BIT(1)
+#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2		BIT(2)
+#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE			BIT(4)
+#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT			BIT(1)
 
 /* DualSense hardware limits */
 #define DS_ACC_RES_PER_G	8192
@@ -315,7 +328,9 @@ struct dualsense_output_report {
  * Contact IDs, with highest bit set are 'inactive'
  * and any associated data is then invalid.
  */
-#define DS4_TOUCH_POINT_INACTIVE BIT(7)
+#define DS4_TOUCH_POINT_INACTIVE	BIT(7)
+#define DS4_TOUCH_POINT_X(hi, lo)	DS_TOUCH_POINT_X(hi, lo)
+#define DS4_TOUCH_POINT_Y(hi, lo)	DS_TOUCH_POINT_Y(hi, lo)
 
 /* Status field of DualShock4 input report. */
 #define DS4_STATUS0_BATTERY_CAPACITY	GENMASK(3, 0)
@@ -1194,7 +1209,8 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
 		 * Highest 4-bit is a sequence number, which needs to be increased
 		 * every report. Lowest 4-bit is tag and can be zero for now.
 		 */
-		bt->seq_tag = (ds->output_seq << 4) | 0x0;
+		bt->seq_tag = FIELD_PREP(DS_OUTPUT_SEQ_NO, ds->output_seq) |
+			      FIELD_PREP(DS_OUTPUT_SEQ_TAG, 0x0);
 		if (++ds->output_seq == 16)
 			ds->output_seq = 0;
 
@@ -1439,11 +1455,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
 		input_mt_report_slot_state(ds->touchpad, MT_TOOL_FINGER, active);
 
 		if (active) {
-			int x = (point->x_hi << 8) | point->x_lo;
-			int y = (point->y_hi << 4) | point->y_lo;
-
-			input_report_abs(ds->touchpad, ABS_MT_POSITION_X, x);
-			input_report_abs(ds->touchpad, ABS_MT_POSITION_Y, y);
+			input_report_abs(ds->touchpad, ABS_MT_POSITION_X,
+					 DS_TOUCH_POINT_X(point->x_hi, point->x_lo));
+			input_report_abs(ds->touchpad, ABS_MT_POSITION_Y,
+					 DS_TOUCH_POINT_Y(point->y_hi, point->y_lo));
 		}
 	}
 	input_mt_sync_frame(ds->touchpad);
@@ -2351,11 +2366,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
 			input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
 
 			if (active) {
-				int x = (point->x_hi << 8) | point->x_lo;
-				int y = (point->y_hi << 4) | point->y_lo;
-
-				input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
-				input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
+				input_report_abs(ds4->touchpad, ABS_MT_POSITION_X,
+						 DS4_TOUCH_POINT_X(point->x_hi, point->x_lo));
+				input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y,
+						 DS4_TOUCH_POINT_Y(point->y_hi, point->y_lo));
 			}
 		}
 		input_mt_sync_frame(ds4->touchpad);

-- 
2.49.0
Re: [PATCH 01/11] HID: playstation: Make use of bitfield macros
Posted by Roderick Colenbrander 6 months, 1 week ago
Hi Christian,

Thanks for the effort into this patch. Personally this is a patch I
would rather drop. There is the trade-off between magical numbers and
code readability. I think this patch makes things much less readable.
Having to introduce more macros and FIELD_PREP vs a single bitshift
here and there.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:52 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Improve code readability by replacing open coded bit operations with the
> equivalent bitfield macros.
>
> While at it, vertically align all DS_OUTPUT_* bit constants.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/hid/hid-playstation.c | 68 ++++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 1468fb11e39dffc883181663a4ad44252e0a7ebb..538194ce8902fe1383b57ac59743f32838dcb0df 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -5,6 +5,7 @@
>   *  Copyright (c) 2020-2022 Sony Interactive Entertainment
>   */
>
> +#include <linux/bitfield.h>
>  #include <linux/bits.h>
>  #include <linux/crc32.h>
>  #include <linux/device.h>
> @@ -116,29 +117,41 @@ struct ps_led_info {
>  #define DS_STATUS_CHARGING_SHIFT       4
>
>  /* Feature version from DualSense Firmware Info report. */
> -#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
> -
> +#define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
> +#define DS_FEATURE_VERSION_MAJOR               GENMASK(15, 8)
> +#define DS_FEATURE_VERSION(major, minor)       (FIELD_PREP(DS_FEATURE_VERSION_MAJOR, major) | \
> +                                                FIELD_PREP(DS_FEATURE_VERSION_MINOR, minor))
>  /*
>   * Status of a DualSense touch point contact.
>   * Contact IDs, with highest bit set are 'inactive'
>   * and any associated data is then invalid.
>   */
> -#define DS_TOUCH_POINT_INACTIVE BIT(7)
> +#define DS_TOUCH_POINT_INACTIVE                        BIT(7)
> +#define DS_TOUCH_POINT_X_LO                    GENMASK(7, 0)
> +#define DS_TOUCH_POINT_X_HI                    GENMASK(11, 8)
> +#define DS_TOUCH_POINT_X(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_X_HI, hi) | \
> +                                                FIELD_PREP(DS_TOUCH_POINT_X_LO, lo))
> +#define DS_TOUCH_POINT_Y_LO                    GENMASK(3, 0)
> +#define DS_TOUCH_POINT_Y_HI                    GENMASK(11, 4)
> +#define DS_TOUCH_POINT_Y(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_Y_HI, hi) | \
> +                                                FIELD_PREP(DS_TOUCH_POINT_Y_LO, lo))
>
>   /* Magic value required in tag field of Bluetooth output report. */
> -#define DS_OUTPUT_TAG 0x10
> +#define DS_OUTPUT_TAG                          0x10
> +#define DS_OUTPUT_SEQ_TAG                      GENMASK(3, 0)
> +#define DS_OUTPUT_SEQ_NO                       GENMASK(7, 4)
>  /* Flags for DualSense output report. */
> -#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
> -#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
> -#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
> -#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
> -#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
> -#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
> -#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
> -#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
> -#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
> -#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
> -#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
> +#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION             BIT(0)
> +#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT                   BIT(1)
> +#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE      BIT(0)
> +#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE                BIT(1)
> +#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE          BIT(2)
> +#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS                     BIT(3)
> +#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE  BIT(4)
> +#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE    BIT(1)
> +#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2            BIT(2)
> +#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE                  BIT(4)
> +#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT                     BIT(1)
>
>  /* DualSense hardware limits */
>  #define DS_ACC_RES_PER_G       8192
> @@ -315,7 +328,9 @@ struct dualsense_output_report {
>   * Contact IDs, with highest bit set are 'inactive'
>   * and any associated data is then invalid.
>   */
> -#define DS4_TOUCH_POINT_INACTIVE BIT(7)
> +#define DS4_TOUCH_POINT_INACTIVE       BIT(7)
> +#define DS4_TOUCH_POINT_X(hi, lo)      DS_TOUCH_POINT_X(hi, lo)
> +#define DS4_TOUCH_POINT_Y(hi, lo)      DS_TOUCH_POINT_Y(hi, lo)
>
>  /* Status field of DualShock4 input report. */
>  #define DS4_STATUS0_BATTERY_CAPACITY   GENMASK(3, 0)
> @@ -1194,7 +1209,8 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
>                  * Highest 4-bit is a sequence number, which needs to be increased
>                  * every report. Lowest 4-bit is tag and can be zero for now.
>                  */
> -               bt->seq_tag = (ds->output_seq << 4) | 0x0;
> +               bt->seq_tag = FIELD_PREP(DS_OUTPUT_SEQ_NO, ds->output_seq) |
> +                             FIELD_PREP(DS_OUTPUT_SEQ_TAG, 0x0);
>                 if (++ds->output_seq == 16)
>                         ds->output_seq = 0;
>
> @@ -1439,11 +1455,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>                 input_mt_report_slot_state(ds->touchpad, MT_TOOL_FINGER, active);
>
>                 if (active) {
> -                       int x = (point->x_hi << 8) | point->x_lo;
> -                       int y = (point->y_hi << 4) | point->y_lo;
> -
> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X, x);
> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y, y);
> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X,
> +                                        DS_TOUCH_POINT_X(point->x_hi, point->x_lo));
> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y,
> +                                        DS_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>                 }
>         }
>         input_mt_sync_frame(ds->touchpad);
> @@ -2351,11 +2366,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>                         input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
>
>                         if (active) {
> -                               int x = (point->x_hi << 8) | point->x_lo;
> -                               int y = (point->y_hi << 4) | point->y_lo;
> -
> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X,
> +                                                DS4_TOUCH_POINT_X(point->x_hi, point->x_lo));
> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y,
> +                                                DS4_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>                         }
>                 }
>                 input_mt_sync_frame(ds4->touchpad);
>
> --
> 2.49.0
>
>
Re: [PATCH 01/11] HID: playstation: Make use of bitfield macros
Posted by Cristian Ciocaltea 6 months, 1 week ago
Hi Roderick,

On 6/10/25 7:07 AM, Roderick Colenbrander wrote:
> Hi Christian,
> 
> Thanks for the effort into this patch. Personally this is a patch I
> would rather drop. There is the trade-off between magical numbers and
> code readability. I think this patch makes things much less readable.
> Having to introduce more macros and FIELD_PREP vs a single bitshift
> here and there.

I think the usage of bitfield macros is a general recommendation of 
making code more robust, and this should also help improve overall 
readability - e.g. this reduced code duplication by replacing the open 
coded bit operations for computing touch point coordinates.

Sorry for the noise generated by the alignment correction!

Thanks,
Cristian

> 
> Thanks,
> Roderick
> 
> On Mon, May 26, 2025 at 5:52 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
>>
>> Improve code readability by replacing open coded bit operations with the
>> equivalent bitfield macros.
>>
>> While at it, vertically align all DS_OUTPUT_* bit constants.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/hid/hid-playstation.c | 68 ++++++++++++++++++++++++++-----------------
>>  1 file changed, 41 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
>> index 1468fb11e39dffc883181663a4ad44252e0a7ebb..538194ce8902fe1383b57ac59743f32838dcb0df 100644
>> --- a/drivers/hid/hid-playstation.c
>> +++ b/drivers/hid/hid-playstation.c
>> @@ -5,6 +5,7 @@
>>   *  Copyright (c) 2020-2022 Sony Interactive Entertainment
>>   */
>>
>> +#include <linux/bitfield.h>
>>  #include <linux/bits.h>
>>  #include <linux/crc32.h>
>>  #include <linux/device.h>
>> @@ -116,29 +117,41 @@ struct ps_led_info {
>>  #define DS_STATUS_CHARGING_SHIFT       4
>>
>>  /* Feature version from DualSense Firmware Info report. */
>> -#define DS_FEATURE_VERSION(major, minor) ((major & 0xff) << 8 | (minor & 0xff))
>> -
>> +#define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
>> +#define DS_FEATURE_VERSION_MAJOR               GENMASK(15, 8)
>> +#define DS_FEATURE_VERSION(major, minor)       (FIELD_PREP(DS_FEATURE_VERSION_MAJOR, major) | \
>> +                                                FIELD_PREP(DS_FEATURE_VERSION_MINOR, minor))
>>  /*
>>   * Status of a DualSense touch point contact.
>>   * Contact IDs, with highest bit set are 'inactive'
>>   * and any associated data is then invalid.
>>   */
>> -#define DS_TOUCH_POINT_INACTIVE BIT(7)
>> +#define DS_TOUCH_POINT_INACTIVE                        BIT(7)
>> +#define DS_TOUCH_POINT_X_LO                    GENMASK(7, 0)
>> +#define DS_TOUCH_POINT_X_HI                    GENMASK(11, 8)
>> +#define DS_TOUCH_POINT_X(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_X_HI, hi) | \
>> +                                                FIELD_PREP(DS_TOUCH_POINT_X_LO, lo))
>> +#define DS_TOUCH_POINT_Y_LO                    GENMASK(3, 0)
>> +#define DS_TOUCH_POINT_Y_HI                    GENMASK(11, 4)
>> +#define DS_TOUCH_POINT_Y(hi, lo)               (FIELD_PREP(DS_TOUCH_POINT_Y_HI, hi) | \
>> +                                                FIELD_PREP(DS_TOUCH_POINT_Y_LO, lo))
>>
>>   /* Magic value required in tag field of Bluetooth output report. */
>> -#define DS_OUTPUT_TAG 0x10
>> +#define DS_OUTPUT_TAG                          0x10
>> +#define DS_OUTPUT_SEQ_TAG                      GENMASK(3, 0)
>> +#define DS_OUTPUT_SEQ_NO                       GENMASK(7, 4)
>>  /* Flags for DualSense output report. */
>> -#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION BIT(0)
>> -#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT BIT(1)
>> -#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE BIT(0)
>> -#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE BIT(1)
>> -#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE BIT(2)
>> -#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS BIT(3)
>> -#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE BIT(4)
>> -#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE BIT(1)
>> -#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2 BIT(2)
>> -#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE BIT(4)
>> -#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG0_COMPATIBLE_VIBRATION             BIT(0)
>> +#define DS_OUTPUT_VALID_FLAG0_HAPTICS_SELECT                   BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG1_MIC_MUTE_LED_CONTROL_ENABLE      BIT(0)
>> +#define DS_OUTPUT_VALID_FLAG1_POWER_SAVE_CONTROL_ENABLE                BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG1_LIGHTBAR_CONTROL_ENABLE          BIT(2)
>> +#define DS_OUTPUT_VALID_FLAG1_RELEASE_LEDS                     BIT(3)
>> +#define DS_OUTPUT_VALID_FLAG1_PLAYER_INDICATOR_CONTROL_ENABLE  BIT(4)
>> +#define DS_OUTPUT_VALID_FLAG2_LIGHTBAR_SETUP_CONTROL_ENABLE    BIT(1)
>> +#define DS_OUTPUT_VALID_FLAG2_COMPATIBLE_VIBRATION2            BIT(2)
>> +#define DS_OUTPUT_POWER_SAVE_CONTROL_MIC_MUTE                  BIT(4)
>> +#define DS_OUTPUT_LIGHTBAR_SETUP_LIGHT_OUT                     BIT(1)
>>
>>  /* DualSense hardware limits */
>>  #define DS_ACC_RES_PER_G       8192
>> @@ -315,7 +328,9 @@ struct dualsense_output_report {
>>   * Contact IDs, with highest bit set are 'inactive'
>>   * and any associated data is then invalid.
>>   */
>> -#define DS4_TOUCH_POINT_INACTIVE BIT(7)
>> +#define DS4_TOUCH_POINT_INACTIVE       BIT(7)
>> +#define DS4_TOUCH_POINT_X(hi, lo)      DS_TOUCH_POINT_X(hi, lo)
>> +#define DS4_TOUCH_POINT_Y(hi, lo)      DS_TOUCH_POINT_Y(hi, lo)
>>
>>  /* Status field of DualShock4 input report. */
>>  #define DS4_STATUS0_BATTERY_CAPACITY   GENMASK(3, 0)
>> @@ -1194,7 +1209,8 @@ static void dualsense_init_output_report(struct dualsense *ds, struct dualsense_
>>                  * Highest 4-bit is a sequence number, which needs to be increased
>>                  * every report. Lowest 4-bit is tag and can be zero for now.
>>                  */
>> -               bt->seq_tag = (ds->output_seq << 4) | 0x0;
>> +               bt->seq_tag = FIELD_PREP(DS_OUTPUT_SEQ_NO, ds->output_seq) |
>> +                             FIELD_PREP(DS_OUTPUT_SEQ_TAG, 0x0);
>>                 if (++ds->output_seq == 16)
>>                         ds->output_seq = 0;
>>
>> @@ -1439,11 +1455,10 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>>                 input_mt_report_slot_state(ds->touchpad, MT_TOOL_FINGER, active);
>>
>>                 if (active) {
>> -                       int x = (point->x_hi << 8) | point->x_lo;
>> -                       int y = (point->y_hi << 4) | point->y_lo;
>> -
>> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X, x);
>> -                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y, y);
>> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_X,
>> +                                        DS_TOUCH_POINT_X(point->x_hi, point->x_lo));
>> +                       input_report_abs(ds->touchpad, ABS_MT_POSITION_Y,
>> +                                        DS_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>>                 }
>>         }
>>         input_mt_sync_frame(ds->touchpad);
>> @@ -2351,11 +2366,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *
>>                         input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active);
>>
>>                         if (active) {
>> -                               int x = (point->x_hi << 8) | point->x_lo;
>> -                               int y = (point->y_hi << 4) | point->y_lo;
>> -
>> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x);
>> -                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y);
>> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_X,
>> +                                                DS4_TOUCH_POINT_X(point->x_hi, point->x_lo));
>> +                               input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y,
>> +                                                DS4_TOUCH_POINT_Y(point->y_hi, point->y_lo));
>>                         }
>>                 }
>>                 input_mt_sync_frame(ds4->touchpad);
>>
>> --
>> 2.49.0
>>
>>