[PATCH] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[PATCH] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Martino Fontana]

When suspending the computer, a Switch Pro Controller connected via USB will lose its internal status.
However, because the USB connection was technically never lost, when resuming the computer, the driver will attempt to communicate with the controller as if nothing happened (and fail).
Because of this, the user was forced to manually disconnect the controller (or to press the sync button on the controller to power it off), so that it can be re-initialized.

With this patch, the controller will be automatically re-initialized after resuming from suspend.
Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233

Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
 drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
 1 file changed, 106 insertions(+), 72 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 250f5d2f8..586c7f8d7 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
 	struct joycon_input_report *report;
 
 	req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
+	mutex_lock(&ctlr->output_mutex);
 	ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
+	mutex_unlock(&ctlr->output_mutex);
 	if (ret) {
 		hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
 		return ret;
@@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
 	return 0;
 }
 
+static int joycon_init(struct hid_device *hdev)
+{
+	struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
+	int ret = 0;
+
+	mutex_lock(&ctlr->output_mutex);
+	/* if handshake command fails, assume ble pro controller */
+	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
+	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
+		hid_dbg(hdev, "detected USB controller\n");
+		/* set baudrate for improved latency */
+		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
+		if (ret) {
+			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
+			goto err_mutex;
+		}
+		/* handshake */
+		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
+		if (ret) {
+			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
+			goto err_mutex;
+		}
+		/*
+		 * Set no timeout (to keep controller in USB mode).
+		 * This doesn't send a response, so ignore the timeout.
+		 */
+		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
+	} else if (jc_type_is_chrggrip(ctlr)) {
+		hid_err(hdev, "Failed charging grip handshake\n");
+		ret = -ETIMEDOUT;
+		goto err_mutex;
+	}
+
+	/* get controller calibration data, and parse it */
+	ret = joycon_request_calibration(ctlr);
+	if (ret) {
+		/*
+		 * We can function with default calibration, but it may be
+		 * inaccurate. Provide a warning, and continue on.
+		 */
+		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+	}
+
+	/* get IMU calibration data, and parse it */
+	ret = joycon_request_imu_calibration(ctlr);
+	if (ret) {
+		/*
+		 * We can function with default calibration, but it may be
+		 * inaccurate. Provide a warning, and continue on.
+		 */
+		hid_warn(hdev, "Unable to read IMU calibration data\n");
+	}
+
+	/* Set the reporting mode to 0x30, which is the full report mode */
+	ret = joycon_set_report_mode(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	/* Enable rumble */
+	ret = joycon_enable_rumble(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	/* Enable the IMU */
+	ret = joycon_enable_imu(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	mutex_unlock(&ctlr->output_mutex);
+	return 0;
+
+err_mutex:
+	mutex_unlock(&ctlr->output_mutex);
+	return ret;
+}
+
 /* Common handler for parsing inputs */
 static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
 							      int size)
@@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 
 	hid_device_io_start(hdev);
 
-	/* Initialize the controller */
-	mutex_lock(&ctlr->output_mutex);
-	/* if handshake command fails, assume ble pro controller */
-	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
-	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
-		hid_dbg(hdev, "detected USB controller\n");
-		/* set baudrate for improved latency */
-		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
-		if (ret) {
-			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
-			goto err_mutex;
-		}
-		/* handshake */
-		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
-		if (ret) {
-			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
-			goto err_mutex;
-		}
-		/*
-		 * Set no timeout (to keep controller in USB mode).
-		 * This doesn't send a response, so ignore the timeout.
-		 */
-		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
-	} else if (jc_type_is_chrggrip(ctlr)) {
-		hid_err(hdev, "Failed charging grip handshake\n");
-		ret = -ETIMEDOUT;
-		goto err_mutex;
-	}
-
-	/* get controller calibration data, and parse it */
-	ret = joycon_request_calibration(ctlr);
-	if (ret) {
-		/*
-		 * We can function with default calibration, but it may be
-		 * inaccurate. Provide a warning, and continue on.
-		 */
-		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
-	}
-
-	/* get IMU calibration data, and parse it */
-	ret = joycon_request_imu_calibration(ctlr);
-	if (ret) {
-		/*
-		 * We can function with default calibration, but it may be
-		 * inaccurate. Provide a warning, and continue on.
-		 */
-		hid_warn(hdev, "Unable to read IMU calibration data\n");
-	}
-
-	/* Set the reporting mode to 0x30, which is the full report mode */
-	ret = joycon_set_report_mode(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
-		goto err_mutex;
-	}
-
-	/* Enable rumble */
-	ret = joycon_enable_rumble(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
-		goto err_mutex;
-	}
-
-	/* Enable the IMU */
-	ret = joycon_enable_imu(ctlr);
+	ret = joycon_init(hdev);
 	if (ret) {
-		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
-		goto err_mutex;
+		hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
+		goto err_close;
 	}
 
 	ret = joycon_read_info(ctlr);
 	if (ret) {
 		hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
 			ret);
