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@xxxxxxxxx> wrote: > > 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. > > > > Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233 > > > > Signed-off-by: Martino Fontana <tinozzo123@xxxxxxxxx> > > 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@xxxxxxxxxx/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); > > > >
