[PATCH 0/3] hid: sony: Fix duplicate check logic and other cleanups

STALE3656d

5 messages, 2 authors, 2016-09-26 · open the first message on its own page

[PATCH 0/3] hid: sony: Fix duplicate check logic and other cleanups

From: Frank Praznik <hidden>
Date: 2016-09-23 00:18:41

This set of patches fixes a case where the duplicate check incorrectly
rejects valid devices and cleans up the behavior when a DualShock 3 is
initially connected.  It also adds a bit of documentation regarding the
DualShock 4 reporting rate as there have been a few cases where users
needed to slow it down since the full rate can be too much for some
slow embedded devices.

[PATCH 1/3] hid: sony: Relax duplicate checking for USB-only devices

From: Frank Praznik <hidden>
Date: 2016-09-23 00:18:37

Some USB-only devices which masquerade as Sixaxis controllers report the
same generic Bluetooth address for all hardware when queried via the HID
report. This causes these devices to be wrongly rejected as duplicates
when more than one is connected at once.

This introduces a connection type comparison when checking for duplicates
and only rejects the newly connected device if the existing matching
device is connected using a different connection protocol.

The results of the connection type comparison are also used when
registering power supply info as the device Bluetooth address is used
as the unique identifier string.  In cases where more than one valid
device has the same Bluetooth address the device ID is now appended
to the power supply name string to avoid name collisions when
registering the power supply information.

Signed-off-by: Frank Praznik <redacted>
---
 drivers/hid/hid-sony.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 310436a..92f27b7 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -65,6 +65,8 @@
 				MOTION_CONTROLLER_BT | NAVIGATION_CONTROLLER)
 #define SONY_FF_SUPPORT (SIXAXIS_CONTROLLER | DUALSHOCK4_CONTROLLER |\
 				MOTION_CONTROLLER)
+#define SONY_BT_DEVICE (SIXAXIS_CONTROLLER_BT | DUALSHOCK4_CONTROLLER_BT |\
+			MOTION_CONTROLLER_BT | NAVIGATION_CONTROLLER_BT)
 
 #define MAX_LEDS 4
 