-		goto err_mutex;
+		goto err_close;
 	}
 
-	mutex_unlock(&ctlr->output_mutex);
-
 	/* Initialize the leds */
 	ret = joycon_leds_create(ctlr);
 	if (ret) {
@@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 	hid_dbg(hdev, "probe - success\n");
 	return 0;
 
-err_mutex:
-	mutex_unlock(&ctlr->output_mutex);
 err_close:
 	hid_hw_close(hdev);
 err_stop:
@@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
 	hid_hw_stop(hdev);
 }
 
+#ifdef CONFIG_PM
+
+static int nintendo_hid_resume(struct hid_device *hdev)
+{
+	int ret = joycon_init(hdev);
+
+	if (ret)
+		hid_err(hdev, "Failed to restore controller after resume");
+
+	return ret;
+}
+
+#endif
+
 static const struct hid_device_id nintendo_hid_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
 			 USB_DEVICE_ID_NINTENDO_PROCON) },
@@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
 	.probe		= nintendo_hid_probe,
 	.remove		= nintendo_hid_remove,
 	.raw_event	= nintendo_hid_event,
+
+#ifdef CONFIG_PM
+	.resume	          = nintendo_hid_resume,
+#endif
 };
 module_hid_driver(nintendo_hid_driver);
 
-- 
2.41.0

Re: [PATCH] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Silvan Jegen]

Hi

Thanks for the patch!

Some comments below.

Martino Fontana <tinozzo123@gmail.com> wrote:
> When suspending the computer, a Switch Pro Controller connected via USB will lose its internal status.
> However, because the USB connection was technically never lost, when resuming the computer, the driver will attempt to communicate with the controller as if nothing happened (and fail).
> Because of this, the user was forced to manually disconnect the controller (or to press the sync button on the controller to power it off), so that it can be re-initialized.
> 
> With this patch, the controller will be automatically re-initialized after resuming from suspend.

The commit message would read nicer when wrapped around 78 chars or so.


> Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233
> 
> Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
> ---
>  drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
>  1 file changed, 106 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 250f5d2f8..586c7f8d7 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
>  	struct joycon_input_report *report;
>  
>  	req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
> +	mutex_lock(&ctlr->output_mutex);
>  	ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
> +	mutex_unlock(&ctlr->output_mutex);
>  	if (ret) {
>  		hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
>  		return ret;
> @@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
>  	return 0;
>  }
>  
> +static int joycon_init(struct hid_device *hdev)
> +{
> +	struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> +	int ret = 0;
> +
> +	mutex_lock(&ctlr->output_mutex);
> +	/* if handshake command fails, assume ble pro controller */
> +	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> +	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> +		hid_dbg(hdev, "detected USB controller\n");
> +		/* set baudrate for improved latency */
> +		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> +		if (ret) {
> +			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> +			goto err_mutex;
> +		}
> +		/* handshake */
> +		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> +		if (ret) {
> +			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> +			goto err_mutex;
> +		}
> +		/*
> +		 * Set no timeout (to keep controller in USB mode).
> +		 * This doesn't send a response, so ignore the timeout.
> +		 */
> +		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> +	} else if (jc_type_is_chrggrip(ctlr)) {
> +		hid_err(hdev, "Failed charging grip handshake\n");
> +		ret = -ETIMEDOUT;
> +		goto err_mutex;
> +	}
> +
> +	/* get controller calibration data, and parse it */
> +	ret = joycon_request_calibration(ctlr);
> +	if (ret) {
> +		/*
> +		 * We can function with default calibration, but it may be
> +		 * inaccurate. Provide a warning, and continue on.
> +		 */
> +		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> +	}
> +
> +	/* get IMU calibration data, and parse it */
> +	ret = joycon_request_imu_calibration(ctlr);
> +	if (ret) {
> +		/*
> +		 * We can function with default calibration, but it may be
> +		 * inaccurate. Provide a warning, and continue on.
> +		 */
> +		hid_warn(hdev, "Unable to read IMU calibration data\n");
> +	}
> +
> +	/* Set the reporting mode to 0x30, which is the full report mode */
> +	ret = joycon_set_report_mode(ctlr);
> +	if (ret) {
> +		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> +		goto err_mutex;
> +	}
> +
> +	/* Enable rumble */
> +	ret = joycon_enable_rumble(ctlr);
> +	if (ret) {
> +		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> +		goto err_mutex;
> +	}
> +
> +	/* Enable the IMU */
> +	ret = joycon_enable_imu(ctlr);
> +	if (ret) {
> +		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> +		goto err_mutex;
> +	}
> +
> +	mutex_unlock(&ctlr->output_mutex);
> +	return 0;
> +
> +err_mutex:
> +	mutex_unlock(&ctlr->output_mutex);
> +	return ret;
> +}
> +
>  /* Common handler for parsing inputs */
>  static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
>  							      int size)
> @@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>  
>  	hid_device_io_start(hdev);
>  
> -	/* Initialize the controller */
> -	mutex_lock(&ctlr->output_mutex);
> -	/* if handshake command fails, assume ble pro controller */
> -	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> -	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> -		hid_dbg(hdev, "detected USB controller\n");
> -		/* set baudrate for improved latency */
> -		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> -		if (ret) {
> -			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> -			goto err_mutex;
> -		}
> -		/* handshake */
> -		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> -		if (ret) {
> -			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> -			goto err_mutex;
> -		}
> -		/*
> -		 * Set no timeout (to keep controller in USB mode).
> -		 * This doesn't send a response, so ignore the timeout.
> -		 */
> -		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> -	} else if (jc_type_is_chrggrip(ctlr)) {
> -		hid_err(hdev, "Failed charging grip handshake\n");
> -		ret = -ETIMEDOUT;
> -		goto err_mutex;
> -	}
> -
> -	/* get controller calibration data, and parse it */
> -	ret = joycon_request_calibration(ctlr);
> -	if (ret) {
> -		/*
> -		 * We can function with default calibration, but it may be
> -		 * inaccurate. Provide a warning, and continue on.
> -		 */
> -		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> -	}
> -
> -	/* get IMU calibration data, and parse it */
> -	ret = joycon_request_imu_calibration(ctlr);
> -	if (ret) {
> -		/*
> -		 * We can function with default calibration, but it may be
> -		 * inaccurate. Provide a warning, and continue on.
> -		 */
> -		hid_warn(hdev, "Unable to read IMU calibration data\n");
> -	}
> -
> -	/* Set the reporting mode to 0x30, which is the full report mode */
> -	ret = joycon_set_report_mode(ctlr);
> -	if (ret) {
> -		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> -		goto err_mutex;
> -	}
> -
> -	/* Enable rumble */
> -	ret = joycon_enable_rumble(ctlr);
> -	if (ret) {
> -		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> -		goto err_mutex;
> -	}
> -
> -	/* Enable the IMU */
> -	ret = joycon_enable_imu(ctlr);
> +	ret = joycon_init(hdev);
>  	if (ret) {
> -		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> -		goto err_mutex;
> +		hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
> +		goto err_close;
>  	}
>  
>  	ret = joycon_read_info(ctlr);
>  	if (ret) {
>  		hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
>  			ret);
> -		goto err_mutex;
> +		goto err_close;
>  	}
>  
> -	mutex_unlock(&ctlr->output_mutex);
> -
>  	/* Initialize the leds */
>  	ret = joycon_leds_create(ctlr);
>  	if (ret) {
> @@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>  	hid_dbg(hdev, "probe - success\n");
>  	return 0;
>  
> -err_mutex:
> -	mutex_unlock(&ctlr->output_mutex);
>  err_close:
>  	hid_hw_close(hdev);
>  err_stop:
> @@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
>  	hid_hw_stop(hdev);
>  }
>  
> +#ifdef CONFIG_PM
> +
> +static int nintendo_hid_resume(struct hid_device *hdev)
> +{
> +	int ret = joycon_init(hdev);
> +

Removing this empty line would be more consistent with the rest of
the code.


> +	if (ret)
> +		hid_err(hdev, "Failed to restore controller after resume");
> +
> +	return ret;
> +}
> +
> +#endif
> +
>  static const struct hid_device_id nintendo_hid_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>  			 USB_DEVICE_ID_NINTENDO_PROCON) },
> @@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
>  	.probe		= nintendo_hid_probe,
>  	.remove		= nintendo_hid_remove,
>  	.raw_event	= nintendo_hid_event,
> +
> +#ifdef CONFIG_PM
> +	.resume	          = nintendo_hid_resume,

We shouldn't mix tabs and spaces for formatting here.


Cheers,
Silvan

> +#endif
>  };
>  module_hid_driver(nintendo_hid_driver);
>  

[PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Martino Fontana]

When suspending the computer, a Switch Pro Controller connected via USB will
lose its internal status. However, because the USB connection was technically
never lost, when resuming the computer, the driver will attempt to communicate
with the controller as if nothing happened (and fail).
Because of this, the user was forced to manually disconnect the controller
(or to press the sync button on the controller to power it off), so that it
can be re-initialized.

With this patch, the controller will be automatically re-initialized after
resuming from suspend.

Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233

Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
 drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
 1 file changed, 106 insertions(+), 72 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 250f5d2f8..a5ebe857a 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
 	struct joycon_input_report *report;
 
 	req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
