| 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
| ||
|
Message-ID: <CAEc3jaCEENSMbWFcQyjWLP+4UPv3_2inARZLJsmXYF7zVbhkug@mail.gmail.com>
Date: Sun, 8 Jun 2025 11:08:13 -0700
From: Roderick Colenbrander <thunderbird2k@...il.com>
To: Vadym Tytan <titanv3585@...il.com>
Cc: Roderick Colenbrander <roderick.colenbrander@...y.com>, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>, linux-kernel@...r.kernel.org,
linux-input@...r.kernel.org
Subject: Re: [PATCH] HID: playstation: DS4: Add BT poll interval adjustment
Hi,
Sorry for the late reply (business travel / vacation). This is the
only code I left out during hid-playstation creation. At the time when
I added this to hid-sony it felt like a good idea. But we later found
out the console wasn't even using this code itself. We haven't had a
need for it in various embedded projects. The code had significant
impact on battery (especially when set to like 1ms). The timing itself
was not that stable, it made it better from the hardware side
(measured using a BT packet sniffer), but there is overhead in
bluetooth stacks, which makes the timing not that predictable
Do you have a strong use case or reason to add it back?
Thanks,
Roderick
On Thu, May 8, 2025 at 2:43 PM Vadym Tytan <titanv3585@...il.com> wrote:
>
> Between v6.1 and v6.2 versions of Linux kernel,
> DualShock4 related code was moved from `hid-sony.c`
> to `hid-playstation.c` (near DualSense code) and
> Bluetooth poll interval adjustment functionality was lost
>
> Signed-off-by: Vadym Tytan <titanv3585@...il.com>
> ---
> CREDITS | 4 +++
> drivers/hid/hid-playstation.c | 64 +++++++++++++++++++++++++++++++++--
> 2 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/CREDITS b/CREDITS
> index f74d230992d6..137c3636e318 100644
> --- a/CREDITS
> +++ b/CREDITS
> @@ -4001,6 +4001,10 @@ S: 44 Campbell Park Crescent
> S: Edinburgh EH13 0HT
> S: United Kingdom
>
> +N: Vadym Tytan
> +E: titanv3585@...il.com
> +D: Minor game controllers features
> +
> N: Thomas Uhl
> E: uhl@...1.rz.fh-heilbronn.de
> D: Application programmer
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 1468fb11e39d..fd51f00b0516 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -337,7 +337,7 @@ struct dualsense_output_report {
> * 0x3F - disabled
> */
> #define DS4_OUTPUT_HWCTL_BT_POLL_MASK 0x3F
> -/* Default to 4ms poll interval, which is same as USB (not adjustable). */
> +/* Default to 4ms poll interval, which is same as USB (adjustable). */
The not adjustable referred to USB, which is not adjustable. So keep
that one the way it is.
> #define DS4_BT_DEFAULT_POLL_INTERVAL_MS 4
> #define DS4_OUTPUT_HWCTL_CRC32 0x40
> #define DS4_OUTPUT_HWCTL_HID 0x80
> @@ -542,6 +542,7 @@ static inline void dualsense_schedule_work(struct dualsense *ds);
> static inline void dualshock4_schedule_work(struct dualshock4 *ds4);
> static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue);
> static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4);
> +static int dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval);
>
> /*
> * Add a new ps_device to ps_devices if it doesn't exist.
> @@ -1738,6 +1739,43 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
> return ERR_PTR(ret);
> }
>
> +static ssize_t dualshock4_show_poll_interval(struct device *dev,
> + struct device_attribute
> + *attr, char *buf)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct ps_device *ps_dev = hid_get_drvdata(hdev);
> + struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
> +
> + return sysfs_emit(buf, "%i\n", ds4->bt_poll_interval);
> +}
> +
> +static ssize_t dualshock4_store_poll_interval(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct ps_device *ps_dev = hid_get_drvdata(hdev);
> + struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
> + int ret;
> + u8 interval;
> +
> + if (kstrtou8(buf, 0, &interval))
> + return -EINVAL;
> +
> + ret = dualshock4_set_bt_poll_interval(ds4, interval);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +struct device_attribute dev_attr_dualshock4_bt_poll_interval = {
> + .attr = { .name = "bt_poll_interval", .mode = 0644 },
> + .show = dualshock4_show_poll_interval,
> + .store = dualshock4_store_poll_interval,
> +};
> +
> static void dualshock4_dongle_calibration_work(struct work_struct *work)
> {
> struct dualshock4 *ds4 = container_of(work, struct dualshock4, dongle_hotplug_worker);
> @@ -2493,6 +2531,9 @@ static void dualshock4_remove(struct ps_device *ps_dev)
> struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base);
> unsigned long flags;
>
> + if (ps_dev->hdev->bus == BUS_BLUETOOTH)
> + device_remove_file(&ps_dev->hdev->dev, &dev_attr_dualshock4_bt_poll_interval);
> +
> spin_lock_irqsave(&ds4->base.lock, flags);
> ds4->output_worker_initialized = false;
> spin_unlock_irqrestore(&ds4->base.lock, flags);
> @@ -2513,11 +2554,16 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4)
> spin_unlock_irqrestore(&ds4->base.lock, flags);
> }
>
> -static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)
> +static int dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval)
> {
> + if (interval >= DS4_OUTPUT_HWCTL_BT_POLL_MASK)
> + return -EINVAL;
> +
> ds4->bt_poll_interval = interval;
> ds4->update_bt_poll_interval = true;
> dualshock4_schedule_work(ds4);
> +
> + return 0;
> }
>
> /* Set default lightbar color based on player. */
> @@ -2659,7 +2705,17 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
> goto err;
> }
>
> - dualshock4_set_bt_poll_interval(ds4, DS4_BT_DEFAULT_POLL_INTERVAL_MS);
> + if (hdev->bus == BUS_BLUETOOTH) {
> + ret = dualshock4_set_bt_poll_interval(ds4, DS4_BT_DEFAULT_POLL_INTERVAL_MS);
> + if (ret) {
> + hid_err(hdev, "Failed to set poll interval for DualShock4: %d\n", ret);
> + goto err;
> + }
> +
> + ret = device_create_file(&hdev->dev, &dev_attr_dualshock4_bt_poll_interval);
> + if (ret)
> + hid_warn(hdev, "can't create sysfs bt_poll_interval attribute err: %d\n", ret);
> + }
>
> ret = ps_device_set_player_id(ps_dev);
> if (ret) {
> @@ -2678,6 +2734,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
> return &ds4->base;
>
> err:
> + if (hdev->bus == BUS_BLUETOOTH)
> + device_remove_file(&hdev->dev, &dev_attr_dualshock4_bt_poll_interval);
> ps_devices_list_remove(ps_dev);
> return ERR_PTR(ret);
> }
> --
> 2.49.0
>
>
Powered by blists - more mailing lists