linux-kernel - Re: [PATCH v2 1/5] HID: steelseries: preparation for adding SteelSeries Arctis 9 support

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: <ee985d9040ce54478ad3db448850ea8676fee642.camel@hadess.net>
Date: Mon, 13 Jan 2025 14:42:21 +0100
From: Bastien Nocera <hadess@...ess.net>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>, Christian Mayer
	 <git@...er-bgk.de>, linux-input@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Jiri Kosina <jikos@...nel.org>, Benjamin
 Tissoires <benjamin.tissoires@...hat.com>
Subject: Re: [PATCH v2 1/5] HID: steelseries: preparation for adding
 SteelSeries Arctis 9 support

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@...er-bgk.de>

Feel free to add my:
Reviewed-by: Bastien Nocera <hadess@...ess.net>
Tested-by: Bastien Nocera <hadess@...ess.net>

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?
> 
> > +
> >   	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);
> >   }
> >   
>