+	mutex_lock(&ctlr->output_mutex);
 	ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
+	mutex_unlock(&ctlr->output_mutex);
 	if (ret) {
 		hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
 		return ret;
@@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
 	return 0;
 }
 
+static int joycon_init(struct hid_device *hdev)
+{
+	struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
+	int ret = 0;
+
+	mutex_lock(&ctlr->output_mutex);
+	/* if handshake command fails, assume ble pro controller */
+	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
+	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
+		hid_dbg(hdev, "detected USB controller\n");
+		/* set baudrate for improved latency */
+		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
+		if (ret) {
+			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
+			goto err_mutex;
+		}
+		/* handshake */
+		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
+		if (ret) {
+			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
+			goto err_mutex;
+		}
+		/*
+		 * Set no timeout (to keep controller in USB mode).
+		 * This doesn't send a response, so ignore the timeout.
+		 */
+		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
+	} else if (jc_type_is_chrggrip(ctlr)) {
+		hid_err(hdev, "Failed charging grip handshake\n");
+		ret = -ETIMEDOUT;
+		goto err_mutex;
+	}
+
+	/* get controller calibration data, and parse it */
+	ret = joycon_request_calibration(ctlr);
+	if (ret) {
+		/*
+		 * We can function with default calibration, but it may be
+		 * inaccurate. Provide a warning, and continue on.
+		 */
+		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+	}
+
+	/* get IMU calibration data, and parse it */
+	ret = joycon_request_imu_calibration(ctlr);
+	if (ret) {
+		/*
+		 * We can function with default calibration, but it may be
+		 * inaccurate. Provide a warning, and continue on.
+		 */
+		hid_warn(hdev, "Unable to read IMU calibration data\n");
+	}
+
+	/* Set the reporting mode to 0x30, which is the full report mode */
+	ret = joycon_set_report_mode(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	/* Enable rumble */
+	ret = joycon_enable_rumble(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	/* Enable the IMU */
+	ret = joycon_enable_imu(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	mutex_unlock(&ctlr->output_mutex);
+	return 0;
+
+err_mutex:
+	mutex_unlock(&ctlr->output_mutex);
+	return ret;
+}
+
 /* Common handler for parsing inputs */
 static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
 							      int size)
@@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 
 	hid_device_io_start(hdev);
 
-	/* Initialize the controller */
-	mutex_lock(&ctlr->output_mutex);
-	/* if handshake command fails, assume ble pro controller */
-	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
-	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
-		hid_dbg(hdev, "detected USB controller\n");
-		/* set baudrate for improved latency */
-		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
-		if (ret) {
-			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
-			goto err_mutex;
-		}
-		/* handshake */
-		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
-		if (ret) {
-			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
-			goto err_mutex;
-		}
-		/*
-		 * Set no timeout (to keep controller in USB mode).
-		 * This doesn't send a response, so ignore the timeout.
-		 */
-		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
-	} else if (jc_type_is_chrggrip(ctlr)) {
-		hid_err(hdev, "Failed charging grip handshake\n");
-		ret = -ETIMEDOUT;
-		goto err_mutex;
-	}
-
-	/* get controller calibration data, and parse it */
-	ret = joycon_request_calibration(ctlr);
-	if (ret) {
-		/*
-		 * We can function with default calibration, but it may be
-		 * inaccurate. Provide a warning, and continue on.
-		 */
-		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
-	}
-
-	/* get IMU calibration data, and parse it */
-	ret = joycon_request_imu_calibration(ctlr);
-	if (ret) {
-		/*
-		 * We can function with default calibration, but it may be
-		 * inaccurate. Provide a warning, and continue on.
-		 */
-		hid_warn(hdev, "Unable to read IMU calibration data\n");
-	}
-
-	/* Set the reporting mode to 0x30, which is the full report mode */
-	ret = joycon_set_report_mode(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
-		goto err_mutex;
-	}
-
-	/* Enable rumble */
-	ret = joycon_enable_rumble(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
-		goto err_mutex;
-	}
-
-	/* Enable the IMU */
-	ret = joycon_enable_imu(ctlr);
+	ret = joycon_init(hdev);
 	if (ret) {
-		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
-		goto err_mutex;
+		hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
+		goto err_close;
 	}
 
 	ret = joycon_read_info(ctlr);
 	if (ret) {
 		hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
 			ret);
-		goto err_mutex;
+		goto err_close;
 	}
 
-	mutex_unlock(&ctlr->output_mutex);
-
 	/* Initialize the leds */
 	ret = joycon_leds_create(ctlr);
 	if (ret) {
@@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 	hid_dbg(hdev, "probe - success\n");
 	return 0;
 
-err_mutex:
-	mutex_unlock(&ctlr->output_mutex);
 err_close:
 	hid_hw_close(hdev);
 err_stop:
@@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
 	hid_hw_stop(hdev);
 }
 
