Hi Martino, On Sun, Sep 24, 2023 at 10:13 AM Martino Fontana <tinozzo123@xxxxxxxxx> 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. > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=216233 > > Signed-off-by: Martino Fontana <tinozzo123@xxxxxxxxx> > > --- > Changes for v2 and v3: Applied suggestions > > drivers/hid/hid-nintendo.c | 175 ++++++++++++++++++++++--------------- > 1 file changed, 103 insertions(+), 72 deletions(-) > > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c > index 250f5d2f8..10468f727 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,85 @@ 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 out_unlock; > + } > + /* handshake */ > + ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ); > + if (ret) { > + hid_err(hdev, "Failed handshake; ret=%d\n", ret); > + goto out_unlock; > + } > + /* > + * 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 out_unlock; > + } > + > + /* 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 out_unlock; > + } > + > + /* Enable rumble */ > + ret = joycon_enable_rumble(ctlr); > + if (ret) { > + hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret); > + goto out_unlock; > + } > + > + /* Enable the IMU */ > + ret = joycon_enable_imu(ctlr); > + if (ret) { > + hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret); > + goto out_unlock; > + } > + > +out_unlock: > + 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 +2329,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); > + ret = joycon_init(hdev); > 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; > + 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 +2367,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 +2396,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 +2431,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.42.0 > Thanks for adding the resume hook for usb controllers. Looks good to me. Reviewed-by: Daniel J. Ogorchock <djogorchock@xxxxxxxxx>
