Re: [STABLE v5.15-v5.10 5/6] HID: steam: Refactor registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hid: steam: initial connection status dropped due to missing hid_device_io_start
--

commit 06e6c231f0f1bba41bfa16d27899ea2f3bb451a9
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: steam: Refactor registration

This commit refactors and simplifies the registration/unregistration flow.
It relies on the client open state and a single boolean flag to manage
registration logic, and ensures the battery device is unregistered if later
initialization steps fail.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c

[ ... ]

> @@ -739,20 +768,33 @@ static int steam_probe(struct hid_device *hdev,
[ ... ]
>  	if (steam->quirks & STEAM_QUIRK_WIRELESS) {
>  		hid_info(hdev, "Steam wireless receiver connected");
>  		/* If using a wireless adaptor ask for connection status */
>  		steam->connected = false;
>  		steam_request_conn_status(steam);
>  	} else {
>  		/* A wired connection is always present */
>  		steam->connected = true;
>  		ret = steam_register(steam);
>  		if (ret) {
>  			hid_err(hdev,
>  				"%s:steam_register failed with error %d\n",
>  				__func__, ret);
> -			goto input_register_fail;
> +			goto err_hw_close;
>  		}
>  	}
>  
> +	steam->client_hdev = steam_create_client_hid(hdev);
> +	if (IS_ERR(steam->client_hdev)) {
> +		ret = PTR_ERR(steam->client_hdev);
> +		goto err_stream_unregister;
> +	}
> +	steam->client_hdev->driver_data = steam;
> +
> +	ret = hid_add_device(steam->client_hdev);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will the earlier asynchronous status request be dropped because the HID core
input gate is still closed?

By moving the hid_add_device(steam->client_hdev) call to execute after
steam_request_conn_status(), the steam_probe() function takes longer to
return.

The hardware replies asynchronously with a STEAM_EV_CONNECT event on the
interrupt endpoint. Because hid_device_io_start() has not been called, the
HID core driver_input_lock remains held while the rest of steam_probe()
executes.

If the interrupt URB completes during this widened window, won't the HID
core unconditionally drop the connection status report, leaving the wireless
controller unresponsive?

> +	if (ret)
> +		goto err_destroy;
> +
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013091215.529531-1-lee@kernel.org?part=5