Le 15/01/2025 à 18:20, Christian Mayer a écrit :
Am 13.01.25 um 14:42 schrieb Bastien Nocera:
On Sun, 2025-01-12 at 13:02 +0100, Christophe JAILLET wrote:
Le 12/01/2025 à 12:44, Christian Mayer a écrit :
Refactor code and add calls to hid_hw_open/hid_hw_closed in
preparation
for adding support for the SteelSeries Arctis 9 headset.
Signed-off-by: Christian Mayer <git@xxxxxxxxxxxx>
Feel free to add my:
Reviewed-by: Bastien Nocera <hadess@xxxxxxxxxx>
Tested-by: Bastien Nocera <hadess@xxxxxxxxxx>
once you've made the change Christophe mentions.
---
drivers/hid/hid-steelseries.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-
steelseries.c
index f9ff5be94309..dc4ab55d7c22 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -377,20 +377,21 @@ static void steelseries_srws1_remove(struct
hid_device *hdev)
#define ARCTIS_1_BATTERY_RESPONSE_LEN 8
static const char arctis_1_battery_request[] = { 0x06, 0x12 };
-static int steelseries_headset_arctis_1_fetch_battery(struct
hid_device *hdev)
+static int steelseries_headset_request_battery(struct hid_device
*hdev,
+ const char *request, size_t len)
{
u8 *write_buf;
int ret;
/* Request battery information */
- write_buf = kmemdup(arctis_1_battery_request,
sizeof(arctis_1_battery_request), GFP_KERNEL);
+ write_buf = kmemdup(request, len, GFP_KERNEL);
if (!write_buf)
return -ENOMEM;
- ret = hid_hw_raw_request(hdev,
arctis_1_battery_request[0],
- write_buf,
sizeof(arctis_1_battery_request),
+ hid_dbg(hdev, "Sending battery request report");
+ ret = hid_hw_raw_request(hdev, request[0], write_buf, len,
HID_OUTPUT_REPORT,
HID_REQ_SET_REPORT);
- if (ret < (int)sizeof(arctis_1_battery_request)) {
+ if (ret < (int)len) {
hid_err(hdev, "hid_hw_raw_request() failed with
%d\n", ret);
ret = -ENODATA;
}
@@ -404,7 +405,8 @@ static void
steelseries_headset_fetch_battery(struct hid_device *hdev)
int ret = 0;
if (sd->quirks & STEELSERIES_ARCTIS_1)
- ret =
steelseries_headset_arctis_1_fetch_battery(hdev);
+ ret = steelseries_headset_request_battery(hdev,
+ arctis_1_battery_request,
sizeof(arctis_1_battery_request));
if (ret < 0)
hid_dbg(hdev,
@@ -554,6 +556,10 @@ static int steelseries_probe(struct hid_device
*hdev, const struct hid_device_id
if (ret)
return ret;
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
Should we call hid_hw_stop() if
steelseries_headset_battery_register()
below fails, as done in the remove funcion?
Did you mean hid_hw_close instead of hid_hw_stop?
Yes, I mean hid_hw_close()
If the battery_register fails, the driver will still get assigned to the
device, because ret is not getting set and is still 0 from hid_hw_open,
right?
Correct. I miss-read this part.
If steelseries_headset_battery_register() fails, the probe still succeeds.
hid_hw_close doesn't get called before hid_hw_stop and the remove
function will get called when the device disconnects.
That doesn't seem right to me, or am i wrong?
I think you are right.
Not sure it is 100% future proof, but it looks correct.
Sorry for the noise.
CJ
+
if (steelseries_headset_battery_register(sd) < 0)
hid_err(sd->hdev,
"Failed to register battery for
headset\n");
@@ -580,6 +586,7 @@ static void steelseries_remove(struct
hid_device *hdev)
cancel_delayed_work_sync(&sd->battery_work);
+ hid_hw_close(hdev);
hid_hw_stop(hdev);
}