@@ -2039,8 +2041,11 @@ static int sony_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
-static int sony_battery_probe(struct sony_sc *sc)
+static int sony_battery_probe(struct sony_sc *sc, int append_dev_id)
 {
+	const char *battery_str_fmt = append_dev_id ?
+		"sony_controller_battery_%pMR_%i" :
+		"sony_controller_battery_%pMR";
 	struct power_supply_config psy_cfg = { .drv_data = sc, };
 	struct hid_device *hdev = sc->hdev;
 	int ret;
@@ -2056,9 +2061,8 @@ static int sony_battery_probe(struct sony_sc *sc)
 	sc->battery_desc.get_property = sony_battery_get_property;
 	sc->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
 	sc->battery_desc.use_for_apm = 0;
-	sc->battery_desc.name = kasprintf(GFP_KERNEL,
-					  "sony_controller_battery_%pMR",
-					  sc->mac_address);
+	sc->battery_desc.name = kasprintf(GFP_KERNEL, battery_str_fmt,
+					  sc->mac_address, sc->device_id);
 	if (!sc->battery_desc.name)
 		return -ENOMEM;
 
@@ -2094,7 +2098,21 @@ static void sony_battery_remove(struct sony_sc *sc)
  * it will show up as two devices. A global list of connected controllers and
  * their MAC addresses is maintained to ensure that a device is only connected
  * once.
+ *
+ * Some USB-only devices masquerade as Sixaxis controllers and all have the
+ * same dummy Bluetooth address, so a comparison of the connection type is
+ * required.  Devices are only rejected in the case where two devices have
+ * matching Bluetooth addresses on different bus types.
  */
+static inline int sony_compare_connection_type(struct sony_sc *sc0,
+						struct sony_sc *sc1)
+{
+	const int sc0_not_bt = !(sc0->quirks & SONY_BT_DEVICE);
+	const int sc1_not_bt = !(sc1->quirks & SONY_BT_DEVICE);
+
+	return sc0_not_bt == sc1_not_bt;
+}
+
 static int sony_check_add_dev_list(struct sony_sc *sc)
 {
 	struct sony_sc *entry;
@@ -2107,9 +2125,14 @@ static int sony_check_add_dev_list(struct sony_sc *sc)
 		ret = memcmp(sc->mac_address, entry->mac_address,
 				sizeof(sc->mac_address));
 		if (!ret) {
-			ret = -EEXIST;
-			hid_info(sc->hdev, "controller with MAC address %pMR already connected\n",
+			if (sony_compare_connection_type(sc, entry)) {
+				ret = 1;
+			} else {
+				ret = -EEXIST;
+				hid_info(sc->hdev,
+				"controller with MAC address %pMR already connected\n",
 				sc->mac_address);
+			}
 			goto unlock;
 		}
 	}
@@ -2285,6 +2308,7 @@ static inline void sony_cancel_work_sync(struct sony_sc *sc)
 static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
+	int append_dev_id;
 	unsigned long quirks = id->driver_data;
 	struct sony_sc *sc;
 	unsigned int connect_mask = HID_CONNECT_DEFAULT;
@@ -2379,7 +2403,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (ret < 0)
 		goto err_stop;
 
-	ret = sony_check_add(sc);
+	ret = append_dev_id = sony_check_add(sc);
 	if (ret < 0)
 		goto err_stop;
 
@@ -2390,7 +2414,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (sc->quirks & SONY_BATTERY_SUPPORT) {
-		ret = sony_battery_probe(sc);
+		ret = sony_battery_probe(sc, append_dev_id);
 		if (ret < 0)
 			goto err_stop;
 
-- 
2.7.4

[PATCH 2/3] hid: sony: Defer the initial USB Sixaxis output report

From: Frank Praznik <hidden>
Date: 2016-09-23 00:18:38

When initially connected via USB the Sixaxis isn't fully initialized
until the PS logo button is pressed and won't send any input reports
nor will any state set by output reports be retained.

This adds a 'defer_initialization' flag to the sony_sc struct which,
when set, will delay sending any output reports until the first input
report has arrived. This flag is used with the USB Sixaxis to ensure
that any state sent will persist since, until the PS button is pushed,
any changes sent to the controller via an output report will be lost
after a couple of seconds.

The initial state of the controller is still configured at the time
of the initial connection and won't be internally modified after that,
so any state set by the user between that time and the recepit of the
first input report won't be lost.

Signed-off-by: Frank Praznik <redacted>
---

 - A variant of this patch has been in the SteamOS kernel for some
   time, so it's well-tested at this point.

 drivers/hid/hid-sony.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 92f27b7..9b96f95 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1050,6 +1050,7 @@ struct sony_sc {
 
 	u8 mac_address[6];
 	u8 worker_initialized;
+	u8 defer_initialization;
 	u8 cable_state;
 	u8 battery_charging;
 	u8 battery_capacity;
@@ -1060,6 +1061,12 @@ struct sony_sc {
 	u8 led_count;
 };
 
+static inline void sony_schedule_work(struct sony_sc *sc)
+{
+	if (!sc->defer_initialization)
+		schedule_work(&sc->state_worker);
+}
+
 static u8 *sixaxis_fixup(struct hid_device *hdev, u8 *rdesc,
 			     unsigned int *rsize)
 {
@@ -1319,6 +1326,11 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
 		dualshock4_parse_report(sc, rd, size);
 	}
 
+	if (sc->defer_initialization) {
+		sc->defer_initialization = 0;
+		sony_schedule_work(sc);
+	}
+
 	return 0;
 }
 
@@ -1556,7 +1568,7 @@ static void buzz_set_leds(struct sony_sc *sc)
 static void sony_set_leds(struct sony_sc *sc)
 {
 	if (!(sc->quirks & BUZZ_CONTROLLER))
-		schedule_work(&sc->state_worker);
+		sony_schedule_work(sc);
 	else
 		buzz_set_leds(sc);
 }
@@ -1667,7 +1679,7 @@ static int sony_led_blink_set(struct led_classdev *led, unsigned long *delay_on,
 		new_off != drv_data->led_delay_off[n]) {
 		drv_data->led_delay_on[n] = new_on;
 		drv_data->led_delay_off[n] = new_off;
-		schedule_work(&drv_data->state_worker);
+		sony_schedule_work(drv_data);
 	}
 
 	return 0;
@@ -1978,7 +1990,7 @@ static int sony_play_effect(struct input_dev *dev, void *data,
 	sc->left = effect->u.rumble.strong_magnitude / 256;
 	sc->right = effect->u.rumble.weak_magnitude / 256;
 
-	schedule_work(&sc->state_worker);
+	sony_schedule_work(sc);
 	return 0;
 }
 
@@ -2365,9 +2377,16 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		 * the Sixaxis does not want the report_id as part of the data
 		 * packet, so we have to discard buf[0] when sending the actual
 		 * control message, even for numbered reports, humpf!
+		 *
+		 * Additionally, the Sixaxis on USB isn't properly initialized
+		 * until the PS logo button is pressed and as such won't retain
+		 * any state set by an output report, so the initial
+		 * configuration report is deferred until the first input
+		 * report arrives.
 		 */
 		hdev->quirks |= HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP;
 		hdev->quirks |= HID_QUIRK_SKIP_OUTPUT_REPORT_ID;
+		sc->defer_initialization = 1;
 		ret = sixaxis_set_operational_usb(hdev);
 		sony_init_output_report(sc, sixaxis_send_output_report);
 	} else if ((sc->quirks & SIXAXIS_CONTROLLER_BT) ||
@@ -2510,8 +2529,10 @@ static int sony_resume(struct hid_device *hdev)
 		 * reinitialized on resume or they won't behave properly.
 		 */
 		if ((sc->quirks & SIXAXIS_CONTROLLER_USB) ||
-			(sc->quirks & NAVIGATION_CONTROLLER_USB))
+			(sc->quirks & NAVIGATION_CONTROLLER_USB)) {
 			sixaxis_set_operational_usb(sc->hdev);
+			sc->defer_initialization = 1;
+		}
 
 		sony_set_leds(sc);
 	}
-- 
2.7.4

[PATCH 3/3] hid: sony: Update copyright and add Dualshock 4 rate control note

From: Frank Praznik <hidden>
Date: 2016-09-23 00:18:39

Update the copyright notice with the current year and add a note
about values for controlling the Dualshock 4 reporting rate.

Processing reports at the default full rate of 1000hz can be too
demanding for some low-power embedded processors so noting
alternate values for people working with this hardware can be useful.

Thanks to Rostislav Pehlivanov for finding these values.

Signed-off-by: Frank Praznik <redacted>
---
 drivers/hid/hid-sony.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 9b96f95..9bf4e36 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -8,7 +8,7 @@
  *  Copyright (c) 2012 David Dillow <dave@thedillows.org>
  *  Copyright (c) 2006-2013 Jiri Kosina
  *  Copyright (c) 2013 Colin Leitner <colin.leitner@gmail.com>
- *  Copyright (c) 2014 Frank Praznik <frank.praznik@gmail.com>
+ *  Copyright (c) 2014-2016 Frank Praznik <frank.praznik@gmail.com>
  */
 
 /*
@@ -1879,6 +1879,17 @@ static void dualshock4_send_output_report(struct sony_sc *sc)
 	u8 *buf = sc->output_report_dmabuf;
 	int offset;
 
+	/*
+	 * NOTE: The buf[1] field of the Bluetooth report controls
+	 * the Dualshock 4 reporting rate.
+	 *
+	 * Known values include:
+	 *
+	 * 0x80 - 1000hz (full speed)
+	 * 0xA0 - 31hz
+	 * 0xB0 - 20hz
+	 * 0xD0 - 66hz
+	 */
 	if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
 		memset(buf, 0, DS4_REPORT_0x05_SIZE);
 		buf[0] = 0x05;
-- 
2.7.4

Re: [PATCH 0/3] hid: sony: Fix duplicate check logic and other cleanups

From: Jiri Kosina <jikos@kernel.org>
Date: 2016-09-26 13:36:37

On Thu, 22 Sep 2016, Frank Praznik wrote:
This set of patches fixes a case where the duplicate check incorrectly
rejects valid devices and cleans up the behavior when a DualShock 3 is
initially connected.  It also adds a bit of documentation regarding the
DualShock 4 reporting rate as there have been a few cases where users
needed to slow it down since the full rate can be too much for some
slow embedded devices.
Applied to hid.git#for-4.9/sony. Thanks,

-- 
Jiri Kosina
SUSE Labs
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help