+#ifdef CONFIG_PM
+
+static int nintendo_hid_resume(struct hid_device *hdev)
+{
+	int ret = joycon_init(hdev);
+
+	if (ret)
+		hid_err(hdev, "Failed to restore controller after resume");
+
+	return ret;
+}
+
+#endif
+
 static const struct hid_device_id nintendo_hid_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
 			 USB_DEVICE_ID_NINTENDO_PROCON) },
@@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
 	.probe		= nintendo_hid_probe,
 	.remove		= nintendo_hid_remove,
 	.raw_event	= nintendo_hid_event,
+
+#ifdef CONFIG_PM
+	.resume		= nintendo_hid_resume,
+#endif
 };
 module_hid_driver(nintendo_hid_driver);
 
-- 
2.41.0

Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Silvan Jegen]

Martino Fontana <tinozzo123@gmail.com> wrote:
> When suspending the computer, a Switch Pro Controller connected via USB will
> lose its internal status. However, because the USB connection was technically
> never lost, when resuming the computer, the driver will attempt to communicate
> with the controller as if nothing happened (and fail).
> Because of this, the user was forced to manually disconnect the controller
> (or to press the sync button on the controller to power it off), so that it
> can be re-initialized.
> 
> With this patch, the controller will be automatically re-initialized after
> resuming from suspend.
> 
> Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233
> 
> Signed-off-by: Martino Fontana <tinozzo123@gmail.com>

It would be good to add a small section about what changed between v1
and v2 of the patch. Here is an example for how this can be done.

https://lore.kernel.org/lkml/cover.b24362332ec6099bc8db4e8e06a67545c653291d.1689842332.git-series.apopple@nvidia.com/T/#m73dd8d44f40742e67cbb0d4f030a90b6264a88d3

It's probably not worth resending this patch just for this though. Just
something to keep in mind if there will be another patch version needed.

Cheers,
Silvan

