linux-kernel - Re: [PATCH 09/11] HID: playstation: Rename DualSense input report status field

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEc3jaC-jm0kG3oM1z5BgW4FnXfgKztFg3rFyNUzKHieXxFh-Q@mail.gmail.com>
Date: Mon, 9 Jun 2025 21:40:03 -0700
From: Roderick Colenbrander <thunderbird2k@...il.com>
To: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
Cc: Roderick Colenbrander <roderick.colenbrander@...y.com>, Jiri Kosina <jikos@...nel.org>, 
	Benjamin Tissoires <bentiss@...nel.org>, Henrik Rydberg <rydberg@...math.org>, kernel@...labora.com, 
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/11] HID: playstation: Rename DualSense input report
 status field

Hi Christian,

At the time I didn't document all parts of the HID report as not
everything was needed. When adding in the audio stuff, let's call this
one by its official name. There are officially 3 bytes of status, so
the correct thing is to technically make it a 3 byte array of status.
The official register names contain DS_STATUS0_.., DS_STATUS1_..,
etcetera in the name, so I would like to keep things aligned with our
datasheets.

Thanks,
Roderick

On Mon, May 26, 2025 at 5:54 AM Cristian Ciocaltea
<cristian.ciocaltea@...labora.com> wrote:
>
> The name of the 'status' member of struct dualsense_input_report is too
> generic, since it is only used to provide battery data.
>
> In preparation to support handling additional (non-battery) status
> information and avoid ambiguity, rename the field to status_battery.
> For consistency, also rename the related DS_STATUS_* bitfield macros.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> ---
>  drivers/hid/hid-playstation.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 937b14679c8b17c5d3d245eae1cc4e0e56dadb71..b7777d3230b2fe277a4a2217ef6b1eff9cfad182 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -112,10 +112,10 @@ struct ps_led_info {
>  #define DS_BUTTONS2_TOUCHPAD   BIT(1)
>  #define DS_BUTTONS2_MIC_MUTE   BIT(2)
>
> -/* Status field of DualSense input report. */
> -#define DS_STATUS_BATTERY_CAPACITY     GENMASK(3, 0)
> -#define DS_STATUS_CHARGING             GENMASK(7, 4)
> -#define DS_STATUS_CHARGING_SHIFT       4
> +/* Battery status field of DualSense input report. */
> +#define DS_STATUS_BATTERY_CAPACITY             GENMASK(3, 0)
> +#define DS_STATUS_BATTERY_CHARGING             GENMASK(7, 4)
> +#define DS_STATUS_BATTERY_CHARGING_SHIFT       4
>
>  /* Feature version from DualSense Firmware Info report. */
>  #define DS_FEATURE_VERSION_MINOR               GENMASK(7, 0)
> @@ -236,7 +236,7 @@ struct dualsense_input_report {
>         struct dualsense_touch_point points[2];
>
>         u8 reserved3[12];
> -       u8 status;
> +       u8 status_battery;
>         u8 reserved4[10];
>  } __packed;
>  /* Common input report size shared equals the size of the USB report minus 1 byte for ReportID. */
> @@ -1462,8 +1462,9 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>         input_report_key(ds->touchpad, BTN_LEFT, ds_report->buttons[2] & DS_BUTTONS2_TOUCHPAD);
>         input_sync(ds->touchpad);
>
> -       battery_data = ds_report->status & DS_STATUS_BATTERY_CAPACITY;
> -       charging_status = (ds_report->status & DS_STATUS_CHARGING) >> DS_STATUS_CHARGING_SHIFT;
> +       battery_data = ds_report->status_battery & DS_STATUS_BATTERY_CAPACITY;
> +       charging_status = ds_report->status_battery & DS_STATUS_BATTERY_CHARGING;
> +       charging_status >>= DS_STATUS_BATTERY_CHARGING_SHIFT;
>
>         switch (charging_status) {
>         case 0x0:
>
> --
> 2.49.0
>
>