> ---
>  drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
>  1 file changed, 106 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 250f5d2f8..a5ebe857a 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
>  	struct joycon_input_report *report;
>  
>  	req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
> +	mutex_lock(&ctlr->output_mutex);
>  	ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
> +	mutex_unlock(&ctlr->output_mutex);
>  	if (ret) {
>  		hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
>  		return ret;
> @@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
>  	return 0;
>  }
>  
> +static int joycon_init(struct hid_device *hdev)
> +{
> +	struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> +	int ret = 0;
> +
> +	mutex_lock(&ctlr->output_mutex);
> +	/* if handshake command fails, assume ble pro controller */
> +	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> +	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> +		hid_dbg(hdev, "detected USB controller\n");
> +		/* set baudrate for improved latency */
> +		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> +		if (ret) {
> +			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> +			goto err_mutex;
> +		}
> +		/* handshake */
> +		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> +		if (ret) {
> +			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> +			goto err_mutex;
> +		}
> +		/*
> +		 * Set no timeout (to keep controller in USB mode).
> +		 * This doesn't send a response, so ignore the timeout.
> +		 */
> +		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> +	} else if (jc_type_is_chrggrip(ctlr)) {
> +		hid_err(hdev, "Failed charging grip handshake\n");
> +		ret = -ETIMEDOUT;
> +		goto err_mutex;
> +	}
> +
> +	/* get controller calibration data, and parse it */
> +	ret = joycon_request_calibration(ctlr);
> +	if (ret) {
> +		/*
> +		 * We can function with default calibration, but it may be
> +		 * inaccurate. Provide a warning, and continue on.
> +		 */
> +		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> +	}
> +
> +	/* get IMU calibration data, and parse it */
> +	ret = joycon_request_imu_calibration(ctlr);
> +	if (ret) {
> +		/*
> +		 * We can function with default calibration, but it may be
> +		 * inaccurate. Provide a warning, and continue on.
> +		 */
> +		hid_warn(hdev, "Unable to read IMU calibration data\n");
> +	}
> +
> +	/* Set the reporting mode to 0x30, which is the full report mode */
> +	ret = joycon_set_report_mode(ctlr);
> +	if (ret) {
> +		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> +		goto err_mutex;
> +	}
> +
> +	/* Enable rumble */
> +	ret = joycon_enable_rumble(ctlr);
> +	if (ret) {
> +		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> +		goto err_mutex;
> +	}
> +
> +	/* Enable the IMU */
> +	ret = joycon_enable_imu(ctlr);
> +	if (ret) {
> +		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> +		goto err_mutex;
> +	}
> +
> +	mutex_unlock(&ctlr->output_mutex);
> +	return 0;
> +
> +err_mutex:
> +	mutex_unlock(&ctlr->output_mutex);
> +	return ret;
> +}
> +
>  /* Common handler for parsing inputs */
>  static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
>  							      int size)
> @@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>  
>  	hid_device_io_start(hdev);
>  
> -	/* Initialize the controller */
> -	mutex_lock(&ctlr->output_mutex);
> -	/* if handshake command fails, assume ble pro controller */
> -	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> -	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> -		hid_dbg(hdev, "detected USB controller\n");
> -		/* set baudrate for improved latency */
> -		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> -		if (ret) {
> -			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> -			goto err_mutex;
> -		}
> -		/* handshake */
> -		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> -		if (ret) {
> -			hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> -			goto err_mutex;
> -		}
> -		/*
> -		 * Set no timeout (to keep controller in USB mode).
> -		 * This doesn't send a response, so ignore the timeout.
> -		 */
> -		joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> -	} else if (jc_type_is_chrggrip(ctlr)) {
> -		hid_err(hdev, "Failed charging grip handshake\n");
> -		ret = -ETIMEDOUT;
> -		goto err_mutex;
> -	}
> -
> -	/* get controller calibration data, and parse it */
> -	ret = joycon_request_calibration(ctlr);
> -	if (ret) {
> -		/*
> -		 * We can function with default calibration, but it may be
> -		 * inaccurate. Provide a warning, and continue on.
> -		 */
> -		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> -	}
> -
> -	/* get IMU calibration data, and parse it */
> -	ret = joycon_request_imu_calibration(ctlr);
> -	if (ret) {
> -		/*
> -		 * We can function with default calibration, but it may be
> -		 * inaccurate. Provide a warning, and continue on.
> -		 */
> -		hid_warn(hdev, "Unable to read IMU calibration data\n");
> -	}
> -
> -	/* Set the reporting mode to 0x30, which is the full report mode */
> -	ret = joycon_set_report_mode(ctlr);
> -	if (ret) {
> -		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> -		goto err_mutex;
> -	}
> -
> -	/* Enable rumble */
> -	ret = joycon_enable_rumble(ctlr);
> -	if (ret) {
> -		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> -		goto err_mutex;
> -	}
> -
> -	/* Enable the IMU */
> -	ret = joycon_enable_imu(ctlr);
> +	ret = joycon_init(hdev);
>  	if (ret) {
> -		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> -		goto err_mutex;
> +		hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
> +		goto err_close;
>  	}
>  
>  	ret = joycon_read_info(ctlr);
>  	if (ret) {
>  		hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
>  			ret);
> -		goto err_mutex;
> +		goto err_close;
>  	}
>  
> -	mutex_unlock(&ctlr->output_mutex);
> -
>  	/* Initialize the leds */
>  	ret = joycon_leds_create(ctlr);
>  	if (ret) {
> @@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>  	hid_dbg(hdev, "probe - success\n");
>  	return 0;
>  
> -err_mutex:
> -	mutex_unlock(&ctlr->output_mutex);
>  err_close:
>  	hid_hw_close(hdev);
>  err_stop:
> @@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
>  	hid_hw_stop(hdev);
>  }
>  
> +#ifdef CONFIG_PM
> +
> +static int nintendo_hid_resume(struct hid_device *hdev)
> +{
> +	int ret = joycon_init(hdev);
> +
> +	if (ret)
> +		hid_err(hdev, "Failed to restore controller after resume");
> +
> +	return ret;
> +}
> +
> +#endif
> +
>  static const struct hid_device_id nintendo_hid_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>  			 USB_DEVICE_ID_NINTENDO_PROCON) },
> @@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
>  	.probe		= nintendo_hid_probe,
>  	.remove		= nintendo_hid_remove,
>  	.raw_event	= nintendo_hid_event,
> +
> +#ifdef CONFIG_PM
> +	.resume		= nintendo_hid_resume,
> +#endif
>  };
>  module_hid_driver(nintendo_hid_driver);
>  

Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Martino Fontana]

It is my first patch on the Linux kernel, so I just did kinda what I
would do on GitHub (amend and force push).
What should I do here in case of trivial adjustments?

On Fri, Jul 21, 2023 at 9:02 PM Silvan Jegen <s.jegen@gmail.com> wrote:
>
> Martino Fontana <tinozzo123@gmail.com> wrote:
> > When suspending the computer, a Switch Pro Controller connected via USB will
> > lose its internal status. However, because the USB connection was technically
> > never lost, when resuming the computer, the driver will attempt to communicate
> > with the controller as if nothing happened (and fail).
> > Because of this, the user was forced to manually disconnect the controller
> > (or to press the sync button on the controller to power it off), so that it
> > can be re-initialized.
> >
> > With this patch, the controller will be automatically re-initialized after
> > resuming from suspend.
> >
> > Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233
> >
> > Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
>
> It would be good to add a small section about what changed between v1
> and v2 of the patch. Here is an example for how this can be done.
>
> https://lore.kernel.org/lkml/cover.b24362332ec6099bc8db4e8e06a67545c653291d.1689842332.git-series.apopple@nvidia.com/T/#m73dd8d44f40742e67cbb0d4f030a90b6264a88d3
>
> It's probably not worth resending this patch just for this though. Just
> something to keep in mind if there will be another patch version needed.
>
> Cheers,
> Silvan
>
> > ---
> >  drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
> >  1 file changed, 106 insertions(+), 72 deletions(-)
> >
> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> > index 250f5d2f8..a5ebe857a 100644
> > --- a/drivers/hid/hid-nintendo.c
> > +++ b/drivers/hid/hid-nintendo.c
> > @@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> >       struct joycon_input_report *report;
> >
> >       req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
> > +     mutex_lock(&ctlr->output_mutex);
> >       ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
> > +     mutex_unlock(&ctlr->output_mutex);
> >       if (ret) {
> >               hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
> >               return ret;
> > @@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> >       return 0;
> >  }
> >
> > +static int joycon_init(struct hid_device *hdev)
> > +{
> > +     struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> > +     int ret = 0;
> > +
> > +     mutex_lock(&ctlr->output_mutex);
> > +     /* if handshake command fails, assume ble pro controller */
> > +     if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> > +         !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> > +             hid_dbg(hdev, "detected USB controller\n");
> > +             /* set baudrate for improved latency */
> > +             ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> > +             if (ret) {
> > +                     hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> > +                     goto err_mutex;
> > +             }
> > +             /* handshake */
> > +             ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> > +             if (ret) {
> > +                     hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> > +                     goto err_mutex;
> > +             }
> > +             /*
> > +              * Set no timeout (to keep controller in USB mode).
> > +              * This doesn't send a response, so ignore the timeout.
> > +              */
> > +             joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> > +     } else if (jc_type_is_chrggrip(ctlr)) {
> > +             hid_err(hdev, "Failed charging grip handshake\n");
> > +             ret = -ETIMEDOUT;
> > +             goto err_mutex;
> > +     }
> > +
> > +     /* get controller calibration data, and parse it */
> > +     ret = joycon_request_calibration(ctlr);
> > +     if (ret) {
> > +             /*
> > +              * We can function with default calibration, but it may be
> > +              * inaccurate. Provide a warning, and continue on.
> > +              */
> > +             hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> > +     }
> > +
> > +     /* get IMU calibration data, and parse it */
> > +     ret = joycon_request_imu_calibration(ctlr);
> > +     if (ret) {
> > +             /*
> > +              * We can function with default calibration, but it may be
> > +              * inaccurate. Provide a warning, and continue on.
> > +              */
> > +             hid_warn(hdev, "Unable to read IMU calibration data\n");
> > +     }
> > +
> > +     /* Set the reporting mode to 0x30, which is the full report mode */
> > +     ret = joycon_set_report_mode(ctlr);
> > +     if (ret) {
> > +             hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> > +             goto err_mutex;
> > +     }
> > +
> > +     /* Enable rumble */
> > +     ret = joycon_enable_rumble(ctlr);
> > +     if (ret) {
> > +             hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> > +             goto err_mutex;
> > +     }
> > +
> > +     /* Enable the IMU */
> > +     ret = joycon_enable_imu(ctlr);
> > +     if (ret) {
> > +             hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> > +             goto err_mutex;
> > +     }
> > +
> > +     mutex_unlock(&ctlr->output_mutex);
> > +     return 0;
> > +
> > +err_mutex:
> > +     mutex_unlock(&ctlr->output_mutex);
> > +     return ret;
> > +}
> > +
> >  /* Common handler for parsing inputs */
> >  static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
> >                                                             int size)
> > @@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> >
> >       hid_device_io_start(hdev);
> >
> > -     /* Initialize the controller */
> > -     mutex_lock(&ctlr->output_mutex);
> > -     /* if handshake command fails, assume ble pro controller */
> > -     if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> > -         !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> > -             hid_dbg(hdev, "detected USB controller\n");
> > -             /* set baudrate for improved latency */
> > -             ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> > -             if (ret) {
> > -                     hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> > -                     goto err_mutex;
> > -             }
> > -             /* handshake */
> > -             ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> > -             if (ret) {
> > -                     hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> > -                     goto err_mutex;
> > -             }
> > -             /*
> > -              * Set no timeout (to keep controller in USB mode).
> > -              * This doesn't send a response, so ignore the timeout.
> > -              */
> > -             joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> > -     } else if (jc_type_is_chrggrip(ctlr)) {
> > -             hid_err(hdev, "Failed charging grip handshake\n");
> > -             ret = -ETIMEDOUT;
> > -             goto err_mutex;
> > -     }
> > -
> > -     /* get controller calibration data, and parse it */
> > -     ret = joycon_request_calibration(ctlr);
> > -     if (ret) {
> > -             /*
> > -              * We can function with default calibration, but it may be
> > -              * inaccurate. Provide a warning, and continue on.
> > -              */
> > -             hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> > -     }
> > -
> > -     /* get IMU calibration data, and parse it */
> > -     ret = joycon_request_imu_calibration(ctlr);
> > -     if (ret) {
> > -             /*
> > -              * We can function with default calibration, but it may be
> > -              * inaccurate. Provide a warning, and continue on.
> > -              */
> > -             hid_warn(hdev, "Unable to read IMU calibration data\n");
> > -     }
> > -
> > -     /* Set the reporting mode to 0x30, which is the full report mode */
> > -     ret = joycon_set_report_mode(ctlr);
> > -     if (ret) {
> > -             hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> > -             goto err_mutex;
> > -     }
> > -
> > -     /* Enable rumble */
> > -     ret = joycon_enable_rumble(ctlr);
> > -     if (ret) {
> > -             hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> > -             goto err_mutex;
> > -     }
> > -
> > -     /* Enable the IMU */
> > -     ret = joycon_enable_imu(ctlr);
> > +     ret = joycon_init(hdev);
> >       if (ret) {
> > -             hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> > -             goto err_mutex;
> > +             hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
> > +             goto err_close;
> >       }
> >
> >       ret = joycon_read_info(ctlr);
> >       if (ret) {
> >               hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
> >                       ret);
> > -             goto err_mutex;
> > +             goto err_close;
> >       }
> >
> > -     mutex_unlock(&ctlr->output_mutex);
> > -
> >       /* Initialize the leds */
> >       ret = joycon_leds_create(ctlr);
> >       if (ret) {
> > @@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> >       hid_dbg(hdev, "probe - success\n");
> >       return 0;
> >
> > -err_mutex:
> > -     mutex_unlock(&ctlr->output_mutex);
> >  err_close:
> >       hid_hw_close(hdev);
> >  err_stop:
> > @@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
> >       hid_hw_stop(hdev);
> >  }
> >
> > +#ifdef CONFIG_PM
> > +
> > +static int nintendo_hid_resume(struct hid_device *hdev)
> > +{
> > +     int ret = joycon_init(hdev);
> > +
> > +     if (ret)
> > +             hid_err(hdev, "Failed to restore controller after resume");
> > +
> > +     return ret;
> > +}
> > +
> > +#endif
> > +
> >  static const struct hid_device_id nintendo_hid_devices[] = {
> >       { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >                        USB_DEVICE_ID_NINTENDO_PROCON) },
> > @@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
> >       .probe          = nintendo_hid_probe,
> >       .remove         = nintendo_hid_remove,
> >       .raw_event      = nintendo_hid_event,
> > +
> > +#ifdef CONFIG_PM
> > +     .resume         = nintendo_hid_resume,
> > +#endif
> >  };
> >  module_hid_driver(nintendo_hid_driver);
> >
>
>

Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Silvan Jegen]

Heyho

Martino Fontana <tinozzo123@gmail.com> wrote:
> It is my first patch on the Linux kernel, so I just did kinda what I
> would do on GitHub (amend and force push).
> What should I do here in case of trivial adjustments?

I would leave it as is and only respin this one if either there are more
comments on the old version or if the maintainer tells you to.

BTW, just as a heads-up: posting an email reply at the top of the quoted
email (aka "top-posting") is frowned upon in the Linux mailing lists. See

https://people.kernel.org/tglx/

for some pointers regarding netiquette.

Cheers,
Silvan

Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend

[Martino Fontana]

Hi

Is this patch (alongside "cleanup LED code") expected to land for Linux 6.6?
I'm asking because I can't find the patch in `for-next`.
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-next
(Since it's my first patch here, I don't know if it's supposed to be seen
there.)

Cheers

On Sun, 23 Jul 2023 at 20:58, Silvan Jegen <s.jegen@gmail.com> wrote:
>
> Heyho
>
> Martino Fontana <tinozzo123@gmail.com> wrote:
> > It is my first patch on the Linux kernel, so I just did kinda what I
> > would do on GitHub (amend and force push).
> > What should I do here in case of trivial adjustments?
>
> I would leave it as is and only respin this one if either there are more
> comments on the old version or if the maintainer tells you to.
>
> BTW, just as a heads-up: posting an email reply at the top of the quoted
> email (aka "top-posting") is frowned upon in the Linux mailing lists. See
>
> https://people.kernel.org/tglx/
>
> for some pointers regarding netiquette.
>
> Cheers,
> Silvan