[v3,1/3] HID: switchcon: add nintendo switch controller driver
diff mbox series

Message ID 20190128040421.31878-2-djogorchock@gmail.com
State New
Headers show
Series
  • [v3,1/3] HID: switchcon: add nintendo switch controller driver
Related show

Commit Message

Daniel Ogorchock Jan. 28, 2019, 4:04 a.m. UTC
The switchcon driver supports the Nintendo Switch Pro Controllers and
the Joy-Cons. The Pro Controllers can be used over USB or Bluetooth.

The Joy-Cons each create their own, independent input devices, so it is
up to userspace to combine them if desired.

Signed-off-by: Daniel J. Ogorchock <djogorchock@gmail.com>
---
 MAINTAINERS                 |   6 +
 drivers/hid/Kconfig         |  11 +
 drivers/hid/Makefile        |   1 +
 drivers/hid/hid-ids.h       |   3 +
 drivers/hid/hid-switchcon.c | 827 ++++++++++++++++++++++++++++++++++++
 5 files changed, 848 insertions(+)
 create mode 100644 drivers/hid/hid-switchcon.c

Comments

Roderick Colenbrander Jan. 28, 2019, 5:10 a.m. UTC | #1
Hi Daniel,

Thanks for updating the patch series. It is definitely improved and
easier to read. See some of my comments below.

One other thing I noticed, see the comment in v2 review about breaking
userspace. I think since you change the button / stick mapping
compared to what SDL2 supports for Pro controller. You need to do
something e.g. different version like hid-sony does.

Out of curiosity did you test the driver on Bluetooth? The reason I'm
asking is, because I suspect you would have hit a deadlock issue,
which we hit in hid-sony before. I'm not sure how you got around that
one..

Let me explain the issue. When using Bluetooth, many Bluetooth stacks
rely on uhid (either Bluez on desktop and even Android's one). I
recall when the BT stack requests uhid to "create" an input device,
this ultimately triggers "_probe" in your driver. hid-sony at the time
and similar for hid-switchcon triggers various requests to the
hardware during probe. However uhid is single threaded and it is
holding a lock. This caused a deadlock and caused hardware request
timeouts. I don't see how this is not happening for you.

The solution in hid-sony at the time was to move logic to
"_input_configured", which had slightly different timing. (It is
triggered if I recall by hid_hw_start or a call like that and avoided
the deadlock).

Thanks,
Roderick

On Sun, Jan 27, 2019 at 8:05 PM Daniel J. Ogorchock
<djogorchock@gmail.com> wrote:
>
> The switchcon driver supports the Nintendo Switch Pro Controllers and
> the Joy-Cons. The Pro Controllers can be used over USB or Bluetooth.
>
> The Joy-Cons each create their own, independent input devices, so it is
> up to userspace to combine them if desired.
>
> Signed-off-by: Daniel J. Ogorchock <djogorchock@gmail.com>
> ---
>  MAINTAINERS                 |   6 +
>  drivers/hid/Kconfig         |  11 +
>  drivers/hid/Makefile        |   1 +
>  drivers/hid/hid-ids.h       |   3 +
>  drivers/hid/hid-switchcon.c | 827 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 848 insertions(+)
>  create mode 100644 drivers/hid/hid-switchcon.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f3a5c97e3419..001817e2e08a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14397,6 +14397,12 @@ F:     kernel/dma/swiotlb.c
>  F:     arch/*/kernel/pci-swiotlb.c
>  F:     include/linux/swiotlb.h
>
> +SWITCHCON HID DRIVER
> +M:     Daniel J. Ogorchock <djogorchock@gmail.com>
> +L:     linux-input@vger.kernel.org
> +S:     Maintained
> +F:     drivers/hid/hid-switchcon*
> +
>  SWITCHDEV
>  M:     Jiri Pirko <jiri@resnulli.us>
>  M:     Ivan Vecera <ivecera@redhat.com>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 41e9935fc584..9702c07802a5 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -943,6 +943,17 @@ config SMARTJOYPLUS_FF
>         Say Y here if you have a SmartJoy PLUS PS2/USB adapter and want to
>         enable force feedback support for it.
>
> +config HID_SWITCHCON
> +       tristate "Nintendo Joy-Con and Pro Controller support"
> +       depends on HID
> +       help
> +       Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
> +       All controllers support bluetooth, and the Pro Controller also supports
> +       its USB mode.
> +
> +       To compile this driver as a module, choose M here: the
> +       module will be called hid-switchcon.
> +
>  config HID_TIVO
>         tristate "TiVo Slide Bluetooth remote control support"
>         depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 896a51ce7ce0..e708e682c4b8 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
>  obj-$(CONFIG_HID_STEAM)                += hid-steam.o
>  obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
>  obj-$(CONFIG_HID_SUNPLUS)      += hid-sunplus.o
> +obj-$(CONFIG_HID_SWITCHCON)    += hid-switchcon.o
>  obj-$(CONFIG_HID_GREENASIA)    += hid-gaff.o
>  obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
>  obj-$(CONFIG_HID_TIVO)         += hid-tivo.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 27519eb8ee63..7924f7d502a8 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -846,6 +846,9 @@
>  #define USB_VENDOR_ID_NINTENDO         0x057e
>  #define USB_DEVICE_ID_NINTENDO_WIIMOTE 0x0306
>  #define USB_DEVICE_ID_NINTENDO_WIIMOTE2        0x0330
> +#define USB_DEVICE_ID_NINTENDO_JOYCONL 0x2006
> +#define USB_DEVICE_ID_NINTENDO_JOYCONR  0x2007
> +#define USB_DEVICE_ID_NINTENDO_PROCON   0x2009
>
>  #define USB_VENDOR_ID_NOVATEK          0x0603
>  #define USB_DEVICE_ID_NOVATEK_PCT      0x0600
> diff --git a/drivers/hid/hid-switchcon.c b/drivers/hid/hid-switchcon.c
> new file mode 100644
> index 000000000000..f70b7cf95021
> --- /dev/null
> +++ b/drivers/hid/hid-switchcon.c
> @@ -0,0 +1,827 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
> + *
> + * Copyright (c) 2019 Daniel J. Ogorchock <djogorchock@gmail.com>
> + *
> + * The following resources/projects were referenced for this driver:
> + *   https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> + *   https://gitlab.com/pjranki/joycon-linux-kernel (Peter Rankin)
> + *   https://github.com/FrotBot/SwitchProConLinuxUSB
> + *   https://github.com/MTCKC/ProconXInput
> + *   hid-wiimote kernel hid driver
> + *   hid-logitech-hidpp driver
> + *
> + * This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
> + * Pro Controllers can either be used over USB or Bluetooth.
> + *
> + * The driver will retrieve the factory calibration info from the controllers,
> + * so little to no user calibration should be required.
> + *
> + */
> +
> +#include "hid-ids.h"
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +
> +/*
> + * Reference the url below for the following HID report defines:
> + * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> + */
> +
> +/* Output Reports */
> +#define SC_OUTPUT_RUMBLE_AND_SUBCMD    0x01
> +#define SC_OUTPUT_FW_UPDATE_PKT                0x03
> +#define SC_OUTPUT_RUMBLE_ONLY          0x10
> +#define SC_OUTPUT_MCU_DATA             0x11
> +#define SC_OUTPUT_USB_CMD              0x80
> +
> +/* Subcommand IDs */
> +#define SC_SUBCMD_STATE                        0x00
> +#define SC_SUBCMD_MANUAL_BT_PAIRING    0x01
> +#define SC_SUBCMD_REQ_DEV_INFO         0x02
> +#define SC_SUBCMD_SET_REPORT_MODE      0x03
> +#define SC_SUBCMD_TRIGGERS_ELAPSED     0x04
> +#define SC_SUBCMD_GET_PAGE_LIST_STATE  0x05
> +#define SC_SUBCMD_SET_HCI_STATE                0x06
> +#define SC_SUBCMD_RESET_PAIRING_INFO   0x07
> +#define SC_SUBCMD_LOW_POWER_MODE       0x08
> +#define SC_SUBCMD_SPI_FLASH_READ       0x10
> +#define SC_SUBCMD_SPI_FLASH_WRITE      0x11
> +#define SC_SUBCMD_RESET_MCU            0x20
> +#define SC_SUBCMD_SET_MCU_CONFIG       0x21
> +#define SC_SUBCMD_SET_MCU_STATE                0x22
> +#define SC_SUBCMD_SET_PLAYER_LIGHTS    0x30
> +#define SC_SUBCMD_GET_PLAYER_LIGHTS    0x31
> +#define SC_SUBCMD_SET_HOME_LIGHT       0x38
> +#define SC_SUBCMD_ENABLE_IMU           0x40
> +#define SC_SUBCMD_SET_IMU_SENSITIVITY  0x41
> +#define SC_SUBCMD_WRITE_IMU_REG                0x42
> +#define SC_SUBCMD_READ_IMU_REG         0x43
> +#define SC_SUBCMD_ENABLE_VIBRATION     0x48
> +#define SC_SUBCMD_GET_REGULATED_VOLTAGE        0x50
> +
> +/* Input Reports */
> +#define SC_INPUT_BUTTON_EVENT          0x3F
> +#define SC_INPUT_SUBCMD_REPLY          0x21
> +#define SC_INPUT_IMU_DATA              0x30
> +#define SC_INPUT_MCU_DATA              0x31
> +#define SC_INPUT_USB_RESPONSE          0x81
> +
> +/* Feature Reports */
> +#define SC_FEATURE_LAST_SUBCMD         0x02
> +#define SC_FEATURE_OTA_FW_UPGRADE      0x70
> +#define SC_FEATURE_SETUP_MEM_READ      0x71
> +#define SC_FEATURE_MEM_READ            0x72
> +#define SC_FEATURE_ERASE_MEM_SECTOR    0x73
> +#define SC_FEATURE_MEM_WRITE           0x74
> +#define SC_FEATURE_LAUNCH              0x75
> +
> +/* USB Commands */
> +#define SC_USB_CMD_CONN_STATUS         0x01
> +#define SC_USB_CMD_HANDSHAKE           0x02
> +#define SC_USB_CMD_BAUDRATE_3M         0x03
> +#define SC_USB_CMD_NO_TIMEOUT          0x04
> +#define SC_USB_CMD_EN_TIMEOUT          0x05
> +#define SC_USB_RESET                   0x06
> +#define SC_USB_PRE_HANDSHAKE           0x91
> +#define SC_USB_SEND_UART               0x92
> +
> +/* SPI storage addresses of factory calibration data */
> +#define SC_CAL_DATA_START              0x603d
> +#define SC_CAL_DATA_END                        0x604e
> +#define SC_CAL_DATA_SIZE       (SC_CAL_DATA_END - SC_CAL_DATA_START + 1)
> +
> +
> +/* The raw analog joystick values will be mapped in terms of this magnitude */
> +#define SC_MAX_STICK_MAG       32767
> +#define SC_STICK_FUZZ          250
> +#define SC_STICK_FLAT          500
> +
> +/* States for controller state machine */
> +enum switchcon_ctlr_state {
> +       SWITCHCON_CTLR_STATE_INIT,
> +       SWITCHCON_CTLR_STATE_READ,
> +};
> +
> +struct switchcon_stick_cal {
> +       s32 max;
> +       s32 min;
> +       s32 center;
> +};
> +
> +/*
> + * All the controller's button values are stored in a u32.
> + * They can be accessed with bitwise ANDs.
> + */
> +#define SC_BTN_Y       BIT(0)
> +#define SC_BTN_X       BIT(1)
> +#define SC_BTN_B       BIT(2)
> +#define SC_BTN_A       BIT(3)
> +#define SC_BTN_SR_R    BIT(4)
> +#define SC_BTN_SL_R    BIT(5)
> +#define SC_BTN_R       BIT(6)
> +#define SC_BTN_ZR      BIT(7)
> +#define SC_BTN_MINUS   BIT(8)
> +#define SC_BTN_PLUS    BIT(9)
> +#define SC_BTN_RSTICK  BIT(10)
> +#define SC_BTN_LSTICK  BIT(11)
> +#define SC_BTN_HOME    BIT(12)
> +#define SC_BTN_CAP     BIT(13) /* capture button */
> +#define SC_BTN_DOWN    BIT(16)
> +#define SC_BTN_UP      BIT(17)
> +#define SC_BTN_RIGHT   BIT(18)
> +#define SC_BTN_LEFT    BIT(19)
> +#define SC_BTN_SR_L    BIT(20)
> +#define SC_BTN_SL_L    BIT(21)
> +#define SC_BTN_L       BIT(22)
> +#define SC_BTN_ZL      BIT(23)
> +
> +enum switchcon_ctlr_type {
> +       SWITCHCON_CTLR_TYPE_PROCON,
> +       SWITCHCON_CTLR_TYPE_JOYCON_L,
> +       SWITCHCON_CTLR_TYPE_JOYCON_R,
> +};

I think there is little use in having this enum. See further comments
below, hdev seems to be sufficient for this driver.

> +
> +static const char * const switchcon_input_names[] = {
> +       "Nintendo Switch Pro Controller",
> +       "Nintendo Switch Left Joy-Con",
> +       "Nintendo Switch Right Joy-Con",
> +};
> +
> +enum switchcon_msg_type {
> +       SWITCHCON_MSG_TYPE_NONE,
> +       SWITCHCON_MSG_TYPE_USB,
> +       SWITCHCON_MSG_TYPE_SUBCMD,
> +};
> +
> +struct switchcon_subcmd_request {
> +       u8 output_id; /* must be 0x01 for subcommand, 0x10 for rumble only */
> +       u8 packet_num; /* incremented every send */
> +       u8 rumble_data[8];
> +       u8 subcmd_id;
> +       /* data is here */
> +} __packed;
> +
> +/* should pass in pointer to a struct switchcon_subcmd_request */
> +#define SC_SUBCMD_REQ_GET_DATA(req) \
> +       ((u8 *)(req) + sizeof(struct switchcon_subcmd_request))
> +
> +struct switchcon_subcmd_reply {
> +       u8 ack; /* MSB 1 for ACK, 0 for NACK */
> +       u8 id; /* id of requested subcmd */
> +       /* data is here, can be up to 35 bytes */
> +} __packed;
> +
> +/* should pass in pointer to a struct switchcon_subcmd_reply */
> +#define SC_SUBCMD_REPLY_GET_DATA(reply) \
> +       ((u8 *)(reply) + sizeof(struct switchcon_subcmd_reply))
> +
> +struct switchcon_input_report {
> +       u8 id;
> +       u8 timer;
> +       u8 bat_con; /* battery and connection info */
> +       u8 button_status[3];
> +       u8 left_stick[3];
> +       u8 right_stick[3];
> +       u8 vibrator_report;
> +
> +       /*
> +        * If support for firmware updates, gyroscope data, and/or NFC/IR
> +        * are added in the future, this can be swapped for a union.
> +        */
> +       struct switchcon_subcmd_reply reply;
> +} __packed;
> +
> +#define SC_MAX_RESP_SIZE (sizeof(struct switchcon_input_report) + 35)
> +
> +/* Each physical controller is associated with a switchcon_ctlr struct */
> +struct switchcon_ctlr {
> +       struct hid_device *hdev;
> +       struct input_dev *input;
> +       enum switchcon_ctlr_type type;
> +       enum switchcon_ctlr_state ctlr_state;
> +
> +       /* The following members are used for synchronous sends/receives */
> +       enum switchcon_msg_type msg_type;
> +       u8 subcmd_num;
> +       struct mutex output_mutex;
> +       u8 input_buf[SC_MAX_RESP_SIZE];
> +       wait_queue_head_t wait;
> +       bool received_resp;
> +       u8 usb_ack_match;
> +       u8 subcmd_ack_match;
> +
> +       /* factory calibration data */
> +       struct switchcon_stick_cal left_stick_cal_x;
> +       struct switchcon_stick_cal left_stick_cal_y;
> +       struct switchcon_stick_cal right_stick_cal_x;
> +       struct switchcon_stick_cal right_stick_cal_y;
> +
> +};
> +
> +static int __switchcon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
> +{
> +       u8 *buf;
> +       int ret;
> +
> +       buf = kmemdup(data, len, GFP_KERNEL);
> +       if (!buf)
> +               return -ENOMEM;
> +       ret = hid_hw_output_report(hdev, buf, len);
> +       kfree(buf);
> +       if (ret < 0)
> +               hid_dbg(hdev, "Failed to send output report ret=%d\n", ret);
> +       return ret;
> +}
> +
> +static int switchcon_hid_send_sync(struct switchcon_ctlr *ctlr, u8 *data,
> +                                                               size_t len)
> +{
> +       int ret;
> +
> +       ret = __switchcon_hid_send(ctlr->hdev, data, len);
> +       if (ret < 0) {
> +               memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
> +               return ret;
> +       }
> +
> +       if (!wait_event_timeout(ctlr->wait, ctlr->received_resp, HZ)) {
> +               hid_dbg(ctlr->hdev, "syncronous send/receive timed out\n");
> +               memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
> +               return -ETIMEDOUT;
> +       }
> +
> +       ctlr->received_resp = false;
> +       return 0;
> +}
> +
> +static int switchcon_send_usb(struct switchcon_ctlr *ctlr, u8 cmd)
> +{
> +       int ret;
> +       u8 buf[2] = {SC_OUTPUT_USB_CMD};
> +
> +       buf[1] = cmd;
> +       ctlr->usb_ack_match = cmd;
> +       ctlr->msg_type = SWITCHCON_MSG_TYPE_USB;
> +       ret = switchcon_hid_send_sync(ctlr, buf, sizeof(buf));
> +       if (ret)
> +               hid_dbg(ctlr->hdev, "send usb command failed; ret=%d\n", ret);
> +       return ret;
> +}
> +
> +static int switchcon_send_subcmd(struct switchcon_ctlr *ctlr,
> +                                struct switchcon_subcmd_request *subcmd,
> +                                size_t data_len)
> +{
> +       int ret;
> +
> +       subcmd->output_id = SC_OUTPUT_RUMBLE_AND_SUBCMD;
> +       subcmd->packet_num = ctlr->subcmd_num;
> +       if (++ctlr->subcmd_num > 0xF)
> +               ctlr->subcmd_num = 0;
> +       ctlr->subcmd_ack_match = subcmd->subcmd_id;
> +       ctlr->msg_type = SWITCHCON_MSG_TYPE_SUBCMD;
> +
> +       ret = switchcon_hid_send_sync(ctlr, (u8 *)subcmd,
> +                                     sizeof(*subcmd) + data_len);
> +       if (ret)
> +               hid_dbg(ctlr->hdev, "send subcommand failed; ret=%d\n", ret);
> +       return ret;
> +}
> +
> +/* Supply nibbles for flash and on. Ones correspond to active */
> +static int switchcon_set_player_leds(struct switchcon_ctlr *ctlr,
> +                                               u8 flash, u8 on)
> +{
> +       struct switchcon_subcmd_request *req;
> +       u8 buffer[sizeof(*req) + 1] = { 0 };
> +
> +       req = (struct switchcon_subcmd_request *)buffer;
> +       req->subcmd_id = SC_SUBCMD_SET_PLAYER_LIGHTS;
> +       SC_SUBCMD_REQ_GET_DATA(req)[0] = (flash << 4) | on;
> +
> +       hid_dbg(ctlr->hdev, "setting player leds\n");
> +       return switchcon_send_subcmd(ctlr, req, 1);
> +}
> +
> +static int switchcon_request_calibration(struct switchcon_ctlr *ctlr)
> +{
> +       struct switchcon_subcmd_request *req;
> +       u8 buffer[sizeof(*req) + 5] = { 0 };
> +       u8 *data;
> +
> +       req = (struct switchcon_subcmd_request *)buffer;
> +       req->subcmd_id = SC_SUBCMD_SPI_FLASH_READ;
> +       data = SC_SUBCMD_REQ_GET_DATA(req);
> +       data[0] = 0xFF & SC_CAL_DATA_START;
> +       data[1] = 0xFF & (SC_CAL_DATA_START >> 8);
> +       data[2] = 0xFF & (SC_CAL_DATA_START >> 16);
> +       data[3] = 0xFF & (SC_CAL_DATA_START >> 24);
> +       data[4] = SC_CAL_DATA_SIZE;
> +
> +       hid_dbg(ctlr->hdev, "requesting cal data\n");
> +       return switchcon_send_subcmd(ctlr, req, 5);
> +}
> +
> +static int switchcon_set_report_mode(struct switchcon_ctlr *ctlr)
> +{
> +       struct switchcon_subcmd_request *req;
> +       u8 buffer[sizeof(*req) + 1] = { 0 };
> +
> +       req = (struct switchcon_subcmd_request *)buffer;
> +       req->subcmd_id = SC_SUBCMD_SET_REPORT_MODE;
> +       SC_SUBCMD_REQ_GET_DATA(req)[0] = 0x30; /* standard, full report mode */
> +
> +       hid_dbg(ctlr->hdev, "setting controller report mode\n");
> +       return switchcon_send_subcmd(ctlr, req, 1);
> +}
> +
> +static int switchcon_map_stick_val(struct switchcon_stick_cal *cal, s32 val)
> +{
> +       s32 center = cal->center;
> +       s32 min = cal->min;
> +       s32 max = cal->max;
> +       int new_val;
> +
> +       if (val > center) {
> +               new_val = (val - center) * SC_MAX_STICK_MAG;
> +               new_val /= (max - center);
> +       } else {
> +               new_val = (center - val) * -SC_MAX_STICK_MAG;
> +               new_val /= (center - min);
> +       }
> +       new_val = clamp(new_val, -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG);
> +       return new_val;
> +}
> +
> +static void switchcon_parse_report(struct switchcon_ctlr *ctlr, u8 *data)
> +{
> +       struct input_dev *dev = ctlr->input;
> +       enum switchcon_ctlr_type type = ctlr->type;
> +       u32 btns;
> +
> +       btns = hid_field_extract(ctlr->hdev, data + 3, 0, 24);
> +
> +       if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> +           type == SWITCHCON_CTLR_TYPE_JOYCON_L) {
> +               u16 raw_l_x;
> +               u16 raw_l_y;
> +               int s_l_x;
> +               int s_l_y;

Naming wise, I would probably rename "raw_l_x" to "raw_x" (abs_x later
implies left a bit). Similar for "s_l_x" probably just call it "x".
Similar later for the right stick.

> +
> +               /* get raw stick values */
> +               raw_l_x = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> +               raw_l_y = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> +               /* map the stick values */
> +               s_l_x = switchcon_map_stick_val(&ctlr->left_stick_cal_x,
> +                                               raw_l_x);
> +               s_l_y = -switchcon_map_stick_val(&ctlr->left_stick_cal_y,
> +                                               raw_l_y);
> +               /* report sticks */
> +               input_report_abs(dev, ABS_X, s_l_x);
> +               input_report_abs(dev, ABS_Y, s_l_y);
> +
> +               /* report buttons */
> +               input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
> +                                             btns & SC_BTN_SL_L);
> +               input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
> +                                             btns & SC_BTN_SR_L);
> +               input_report_key(dev, BTN_TL2, btns & SC_BTN_ZL);
> +               input_report_key(dev, BTN_SELECT, btns & SC_BTN_MINUS);
> +               input_report_key(dev, BTN_THUMBL, btns & SC_BTN_LSTICK);
> +               input_report_key(dev, BTN_Z, btns & SC_BTN_CAP);
> +               input_report_key(dev, BTN_DPAD_DOWN, btns & SC_BTN_DOWN);
> +               input_report_key(dev, BTN_DPAD_UP, btns & SC_BTN_UP);
> +               input_report_key(dev, BTN_DPAD_RIGHT, btns & SC_BTN_RIGHT);
> +               input_report_key(dev, BTN_DPAD_LEFT, btns & SC_BTN_LEFT);
> +       }
> +       if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> +           type == SWITCHCON_CTLR_TYPE_JOYCON_R) {
> +               u16 raw_r_x;
> +               u16 raw_r_y;
> +               int s_r_x;
> +               int s_r_y;

"raw_rx" and "rx" perhaps?

> +
> +               /* get raw stick values */
> +               raw_r_x = hid_field_extract(ctlr->hdev, (data + 9), 0, 12);
> +               raw_r_y = hid_field_extract(ctlr->hdev, (data + 10), 4, 12);
> +               /* map stick values */
> +               s_r_x = switchcon_map_stick_val(&ctlr->right_stick_cal_x,
> +                                               raw_r_x);
> +               s_r_y = -switchcon_map_stick_val(&ctlr->right_stick_cal_y,
> +                                               raw_r_y);
> +               /* report sticks */
> +               input_report_abs(dev, ABS_RX, s_r_x);
> +               input_report_abs(dev, ABS_RY, s_r_y);
> +
> +               /* report buttons */
> +               input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
> +                                             btns & SC_BTN_SL_R);
> +               input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
> +                                             btns & SC_BTN_SR_R);
> +               input_report_key(dev, BTN_TR2, btns & SC_BTN_ZR);
> +               input_report_key(dev, BTN_START, btns & SC_BTN_PLUS);
> +               input_report_key(dev, BTN_THUMBR, btns & SC_BTN_RSTICK);
> +               input_report_key(dev, BTN_MODE, btns & SC_BTN_HOME);
> +               input_report_key(dev, BTN_WEST, btns & SC_BTN_Y);
> +               input_report_key(dev, BTN_NORTH, btns & SC_BTN_X);
> +               input_report_key(dev, BTN_EAST, btns & SC_BTN_A);
> +               input_report_key(dev, BTN_SOUTH, btns & SC_BTN_B);
> +       }
> +
> +       input_sync(dev);
> +}
> +
> +
> +static const unsigned int switchcon_button_inputs[] = {
> +       BTN_SELECT, BTN_Z, BTN_THUMBL,
> +       BTN_START, BTN_MODE, BTN_THUMBR,
> +       BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
> +       BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
> +       BTN_TL, BTN_TR, BTN_TL2, BTN_TR2,
> +       0 /* 0 signals end of array */
> +};
> +
> +static DEFINE_MUTEX(switchcon_input_num_mutex);
> +static int switchcon_input_create(struct switchcon_ctlr *ctlr)
> +{
> +       struct hid_device *hdev;
> +       static int input_num = 1;
> +       int ret;
> +       int i;
> +
> +       hdev = ctlr->hdev;
> +       ctlr->input = input_allocate_device();
> +       if (!ctlr->input) {
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +       ctlr->input->dev.parent = &hdev->dev;
> +       ctlr->input->id.bustype = hdev->bus;
> +       ctlr->input->id.vendor = hdev->vendor;
> +       ctlr->input->id.product = hdev->product;
> +       ctlr->input->id.version = hdev->version;
> +       ctlr->input->name = switchcon_input_names[ctlr->type];
> +       input_set_drvdata(ctlr->input, ctlr);
> +
> +
> +       /* set up sticks */
> +       input_set_abs_params(ctlr->input, ABS_X,
> +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> +       input_set_abs_params(ctlr->input, ABS_Y,
> +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> +       input_set_abs_params(ctlr->input, ABS_RX,
> +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> +       input_set_abs_params(ctlr->input, ABS_RY,
> +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> +       /* set up buttons */
> +       for (i = 0; switchcon_button_inputs[i] > 0; i++)
> +               input_set_capability(ctlr->input, EV_KEY,
> +                                    switchcon_button_inputs[i]);
> +
> +       ret = input_register_device(ctlr->input);
> +       if (ret)
> +               goto err_input;
> +
> +       /* Set the default controller player leds based on controller number */
> +       mutex_lock(&switchcon_input_num_mutex);
> +       mutex_lock(&ctlr->output_mutex);
> +       ret = switchcon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
> +       if (ret)
> +               hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
> +       mutex_unlock(&ctlr->output_mutex);
> +       if (++input_num > 4)
> +               input_num = 1;
> +       mutex_unlock(&switchcon_input_num_mutex);
> +
> +       return 0;
> +
> +err_input:
> +       input_free_device(ctlr->input);
> +err:
> +       return ret;
> +}
> +
> +/* data input must have at least 9 bytes */
> +static void switchcon_parse_lstick_calibration(u8 *data,
> +                                              struct switchcon_ctlr *ctlr)
> +{
> +       struct switchcon_stick_cal *cal_x = &ctlr->left_stick_cal_x;
> +       struct switchcon_stick_cal *cal_y = &ctlr->left_stick_cal_y;
> +       s32 x_max_above;
> +       s32 x_min_below;
> +       s32 y_max_above;
> +       s32 y_min_below;
> +
> +       x_max_above = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
> +       y_max_above = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
> +       cal_x->center = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
> +       cal_y->center = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
> +       x_min_below = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> +       y_min_below = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> +       cal_x->max = cal_x->center + x_max_above;
> +       cal_x->min = cal_x->center - x_min_below;
> +       cal_y->max = cal_y->center + y_max_above;
> +       cal_y->min = cal_y->center - y_min_below;
> +}
> +
> +/* data input must have at least 9 bytes */
> +static void switchcon_parse_rstick_calibration(u8 *data,
> +                                              struct switchcon_ctlr *ctlr)
> +{
> +       struct switchcon_stick_cal *cal_x = &ctlr->right_stick_cal_x;
> +       struct switchcon_stick_cal *cal_y = &ctlr->right_stick_cal_y;
> +       s32 x_max_above;
> +       s32 x_min_below;
> +       s32 y_max_above;
> +       s32 y_min_below;
> +
> +       cal_x->center = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
> +       cal_y->center = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
> +       x_min_below = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
> +       y_min_below = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
> +       x_max_above = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> +       y_max_above = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> +       cal_x->max = cal_x->center + x_max_above;
> +       cal_x->min = cal_x->center - x_min_below;
> +       cal_y->max = cal_y->center + y_max_above;
> +       cal_y->min = cal_y->center - y_min_below;
> +}
> +
> +/* Common handler for parsing inputs */
> +static int switchcon_ctlr_read_handler(struct switchcon_ctlr *ctlr,
> +                                               u8 *data, int size)
> +{
> +       int ret = 0;
> +
> +       switch (data[0]) {
> +       case SC_INPUT_SUBCMD_REPLY:
> +       case SC_INPUT_IMU_DATA:
> +       case SC_INPUT_MCU_DATA:
> +               if (size >= 12) /* make sure it contains the input report */
> +                       switchcon_parse_report(ctlr, data);
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       return ret;
> +}
> +
> +static int switchcon_ctlr_handle_event(struct switchcon_ctlr *ctlr, u8 *data,
> +                                                                   int size)
> +{
> +       int ret = 0;
> +       bool match = false;
> +       int copy_size;
> +       struct switchcon_input_report *report;
> +
> +       if (unlikely(mutex_is_locked(&ctlr->output_mutex)) &&
> +           ctlr->msg_type != SWITCHCON_MSG_TYPE_NONE) {
> +               switch (ctlr->msg_type) {
> +               case SWITCHCON_MSG_TYPE_USB:
> +                       if (size < 2)
> +                               break;
> +                       if (data[0] == SC_INPUT_USB_RESPONSE &&
> +                           data[1] == ctlr->usb_ack_match)
> +                               match = true;
> +                       break;
> +               case SWITCHCON_MSG_TYPE_SUBCMD:
> +                       if (size < sizeof(struct switchcon_input_report) ||
> +                           data[0] != SC_INPUT_SUBCMD_REPLY)
> +                               break;
> +                       report = (struct switchcon_input_report *)data;
> +                       if (report->reply.id == ctlr->subcmd_ack_match)
> +                               match = true;
> +                       break;
> +               default:
> +                       break;
> +               }
> +
> +               if (match) {
> +                       if (size > SC_MAX_RESP_SIZE)
> +                               copy_size = SC_MAX_RESP_SIZE;
> +                       else
> +                               copy_size = size;
> +                       memcpy(ctlr->input_buf, data, copy_size);
> +                       ctlr->msg_type = SWITCHCON_MSG_TYPE_NONE;
> +                       ctlr->received_resp = true;
> +                       wake_up(&ctlr->wait);
> +
> +                       /* This message has been handled */
> +                       return 1;
> +               }
> +       }
> +
> +       if (ctlr->ctlr_state == SWITCHCON_CTLR_STATE_READ)
> +               ret = switchcon_ctlr_read_handler(ctlr, data, size);
> +
> +       return ret;
> +}
> +
> +static int switchcon_hid_event(struct hid_device *hdev,
> +                       struct hid_report *report, u8 *raw_data, int size)
> +{
> +       struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
> +
> +       if (size < 1)
> +               return -EINVAL;
> +
> +       return switchcon_ctlr_handle_event(ctlr, raw_data, size);
> +}
> +
> +static struct switchcon_ctlr *switchcon_ctlr_create(struct hid_device *hdev)
> +{
> +       struct switchcon_ctlr *ctlr;
> +
> +       ctlr = devm_kzalloc(&hdev->dev, sizeof(*ctlr), GFP_KERNEL);
> +       if (!ctlr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       switch (hdev->product) {
> +       case USB_DEVICE_ID_NINTENDO_PROCON:
> +               ctlr->type = SWITCHCON_CTLR_TYPE_PROCON;
> +               break;
> +       case USB_DEVICE_ID_NINTENDO_JOYCONL:
> +               ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_L;
> +               break;
> +       case USB_DEVICE_ID_NINTENDO_JOYCONR:
> +               ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_R;
> +               break;
> +       default:
> +               return ERR_PTR(-EINVAL);
> +       }

There is no need to store "ctlr->type". It is checked in one or two
places. Just use hdev in those.

> +       ctlr->hdev = hdev;
> +       ctlr->ctlr_state = SWITCHCON_CTLR_STATE_INIT;
> +       hid_set_drvdata(hdev, ctlr);
> +       mutex_init(&ctlr->output_mutex);
> +       init_waitqueue_head(&ctlr->wait);
> +       return ctlr;
> +}

With removal of the "ctrl->type" logic and some of the other
refactoring of calibration as suggested below in "_probe", there is
probably no good reason to keep a separate create function.

> +
> +static void switchcon_ctlr_destroy(struct switchcon_ctlr *ctlr)
> +{
> +       if (ctlr->input)
> +               input_unregister_device(ctlr->input);
> +       mutex_destroy(&ctlr->output_mutex);
> +}
> +
> +static int switchcon_hid_probe(struct hid_device *hdev,
> +                              const struct hid_device_id *id)
> +{
> +       int ret;
> +       struct switchcon_ctlr *ctlr;
> +       struct switchcon_input_report *report;
> +       u8 *raw_cal;
> +
> +       hid_dbg(hdev, "probe - start\n");
> +
> +       ctlr = switchcon_ctlr_create(hdev);
> +       if (IS_ERR(ctlr)) {
> +               hid_err(hdev, "Failed to create new controller\n");
> +               ret = PTR_ERR(ctlr);
> +               goto err;
> +       }
> +
> +       ret = hid_parse(hdev);
> +       if (ret) {
> +               hid_err(hdev, "HID parse failed\n");
> +               goto err;
> +       }
> +
> +       ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> +       if (ret) {
> +               hid_err(hdev, "HW start failed\n");
> +               goto err;
> +       }
> +
> +       ret = hid_hw_open(hdev);
> +       if (ret) {
> +               hid_err(hdev, "cannot start hardware I/O\n");
> +               goto err_stop;
> +       }
> +
> +       hid_device_io_start(hdev);
> +
> +       /* Initialize the controller */
> +       mutex_lock(&ctlr->output_mutex);
> +       /* if baudrate command fails, assume ble pro controller */
> +       if (ctlr->type == SWITCHCON_CTLR_TYPE_PROCON &&
> +           !switchcon_send_usb(ctlr, SC_USB_CMD_BAUDRATE_3M)) {
> +               /* handshake */
> +               ret = switchcon_send_usb(ctlr, SC_USB_CMD_HANDSHAKE);
> +               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.
> +                */
> +               switchcon_send_usb(ctlr, SC_USB_CMD_NO_TIMEOUT);
> +       }
> +
> +       /* get controller calibration data, and parse it */
> +       ret = switchcon_request_calibration(ctlr);
> +       if (ret) {
> +               hid_err(hdev, "Failed to retrieve calibration; ret=%d\n", ret);
> +               goto err_mutex;
> +       }
> +       report = (struct switchcon_input_report *)ctlr->input_buf;
> +       raw_cal = SC_SUBCMD_REPLY_GET_DATA(&report->reply) + 5;
> +       switchcon_parse_lstick_calibration(raw_cal, ctlr);
> +       switchcon_parse_rstick_calibration(raw_cal + 9, ctlr);

Move the lstick/rstick code into the calibration function. I think
there is also little reason to keep the lstick/rstick calibration
helper functions. They are only used once, so make a slightly bigger
overall calibration function hiding all this detail.

> +       hid_info(ctlr->hdev, "calibration:\n"
> +                            "l_x_c=%d l_x_max=%d l_x_min=%d\n"
> +                            "l_y_c=%d l_y_max=%d l_y_min=%d\n"
> +                            "r_x_c=%d r_x_max=%d r_x_min=%d\n"
> +                            "r_y_c=%d r_y_max=%d r_y_min=%d\n",
> +                            ctlr->left_stick_cal_x.center,
> +                            ctlr->left_stick_cal_x.max,
> +                            ctlr->left_stick_cal_x.min,
> +                            ctlr->left_stick_cal_y.center,
> +                            ctlr->left_stick_cal_y.max,
> +                            ctlr->left_stick_cal_y.min,
> +                            ctlr->right_stick_cal_x.center,
> +                            ctlr->right_stick_cal_x.max,
> +                            ctlr->right_stick_cal_x.min,
> +                            ctlr->right_stick_cal_y.center,
> +                            ctlr->right_stick_cal_y.max,
> +                            ctlr->right_stick_cal_y.min);

I would move more of this logic into your calibration function. Not
sure how much sense it makes to print the calibrarion data. Probably
print it using hid_dbg, so it is only printed on debug builds.

> +
> +       /* Set the reporting mode to 0x30, which is the full report mode */
> +       ret = switchcon_set_report_mode(ctlr);
> +       if (ret) {
> +               hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> +               goto err_mutex;
> +       }
> +
> +       mutex_unlock(&ctlr->output_mutex);
> +
> +       ret = switchcon_input_create(ctlr);
> +       if (ret) {
> +               hid_err(hdev, "Failed to create input device; ret=%d\n", ret);
> +               goto err_close;
> +       }
> +
> +       ctlr->ctlr_state = SWITCHCON_CTLR_STATE_READ;
> +
> +       hid_dbg(hdev, "probe - success\n");
> +       return 0;
> +
> +err_mutex:
> +       mutex_unlock(&ctlr->output_mutex);
> +err_close:
> +       switchcon_ctlr_destroy(ctlr);
> +       hid_hw_close(hdev);
> +err_stop:
> +       hid_hw_stop(hdev);
> +err:
> +       hid_err(hdev, "probe - fail = %d\n", ret);
> +       return ret;
> +}
> +
> +static void switchcon_hid_remove(struct hid_device *hdev)
> +{
> +       struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
> +
> +       hid_dbg(hdev, "remove\n");
> +       hid_hw_close(hdev);
> +       hid_hw_stop(hdev);
> +       switchcon_ctlr_destroy(ctlr);
> +}
> +
> +static const struct hid_device_id switchcon_hid_devices[] = {
> +       { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> +                        USB_DEVICE_ID_NINTENDO_PROCON) },
> +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> +                        USB_DEVICE_ID_NINTENDO_PROCON) },
> +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> +                        USB_DEVICE_ID_NINTENDO_JOYCONL) },
> +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> +                        USB_DEVICE_ID_NINTENDO_JOYCONR) },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(hid, switchcon_hid_devices);
> +
> +static struct hid_driver switchcon_hid_driver = {
> +       .name           = "switchcon",
> +       .id_table       = switchcon_hid_devices,
> +       .probe          = switchcon_hid_probe,
> +       .remove         = switchcon_hid_remove,
> +       .raw_event      = switchcon_hid_event,
> +};
> +module_hid_driver(switchcon_hid_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
> +MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
> --
> 2.20.1
>
Daniel Ogorchock Jan. 28, 2019, 5:41 a.m. UTC | #2
Hi Roderick,

Thanks for the again prompt response.

On Sun, Jan 27, 2019 at 11:11 PM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> Hi Daniel,
>
> Thanks for updating the patch series. It is definitely improved and
> easier to read. See some of my comments below.
>
> One other thing I noticed, see the comment in v2 review about breaking
> userspace. I think since you change the button / stick mapping
> compared to what SDL2 supports for Pro controller. You need to do
> something e.g. different version like hid-sony does.
>

Ah yes, I had read your initial email but forgot about it entirely when making
the v3 changes. I'll address that in v4.

> Out of curiosity did you test the driver on Bluetooth? The reason I'm
> asking is, because I suspect you would have hit a deadlock issue,
> which we hit in hid-sony before. I'm not sure how you got around that
> one..
>
> Let me explain the issue. When using Bluetooth, many Bluetooth stacks
> rely on uhid (either Bluez on desktop and even Android's one). I
> recall when the BT stack requests uhid to "create" an input device,
> this ultimately triggers "_probe" in your driver. hid-sony at the time
> and similar for hid-switchcon triggers various requests to the
> hardware during probe. However uhid is single threaded and it is
> holding a lock. This caused a deadlock and caused hardware request
> timeouts. I don't see how this is not happening for you.
>
> The solution in hid-sony at the time was to move logic to
> "_input_configured", which had slightly different timing. (It is
> triggered if I recall by hid_hw_start or a call like that and avoided
> the deadlock).
>

Interesting. I've been using bluetooth extensively and haven't run
into this issue.
I've found a 2016 patch from you which reports to fix this problem here
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8442b8809cd81d9d7e99ee58f44967b94b7c4168

Is this referring to the same issue? If so, looks like what you did is
working for me.

> Thanks,
> Roderick
>
> On Sun, Jan 27, 2019 at 8:05 PM Daniel J. Ogorchock
> <djogorchock@gmail.com> wrote:
> >
> > The switchcon driver supports the Nintendo Switch Pro Controllers and
> > the Joy-Cons. The Pro Controllers can be used over USB or Bluetooth.
> >
> > The Joy-Cons each create their own, independent input devices, so it is
> > up to userspace to combine them if desired.
> >
> > Signed-off-by: Daniel J. Ogorchock <djogorchock@gmail.com>
> > ---
> >  MAINTAINERS                 |   6 +
> >  drivers/hid/Kconfig         |  11 +
> >  drivers/hid/Makefile        |   1 +
> >  drivers/hid/hid-ids.h       |   3 +
> >  drivers/hid/hid-switchcon.c | 827 ++++++++++++++++++++++++++++++++++++
> >  5 files changed, 848 insertions(+)
> >  create mode 100644 drivers/hid/hid-switchcon.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f3a5c97e3419..001817e2e08a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14397,6 +14397,12 @@ F:     kernel/dma/swiotlb.c
> >  F:     arch/*/kernel/pci-swiotlb.c
> >  F:     include/linux/swiotlb.h
> >
> > +SWITCHCON HID DRIVER
> > +M:     Daniel J. Ogorchock <djogorchock@gmail.com>
> > +L:     linux-input@vger.kernel.org
> > +S:     Maintained
> > +F:     drivers/hid/hid-switchcon*
> > +
> >  SWITCHDEV
> >  M:     Jiri Pirko <jiri@resnulli.us>
> >  M:     Ivan Vecera <ivecera@redhat.com>
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 41e9935fc584..9702c07802a5 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -943,6 +943,17 @@ config SMARTJOYPLUS_FF
> >         Say Y here if you have a SmartJoy PLUS PS2/USB adapter and want to
> >         enable force feedback support for it.
> >
> > +config HID_SWITCHCON
> > +       tristate "Nintendo Joy-Con and Pro Controller support"
> > +       depends on HID
> > +       help
> > +       Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
> > +       All controllers support bluetooth, and the Pro Controller also supports
> > +       its USB mode.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called hid-switchcon.
> > +
> >  config HID_TIVO
> >         tristate "TiVo Slide Bluetooth remote control support"
> >         depends on HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index 896a51ce7ce0..e708e682c4b8 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -103,6 +103,7 @@ obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
> >  obj-$(CONFIG_HID_STEAM)                += hid-steam.o
> >  obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
> >  obj-$(CONFIG_HID_SUNPLUS)      += hid-sunplus.o
> > +obj-$(CONFIG_HID_SWITCHCON)    += hid-switchcon.o
> >  obj-$(CONFIG_HID_GREENASIA)    += hid-gaff.o
> >  obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
> >  obj-$(CONFIG_HID_TIVO)         += hid-tivo.o
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 27519eb8ee63..7924f7d502a8 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -846,6 +846,9 @@
> >  #define USB_VENDOR_ID_NINTENDO         0x057e
> >  #define USB_DEVICE_ID_NINTENDO_WIIMOTE 0x0306
> >  #define USB_DEVICE_ID_NINTENDO_WIIMOTE2        0x0330
> > +#define USB_DEVICE_ID_NINTENDO_JOYCONL 0x2006
> > +#define USB_DEVICE_ID_NINTENDO_JOYCONR  0x2007
> > +#define USB_DEVICE_ID_NINTENDO_PROCON   0x2009
> >
> >  #define USB_VENDOR_ID_NOVATEK          0x0603
> >  #define USB_DEVICE_ID_NOVATEK_PCT      0x0600
> > diff --git a/drivers/hid/hid-switchcon.c b/drivers/hid/hid-switchcon.c
> > new file mode 100644
> > index 000000000000..f70b7cf95021
> > --- /dev/null
> > +++ b/drivers/hid/hid-switchcon.c
> > @@ -0,0 +1,827 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
> > + *
> > + * Copyright (c) 2019 Daniel J. Ogorchock <djogorchock@gmail.com>
> > + *
> > + * The following resources/projects were referenced for this driver:
> > + *   https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> > + *   https://gitlab.com/pjranki/joycon-linux-kernel (Peter Rankin)
> > + *   https://github.com/FrotBot/SwitchProConLinuxUSB
> > + *   https://github.com/MTCKC/ProconXInput
> > + *   hid-wiimote kernel hid driver
> > + *   hid-logitech-hidpp driver
> > + *
> > + * This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
> > + * Pro Controllers can either be used over USB or Bluetooth.
> > + *
> > + * The driver will retrieve the factory calibration info from the controllers,
> > + * so little to no user calibration should be required.
> > + *
> > + */
> > +
> > +#include "hid-ids.h"
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/hid.h>
> > +#include <linux/input.h>
> > +#include <linux/module.h>
> > +#include <linux/spinlock.h>
> > +
> > +/*
> > + * Reference the url below for the following HID report defines:
> > + * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> > + */
> > +
> > +/* Output Reports */
> > +#define SC_OUTPUT_RUMBLE_AND_SUBCMD    0x01
> > +#define SC_OUTPUT_FW_UPDATE_PKT                0x03
> > +#define SC_OUTPUT_RUMBLE_ONLY          0x10
> > +#define SC_OUTPUT_MCU_DATA             0x11
> > +#define SC_OUTPUT_USB_CMD              0x80
> > +
> > +/* Subcommand IDs */
> > +#define SC_SUBCMD_STATE                        0x00
> > +#define SC_SUBCMD_MANUAL_BT_PAIRING    0x01
> > +#define SC_SUBCMD_REQ_DEV_INFO         0x02
> > +#define SC_SUBCMD_SET_REPORT_MODE      0x03
> > +#define SC_SUBCMD_TRIGGERS_ELAPSED     0x04
> > +#define SC_SUBCMD_GET_PAGE_LIST_STATE  0x05
> > +#define SC_SUBCMD_SET_HCI_STATE                0x06
> > +#define SC_SUBCMD_RESET_PAIRING_INFO   0x07
> > +#define SC_SUBCMD_LOW_POWER_MODE       0x08
> > +#define SC_SUBCMD_SPI_FLASH_READ       0x10
> > +#define SC_SUBCMD_SPI_FLASH_WRITE      0x11
> > +#define SC_SUBCMD_RESET_MCU            0x20
> > +#define SC_SUBCMD_SET_MCU_CONFIG       0x21
> > +#define SC_SUBCMD_SET_MCU_STATE                0x22
> > +#define SC_SUBCMD_SET_PLAYER_LIGHTS    0x30
> > +#define SC_SUBCMD_GET_PLAYER_LIGHTS    0x31
> > +#define SC_SUBCMD_SET_HOME_LIGHT       0x38
> > +#define SC_SUBCMD_ENABLE_IMU           0x40
> > +#define SC_SUBCMD_SET_IMU_SENSITIVITY  0x41
> > +#define SC_SUBCMD_WRITE_IMU_REG                0x42
> > +#define SC_SUBCMD_READ_IMU_REG         0x43
> > +#define SC_SUBCMD_ENABLE_VIBRATION     0x48
> > +#define SC_SUBCMD_GET_REGULATED_VOLTAGE        0x50
> > +
> > +/* Input Reports */
> > +#define SC_INPUT_BUTTON_EVENT          0x3F
> > +#define SC_INPUT_SUBCMD_REPLY          0x21
> > +#define SC_INPUT_IMU_DATA              0x30
> > +#define SC_INPUT_MCU_DATA              0x31
> > +#define SC_INPUT_USB_RESPONSE          0x81
> > +
> > +/* Feature Reports */
> > +#define SC_FEATURE_LAST_SUBCMD         0x02
> > +#define SC_FEATURE_OTA_FW_UPGRADE      0x70
> > +#define SC_FEATURE_SETUP_MEM_READ      0x71
> > +#define SC_FEATURE_MEM_READ            0x72
> > +#define SC_FEATURE_ERASE_MEM_SECTOR    0x73
> > +#define SC_FEATURE_MEM_WRITE           0x74
> > +#define SC_FEATURE_LAUNCH              0x75
> > +
> > +/* USB Commands */
> > +#define SC_USB_CMD_CONN_STATUS         0x01
> > +#define SC_USB_CMD_HANDSHAKE           0x02
> > +#define SC_USB_CMD_BAUDRATE_3M         0x03
> > +#define SC_USB_CMD_NO_TIMEOUT          0x04
> > +#define SC_USB_CMD_EN_TIMEOUT          0x05
> > +#define SC_USB_RESET                   0x06
> > +#define SC_USB_PRE_HANDSHAKE           0x91
> > +#define SC_USB_SEND_UART               0x92
> > +
> > +/* SPI storage addresses of factory calibration data */
> > +#define SC_CAL_DATA_START              0x603d
> > +#define SC_CAL_DATA_END                        0x604e
> > +#define SC_CAL_DATA_SIZE       (SC_CAL_DATA_END - SC_CAL_DATA_START + 1)
> > +
> > +
> > +/* The raw analog joystick values will be mapped in terms of this magnitude */
> > +#define SC_MAX_STICK_MAG       32767
> > +#define SC_STICK_FUZZ          250
> > +#define SC_STICK_FLAT          500
> > +
> > +/* States for controller state machine */
> > +enum switchcon_ctlr_state {
> > +       SWITCHCON_CTLR_STATE_INIT,
> > +       SWITCHCON_CTLR_STATE_READ,
> > +};
> > +
> > +struct switchcon_stick_cal {
> > +       s32 max;
> > +       s32 min;
> > +       s32 center;
> > +};
> > +
> > +/*
> > + * All the controller's button values are stored in a u32.
> > + * They can be accessed with bitwise ANDs.
> > + */
> > +#define SC_BTN_Y       BIT(0)
> > +#define SC_BTN_X       BIT(1)
> > +#define SC_BTN_B       BIT(2)
> > +#define SC_BTN_A       BIT(3)
> > +#define SC_BTN_SR_R    BIT(4)
> > +#define SC_BTN_SL_R    BIT(5)
> > +#define SC_BTN_R       BIT(6)
> > +#define SC_BTN_ZR      BIT(7)
> > +#define SC_BTN_MINUS   BIT(8)
> > +#define SC_BTN_PLUS    BIT(9)
> > +#define SC_BTN_RSTICK  BIT(10)
> > +#define SC_BTN_LSTICK  BIT(11)
> > +#define SC_BTN_HOME    BIT(12)
> > +#define SC_BTN_CAP     BIT(13) /* capture button */
> > +#define SC_BTN_DOWN    BIT(16)
> > +#define SC_BTN_UP      BIT(17)
> > +#define SC_BTN_RIGHT   BIT(18)
> > +#define SC_BTN_LEFT    BIT(19)
> > +#define SC_BTN_SR_L    BIT(20)
> > +#define SC_BTN_SL_L    BIT(21)
> > +#define SC_BTN_L       BIT(22)
> > +#define SC_BTN_ZL      BIT(23)
> > +
> > +enum switchcon_ctlr_type {
> > +       SWITCHCON_CTLR_TYPE_PROCON,
> > +       SWITCHCON_CTLR_TYPE_JOYCON_L,
> > +       SWITCHCON_CTLR_TYPE_JOYCON_R,
> > +};
>
> I think there is little use in having this enum. See further comments
> below, hdev seems to be sufficient for this driver.
>

I hadn't changed it due to Benjamin's comments on v2 about the potential for
future 3rd party controllers. That said, the enum could always be
re-added if that
comes to pass. I guess everyone can weigh in opinions.

> > +
> > +static const char * const switchcon_input_names[] = {
> > +       "Nintendo Switch Pro Controller",
> > +       "Nintendo Switch Left Joy-Con",
> > +       "Nintendo Switch Right Joy-Con",
> > +};
> > +
> > +enum switchcon_msg_type {
> > +       SWITCHCON_MSG_TYPE_NONE,
> > +       SWITCHCON_MSG_TYPE_USB,
> > +       SWITCHCON_MSG_TYPE_SUBCMD,
> > +};
> > +
> > +struct switchcon_subcmd_request {
> > +       u8 output_id; /* must be 0x01 for subcommand, 0x10 for rumble only */
> > +       u8 packet_num; /* incremented every send */
> > +       u8 rumble_data[8];
> > +       u8 subcmd_id;
> > +       /* data is here */
> > +} __packed;
> > +
> > +/* should pass in pointer to a struct switchcon_subcmd_request */
> > +#define SC_SUBCMD_REQ_GET_DATA(req) \
> > +       ((u8 *)(req) + sizeof(struct switchcon_subcmd_request))
> > +
> > +struct switchcon_subcmd_reply {
> > +       u8 ack; /* MSB 1 for ACK, 0 for NACK */
> > +       u8 id; /* id of requested subcmd */
> > +       /* data is here, can be up to 35 bytes */
> > +} __packed;
> > +
> > +/* should pass in pointer to a struct switchcon_subcmd_reply */
> > +#define SC_SUBCMD_REPLY_GET_DATA(reply) \
> > +       ((u8 *)(reply) + sizeof(struct switchcon_subcmd_reply))
> > +
> > +struct switchcon_input_report {
> > +       u8 id;
> > +       u8 timer;
> > +       u8 bat_con; /* battery and connection info */
> > +       u8 button_status[3];
> > +       u8 left_stick[3];
> > +       u8 right_stick[3];
> > +       u8 vibrator_report;
> > +
> > +       /*
> > +        * If support for firmware updates, gyroscope data, and/or NFC/IR
> > +        * are added in the future, this can be swapped for a union.
> > +        */
> > +       struct switchcon_subcmd_reply reply;
> > +} __packed;
> > +
> > +#define SC_MAX_RESP_SIZE (sizeof(struct switchcon_input_report) + 35)
> > +
> > +/* Each physical controller is associated with a switchcon_ctlr struct */
> > +struct switchcon_ctlr {
> > +       struct hid_device *hdev;
> > +       struct input_dev *input;
> > +       enum switchcon_ctlr_type type;
> > +       enum switchcon_ctlr_state ctlr_state;
> > +
> > +       /* The following members are used for synchronous sends/receives */
> > +       enum switchcon_msg_type msg_type;
> > +       u8 subcmd_num;
> > +       struct mutex output_mutex;
> > +       u8 input_buf[SC_MAX_RESP_SIZE];
> > +       wait_queue_head_t wait;
> > +       bool received_resp;
> > +       u8 usb_ack_match;
> > +       u8 subcmd_ack_match;
> > +
> > +       /* factory calibration data */
> > +       struct switchcon_stick_cal left_stick_cal_x;
> > +       struct switchcon_stick_cal left_stick_cal_y;
> > +       struct switchcon_stick_cal right_stick_cal_x;
> > +       struct switchcon_stick_cal right_stick_cal_y;
> > +
> > +};
> > +
> > +static int __switchcon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
> > +{
> > +       u8 *buf;
> > +       int ret;
> > +
> > +       buf = kmemdup(data, len, GFP_KERNEL);
> > +       if (!buf)
> > +               return -ENOMEM;
> > +       ret = hid_hw_output_report(hdev, buf, len);
> > +       kfree(buf);
> > +       if (ret < 0)
> > +               hid_dbg(hdev, "Failed to send output report ret=%d\n", ret);
> > +       return ret;
> > +}
> > +
> > +static int switchcon_hid_send_sync(struct switchcon_ctlr *ctlr, u8 *data,
> > +                                                               size_t len)
> > +{
> > +       int ret;
> > +
> > +       ret = __switchcon_hid_send(ctlr->hdev, data, len);
> > +       if (ret < 0) {
> > +               memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
> > +               return ret;
> > +       }
> > +
> > +       if (!wait_event_timeout(ctlr->wait, ctlr->received_resp, HZ)) {
> > +               hid_dbg(ctlr->hdev, "syncronous send/receive timed out\n");
> > +               memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
> > +               return -ETIMEDOUT;
> > +       }
> > +
> > +       ctlr->received_resp = false;
> > +       return 0;
> > +}
> > +
> > +static int switchcon_send_usb(struct switchcon_ctlr *ctlr, u8 cmd)
> > +{
> > +       int ret;
> > +       u8 buf[2] = {SC_OUTPUT_USB_CMD};
> > +
> > +       buf[1] = cmd;
> > +       ctlr->usb_ack_match = cmd;
> > +       ctlr->msg_type = SWITCHCON_MSG_TYPE_USB;
> > +       ret = switchcon_hid_send_sync(ctlr, buf, sizeof(buf));
> > +       if (ret)
> > +               hid_dbg(ctlr->hdev, "send usb command failed; ret=%d\n", ret);
> > +       return ret;
> > +}
> > +
> > +static int switchcon_send_subcmd(struct switchcon_ctlr *ctlr,
> > +                                struct switchcon_subcmd_request *subcmd,
> > +                                size_t data_len)
> > +{
> > +       int ret;
> > +
> > +       subcmd->output_id = SC_OUTPUT_RUMBLE_AND_SUBCMD;
> > +       subcmd->packet_num = ctlr->subcmd_num;
> > +       if (++ctlr->subcmd_num > 0xF)
> > +               ctlr->subcmd_num = 0;
> > +       ctlr->subcmd_ack_match = subcmd->subcmd_id;
> > +       ctlr->msg_type = SWITCHCON_MSG_TYPE_SUBCMD;
> > +
> > +       ret = switchcon_hid_send_sync(ctlr, (u8 *)subcmd,
> > +                                     sizeof(*subcmd) + data_len);
> > +       if (ret)
> > +               hid_dbg(ctlr->hdev, "send subcommand failed; ret=%d\n", ret);
> > +       return ret;
> > +}
> > +
> > +/* Supply nibbles for flash and on. Ones correspond to active */
> > +static int switchcon_set_player_leds(struct switchcon_ctlr *ctlr,
> > +                                               u8 flash, u8 on)
> > +{
> > +       struct switchcon_subcmd_request *req;
> > +       u8 buffer[sizeof(*req) + 1] = { 0 };
> > +
> > +       req = (struct switchcon_subcmd_request *)buffer;
> > +       req->subcmd_id = SC_SUBCMD_SET_PLAYER_LIGHTS;
> > +       SC_SUBCMD_REQ_GET_DATA(req)[0] = (flash << 4) | on;
> > +
> > +       hid_dbg(ctlr->hdev, "setting player leds\n");
> > +       return switchcon_send_subcmd(ctlr, req, 1);
> > +}
> > +
> > +static int switchcon_request_calibration(struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_subcmd_request *req;
> > +       u8 buffer[sizeof(*req) + 5] = { 0 };
> > +       u8 *data;
> > +
> > +       req = (struct switchcon_subcmd_request *)buffer;
> > +       req->subcmd_id = SC_SUBCMD_SPI_FLASH_READ;
> > +       data = SC_SUBCMD_REQ_GET_DATA(req);
> > +       data[0] = 0xFF & SC_CAL_DATA_START;
> > +       data[1] = 0xFF & (SC_CAL_DATA_START >> 8);
> > +       data[2] = 0xFF & (SC_CAL_DATA_START >> 16);
> > +       data[3] = 0xFF & (SC_CAL_DATA_START >> 24);
> > +       data[4] = SC_CAL_DATA_SIZE;
> > +
> > +       hid_dbg(ctlr->hdev, "requesting cal data\n");
> > +       return switchcon_send_subcmd(ctlr, req, 5);
> > +}
> > +
> > +static int switchcon_set_report_mode(struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_subcmd_request *req;
> > +       u8 buffer[sizeof(*req) + 1] = { 0 };
> > +
> > +       req = (struct switchcon_subcmd_request *)buffer;
> > +       req->subcmd_id = SC_SUBCMD_SET_REPORT_MODE;
> > +       SC_SUBCMD_REQ_GET_DATA(req)[0] = 0x30; /* standard, full report mode */
> > +
> > +       hid_dbg(ctlr->hdev, "setting controller report mode\n");
> > +       return switchcon_send_subcmd(ctlr, req, 1);
> > +}
> > +
> > +static int switchcon_map_stick_val(struct switchcon_stick_cal *cal, s32 val)
> > +{
> > +       s32 center = cal->center;
> > +       s32 min = cal->min;
> > +       s32 max = cal->max;
> > +       int new_val;
> > +
> > +       if (val > center) {
> > +               new_val = (val - center) * SC_MAX_STICK_MAG;
> > +               new_val /= (max - center);
> > +       } else {
> > +               new_val = (center - val) * -SC_MAX_STICK_MAG;
> > +               new_val /= (center - min);
> > +       }
> > +       new_val = clamp(new_val, -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG);
> > +       return new_val;
> > +}
> > +
> > +static void switchcon_parse_report(struct switchcon_ctlr *ctlr, u8 *data)
> > +{
> > +       struct input_dev *dev = ctlr->input;
> > +       enum switchcon_ctlr_type type = ctlr->type;
> > +       u32 btns;
> > +
> > +       btns = hid_field_extract(ctlr->hdev, data + 3, 0, 24);
> > +
> > +       if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> > +           type == SWITCHCON_CTLR_TYPE_JOYCON_L) {
> > +               u16 raw_l_x;
> > +               u16 raw_l_y;
> > +               int s_l_x;
> > +               int s_l_y;
>
> Naming wise, I would probably rename "raw_l_x" to "raw_x" (abs_x later
> implies left a bit). Similar for "s_l_x" probably just call it "x".
> Similar later for the right stick.
>
> > +
> > +               /* get raw stick values */
> > +               raw_l_x = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> > +               raw_l_y = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> > +               /* map the stick values */
> > +               s_l_x = switchcon_map_stick_val(&ctlr->left_stick_cal_x,
> > +                                               raw_l_x);
> > +               s_l_y = -switchcon_map_stick_val(&ctlr->left_stick_cal_y,
> > +                                               raw_l_y);
> > +               /* report sticks */
> > +               input_report_abs(dev, ABS_X, s_l_x);
> > +               input_report_abs(dev, ABS_Y, s_l_y);
> > +
> > +               /* report buttons */
> > +               input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
> > +                                             btns & SC_BTN_SL_L);
> > +               input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
> > +                                             btns & SC_BTN_SR_L);
> > +               input_report_key(dev, BTN_TL2, btns & SC_BTN_ZL);
> > +               input_report_key(dev, BTN_SELECT, btns & SC_BTN_MINUS);
> > +               input_report_key(dev, BTN_THUMBL, btns & SC_BTN_LSTICK);
> > +               input_report_key(dev, BTN_Z, btns & SC_BTN_CAP);
> > +               input_report_key(dev, BTN_DPAD_DOWN, btns & SC_BTN_DOWN);
> > +               input_report_key(dev, BTN_DPAD_UP, btns & SC_BTN_UP);
> > +               input_report_key(dev, BTN_DPAD_RIGHT, btns & SC_BTN_RIGHT);
> > +               input_report_key(dev, BTN_DPAD_LEFT, btns & SC_BTN_LEFT);
> > +       }
> > +       if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> > +           type == SWITCHCON_CTLR_TYPE_JOYCON_R) {
> > +               u16 raw_r_x;
> > +               u16 raw_r_y;
> > +               int s_r_x;
> > +               int s_r_y;
>
> "raw_rx" and "rx" perhaps?
>
> > +
> > +               /* get raw stick values */
> > +               raw_r_x = hid_field_extract(ctlr->hdev, (data + 9), 0, 12);
> > +               raw_r_y = hid_field_extract(ctlr->hdev, (data + 10), 4, 12);
> > +               /* map stick values */
> > +               s_r_x = switchcon_map_stick_val(&ctlr->right_stick_cal_x,
> > +                                               raw_r_x);
> > +               s_r_y = -switchcon_map_stick_val(&ctlr->right_stick_cal_y,
> > +                                               raw_r_y);
> > +               /* report sticks */
> > +               input_report_abs(dev, ABS_RX, s_r_x);
> > +               input_report_abs(dev, ABS_RY, s_r_y);
> > +
> > +               /* report buttons */
> > +               input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
> > +                                             btns & SC_BTN_SL_R);
> > +               input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
> > +                                             btns & SC_BTN_SR_R);
> > +               input_report_key(dev, BTN_TR2, btns & SC_BTN_ZR);
> > +               input_report_key(dev, BTN_START, btns & SC_BTN_PLUS);
> > +               input_report_key(dev, BTN_THUMBR, btns & SC_BTN_RSTICK);
> > +               input_report_key(dev, BTN_MODE, btns & SC_BTN_HOME);
> > +               input_report_key(dev, BTN_WEST, btns & SC_BTN_Y);
> > +               input_report_key(dev, BTN_NORTH, btns & SC_BTN_X);
> > +               input_report_key(dev, BTN_EAST, btns & SC_BTN_A);
> > +               input_report_key(dev, BTN_SOUTH, btns & SC_BTN_B);
> > +       }
> > +
> > +       input_sync(dev);
> > +}
> > +
> > +
> > +static const unsigned int switchcon_button_inputs[] = {
> > +       BTN_SELECT, BTN_Z, BTN_THUMBL,
> > +       BTN_START, BTN_MODE, BTN_THUMBR,
> > +       BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
> > +       BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
> > +       BTN_TL, BTN_TR, BTN_TL2, BTN_TR2,
> > +       0 /* 0 signals end of array */
> > +};
> > +
> > +static DEFINE_MUTEX(switchcon_input_num_mutex);
> > +static int switchcon_input_create(struct switchcon_ctlr *ctlr)
> > +{
> > +       struct hid_device *hdev;
> > +       static int input_num = 1;
> > +       int ret;
> > +       int i;
> > +
> > +       hdev = ctlr->hdev;
> > +       ctlr->input = input_allocate_device();
> > +       if (!ctlr->input) {
> > +               ret = -ENOMEM;
> > +               goto err;
> > +       }
> > +       ctlr->input->dev.parent = &hdev->dev;
> > +       ctlr->input->id.bustype = hdev->bus;
> > +       ctlr->input->id.vendor = hdev->vendor;
> > +       ctlr->input->id.product = hdev->product;
> > +       ctlr->input->id.version = hdev->version;
> > +       ctlr->input->name = switchcon_input_names[ctlr->type];
> > +       input_set_drvdata(ctlr->input, ctlr);
> > +
> > +
> > +       /* set up sticks */
> > +       input_set_abs_params(ctlr->input, ABS_X,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       input_set_abs_params(ctlr->input, ABS_Y,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       input_set_abs_params(ctlr->input, ABS_RX,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       input_set_abs_params(ctlr->input, ABS_RY,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       /* set up buttons */
> > +       for (i = 0; switchcon_button_inputs[i] > 0; i++)
> > +               input_set_capability(ctlr->input, EV_KEY,
> > +                                    switchcon_button_inputs[i]);
> > +
> > +       ret = input_register_device(ctlr->input);
> > +       if (ret)
> > +               goto err_input;
> > +
> > +       /* Set the default controller player leds based on controller number */
> > +       mutex_lock(&switchcon_input_num_mutex);
> > +       mutex_lock(&ctlr->output_mutex);
> > +       ret = switchcon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
> > +       if (ret)
> > +               hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
> > +       mutex_unlock(&ctlr->output_mutex);
> > +       if (++input_num > 4)
> > +               input_num = 1;
> > +       mutex_unlock(&switchcon_input_num_mutex);
> > +
> > +       return 0;
> > +
> > +err_input:
> > +       input_free_device(ctlr->input);
> > +err:
> > +       return ret;
> > +}
> > +
> > +/* data input must have at least 9 bytes */
> > +static void switchcon_parse_lstick_calibration(u8 *data,
> > +                                              struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_stick_cal *cal_x = &ctlr->left_stick_cal_x;
> > +       struct switchcon_stick_cal *cal_y = &ctlr->left_stick_cal_y;
> > +       s32 x_max_above;
> > +       s32 x_min_below;
> > +       s32 y_max_above;
> > +       s32 y_min_below;
> > +
> > +       x_max_above = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
> > +       y_max_above = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
> > +       cal_x->center = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
> > +       cal_y->center = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
> > +       x_min_below = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> > +       y_min_below = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> > +       cal_x->max = cal_x->center + x_max_above;
> > +       cal_x->min = cal_x->center - x_min_below;
> > +       cal_y->max = cal_y->center + y_max_above;
> > +       cal_y->min = cal_y->center - y_min_below;
> > +}
> > +
> > +/* data input must have at least 9 bytes */
> > +static void switchcon_parse_rstick_calibration(u8 *data,
> > +                                              struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_stick_cal *cal_x = &ctlr->right_stick_cal_x;
> > +       struct switchcon_stick_cal *cal_y = &ctlr->right_stick_cal_y;
> > +       s32 x_max_above;
> > +       s32 x_min_below;
> > +       s32 y_max_above;
> > +       s32 y_min_below;
> > +
> > +       cal_x->center = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
> > +       cal_y->center = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
> > +       x_min_below = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
> > +       y_min_below = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
> > +       x_max_above = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> > +       y_max_above = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> > +       cal_x->max = cal_x->center + x_max_above;
> > +       cal_x->min = cal_x->center - x_min_below;
> > +       cal_y->max = cal_y->center + y_max_above;
> > +       cal_y->min = cal_y->center - y_min_below;
> > +}
> > +
> > +/* Common handler for parsing inputs */
> > +static int switchcon_ctlr_read_handler(struct switchcon_ctlr *ctlr,
> > +                                               u8 *data, int size)
> > +{
> > +       int ret = 0;
> > +
> > +       switch (data[0]) {
> > +       case SC_INPUT_SUBCMD_REPLY:
> > +       case SC_INPUT_IMU_DATA:
> > +       case SC_INPUT_MCU_DATA:
> > +               if (size >= 12) /* make sure it contains the input report */
> > +                       switchcon_parse_report(ctlr, data);
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int switchcon_ctlr_handle_event(struct switchcon_ctlr *ctlr, u8 *data,
> > +                                                                   int size)
> > +{
> > +       int ret = 0;
> > +       bool match = false;
> > +       int copy_size;
> > +       struct switchcon_input_report *report;
> > +
> > +       if (unlikely(mutex_is_locked(&ctlr->output_mutex)) &&
> > +           ctlr->msg_type != SWITCHCON_MSG_TYPE_NONE) {
> > +               switch (ctlr->msg_type) {
> > +               case SWITCHCON_MSG_TYPE_USB:
> > +                       if (size < 2)
> > +                               break;
> > +                       if (data[0] == SC_INPUT_USB_RESPONSE &&
> > +                           data[1] == ctlr->usb_ack_match)
> > +                               match = true;
> > +                       break;
> > +               case SWITCHCON_MSG_TYPE_SUBCMD:
> > +                       if (size < sizeof(struct switchcon_input_report) ||
> > +                           data[0] != SC_INPUT_SUBCMD_REPLY)
> > +                               break;
> > +                       report = (struct switchcon_input_report *)data;
> > +                       if (report->reply.id == ctlr->subcmd_ack_match)
> > +                               match = true;
> > +                       break;
> > +               default:
> > +                       break;
> > +               }
> > +
> > +               if (match) {
> > +                       if (size > SC_MAX_RESP_SIZE)
> > +                               copy_size = SC_MAX_RESP_SIZE;
> > +                       else
> > +                               copy_size = size;
> > +                       memcpy(ctlr->input_buf, data, copy_size);
> > +                       ctlr->msg_type = SWITCHCON_MSG_TYPE_NONE;
> > +                       ctlr->received_resp = true;
> > +                       wake_up(&ctlr->wait);
> > +
> > +                       /* This message has been handled */
> > +                       return 1;
> > +               }
> > +       }
> > +
> > +       if (ctlr->ctlr_state == SWITCHCON_CTLR_STATE_READ)
> > +               ret = switchcon_ctlr_read_handler(ctlr, data, size);
> > +
> > +       return ret;
> > +}
> > +
> > +static int switchcon_hid_event(struct hid_device *hdev,
> > +                       struct hid_report *report, u8 *raw_data, int size)
> > +{
> > +       struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
> > +
> > +       if (size < 1)
> > +               return -EINVAL;
> > +
> > +       return switchcon_ctlr_handle_event(ctlr, raw_data, size);
> > +}
> > +
> > +static struct switchcon_ctlr *switchcon_ctlr_create(struct hid_device *hdev)
> > +{
> > +       struct switchcon_ctlr *ctlr;
> > +
> > +       ctlr = devm_kzalloc(&hdev->dev, sizeof(*ctlr), GFP_KERNEL);
> > +       if (!ctlr)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       switch (hdev->product) {
> > +       case USB_DEVICE_ID_NINTENDO_PROCON:
> > +               ctlr->type = SWITCHCON_CTLR_TYPE_PROCON;
> > +               break;
> > +       case USB_DEVICE_ID_NINTENDO_JOYCONL:
> > +               ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_L;
> > +               break;
> > +       case USB_DEVICE_ID_NINTENDO_JOYCONR:
> > +               ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_R;
> > +               break;
> > +       default:
> > +               return ERR_PTR(-EINVAL);
> > +       }
>
> There is no need to store "ctlr->type". It is checked in one or two
> places. Just use hdev in those.
>
> > +       ctlr->hdev = hdev;
> > +       ctlr->ctlr_state = SWITCHCON_CTLR_STATE_INIT;
> > +       hid_set_drvdata(hdev, ctlr);
> > +       mutex_init(&ctlr->output_mutex);
> > +       init_waitqueue_head(&ctlr->wait);
> > +       return ctlr;
> > +}
>
> With removal of the "ctrl->type" logic and some of the other
> refactoring of calibration as suggested below in "_probe", there is
> probably no good reason to keep a separate create function.
>
> > +
> > +static void switchcon_ctlr_destroy(struct switchcon_ctlr *ctlr)
> > +{
> > +       if (ctlr->input)
> > +               input_unregister_device(ctlr->input);
> > +       mutex_destroy(&ctlr->output_mutex);
> > +}
> > +
> > +static int switchcon_hid_probe(struct hid_device *hdev,
> > +                              const struct hid_device_id *id)
> > +{
> > +       int ret;
> > +       struct switchcon_ctlr *ctlr;
> > +       struct switchcon_input_report *report;
> > +       u8 *raw_cal;
> > +
> > +       hid_dbg(hdev, "probe - start\n");
> > +
> > +       ctlr = switchcon_ctlr_create(hdev);
> > +       if (IS_ERR(ctlr)) {
> > +               hid_err(hdev, "Failed to create new controller\n");
> > +               ret = PTR_ERR(ctlr);
> > +               goto err;
> > +       }
> > +
> > +       ret = hid_parse(hdev);
> > +       if (ret) {
> > +               hid_err(hdev, "HID parse failed\n");
> > +               goto err;
> > +       }
> > +
> > +       ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > +       if (ret) {
> > +               hid_err(hdev, "HW start failed\n");
> > +               goto err;
> > +       }
> > +
> > +       ret = hid_hw_open(hdev);
> > +       if (ret) {
> > +               hid_err(hdev, "cannot start hardware I/O\n");
> > +               goto err_stop;
> > +       }
> > +
> > +       hid_device_io_start(hdev);
> > +
> > +       /* Initialize the controller */
> > +       mutex_lock(&ctlr->output_mutex);
> > +       /* if baudrate command fails, assume ble pro controller */
> > +       if (ctlr->type == SWITCHCON_CTLR_TYPE_PROCON &&
> > +           !switchcon_send_usb(ctlr, SC_USB_CMD_BAUDRATE_3M)) {
> > +               /* handshake */
> > +               ret = switchcon_send_usb(ctlr, SC_USB_CMD_HANDSHAKE);
> > +               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.
> > +                */
> > +               switchcon_send_usb(ctlr, SC_USB_CMD_NO_TIMEOUT);
> > +       }
> > +
> > +       /* get controller calibration data, and parse it */
> > +       ret = switchcon_request_calibration(ctlr);
> > +       if (ret) {
> > +               hid_err(hdev, "Failed to retrieve calibration; ret=%d\n", ret);
> > +               goto err_mutex;
> > +       }
> > +       report = (struct switchcon_input_report *)ctlr->input_buf;
> > +       raw_cal = SC_SUBCMD_REPLY_GET_DATA(&report->reply) + 5;
> > +       switchcon_parse_lstick_calibration(raw_cal, ctlr);
> > +       switchcon_parse_rstick_calibration(raw_cal + 9, ctlr);
>
> Move the lstick/rstick code into the calibration function. I think
> there is also little reason to keep the lstick/rstick calibration
> helper functions. They are only used once, so make a slightly bigger
> overall calibration function hiding all this detail.
>
> > +       hid_info(ctlr->hdev, "calibration:\n"
> > +                            "l_x_c=%d l_x_max=%d l_x_min=%d\n"
> > +                            "l_y_c=%d l_y_max=%d l_y_min=%d\n"
> > +                            "r_x_c=%d r_x_max=%d r_x_min=%d\n"
> > +                            "r_y_c=%d r_y_max=%d r_y_min=%d\n",
> > +                            ctlr->left_stick_cal_x.center,
> > +                            ctlr->left_stick_cal_x.max,
> > +                            ctlr->left_stick_cal_x.min,
> > +                            ctlr->left_stick_cal_y.center,
> > +                            ctlr->left_stick_cal_y.max,
> > +                            ctlr->left_stick_cal_y.min,
> > +                            ctlr->right_stick_cal_x.center,
> > +                            ctlr->right_stick_cal_x.max,
> > +                            ctlr->right_stick_cal_x.min,
> > +                            ctlr->right_stick_cal_y.center,
> > +                            ctlr->right_stick_cal_y.max,
> > +                            ctlr->right_stick_cal_y.min);
>
> I would move more of this logic into your calibration function. Not
> sure how much sense it makes to print the calibrarion data. Probably
> print it using hid_dbg, so it is only printed on debug builds.
>
> > +
> > +       /* Set the reporting mode to 0x30, which is the full report mode */
> > +       ret = switchcon_set_report_mode(ctlr);
> > +       if (ret) {
> > +               hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> > +               goto err_mutex;
> > +       }
> > +
> > +       mutex_unlock(&ctlr->output_mutex);
> > +
> > +       ret = switchcon_input_create(ctlr);
> > +       if (ret) {
> > +               hid_err(hdev, "Failed to create input device; ret=%d\n", ret);
> > +               goto err_close;
> > +       }
> > +
> > +       ctlr->ctlr_state = SWITCHCON_CTLR_STATE_READ;
> > +
> > +       hid_dbg(hdev, "probe - success\n");
> > +       return 0;
> > +
> > +err_mutex:
> > +       mutex_unlock(&ctlr->output_mutex);
> > +err_close:
> > +       switchcon_ctlr_destroy(ctlr);
> > +       hid_hw_close(hdev);
> > +err_stop:
> > +       hid_hw_stop(hdev);
> > +err:
> > +       hid_err(hdev, "probe - fail = %d\n", ret);
> > +       return ret;
> > +}
> > +
> > +static void switchcon_hid_remove(struct hid_device *hdev)
> > +{
> > +       struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
> > +
> > +       hid_dbg(hdev, "remove\n");
> > +       hid_hw_close(hdev);
> > +       hid_hw_stop(hdev);
> > +       switchcon_ctlr_destroy(ctlr);
> > +}
> > +
> > +static const struct hid_device_id switchcon_hid_devices[] = {
> > +       { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_PROCON) },
> > +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_PROCON) },
> > +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_JOYCONL) },
> > +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_JOYCONR) },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(hid, switchcon_hid_devices);
> > +
> > +static struct hid_driver switchcon_hid_driver = {
> > +       .name           = "switchcon",
> > +       .id_table       = switchcon_hid_devices,
> > +       .probe          = switchcon_hid_probe,
> > +       .remove         = switchcon_hid_remove,
> > +       .raw_event      = switchcon_hid_event,
> > +};
> > +module_hid_driver(switchcon_hid_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
> > +MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
> > --
> > 2.20.1
> >


Thanks,
Daniel
Bastien Nocera Jan. 28, 2019, 1:15 p.m. UTC | #3
On Sun, 2019-01-27 at 22:04 -0600, Daniel J. Ogorchock wrote:
> The switchcon driver supports the Nintendo Switch Pro Controllers and
> the Joy-Cons. The Pro Controllers can be used over USB or Bluetooth.
> 
> The Joy-Cons each create their own, independent input devices, so it
> is
> up to userspace to combine them if desired.
> 
<snip>
> +static void switchcon_parse_report(struct switchcon_ctlr *ctlr, u8
> *data)
> +{
> +	struct input_dev *dev = ctlr->input;
> +	enum switchcon_ctlr_type type = ctlr->type;
> +	u32 btns;
> +
> +	btns = hid_field_extract(ctlr->hdev, data + 3, 0, 24);
> +
> +	if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> +	    type == SWITCHCON_CTLR_TYPE_JOYCON_L) {

<snip>
> +	}
> +	if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> +	    type == SWITCHCON_CTLR_TYPE_JOYCON_R) {

Did I read this code correctly, in that the 2 joy-cons will each report
their status as if they were independent?

I know that the Switch has a settings panel and probably some platform-
level code to deal with multiple sets of joycons being available, such
a pair attached to the body of the device and another pair used
individually by 2 players.

The "these 2 discrete devices are actually one" case could be handled
by a library that Peter is working on.

However, have you tested the joy-cons with a "charging grip"? Does the
power_supply reporting work for the joy-cons when charging via this
method? Do the joy-cons change behaviour, or work differently when
plugged in to the charging grip? Or do the joy-cons not have a "wired"
mode at all, and all the data is always sent via Bluetooth.

> +		if (match) {
> +			if (size > SC_MAX_RESP_SIZE)
> +				copy_size = SC_MAX_RESP_SIZE;
> +			else
> +				copy_size = size;

Use MIN()?

Cheers
Benjamin Tissoires Jan. 28, 2019, 4:28 p.m. UTC | #4
Hi Roderick,

On Mon, Jan 28, 2019 at 6:11 AM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> Hi Daniel,
>
> Thanks for updating the patch series. It is definitely improved and
> easier to read. See some of my comments below.
>
> One other thing I noticed, see the comment in v2 review about breaking
> userspace. I think since you change the button / stick mapping
> compared to what SDL2 supports for Pro controller. You need to do
> something e.g. different version like hid-sony does.
>
> Out of curiosity did you test the driver on Bluetooth? The reason I'm
> asking is, because I suspect you would have hit a deadlock issue,
> which we hit in hid-sony before. I'm not sure how you got around that
> one..
>
> Let me explain the issue. When using Bluetooth, many Bluetooth stacks
> rely on uhid (either Bluez on desktop and even Android's one). I
> recall when the BT stack requests uhid to "create" an input device,
> this ultimately triggers "_probe" in your driver. hid-sony at the time
> and similar for hid-switchcon triggers various requests to the
> hardware during probe. However uhid is single threaded and it is
> holding a lock. This caused a deadlock and caused hardware request
> timeouts. I don't see how this is not happening for you.

Actually, I think you are missing `hid_device_io_start(hdev);` in hid-sony.c

What this driver does and which is race free (in pseudo code):
probe() {
    hid_parse();
    hid_hw_start(CONNECT_HIDRAW);
    hid_hw_open(hdev);
    hid_device_io_start(hdev);
    // now you can do whatever IO you want, .raw_event() will also be called
    hid_hw_close(hdev);
    switchcon_input_create(ctlr);
}

If you want to rely on HIDINPUT to create the inputs for you and not
prematurely expose a hidraw node, then you can first hid_hwstart with
CONNECT_DRIVER, do your init, then stop the hid device and finally
start with CONNECT_DEFAULT.

Cheers,
Benjamin

>
> The solution in hid-sony at the time was to move logic to
> "_input_configured", which had slightly different timing. (It is
> triggered if I recall by hid_hw_start or a call like that and avoided
> the deadlock).
>
> Thanks,
> Roderick
>
> On Sun, Jan 27, 2019 at 8:05 PM Daniel J. Ogorchock
> <djogorchock@gmail.com> wrote:
> >
> > The switchcon driver supports the Nintendo Switch Pro Controllers and
> > the Joy-Cons. The Pro Controllers can be used over USB or Bluetooth.
> >
> > The Joy-Cons each create their own, independent input devices, so it is
> > up to userspace to combine them if desired.
> >
> > Signed-off-by: Daniel J. Ogorchock <djogorchock@gmail.com>
> > ---
> >  MAINTAINERS                 |   6 +
> >  drivers/hid/Kconfig         |  11 +
> >  drivers/hid/Makefile        |   1 +
> >  drivers/hid/hid-ids.h       |   3 +
> >  drivers/hid/hid-switchcon.c | 827 ++++++++++++++++++++++++++++++++++++
> >  5 files changed, 848 insertions(+)
> >  create mode 100644 drivers/hid/hid-switchcon.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f3a5c97e3419..001817e2e08a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14397,6 +14397,12 @@ F:     kernel/dma/swiotlb.c
> >  F:     arch/*/kernel/pci-swiotlb.c
> >  F:     include/linux/swiotlb.h
> >
> > +SWITCHCON HID DRIVER
> > +M:     Daniel J. Ogorchock <djogorchock@gmail.com>
> > +L:     linux-input@vger.kernel.org
> > +S:     Maintained
> > +F:     drivers/hid/hid-switchcon*
> > +
> >  SWITCHDEV
> >  M:     Jiri Pirko <jiri@resnulli.us>
> >  M:     Ivan Vecera <ivecera@redhat.com>
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 41e9935fc584..9702c07802a5 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -943,6 +943,17 @@ config SMARTJOYPLUS_FF
> >         Say Y here if you have a SmartJoy PLUS PS2/USB adapter and want to
> >         enable force feedback support for it.
> >
> > +config HID_SWITCHCON
> > +       tristate "Nintendo Joy-Con and Pro Controller support"
> > +       depends on HID
> > +       help
> > +       Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
> > +       All controllers support bluetooth, and the Pro Controller also supports
> > +       its USB mode.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called hid-switchcon.
> > +
> >  config HID_TIVO
> >         tristate "TiVo Slide Bluetooth remote control support"
> >         depends on HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > index 896a51ce7ce0..e708e682c4b8 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -103,6 +103,7 @@ obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
> >  obj-$(CONFIG_HID_STEAM)                += hid-steam.o
> >  obj-$(CONFIG_HID_STEELSERIES)  += hid-steelseries.o
> >  obj-$(CONFIG_HID_SUNPLUS)      += hid-sunplus.o
> > +obj-$(CONFIG_HID_SWITCHCON)    += hid-switchcon.o
> >  obj-$(CONFIG_HID_GREENASIA)    += hid-gaff.o
> >  obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
> >  obj-$(CONFIG_HID_TIVO)         += hid-tivo.o
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 27519eb8ee63..7924f7d502a8 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -846,6 +846,9 @@
> >  #define USB_VENDOR_ID_NINTENDO         0x057e
> >  #define USB_DEVICE_ID_NINTENDO_WIIMOTE 0x0306
> >  #define USB_DEVICE_ID_NINTENDO_WIIMOTE2        0x0330
> > +#define USB_DEVICE_ID_NINTENDO_JOYCONL 0x2006
> > +#define USB_DEVICE_ID_NINTENDO_JOYCONR  0x2007
> > +#define USB_DEVICE_ID_NINTENDO_PROCON   0x2009
> >
> >  #define USB_VENDOR_ID_NOVATEK          0x0603
> >  #define USB_DEVICE_ID_NOVATEK_PCT      0x0600
> > diff --git a/drivers/hid/hid-switchcon.c b/drivers/hid/hid-switchcon.c
> > new file mode 100644
> > index 000000000000..f70b7cf95021
> > --- /dev/null
> > +++ b/drivers/hid/hid-switchcon.c
> > @@ -0,0 +1,827 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
> > + *
> > + * Copyright (c) 2019 Daniel J. Ogorchock <djogorchock@gmail.com>
> > + *
> > + * The following resources/projects were referenced for this driver:
> > + *   https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> > + *   https://gitlab.com/pjranki/joycon-linux-kernel (Peter Rankin)
> > + *   https://github.com/FrotBot/SwitchProConLinuxUSB
> > + *   https://github.com/MTCKC/ProconXInput
> > + *   hid-wiimote kernel hid driver
> > + *   hid-logitech-hidpp driver
> > + *
> > + * This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
> > + * Pro Controllers can either be used over USB or Bluetooth.
> > + *
> > + * The driver will retrieve the factory calibration info from the controllers,
> > + * so little to no user calibration should be required.
> > + *
> > + */
> > +
> > +#include "hid-ids.h"
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/hid.h>
> > +#include <linux/input.h>
> > +#include <linux/module.h>
> > +#include <linux/spinlock.h>
> > +
> > +/*
> > + * Reference the url below for the following HID report defines:
> > + * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> > + */
> > +
> > +/* Output Reports */
> > +#define SC_OUTPUT_RUMBLE_AND_SUBCMD    0x01
> > +#define SC_OUTPUT_FW_UPDATE_PKT                0x03
> > +#define SC_OUTPUT_RUMBLE_ONLY          0x10
> > +#define SC_OUTPUT_MCU_DATA             0x11
> > +#define SC_OUTPUT_USB_CMD              0x80
> > +
> > +/* Subcommand IDs */
> > +#define SC_SUBCMD_STATE                        0x00
> > +#define SC_SUBCMD_MANUAL_BT_PAIRING    0x01
> > +#define SC_SUBCMD_REQ_DEV_INFO         0x02
> > +#define SC_SUBCMD_SET_REPORT_MODE      0x03
> > +#define SC_SUBCMD_TRIGGERS_ELAPSED     0x04
> > +#define SC_SUBCMD_GET_PAGE_LIST_STATE  0x05
> > +#define SC_SUBCMD_SET_HCI_STATE                0x06
> > +#define SC_SUBCMD_RESET_PAIRING_INFO   0x07
> > +#define SC_SUBCMD_LOW_POWER_MODE       0x08
> > +#define SC_SUBCMD_SPI_FLASH_READ       0x10
> > +#define SC_SUBCMD_SPI_FLASH_WRITE      0x11
> > +#define SC_SUBCMD_RESET_MCU            0x20
> > +#define SC_SUBCMD_SET_MCU_CONFIG       0x21
> > +#define SC_SUBCMD_SET_MCU_STATE                0x22
> > +#define SC_SUBCMD_SET_PLAYER_LIGHTS    0x30
> > +#define SC_SUBCMD_GET_PLAYER_LIGHTS    0x31
> > +#define SC_SUBCMD_SET_HOME_LIGHT       0x38
> > +#define SC_SUBCMD_ENABLE_IMU           0x40
> > +#define SC_SUBCMD_SET_IMU_SENSITIVITY  0x41
> > +#define SC_SUBCMD_WRITE_IMU_REG                0x42
> > +#define SC_SUBCMD_READ_IMU_REG         0x43
> > +#define SC_SUBCMD_ENABLE_VIBRATION     0x48
> > +#define SC_SUBCMD_GET_REGULATED_VOLTAGE        0x50
> > +
> > +/* Input Reports */
> > +#define SC_INPUT_BUTTON_EVENT          0x3F
> > +#define SC_INPUT_SUBCMD_REPLY          0x21
> > +#define SC_INPUT_IMU_DATA              0x30
> > +#define SC_INPUT_MCU_DATA              0x31
> > +#define SC_INPUT_USB_RESPONSE          0x81
> > +
> > +/* Feature Reports */
> > +#define SC_FEATURE_LAST_SUBCMD         0x02
> > +#define SC_FEATURE_OTA_FW_UPGRADE      0x70
> > +#define SC_FEATURE_SETUP_MEM_READ      0x71
> > +#define SC_FEATURE_MEM_READ            0x72
> > +#define SC_FEATURE_ERASE_MEM_SECTOR    0x73
> > +#define SC_FEATURE_MEM_WRITE           0x74
> > +#define SC_FEATURE_LAUNCH              0x75
> > +
> > +/* USB Commands */
> > +#define SC_USB_CMD_CONN_STATUS         0x01
> > +#define SC_USB_CMD_HANDSHAKE           0x02
> > +#define SC_USB_CMD_BAUDRATE_3M         0x03
> > +#define SC_USB_CMD_NO_TIMEOUT          0x04
> > +#define SC_USB_CMD_EN_TIMEOUT          0x05
> > +#define SC_USB_RESET                   0x06
> > +#define SC_USB_PRE_HANDSHAKE           0x91
> > +#define SC_USB_SEND_UART               0x92
> > +
> > +/* SPI storage addresses of factory calibration data */
> > +#define SC_CAL_DATA_START              0x603d
> > +#define SC_CAL_DATA_END                        0x604e
> > +#define SC_CAL_DATA_SIZE       (SC_CAL_DATA_END - SC_CAL_DATA_START + 1)
> > +
> > +
> > +/* The raw analog joystick values will be mapped in terms of this magnitude */
> > +#define SC_MAX_STICK_MAG       32767
> > +#define SC_STICK_FUZZ          250
> > +#define SC_STICK_FLAT          500
> > +
> > +/* States for controller state machine */
> > +enum switchcon_ctlr_state {
> > +       SWITCHCON_CTLR_STATE_INIT,
> > +       SWITCHCON_CTLR_STATE_READ,
> > +};
> > +
> > +struct switchcon_stick_cal {
> > +       s32 max;
> > +       s32 min;
> > +       s32 center;
> > +};
> > +
> > +/*
> > + * All the controller's button values are stored in a u32.
> > + * They can be accessed with bitwise ANDs.
> > + */
> > +#define SC_BTN_Y       BIT(0)
> > +#define SC_BTN_X       BIT(1)
> > +#define SC_BTN_B       BIT(2)
> > +#define SC_BTN_A       BIT(3)
> > +#define SC_BTN_SR_R    BIT(4)
> > +#define SC_BTN_SL_R    BIT(5)
> > +#define SC_BTN_R       BIT(6)
> > +#define SC_BTN_ZR      BIT(7)
> > +#define SC_BTN_MINUS   BIT(8)
> > +#define SC_BTN_PLUS    BIT(9)
> > +#define SC_BTN_RSTICK  BIT(10)
> > +#define SC_BTN_LSTICK  BIT(11)
> > +#define SC_BTN_HOME    BIT(12)
> > +#define SC_BTN_CAP     BIT(13) /* capture button */
> > +#define SC_BTN_DOWN    BIT(16)
> > +#define SC_BTN_UP      BIT(17)
> > +#define SC_BTN_RIGHT   BIT(18)
> > +#define SC_BTN_LEFT    BIT(19)
> > +#define SC_BTN_SR_L    BIT(20)
> > +#define SC_BTN_SL_L    BIT(21)
> > +#define SC_BTN_L       BIT(22)
> > +#define SC_BTN_ZL      BIT(23)
> > +
> > +enum switchcon_ctlr_type {
> > +       SWITCHCON_CTLR_TYPE_PROCON,
> > +       SWITCHCON_CTLR_TYPE_JOYCON_L,
> > +       SWITCHCON_CTLR_TYPE_JOYCON_R,
> > +};
>
> I think there is little use in having this enum. See further comments
> below, hdev seems to be sufficient for this driver.
>
> > +
> > +static const char * const switchcon_input_names[] = {
> > +       "Nintendo Switch Pro Controller",
> > +       "Nintendo Switch Left Joy-Con",
> > +       "Nintendo Switch Right Joy-Con",
> > +};
> > +
> > +enum switchcon_msg_type {
> > +       SWITCHCON_MSG_TYPE_NONE,
> > +       SWITCHCON_MSG_TYPE_USB,
> > +       SWITCHCON_MSG_TYPE_SUBCMD,
> > +};
> > +
> > +struct switchcon_subcmd_request {
> > +       u8 output_id; /* must be 0x01 for subcommand, 0x10 for rumble only */
> > +       u8 packet_num; /* incremented every send */
> > +       u8 rumble_data[8];
> > +       u8 subcmd_id;
> > +       /* data is here */
> > +} __packed;
> > +
> > +/* should pass in pointer to a struct switchcon_subcmd_request */
> > +#define SC_SUBCMD_REQ_GET_DATA(req) \
> > +       ((u8 *)(req) + sizeof(struct switchcon_subcmd_request))
> > +
> > +struct switchcon_subcmd_reply {
> > +       u8 ack; /* MSB 1 for ACK, 0 for NACK */
> > +       u8 id; /* id of requested subcmd */
> > +       /* data is here, can be up to 35 bytes */
> > +} __packed;
> > +
> > +/* should pass in pointer to a struct switchcon_subcmd_reply */
> > +#define SC_SUBCMD_REPLY_GET_DATA(reply) \
> > +       ((u8 *)(reply) + sizeof(struct switchcon_subcmd_reply))
> > +
> > +struct switchcon_input_report {
> > +       u8 id;
> > +       u8 timer;
> > +       u8 bat_con; /* battery and connection info */
> > +       u8 button_status[3];
> > +       u8 left_stick[3];
> > +       u8 right_stick[3];
> > +       u8 vibrator_report;
> > +
> > +       /*
> > +        * If support for firmware updates, gyroscope data, and/or NFC/IR
> > +        * are added in the future, this can be swapped for a union.
> > +        */
> > +       struct switchcon_subcmd_reply reply;
> > +} __packed;
> > +
> > +#define SC_MAX_RESP_SIZE (sizeof(struct switchcon_input_report) + 35)
> > +
> > +/* Each physical controller is associated with a switchcon_ctlr struct */
> > +struct switchcon_ctlr {
> > +       struct hid_device *hdev;
> > +       struct input_dev *input;
> > +       enum switchcon_ctlr_type type;
> > +       enum switchcon_ctlr_state ctlr_state;
> > +
> > +       /* The following members are used for synchronous sends/receives */
> > +       enum switchcon_msg_type msg_type;
> > +       u8 subcmd_num;
> > +       struct mutex output_mutex;
> > +       u8 input_buf[SC_MAX_RESP_SIZE];
> > +       wait_queue_head_t wait;
> > +       bool received_resp;
> > +       u8 usb_ack_match;
> > +       u8 subcmd_ack_match;
> > +
> > +       /* factory calibration data */
> > +       struct switchcon_stick_cal left_stick_cal_x;
> > +       struct switchcon_stick_cal left_stick_cal_y;
> > +       struct switchcon_stick_cal right_stick_cal_x;
> > +       struct switchcon_stick_cal right_stick_cal_y;
> > +
> > +};
> > +
> > +static int __switchcon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
> > +{
> > +       u8 *buf;
> > +       int ret;
> > +
> > +       buf = kmemdup(data, len, GFP_KERNEL);
> > +       if (!buf)
> > +               return -ENOMEM;
> > +       ret = hid_hw_output_report(hdev, buf, len);
> > +       kfree(buf);
> > +       if (ret < 0)
> > +               hid_dbg(hdev, "Failed to send output report ret=%d\n", ret);
> > +       return ret;
> > +}
> > +
> > +static int switchcon_hid_send_sync(struct switchcon_ctlr *ctlr, u8 *data,
> > +                                                               size_t len)
> > +{
> > +       int ret;
> > +
> > +       ret = __switchcon_hid_send(ctlr->hdev, data, len);
> > +       if (ret < 0) {
> > +               memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
> > +               return ret;
> > +       }
> > +
> > +       if (!wait_event_timeout(ctlr->wait, ctlr->received_resp, HZ)) {
> > +               hid_dbg(ctlr->hdev, "syncronous send/receive timed out\n");
> > +               memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
> > +               return -ETIMEDOUT;
> > +       }
> > +
> > +       ctlr->received_resp = false;
> > +       return 0;
> > +}
> > +
> > +static int switchcon_send_usb(struct switchcon_ctlr *ctlr, u8 cmd)
> > +{
> > +       int ret;
> > +       u8 buf[2] = {SC_OUTPUT_USB_CMD};
> > +
> > +       buf[1] = cmd;
> > +       ctlr->usb_ack_match = cmd;
> > +       ctlr->msg_type = SWITCHCON_MSG_TYPE_USB;
> > +       ret = switchcon_hid_send_sync(ctlr, buf, sizeof(buf));
> > +       if (ret)
> > +               hid_dbg(ctlr->hdev, "send usb command failed; ret=%d\n", ret);
> > +       return ret;
> > +}
> > +
> > +static int switchcon_send_subcmd(struct switchcon_ctlr *ctlr,
> > +                                struct switchcon_subcmd_request *subcmd,
> > +                                size_t data_len)
> > +{
> > +       int ret;
> > +
> > +       subcmd->output_id = SC_OUTPUT_RUMBLE_AND_SUBCMD;
> > +       subcmd->packet_num = ctlr->subcmd_num;
> > +       if (++ctlr->subcmd_num > 0xF)
> > +               ctlr->subcmd_num = 0;
> > +       ctlr->subcmd_ack_match = subcmd->subcmd_id;
> > +       ctlr->msg_type = SWITCHCON_MSG_TYPE_SUBCMD;
> > +
> > +       ret = switchcon_hid_send_sync(ctlr, (u8 *)subcmd,
> > +                                     sizeof(*subcmd) + data_len);
> > +       if (ret)
> > +               hid_dbg(ctlr->hdev, "send subcommand failed; ret=%d\n", ret);
> > +       return ret;
> > +}
> > +
> > +/* Supply nibbles for flash and on. Ones correspond to active */
> > +static int switchcon_set_player_leds(struct switchcon_ctlr *ctlr,
> > +                                               u8 flash, u8 on)
> > +{
> > +       struct switchcon_subcmd_request *req;
> > +       u8 buffer[sizeof(*req) + 1] = { 0 };
> > +
> > +       req = (struct switchcon_subcmd_request *)buffer;
> > +       req->subcmd_id = SC_SUBCMD_SET_PLAYER_LIGHTS;
> > +       SC_SUBCMD_REQ_GET_DATA(req)[0] = (flash << 4) | on;
> > +
> > +       hid_dbg(ctlr->hdev, "setting player leds\n");
> > +       return switchcon_send_subcmd(ctlr, req, 1);
> > +}
> > +
> > +static int switchcon_request_calibration(struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_subcmd_request *req;
> > +       u8 buffer[sizeof(*req) + 5] = { 0 };
> > +       u8 *data;
> > +
> > +       req = (struct switchcon_subcmd_request *)buffer;
> > +       req->subcmd_id = SC_SUBCMD_SPI_FLASH_READ;
> > +       data = SC_SUBCMD_REQ_GET_DATA(req);
> > +       data[0] = 0xFF & SC_CAL_DATA_START;
> > +       data[1] = 0xFF & (SC_CAL_DATA_START >> 8);
> > +       data[2] = 0xFF & (SC_CAL_DATA_START >> 16);
> > +       data[3] = 0xFF & (SC_CAL_DATA_START >> 24);
> > +       data[4] = SC_CAL_DATA_SIZE;
> > +
> > +       hid_dbg(ctlr->hdev, "requesting cal data\n");
> > +       return switchcon_send_subcmd(ctlr, req, 5);
> > +}
> > +
> > +static int switchcon_set_report_mode(struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_subcmd_request *req;
> > +       u8 buffer[sizeof(*req) + 1] = { 0 };
> > +
> > +       req = (struct switchcon_subcmd_request *)buffer;
> > +       req->subcmd_id = SC_SUBCMD_SET_REPORT_MODE;
> > +       SC_SUBCMD_REQ_GET_DATA(req)[0] = 0x30; /* standard, full report mode */
> > +
> > +       hid_dbg(ctlr->hdev, "setting controller report mode\n");
> > +       return switchcon_send_subcmd(ctlr, req, 1);
> > +}
> > +
> > +static int switchcon_map_stick_val(struct switchcon_stick_cal *cal, s32 val)
> > +{
> > +       s32 center = cal->center;
> > +       s32 min = cal->min;
> > +       s32 max = cal->max;
> > +       int new_val;
> > +
> > +       if (val > center) {
> > +               new_val = (val - center) * SC_MAX_STICK_MAG;
> > +               new_val /= (max - center);
> > +       } else {
> > +               new_val = (center - val) * -SC_MAX_STICK_MAG;
> > +               new_val /= (center - min);
> > +       }
> > +       new_val = clamp(new_val, -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG);
> > +       return new_val;
> > +}
> > +
> > +static void switchcon_parse_report(struct switchcon_ctlr *ctlr, u8 *data)
> > +{
> > +       struct input_dev *dev = ctlr->input;
> > +       enum switchcon_ctlr_type type = ctlr->type;
> > +       u32 btns;
> > +
> > +       btns = hid_field_extract(ctlr->hdev, data + 3, 0, 24);
> > +
> > +       if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> > +           type == SWITCHCON_CTLR_TYPE_JOYCON_L) {
> > +               u16 raw_l_x;
> > +               u16 raw_l_y;
> > +               int s_l_x;
> > +               int s_l_y;
>
> Naming wise, I would probably rename "raw_l_x" to "raw_x" (abs_x later
> implies left a bit). Similar for "s_l_x" probably just call it "x".
> Similar later for the right stick.
>
> > +
> > +               /* get raw stick values */
> > +               raw_l_x = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> > +               raw_l_y = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> > +               /* map the stick values */
> > +               s_l_x = switchcon_map_stick_val(&ctlr->left_stick_cal_x,
> > +                                               raw_l_x);
> > +               s_l_y = -switchcon_map_stick_val(&ctlr->left_stick_cal_y,
> > +                                               raw_l_y);
> > +               /* report sticks */
> > +               input_report_abs(dev, ABS_X, s_l_x);
> > +               input_report_abs(dev, ABS_Y, s_l_y);
> > +
> > +               /* report buttons */
> > +               input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
> > +                                             btns & SC_BTN_SL_L);
> > +               input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
> > +                                             btns & SC_BTN_SR_L);
> > +               input_report_key(dev, BTN_TL2, btns & SC_BTN_ZL);
> > +               input_report_key(dev, BTN_SELECT, btns & SC_BTN_MINUS);
> > +               input_report_key(dev, BTN_THUMBL, btns & SC_BTN_LSTICK);
> > +               input_report_key(dev, BTN_Z, btns & SC_BTN_CAP);
> > +               input_report_key(dev, BTN_DPAD_DOWN, btns & SC_BTN_DOWN);
> > +               input_report_key(dev, BTN_DPAD_UP, btns & SC_BTN_UP);
> > +               input_report_key(dev, BTN_DPAD_RIGHT, btns & SC_BTN_RIGHT);
> > +               input_report_key(dev, BTN_DPAD_LEFT, btns & SC_BTN_LEFT);
> > +       }
> > +       if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> > +           type == SWITCHCON_CTLR_TYPE_JOYCON_R) {
> > +               u16 raw_r_x;
> > +               u16 raw_r_y;
> > +               int s_r_x;
> > +               int s_r_y;
>
> "raw_rx" and "rx" perhaps?
>
> > +
> > +               /* get raw stick values */
> > +               raw_r_x = hid_field_extract(ctlr->hdev, (data + 9), 0, 12);
> > +               raw_r_y = hid_field_extract(ctlr->hdev, (data + 10), 4, 12);
> > +               /* map stick values */
> > +               s_r_x = switchcon_map_stick_val(&ctlr->right_stick_cal_x,
> > +                                               raw_r_x);
> > +               s_r_y = -switchcon_map_stick_val(&ctlr->right_stick_cal_y,
> > +                                               raw_r_y);
> > +               /* report sticks */
> > +               input_report_abs(dev, ABS_RX, s_r_x);
> > +               input_report_abs(dev, ABS_RY, s_r_y);
> > +
> > +               /* report buttons */
> > +               input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
> > +                                             btns & SC_BTN_SL_R);
> > +               input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
> > +                                             btns & SC_BTN_SR_R);
> > +               input_report_key(dev, BTN_TR2, btns & SC_BTN_ZR);
> > +               input_report_key(dev, BTN_START, btns & SC_BTN_PLUS);
> > +               input_report_key(dev, BTN_THUMBR, btns & SC_BTN_RSTICK);
> > +               input_report_key(dev, BTN_MODE, btns & SC_BTN_HOME);
> > +               input_report_key(dev, BTN_WEST, btns & SC_BTN_Y);
> > +               input_report_key(dev, BTN_NORTH, btns & SC_BTN_X);
> > +               input_report_key(dev, BTN_EAST, btns & SC_BTN_A);
> > +               input_report_key(dev, BTN_SOUTH, btns & SC_BTN_B);
> > +       }
> > +
> > +       input_sync(dev);
> > +}
> > +
> > +
> > +static const unsigned int switchcon_button_inputs[] = {
> > +       BTN_SELECT, BTN_Z, BTN_THUMBL,
> > +       BTN_START, BTN_MODE, BTN_THUMBR,
> > +       BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
> > +       BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
> > +       BTN_TL, BTN_TR, BTN_TL2, BTN_TR2,
> > +       0 /* 0 signals end of array */
> > +};
> > +
> > +static DEFINE_MUTEX(switchcon_input_num_mutex);
> > +static int switchcon_input_create(struct switchcon_ctlr *ctlr)
> > +{
> > +       struct hid_device *hdev;
> > +       static int input_num = 1;
> > +       int ret;
> > +       int i;
> > +
> > +       hdev = ctlr->hdev;
> > +       ctlr->input = input_allocate_device();
> > +       if (!ctlr->input) {
> > +               ret = -ENOMEM;
> > +               goto err;
> > +       }
> > +       ctlr->input->dev.parent = &hdev->dev;
> > +       ctlr->input->id.bustype = hdev->bus;
> > +       ctlr->input->id.vendor = hdev->vendor;
> > +       ctlr->input->id.product = hdev->product;
> > +       ctlr->input->id.version = hdev->version;
> > +       ctlr->input->name = switchcon_input_names[ctlr->type];
> > +       input_set_drvdata(ctlr->input, ctlr);
> > +
> > +
> > +       /* set up sticks */
> > +       input_set_abs_params(ctlr->input, ABS_X,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       input_set_abs_params(ctlr->input, ABS_Y,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       input_set_abs_params(ctlr->input, ABS_RX,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       input_set_abs_params(ctlr->input, ABS_RY,
> > +                            -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
> > +                            SC_STICK_FUZZ, SC_STICK_FLAT);
> > +       /* set up buttons */
> > +       for (i = 0; switchcon_button_inputs[i] > 0; i++)
> > +               input_set_capability(ctlr->input, EV_KEY,
> > +                                    switchcon_button_inputs[i]);
> > +
> > +       ret = input_register_device(ctlr->input);
> > +       if (ret)
> > +               goto err_input;
> > +
> > +       /* Set the default controller player leds based on controller number */
> > +       mutex_lock(&switchcon_input_num_mutex);
> > +       mutex_lock(&ctlr->output_mutex);
> > +       ret = switchcon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
> > +       if (ret)
> > +               hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
> > +       mutex_unlock(&ctlr->output_mutex);
> > +       if (++input_num > 4)
> > +               input_num = 1;
> > +       mutex_unlock(&switchcon_input_num_mutex);
> > +
> > +       return 0;
> > +
> > +err_input:
> > +       input_free_device(ctlr->input);
> > +err:
> > +       return ret;
> > +}
> > +
> > +/* data input must have at least 9 bytes */
> > +static void switchcon_parse_lstick_calibration(u8 *data,
> > +                                              struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_stick_cal *cal_x = &ctlr->left_stick_cal_x;
> > +       struct switchcon_stick_cal *cal_y = &ctlr->left_stick_cal_y;
> > +       s32 x_max_above;
> > +       s32 x_min_below;
> > +       s32 y_max_above;
> > +       s32 y_min_below;
> > +
> > +       x_max_above = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
> > +       y_max_above = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
> > +       cal_x->center = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
> > +       cal_y->center = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
> > +       x_min_below = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> > +       y_min_below = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> > +       cal_x->max = cal_x->center + x_max_above;
> > +       cal_x->min = cal_x->center - x_min_below;
> > +       cal_y->max = cal_y->center + y_max_above;
> > +       cal_y->min = cal_y->center - y_min_below;
> > +}
> > +
> > +/* data input must have at least 9 bytes */
> > +static void switchcon_parse_rstick_calibration(u8 *data,
> > +                                              struct switchcon_ctlr *ctlr)
> > +{
> > +       struct switchcon_stick_cal *cal_x = &ctlr->right_stick_cal_x;
> > +       struct switchcon_stick_cal *cal_y = &ctlr->right_stick_cal_y;
> > +       s32 x_max_above;
> > +       s32 x_min_below;
> > +       s32 y_max_above;
> > +       s32 y_min_below;
> > +
> > +       cal_x->center = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
> > +       cal_y->center = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
> > +       x_min_below = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
> > +       y_min_below = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
> > +       x_max_above = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
> > +       y_max_above = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
> > +       cal_x->max = cal_x->center + x_max_above;
> > +       cal_x->min = cal_x->center - x_min_below;
> > +       cal_y->max = cal_y->center + y_max_above;
> > +       cal_y->min = cal_y->center - y_min_below;
> > +}
> > +
> > +/* Common handler for parsing inputs */
> > +static int switchcon_ctlr_read_handler(struct switchcon_ctlr *ctlr,
> > +                                               u8 *data, int size)
> > +{
> > +       int ret = 0;
> > +
> > +       switch (data[0]) {
> > +       case SC_INPUT_SUBCMD_REPLY:
> > +       case SC_INPUT_IMU_DATA:
> > +       case SC_INPUT_MCU_DATA:
> > +               if (size >= 12) /* make sure it contains the input report */
> > +                       switchcon_parse_report(ctlr, data);
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int switchcon_ctlr_handle_event(struct switchcon_ctlr *ctlr, u8 *data,
> > +                                                                   int size)
> > +{
> > +       int ret = 0;
> > +       bool match = false;
> > +       int copy_size;
> > +       struct switchcon_input_report *report;
> > +
> > +       if (unlikely(mutex_is_locked(&ctlr->output_mutex)) &&
> > +           ctlr->msg_type != SWITCHCON_MSG_TYPE_NONE) {
> > +               switch (ctlr->msg_type) {
> > +               case SWITCHCON_MSG_TYPE_USB:
> > +                       if (size < 2)
> > +                               break;
> > +                       if (data[0] == SC_INPUT_USB_RESPONSE &&
> > +                           data[1] == ctlr->usb_ack_match)
> > +                               match = true;
> > +                       break;
> > +               case SWITCHCON_MSG_TYPE_SUBCMD:
> > +                       if (size < sizeof(struct switchcon_input_report) ||
> > +                           data[0] != SC_INPUT_SUBCMD_REPLY)
> > +                               break;
> > +                       report = (struct switchcon_input_report *)data;
> > +                       if (report->reply.id == ctlr->subcmd_ack_match)
> > +                               match = true;
> > +                       break;
> > +               default:
> > +                       break;
> > +               }
> > +
> > +               if (match) {
> > +                       if (size > SC_MAX_RESP_SIZE)
> > +                               copy_size = SC_MAX_RESP_SIZE;
> > +                       else
> > +                               copy_size = size;
> > +                       memcpy(ctlr->input_buf, data, copy_size);
> > +                       ctlr->msg_type = SWITCHCON_MSG_TYPE_NONE;
> > +                       ctlr->received_resp = true;
> > +                       wake_up(&ctlr->wait);
> > +
> > +                       /* This message has been handled */
> > +                       return 1;
> > +               }
> > +       }
> > +
> > +       if (ctlr->ctlr_state == SWITCHCON_CTLR_STATE_READ)
> > +               ret = switchcon_ctlr_read_handler(ctlr, data, size);
> > +
> > +       return ret;
> > +}
> > +
> > +static int switchcon_hid_event(struct hid_device *hdev,
> > +                       struct hid_report *report, u8 *raw_data, int size)
> > +{
> > +       struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
> > +
> > +       if (size < 1)
> > +               return -EINVAL;
> > +
> > +       return switchcon_ctlr_handle_event(ctlr, raw_data, size);
> > +}
> > +
> > +static struct switchcon_ctlr *switchcon_ctlr_create(struct hid_device *hdev)
> > +{
> > +       struct switchcon_ctlr *ctlr;
> > +
> > +       ctlr = devm_kzalloc(&hdev->dev, sizeof(*ctlr), GFP_KERNEL);
> > +       if (!ctlr)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       switch (hdev->product) {
> > +       case USB_DEVICE_ID_NINTENDO_PROCON:
> > +               ctlr->type = SWITCHCON_CTLR_TYPE_PROCON;
> > +               break;
> > +       case USB_DEVICE_ID_NINTENDO_JOYCONL:
> > +               ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_L;
> > +               break;
> > +       case USB_DEVICE_ID_NINTENDO_JOYCONR:
> > +               ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_R;
> > +               break;
> > +       default:
> > +               return ERR_PTR(-EINVAL);
> > +       }
>
> There is no need to store "ctlr->type". It is checked in one or two
> places. Just use hdev in those.
>
> > +       ctlr->hdev = hdev;
> > +       ctlr->ctlr_state = SWITCHCON_CTLR_STATE_INIT;
> > +       hid_set_drvdata(hdev, ctlr);
> > +       mutex_init(&ctlr->output_mutex);
> > +       init_waitqueue_head(&ctlr->wait);
> > +       return ctlr;
> > +}
>
> With removal of the "ctrl->type" logic and some of the other
> refactoring of calibration as suggested below in "_probe", there is
> probably no good reason to keep a separate create function.
>
> > +
> > +static void switchcon_ctlr_destroy(struct switchcon_ctlr *ctlr)
> > +{
> > +       if (ctlr->input)
> > +               input_unregister_device(ctlr->input);
> > +       mutex_destroy(&ctlr->output_mutex);
> > +}
> > +
> > +static int switchcon_hid_probe(struct hid_device *hdev,
> > +                              const struct hid_device_id *id)
> > +{
> > +       int ret;
> > +       struct switchcon_ctlr *ctlr;
> > +       struct switchcon_input_report *report;
> > +       u8 *raw_cal;
> > +
> > +       hid_dbg(hdev, "probe - start\n");
> > +
> > +       ctlr = switchcon_ctlr_create(hdev);
> > +       if (IS_ERR(ctlr)) {
> > +               hid_err(hdev, "Failed to create new controller\n");
> > +               ret = PTR_ERR(ctlr);
> > +               goto err;
> > +       }
> > +
> > +       ret = hid_parse(hdev);
> > +       if (ret) {
> > +               hid_err(hdev, "HID parse failed\n");
> > +               goto err;
> > +       }
> > +
> > +       ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > +       if (ret) {
> > +               hid_err(hdev, "HW start failed\n");
> > +               goto err;
> > +       }
> > +
> > +       ret = hid_hw_open(hdev);
> > +       if (ret) {
> > +               hid_err(hdev, "cannot start hardware I/O\n");
> > +               goto err_stop;
> > +       }
> > +
> > +       hid_device_io_start(hdev);
> > +
> > +       /* Initialize the controller */
> > +       mutex_lock(&ctlr->output_mutex);
> > +       /* if baudrate command fails, assume ble pro controller */
> > +       if (ctlr->type == SWITCHCON_CTLR_TYPE_PROCON &&
> > +           !switchcon_send_usb(ctlr, SC_USB_CMD_BAUDRATE_3M)) {
> > +               /* handshake */
> > +               ret = switchcon_send_usb(ctlr, SC_USB_CMD_HANDSHAKE);
> > +               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.
> > +                */
> > +               switchcon_send_usb(ctlr, SC_USB_CMD_NO_TIMEOUT);
> > +       }
> > +
> > +       /* get controller calibration data, and parse it */
> > +       ret = switchcon_request_calibration(ctlr);
> > +       if (ret) {
> > +               hid_err(hdev, "Failed to retrieve calibration; ret=%d\n", ret);
> > +               goto err_mutex;
> > +       }
> > +       report = (struct switchcon_input_report *)ctlr->input_buf;
> > +       raw_cal = SC_SUBCMD_REPLY_GET_DATA(&report->reply) + 5;
> > +       switchcon_parse_lstick_calibration(raw_cal, ctlr);
> > +       switchcon_parse_rstick_calibration(raw_cal + 9, ctlr);
>
> Move the lstick/rstick code into the calibration function. I think
> there is also little reason to keep the lstick/rstick calibration
> helper functions. They are only used once, so make a slightly bigger
> overall calibration function hiding all this detail.
>
> > +       hid_info(ctlr->hdev, "calibration:\n"
> > +                            "l_x_c=%d l_x_max=%d l_x_min=%d\n"
> > +                            "l_y_c=%d l_y_max=%d l_y_min=%d\n"
> > +                            "r_x_c=%d r_x_max=%d r_x_min=%d\n"
> > +                            "r_y_c=%d r_y_max=%d r_y_min=%d\n",
> > +                            ctlr->left_stick_cal_x.center,
> > +                            ctlr->left_stick_cal_x.max,
> > +                            ctlr->left_stick_cal_x.min,
> > +                            ctlr->left_stick_cal_y.center,
> > +                            ctlr->left_stick_cal_y.max,
> > +                            ctlr->left_stick_cal_y.min,
> > +                            ctlr->right_stick_cal_x.center,
> > +                            ctlr->right_stick_cal_x.max,
> > +                            ctlr->right_stick_cal_x.min,
> > +                            ctlr->right_stick_cal_y.center,
> > +                            ctlr->right_stick_cal_y.max,
> > +                            ctlr->right_stick_cal_y.min);
>
> I would move more of this logic into your calibration function. Not
> sure how much sense it makes to print the calibrarion data. Probably
> print it using hid_dbg, so it is only printed on debug builds.
>
> > +
> > +       /* Set the reporting mode to 0x30, which is the full report mode */
> > +       ret = switchcon_set_report_mode(ctlr);
> > +       if (ret) {
> > +               hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> > +               goto err_mutex;
> > +       }
> > +
> > +       mutex_unlock(&ctlr->output_mutex);
> > +
> > +       ret = switchcon_input_create(ctlr);
> > +       if (ret) {
> > +               hid_err(hdev, "Failed to create input device; ret=%d\n", ret);
> > +               goto err_close;
> > +       }
> > +
> > +       ctlr->ctlr_state = SWITCHCON_CTLR_STATE_READ;
> > +
> > +       hid_dbg(hdev, "probe - success\n");
> > +       return 0;
> > +
> > +err_mutex:
> > +       mutex_unlock(&ctlr->output_mutex);
> > +err_close:
> > +       switchcon_ctlr_destroy(ctlr);
> > +       hid_hw_close(hdev);
> > +err_stop:
> > +       hid_hw_stop(hdev);
> > +err:
> > +       hid_err(hdev, "probe - fail = %d\n", ret);
> > +       return ret;
> > +}
> > +
> > +static void switchcon_hid_remove(struct hid_device *hdev)
> > +{
> > +       struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
> > +
> > +       hid_dbg(hdev, "remove\n");
> > +       hid_hw_close(hdev);
> > +       hid_hw_stop(hdev);
> > +       switchcon_ctlr_destroy(ctlr);
> > +}
> > +
> > +static const struct hid_device_id switchcon_hid_devices[] = {
> > +       { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_PROCON) },
> > +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_PROCON) },
> > +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_JOYCONL) },
> > +       { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> > +                        USB_DEVICE_ID_NINTENDO_JOYCONR) },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(hid, switchcon_hid_devices);
> > +
> > +static struct hid_driver switchcon_hid_driver = {
> > +       .name           = "switchcon",
> > +       .id_table       = switchcon_hid_devices,
> > +       .probe          = switchcon_hid_probe,
> > +       .remove         = switchcon_hid_remove,
> > +       .raw_event      = switchcon_hid_event,
> > +};
> > +module_hid_driver(switchcon_hid_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
> > +MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
> > --
> > 2.20.1
> >
Roderick Colenbrander Jan. 28, 2019, 6:42 p.m. UTC | #5
On Mon, Jan 28, 2019 at 8:28 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi Roderick,
>
> On Mon, Jan 28, 2019 at 6:11 AM Roderick Colenbrander
> <thunderbird2k@gmail.com> wrote:
> >
> > Hi Daniel,
> >
> > Thanks for updating the patch series. It is definitely improved and
> > easier to read. See some of my comments below.
> >
> > One other thing I noticed, see the comment in v2 review about breaking
> > userspace. I think since you change the button / stick mapping
> > compared to what SDL2 supports for Pro controller. You need to do
> > something e.g. different version like hid-sony does.
> >
> > Out of curiosity did you test the driver on Bluetooth? The reason I'm
> > asking is, because I suspect you would have hit a deadlock issue,
> > which we hit in hid-sony before. I'm not sure how you got around that
> > one..
> >
> > Let me explain the issue. When using Bluetooth, many Bluetooth stacks
> > rely on uhid (either Bluez on desktop and even Android's one). I
> > recall when the BT stack requests uhid to "create" an input device,
> > this ultimately triggers "_probe" in your driver. hid-sony at the time
> > and similar for hid-switchcon triggers various requests to the
> > hardware during probe. However uhid is single threaded and it is
> > holding a lock. This caused a deadlock and caused hardware request
> > timeouts. I don't see how this is not happening for you.
>
> Actually, I think you are missing `hid_device_io_start(hdev);` in hid-sony.c
>
> What this driver does and which is race free (in pseudo code):
> probe() {
>     hid_parse();
>     hid_hw_start(CONNECT_HIDRAW);
>     hid_hw_open(hdev);
>     hid_device_io_start(hdev);
>     // now you can do whatever IO you want, .raw_event() will also be called
>     hid_hw_close(hdev);
>     switchcon_input_create(ctlr);
> }
>
> If you want to rely on HIDINPUT to create the inputs for you and not
> prematurely expose a hidraw node, then you can first hid_hwstart with
> CONNECT_DRIVER, do your init, then stop the hid device and finally
> start with CONNECT_DEFAULT.
>
> Cheers,
> Benjamin

I dug through my old notes and commit logs again. There were 2
unrelated issues regarding similar code, which had blurred together.
The "uhid" issue I mentioned was indeed resolved by the fix I added at
the time and I/O start related calls helped there. hid-sony probably
could use a hid_device_io_start as you pointed out.

The second issue which there was, was the one where in which
"hid_hw_start" was taking care of input node creation on
CONNECT_DEFAULT. At the time "sony_probe" was doing some remaining
work (creating sysfs options, adding FF..) after hid_hw_start, so this
caused a race condition. This is why we added "sony_input_configured"
as it is called during hid_hw_start. It all starts making sense again.

On a related node for switchcon, what is preferred for device node
creation? Right now switchcon does its own input_device_create. Other
drivers often override the HID mapping through hid callbacks if they
don't like the default mappings. I'm not sure what other benefits
there would be from using functionality from the linux hid layer.

Thanks,
Roderick
Daniel Ogorchock Jan. 29, 2019, 1:42 a.m. UTC | #6
On Mon, Jan 28, 2019 at 7:16 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Sun, 2019-01-27 at 22:04 -0600, Daniel J. Ogorchock wrote:
> > The switchcon driver supports the Nintendo Switch Pro Controllers and
> > the Joy-Cons. The Pro Controllers can be used over USB or Bluetooth.
> >
> > The Joy-Cons each create their own, independent input devices, so it
> > is
> > up to userspace to combine them if desired.
> >
> <snip>
> > +static void switchcon_parse_report(struct switchcon_ctlr *ctlr, u8
> > *data)
> > +{
> > +     struct input_dev *dev = ctlr->input;
> > +     enum switchcon_ctlr_type type = ctlr->type;
> > +     u32 btns;
> > +
> > +     btns = hid_field_extract(ctlr->hdev, data + 3, 0, 24);
> > +
> > +     if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> > +         type == SWITCHCON_CTLR_TYPE_JOYCON_L) {
>
> <snip>
> > +     }
> > +     if (type == SWITCHCON_CTLR_TYPE_PROCON ||
> > +         type == SWITCHCON_CTLR_TYPE_JOYCON_R) {
>
> Did I read this code correctly, in that the 2 joy-cons will each report
> their status as if they were independent?
>
> I know that the Switch has a settings panel and probably some platform-
> level code to deal with multiple sets of joycons being available, such
> a pair attached to the body of the device and another pair used
> individually by 2 players.
>
> The "these 2 discrete devices are actually one" case could be handled
> by a library that Peter is working on.
>

Yes. The v1 patch had the joy-con combination as well as the
horizontal orientation mode built into the driver. It made the driver
needlessly convoluted though, and it was decided that that
functionality belongs in userspace.

> However, have you tested the joy-cons with a "charging grip"? Does the
> power_supply reporting work for the joy-cons when charging via this
> method? Do the joy-cons change behaviour, or work differently when
> plugged in to the charging grip? Or do the joy-cons not have a "wired"
> mode at all, and all the data is always sent via Bluetooth.
>

I actually don't have a charging grip to test with. I believe the
Joy-Cons still operate independently via bluetooth in the grip. The
controllers use the same protocol to report charging/battery/powered
status regardless of controller type, so I believe the battery parsing
code will continue working with the grip. If anyone reading this
happens to have a charging grip and can test it, that would be much
appreciated!

> > +             if (match) {
> > +                     if (size > SC_MAX_RESP_SIZE)
> > +                             copy_size = SC_MAX_RESP_SIZE;
> > +                     else
> > +                             copy_size = size;
>
> Use MIN()?
>
> Cheers
>

Thanks,
Daniel

Patch
diff mbox series

diff --git a/MAINTAINERS b/MAINTAINERS
index f3a5c97e3419..001817e2e08a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14397,6 +14397,12 @@  F:	kernel/dma/swiotlb.c
 F:	arch/*/kernel/pci-swiotlb.c
 F:	include/linux/swiotlb.h
 
+SWITCHCON HID DRIVER
+M:	Daniel J. Ogorchock <djogorchock@gmail.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	drivers/hid/hid-switchcon*
+
 SWITCHDEV
 M:	Jiri Pirko <jiri@resnulli.us>
 M:	Ivan Vecera <ivecera@redhat.com>
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 41e9935fc584..9702c07802a5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -943,6 +943,17 @@  config SMARTJOYPLUS_FF
 	Say Y here if you have a SmartJoy PLUS PS2/USB adapter and want to
 	enable force feedback support for it.
 
+config HID_SWITCHCON
+	tristate "Nintendo Joy-Con and Pro Controller support"
+	depends on HID
+	help
+	Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
+	All controllers support bluetooth, and the Pro Controller also supports
+	its USB mode.
+
+	To compile this driver as a module, choose M here: the
+	module will be called hid-switchcon.
+
 config HID_TIVO
 	tristate "TiVo Slide Bluetooth remote control support"
 	depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 896a51ce7ce0..e708e682c4b8 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -103,6 +103,7 @@  obj-$(CONFIG_HID_SPEEDLINK)	+= hid-speedlink.o
 obj-$(CONFIG_HID_STEAM)		+= hid-steam.o
 obj-$(CONFIG_HID_STEELSERIES)	+= hid-steelseries.o
 obj-$(CONFIG_HID_SUNPLUS)	+= hid-sunplus.o
+obj-$(CONFIG_HID_SWITCHCON)	+= hid-switchcon.o
 obj-$(CONFIG_HID_GREENASIA)	+= hid-gaff.o
 obj-$(CONFIG_HID_THRUSTMASTER)	+= hid-tmff.o
 obj-$(CONFIG_HID_TIVO)		+= hid-tivo.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 27519eb8ee63..7924f7d502a8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -846,6 +846,9 @@ 
 #define USB_VENDOR_ID_NINTENDO		0x057e
 #define USB_DEVICE_ID_NINTENDO_WIIMOTE	0x0306
 #define USB_DEVICE_ID_NINTENDO_WIIMOTE2	0x0330
+#define USB_DEVICE_ID_NINTENDO_JOYCONL	0x2006
+#define USB_DEVICE_ID_NINTENDO_JOYCONR  0x2007
+#define USB_DEVICE_ID_NINTENDO_PROCON   0x2009
 
 #define USB_VENDOR_ID_NOVATEK		0x0603
 #define USB_DEVICE_ID_NOVATEK_PCT	0x0600
diff --git a/drivers/hid/hid-switchcon.c b/drivers/hid/hid-switchcon.c
new file mode 100644
index 000000000000..f70b7cf95021
--- /dev/null
+++ b/drivers/hid/hid-switchcon.c
@@ -0,0 +1,827 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
+ *
+ * Copyright (c) 2019 Daniel J. Ogorchock <djogorchock@gmail.com>
+ *
+ * The following resources/projects were referenced for this driver:
+ *   https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
+ *   https://gitlab.com/pjranki/joycon-linux-kernel (Peter Rankin)
+ *   https://github.com/FrotBot/SwitchProConLinuxUSB
+ *   https://github.com/MTCKC/ProconXInput
+ *   hid-wiimote kernel hid driver
+ *   hid-logitech-hidpp driver
+ *
+ * This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
+ * Pro Controllers can either be used over USB or Bluetooth.
+ *
+ * The driver will retrieve the factory calibration info from the controllers,
+ * so little to no user calibration should be required.
+ *
+ */
+
+#include "hid-ids.h"
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+
+/*
+ * Reference the url below for the following HID report defines:
+ * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
+ */
+
+/* Output Reports */
+#define SC_OUTPUT_RUMBLE_AND_SUBCMD	0x01
+#define SC_OUTPUT_FW_UPDATE_PKT		0x03
+#define SC_OUTPUT_RUMBLE_ONLY		0x10
+#define SC_OUTPUT_MCU_DATA		0x11
+#define SC_OUTPUT_USB_CMD		0x80
+
+/* Subcommand IDs */
+#define SC_SUBCMD_STATE			0x00
+#define SC_SUBCMD_MANUAL_BT_PAIRING	0x01
+#define SC_SUBCMD_REQ_DEV_INFO		0x02
+#define SC_SUBCMD_SET_REPORT_MODE	0x03
+#define SC_SUBCMD_TRIGGERS_ELAPSED	0x04
+#define SC_SUBCMD_GET_PAGE_LIST_STATE	0x05
+#define SC_SUBCMD_SET_HCI_STATE		0x06
+#define SC_SUBCMD_RESET_PAIRING_INFO	0x07
+#define SC_SUBCMD_LOW_POWER_MODE	0x08
+#define SC_SUBCMD_SPI_FLASH_READ	0x10
+#define SC_SUBCMD_SPI_FLASH_WRITE	0x11
+#define SC_SUBCMD_RESET_MCU		0x20
+#define SC_SUBCMD_SET_MCU_CONFIG	0x21
+#define SC_SUBCMD_SET_MCU_STATE		0x22
+#define SC_SUBCMD_SET_PLAYER_LIGHTS	0x30
+#define SC_SUBCMD_GET_PLAYER_LIGHTS	0x31
+#define SC_SUBCMD_SET_HOME_LIGHT	0x38
+#define SC_SUBCMD_ENABLE_IMU		0x40
+#define SC_SUBCMD_SET_IMU_SENSITIVITY	0x41
+#define SC_SUBCMD_WRITE_IMU_REG		0x42
+#define SC_SUBCMD_READ_IMU_REG		0x43
+#define SC_SUBCMD_ENABLE_VIBRATION	0x48
+#define SC_SUBCMD_GET_REGULATED_VOLTAGE	0x50
+
+/* Input Reports */
+#define SC_INPUT_BUTTON_EVENT		0x3F
+#define SC_INPUT_SUBCMD_REPLY		0x21
+#define SC_INPUT_IMU_DATA		0x30
+#define SC_INPUT_MCU_DATA		0x31
+#define SC_INPUT_USB_RESPONSE		0x81
+
+/* Feature Reports */
+#define SC_FEATURE_LAST_SUBCMD		0x02
+#define SC_FEATURE_OTA_FW_UPGRADE	0x70
+#define SC_FEATURE_SETUP_MEM_READ	0x71
+#define SC_FEATURE_MEM_READ		0x72
+#define SC_FEATURE_ERASE_MEM_SECTOR	0x73
+#define SC_FEATURE_MEM_WRITE		0x74
+#define SC_FEATURE_LAUNCH		0x75
+
+/* USB Commands */
+#define SC_USB_CMD_CONN_STATUS		0x01
+#define SC_USB_CMD_HANDSHAKE		0x02
+#define SC_USB_CMD_BAUDRATE_3M		0x03
+#define SC_USB_CMD_NO_TIMEOUT		0x04
+#define SC_USB_CMD_EN_TIMEOUT		0x05
+#define SC_USB_RESET			0x06
+#define SC_USB_PRE_HANDSHAKE		0x91
+#define SC_USB_SEND_UART		0x92
+
+/* SPI storage addresses of factory calibration data */
+#define SC_CAL_DATA_START		0x603d
+#define SC_CAL_DATA_END			0x604e
+#define SC_CAL_DATA_SIZE	(SC_CAL_DATA_END - SC_CAL_DATA_START + 1)
+
+
+/* The raw analog joystick values will be mapped in terms of this magnitude */
+#define SC_MAX_STICK_MAG	32767
+#define SC_STICK_FUZZ		250
+#define SC_STICK_FLAT		500
+
+/* States for controller state machine */
+enum switchcon_ctlr_state {
+	SWITCHCON_CTLR_STATE_INIT,
+	SWITCHCON_CTLR_STATE_READ,
+};
+
+struct switchcon_stick_cal {
+	s32 max;
+	s32 min;
+	s32 center;
+};
+
+/*
+ * All the controller's button values are stored in a u32.
+ * They can be accessed with bitwise ANDs.
+ */
+#define SC_BTN_Y	BIT(0)
+#define SC_BTN_X	BIT(1)
+#define SC_BTN_B	BIT(2)
+#define SC_BTN_A	BIT(3)
+#define SC_BTN_SR_R	BIT(4)
+#define SC_BTN_SL_R	BIT(5)
+#define SC_BTN_R	BIT(6)
+#define SC_BTN_ZR	BIT(7)
+#define SC_BTN_MINUS	BIT(8)
+#define SC_BTN_PLUS	BIT(9)
+#define SC_BTN_RSTICK	BIT(10)
+#define SC_BTN_LSTICK	BIT(11)
+#define SC_BTN_HOME	BIT(12)
+#define SC_BTN_CAP	BIT(13) /* capture button */
+#define SC_BTN_DOWN	BIT(16)
+#define SC_BTN_UP	BIT(17)
+#define SC_BTN_RIGHT	BIT(18)
+#define SC_BTN_LEFT	BIT(19)
+#define SC_BTN_SR_L	BIT(20)
+#define SC_BTN_SL_L	BIT(21)
+#define SC_BTN_L	BIT(22)
+#define SC_BTN_ZL	BIT(23)
+
+enum switchcon_ctlr_type {
+	SWITCHCON_CTLR_TYPE_PROCON,
+	SWITCHCON_CTLR_TYPE_JOYCON_L,
+	SWITCHCON_CTLR_TYPE_JOYCON_R,
+};
+
+static const char * const switchcon_input_names[] = {
+	"Nintendo Switch Pro Controller",
+	"Nintendo Switch Left Joy-Con",
+	"Nintendo Switch Right Joy-Con",
+};
+
+enum switchcon_msg_type {
+	SWITCHCON_MSG_TYPE_NONE,
+	SWITCHCON_MSG_TYPE_USB,
+	SWITCHCON_MSG_TYPE_SUBCMD,
+};
+
+struct switchcon_subcmd_request {
+	u8 output_id; /* must be 0x01 for subcommand, 0x10 for rumble only */
+	u8 packet_num; /* incremented every send */
+	u8 rumble_data[8];
+	u8 subcmd_id;
+	/* data is here */
+} __packed;
+
+/* should pass in pointer to a struct switchcon_subcmd_request */
+#define SC_SUBCMD_REQ_GET_DATA(req) \
+	((u8 *)(req) + sizeof(struct switchcon_subcmd_request))
+
+struct switchcon_subcmd_reply {
+	u8 ack; /* MSB 1 for ACK, 0 for NACK */
+	u8 id; /* id of requested subcmd */
+	/* data is here, can be up to 35 bytes */
+} __packed;
+
+/* should pass in pointer to a struct switchcon_subcmd_reply */
+#define SC_SUBCMD_REPLY_GET_DATA(reply) \
+	((u8 *)(reply) + sizeof(struct switchcon_subcmd_reply))
+
+struct switchcon_input_report {
+	u8 id;
+	u8 timer;
+	u8 bat_con; /* battery and connection info */
+	u8 button_status[3];
+	u8 left_stick[3];
+	u8 right_stick[3];
+	u8 vibrator_report;
+
+	/*
+	 * If support for firmware updates, gyroscope data, and/or NFC/IR
+	 * are added in the future, this can be swapped for a union.
+	 */
+	struct switchcon_subcmd_reply reply;
+} __packed;
+
+#define SC_MAX_RESP_SIZE (sizeof(struct switchcon_input_report) + 35)
+
+/* Each physical controller is associated with a switchcon_ctlr struct */
+struct switchcon_ctlr {
+	struct hid_device *hdev;
+	struct input_dev *input;
+	enum switchcon_ctlr_type type;
+	enum switchcon_ctlr_state ctlr_state;
+
+	/* The following members are used for synchronous sends/receives */
+	enum switchcon_msg_type msg_type;
+	u8 subcmd_num;
+	struct mutex output_mutex;
+	u8 input_buf[SC_MAX_RESP_SIZE];
+	wait_queue_head_t wait;
+	bool received_resp;
+	u8 usb_ack_match;
+	u8 subcmd_ack_match;
+
+	/* factory calibration data */
+	struct switchcon_stick_cal left_stick_cal_x;
+	struct switchcon_stick_cal left_stick_cal_y;
+	struct switchcon_stick_cal right_stick_cal_x;
+	struct switchcon_stick_cal right_stick_cal_y;
+
+};
+
+static int __switchcon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
+{
+	u8 *buf;
+	int ret;
+
+	buf = kmemdup(data, len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+	ret = hid_hw_output_report(hdev, buf, len);
+	kfree(buf);
+	if (ret < 0)
+		hid_dbg(hdev, "Failed to send output report ret=%d\n", ret);
+	return ret;
+}
+
+static int switchcon_hid_send_sync(struct switchcon_ctlr *ctlr, u8 *data,
+								size_t len)
+{
+	int ret;
+
+	ret = __switchcon_hid_send(ctlr->hdev, data, len);
+	if (ret < 0) {
+		memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
+		return ret;
+	}
+
+	if (!wait_event_timeout(ctlr->wait, ctlr->received_resp, HZ)) {
+		hid_dbg(ctlr->hdev, "syncronous send/receive timed out\n");
+		memset(ctlr->input_buf, 0, SC_MAX_RESP_SIZE);
+		return -ETIMEDOUT;
+	}
+
+	ctlr->received_resp = false;
+	return 0;
+}
+
+static int switchcon_send_usb(struct switchcon_ctlr *ctlr, u8 cmd)
+{
+	int ret;
+	u8 buf[2] = {SC_OUTPUT_USB_CMD};
+
+	buf[1] = cmd;
+	ctlr->usb_ack_match = cmd;
+	ctlr->msg_type = SWITCHCON_MSG_TYPE_USB;
+	ret = switchcon_hid_send_sync(ctlr, buf, sizeof(buf));
+	if (ret)
+		hid_dbg(ctlr->hdev, "send usb command failed; ret=%d\n", ret);
+	return ret;
+}
+
+static int switchcon_send_subcmd(struct switchcon_ctlr *ctlr,
+				 struct switchcon_subcmd_request *subcmd,
+				 size_t data_len)
+{
+	int ret;
+
+	subcmd->output_id = SC_OUTPUT_RUMBLE_AND_SUBCMD;
+	subcmd->packet_num = ctlr->subcmd_num;
+	if (++ctlr->subcmd_num > 0xF)
+		ctlr->subcmd_num = 0;
+	ctlr->subcmd_ack_match = subcmd->subcmd_id;
+	ctlr->msg_type = SWITCHCON_MSG_TYPE_SUBCMD;
+
+	ret = switchcon_hid_send_sync(ctlr, (u8 *)subcmd,
+				      sizeof(*subcmd) + data_len);
+	if (ret)
+		hid_dbg(ctlr->hdev, "send subcommand failed; ret=%d\n", ret);
+	return ret;
+}
+
+/* Supply nibbles for flash and on. Ones correspond to active */
+static int switchcon_set_player_leds(struct switchcon_ctlr *ctlr,
+						u8 flash, u8 on)
+{
+	struct switchcon_subcmd_request *req;
+	u8 buffer[sizeof(*req) + 1] = { 0 };
+
+	req = (struct switchcon_subcmd_request *)buffer;
+	req->subcmd_id = SC_SUBCMD_SET_PLAYER_LIGHTS;
+	SC_SUBCMD_REQ_GET_DATA(req)[0] = (flash << 4) | on;
+
+	hid_dbg(ctlr->hdev, "setting player leds\n");
+	return switchcon_send_subcmd(ctlr, req, 1);
+}
+
+static int switchcon_request_calibration(struct switchcon_ctlr *ctlr)
+{
+	struct switchcon_subcmd_request *req;
+	u8 buffer[sizeof(*req) + 5] = { 0 };
+	u8 *data;
+
+	req = (struct switchcon_subcmd_request *)buffer;
+	req->subcmd_id = SC_SUBCMD_SPI_FLASH_READ;
+	data = SC_SUBCMD_REQ_GET_DATA(req);
+	data[0] = 0xFF & SC_CAL_DATA_START;
+	data[1] = 0xFF & (SC_CAL_DATA_START >> 8);
+	data[2] = 0xFF & (SC_CAL_DATA_START >> 16);
+	data[3] = 0xFF & (SC_CAL_DATA_START >> 24);
+	data[4] = SC_CAL_DATA_SIZE;
+
+	hid_dbg(ctlr->hdev, "requesting cal data\n");
+	return switchcon_send_subcmd(ctlr, req, 5);
+}
+
+static int switchcon_set_report_mode(struct switchcon_ctlr *ctlr)
+{
+	struct switchcon_subcmd_request *req;
+	u8 buffer[sizeof(*req) + 1] = { 0 };
+
+	req = (struct switchcon_subcmd_request *)buffer;
+	req->subcmd_id = SC_SUBCMD_SET_REPORT_MODE;
+	SC_SUBCMD_REQ_GET_DATA(req)[0] = 0x30; /* standard, full report mode */
+
+	hid_dbg(ctlr->hdev, "setting controller report mode\n");
+	return switchcon_send_subcmd(ctlr, req, 1);
+}
+
+static int switchcon_map_stick_val(struct switchcon_stick_cal *cal, s32 val)
+{
+	s32 center = cal->center;
+	s32 min = cal->min;
+	s32 max = cal->max;
+	int new_val;
+
+	if (val > center) {
+		new_val = (val - center) * SC_MAX_STICK_MAG;
+		new_val /= (max - center);
+	} else {
+		new_val = (center - val) * -SC_MAX_STICK_MAG;
+		new_val /= (center - min);
+	}
+	new_val = clamp(new_val, -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG);
+	return new_val;
+}
+
+static void switchcon_parse_report(struct switchcon_ctlr *ctlr, u8 *data)
+{
+	struct input_dev *dev = ctlr->input;
+	enum switchcon_ctlr_type type = ctlr->type;
+	u32 btns;
+
+	btns = hid_field_extract(ctlr->hdev, data + 3, 0, 24);
+
+	if (type == SWITCHCON_CTLR_TYPE_PROCON ||
+	    type == SWITCHCON_CTLR_TYPE_JOYCON_L) {
+		u16 raw_l_x;
+		u16 raw_l_y;
+		int s_l_x;
+		int s_l_y;
+
+		/* get raw stick values */
+		raw_l_x = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
+		raw_l_y = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
+		/* map the stick values */
+		s_l_x = switchcon_map_stick_val(&ctlr->left_stick_cal_x,
+						raw_l_x);
+		s_l_y = -switchcon_map_stick_val(&ctlr->left_stick_cal_y,
+						raw_l_y);
+		/* report sticks */
+		input_report_abs(dev, ABS_X, s_l_x);
+		input_report_abs(dev, ABS_Y, s_l_y);
+
+		/* report buttons */
+		input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
+					      btns & SC_BTN_SL_L);
+		input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
+					      btns & SC_BTN_SR_L);
+		input_report_key(dev, BTN_TL2, btns & SC_BTN_ZL);
+		input_report_key(dev, BTN_SELECT, btns & SC_BTN_MINUS);
+		input_report_key(dev, BTN_THUMBL, btns & SC_BTN_LSTICK);
+		input_report_key(dev, BTN_Z, btns & SC_BTN_CAP);
+		input_report_key(dev, BTN_DPAD_DOWN, btns & SC_BTN_DOWN);
+		input_report_key(dev, BTN_DPAD_UP, btns & SC_BTN_UP);
+		input_report_key(dev, BTN_DPAD_RIGHT, btns & SC_BTN_RIGHT);
+		input_report_key(dev, BTN_DPAD_LEFT, btns & SC_BTN_LEFT);
+	}
+	if (type == SWITCHCON_CTLR_TYPE_PROCON ||
+	    type == SWITCHCON_CTLR_TYPE_JOYCON_R) {
+		u16 raw_r_x;
+		u16 raw_r_y;
+		int s_r_x;
+		int s_r_y;
+
+		/* get raw stick values */
+		raw_r_x = hid_field_extract(ctlr->hdev, (data + 9), 0, 12);
+		raw_r_y = hid_field_extract(ctlr->hdev, (data + 10), 4, 12);
+		/* map stick values */
+		s_r_x = switchcon_map_stick_val(&ctlr->right_stick_cal_x,
+						raw_r_x);
+		s_r_y = -switchcon_map_stick_val(&ctlr->right_stick_cal_y,
+						raw_r_y);
+		/* report sticks */
+		input_report_abs(dev, ABS_RX, s_r_x);
+		input_report_abs(dev, ABS_RY, s_r_y);
+
+		/* report buttons */
+		input_report_key(dev, BTN_TL, btns & SC_BTN_L ||
+					      btns & SC_BTN_SL_R);
+		input_report_key(dev, BTN_TR, btns & SC_BTN_R ||
+					      btns & SC_BTN_SR_R);
+		input_report_key(dev, BTN_TR2, btns & SC_BTN_ZR);
+		input_report_key(dev, BTN_START, btns & SC_BTN_PLUS);
+		input_report_key(dev, BTN_THUMBR, btns & SC_BTN_RSTICK);
+		input_report_key(dev, BTN_MODE, btns & SC_BTN_HOME);
+		input_report_key(dev, BTN_WEST, btns & SC_BTN_Y);
+		input_report_key(dev, BTN_NORTH, btns & SC_BTN_X);
+		input_report_key(dev, BTN_EAST, btns & SC_BTN_A);
+		input_report_key(dev, BTN_SOUTH, btns & SC_BTN_B);
+	}
+
+	input_sync(dev);
+}
+
+
+static const unsigned int switchcon_button_inputs[] = {
+	BTN_SELECT, BTN_Z, BTN_THUMBL,
+	BTN_START, BTN_MODE, BTN_THUMBR,
+	BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
+	BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
+	BTN_TL, BTN_TR, BTN_TL2, BTN_TR2,
+	0 /* 0 signals end of array */
+};
+
+static DEFINE_MUTEX(switchcon_input_num_mutex);
+static int switchcon_input_create(struct switchcon_ctlr *ctlr)
+{
+	struct hid_device *hdev;
+	static int input_num = 1;
+	int ret;
+	int i;
+
+	hdev = ctlr->hdev;
+	ctlr->input = input_allocate_device();
+	if (!ctlr->input) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	ctlr->input->dev.parent = &hdev->dev;
+	ctlr->input->id.bustype = hdev->bus;
+	ctlr->input->id.vendor = hdev->vendor;
+	ctlr->input->id.product = hdev->product;
+	ctlr->input->id.version = hdev->version;
+	ctlr->input->name = switchcon_input_names[ctlr->type];
+	input_set_drvdata(ctlr->input, ctlr);
+
+
+	/* set up sticks */
+	input_set_abs_params(ctlr->input, ABS_X,
+			     -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
+			     SC_STICK_FUZZ, SC_STICK_FLAT);
+	input_set_abs_params(ctlr->input, ABS_Y,
+			     -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
+			     SC_STICK_FUZZ, SC_STICK_FLAT);
+	input_set_abs_params(ctlr->input, ABS_RX,
+			     -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
+			     SC_STICK_FUZZ, SC_STICK_FLAT);
+	input_set_abs_params(ctlr->input, ABS_RY,
+			     -SC_MAX_STICK_MAG, SC_MAX_STICK_MAG,
+			     SC_STICK_FUZZ, SC_STICK_FLAT);
+	/* set up buttons */
+	for (i = 0; switchcon_button_inputs[i] > 0; i++)
+		input_set_capability(ctlr->input, EV_KEY,
+				     switchcon_button_inputs[i]);
+
+	ret = input_register_device(ctlr->input);
+	if (ret)
+		goto err_input;
+
+	/* Set the default controller player leds based on controller number */
+	mutex_lock(&switchcon_input_num_mutex);
+	mutex_lock(&ctlr->output_mutex);
+	ret = switchcon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num));
+	if (ret)
+		hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret);
+	mutex_unlock(&ctlr->output_mutex);
+	if (++input_num > 4)
+		input_num = 1;
+	mutex_unlock(&switchcon_input_num_mutex);
+
+	return 0;
+
+err_input:
+	input_free_device(ctlr->input);
+err:
+	return ret;
+}
+
+/* data input must have at least 9 bytes */
+static void switchcon_parse_lstick_calibration(u8 *data,
+					       struct switchcon_ctlr *ctlr)
+{
+	struct switchcon_stick_cal *cal_x = &ctlr->left_stick_cal_x;
+	struct switchcon_stick_cal *cal_y = &ctlr->left_stick_cal_y;
+	s32 x_max_above;
+	s32 x_min_below;
+	s32 y_max_above;
+	s32 y_min_below;
+
+	x_max_above = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
+	y_max_above = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
+	cal_x->center = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
+	cal_y->center = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
+	x_min_below = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
+	y_min_below = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
+	cal_x->max = cal_x->center + x_max_above;
+	cal_x->min = cal_x->center - x_min_below;
+	cal_y->max = cal_y->center + y_max_above;
+	cal_y->min = cal_y->center - y_min_below;
+}
+
+/* data input must have at least 9 bytes */
+static void switchcon_parse_rstick_calibration(u8 *data,
+					       struct switchcon_ctlr *ctlr)
+{
+	struct switchcon_stick_cal *cal_x = &ctlr->right_stick_cal_x;
+	struct switchcon_stick_cal *cal_y = &ctlr->right_stick_cal_y;
+	s32 x_max_above;
+	s32 x_min_below;
+	s32 y_max_above;
+	s32 y_min_below;
+
+	cal_x->center = hid_field_extract(ctlr->hdev, (data + 0), 0, 12);
+	cal_y->center = hid_field_extract(ctlr->hdev, (data + 1), 4, 12);
+	x_min_below = hid_field_extract(ctlr->hdev, (data + 3), 0, 12);
+	y_min_below = hid_field_extract(ctlr->hdev, (data + 4), 4, 12);
+	x_max_above = hid_field_extract(ctlr->hdev, (data + 6), 0, 12);
+	y_max_above = hid_field_extract(ctlr->hdev, (data + 7), 4, 12);
+	cal_x->max = cal_x->center + x_max_above;
+	cal_x->min = cal_x->center - x_min_below;
+	cal_y->max = cal_y->center + y_max_above;
+	cal_y->min = cal_y->center - y_min_below;
+}
+
+/* Common handler for parsing inputs */
+static int switchcon_ctlr_read_handler(struct switchcon_ctlr *ctlr,
+						u8 *data, int size)
+{
+	int ret = 0;
+
+	switch (data[0]) {
+	case SC_INPUT_SUBCMD_REPLY:
+	case SC_INPUT_IMU_DATA:
+	case SC_INPUT_MCU_DATA:
+		if (size >= 12) /* make sure it contains the input report */
+			switchcon_parse_report(ctlr, data);
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static int switchcon_ctlr_handle_event(struct switchcon_ctlr *ctlr, u8 *data,
+								    int size)
+{
+	int ret = 0;
+	bool match = false;
+	int copy_size;
+	struct switchcon_input_report *report;
+
+	if (unlikely(mutex_is_locked(&ctlr->output_mutex)) &&
+	    ctlr->msg_type != SWITCHCON_MSG_TYPE_NONE) {
+		switch (ctlr->msg_type) {
+		case SWITCHCON_MSG_TYPE_USB:
+			if (size < 2)
+				break;
+			if (data[0] == SC_INPUT_USB_RESPONSE &&
+			    data[1] == ctlr->usb_ack_match)
+				match = true;
+			break;
+		case SWITCHCON_MSG_TYPE_SUBCMD:
+			if (size < sizeof(struct switchcon_input_report) ||
+			    data[0] != SC_INPUT_SUBCMD_REPLY)
+				break;
+			report = (struct switchcon_input_report *)data;
+			if (report->reply.id == ctlr->subcmd_ack_match)
+				match = true;
+			break;
+		default:
+			break;
+		}
+
+		if (match) {
+			if (size > SC_MAX_RESP_SIZE)
+				copy_size = SC_MAX_RESP_SIZE;
+			else
+				copy_size = size;
+			memcpy(ctlr->input_buf, data, copy_size);
+			ctlr->msg_type = SWITCHCON_MSG_TYPE_NONE;
+			ctlr->received_resp = true;
+			wake_up(&ctlr->wait);
+
+			/* This message has been handled */
+			return 1;
+		}
+	}
+
+	if (ctlr->ctlr_state == SWITCHCON_CTLR_STATE_READ)
+		ret = switchcon_ctlr_read_handler(ctlr, data, size);
+
+	return ret;
+}
+
+static int switchcon_hid_event(struct hid_device *hdev,
+			struct hid_report *report, u8 *raw_data, int size)
+{
+	struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
+
+	if (size < 1)
+		return -EINVAL;
+
+	return switchcon_ctlr_handle_event(ctlr, raw_data, size);
+}
+
+static struct switchcon_ctlr *switchcon_ctlr_create(struct hid_device *hdev)
+{
+	struct switchcon_ctlr *ctlr;
+
+	ctlr = devm_kzalloc(&hdev->dev, sizeof(*ctlr), GFP_KERNEL);
+	if (!ctlr)
+		return ERR_PTR(-ENOMEM);
+
+	switch (hdev->product) {
+	case USB_DEVICE_ID_NINTENDO_PROCON:
+		ctlr->type = SWITCHCON_CTLR_TYPE_PROCON;
+		break;
+	case USB_DEVICE_ID_NINTENDO_JOYCONL:
+		ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_L;
+		break;
+	case USB_DEVICE_ID_NINTENDO_JOYCONR:
+		ctlr->type = SWITCHCON_CTLR_TYPE_JOYCON_R;
+		break;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+	ctlr->hdev = hdev;
+	ctlr->ctlr_state = SWITCHCON_CTLR_STATE_INIT;
+	hid_set_drvdata(hdev, ctlr);
+	mutex_init(&ctlr->output_mutex);
+	init_waitqueue_head(&ctlr->wait);
+	return ctlr;
+}
+
+static void switchcon_ctlr_destroy(struct switchcon_ctlr *ctlr)
+{
+	if (ctlr->input)
+		input_unregister_device(ctlr->input);
+	mutex_destroy(&ctlr->output_mutex);
+}
+
+static int switchcon_hid_probe(struct hid_device *hdev,
+			       const struct hid_device_id *id)
+{
+	int ret;
+	struct switchcon_ctlr *ctlr;
+	struct switchcon_input_report *report;
+	u8 *raw_cal;
+
+	hid_dbg(hdev, "probe - start\n");
+
+	ctlr = switchcon_ctlr_create(hdev);
+	if (IS_ERR(ctlr)) {
+		hid_err(hdev, "Failed to create new controller\n");
+		ret = PTR_ERR(ctlr);
+		goto err;
+	}
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "HID parse failed\n");
+		goto err;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+	if (ret) {
+		hid_err(hdev, "HW start failed\n");
+		goto err;
+	}
+
+	ret = hid_hw_open(hdev);
+	if (ret) {
+		hid_err(hdev, "cannot start hardware I/O\n");
+		goto err_stop;
+	}
+
+	hid_device_io_start(hdev);
+
+	/* Initialize the controller */
+	mutex_lock(&ctlr->output_mutex);
+	/* if baudrate command fails, assume ble pro controller */
+	if (ctlr->type == SWITCHCON_CTLR_TYPE_PROCON &&
+	    !switchcon_send_usb(ctlr, SC_USB_CMD_BAUDRATE_3M)) {
+		/* handshake */
+		ret = switchcon_send_usb(ctlr, SC_USB_CMD_HANDSHAKE);
+		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.
+		 */
+		switchcon_send_usb(ctlr, SC_USB_CMD_NO_TIMEOUT);
+	}
+
+	/* get controller calibration data, and parse it */
+	ret = switchcon_request_calibration(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to retrieve calibration; ret=%d\n", ret);
+		goto err_mutex;
+	}
+	report = (struct switchcon_input_report *)ctlr->input_buf;
+	raw_cal = SC_SUBCMD_REPLY_GET_DATA(&report->reply) + 5;
+	switchcon_parse_lstick_calibration(raw_cal, ctlr);
+	switchcon_parse_rstick_calibration(raw_cal + 9, ctlr);
+	hid_info(ctlr->hdev, "calibration:\n"
+			     "l_x_c=%d l_x_max=%d l_x_min=%d\n"
+			     "l_y_c=%d l_y_max=%d l_y_min=%d\n"
+			     "r_x_c=%d r_x_max=%d r_x_min=%d\n"
+			     "r_y_c=%d r_y_max=%d r_y_min=%d\n",
+			     ctlr->left_stick_cal_x.center,
+			     ctlr->left_stick_cal_x.max,
+			     ctlr->left_stick_cal_x.min,
+			     ctlr->left_stick_cal_y.center,
+			     ctlr->left_stick_cal_y.max,
+			     ctlr->left_stick_cal_y.min,
+			     ctlr->right_stick_cal_x.center,
+			     ctlr->right_stick_cal_x.max,
+			     ctlr->right_stick_cal_x.min,
+			     ctlr->right_stick_cal_y.center,
+			     ctlr->right_stick_cal_y.max,
+			     ctlr->right_stick_cal_y.min);
+
+	/* Set the reporting mode to 0x30, which is the full report mode */
+	ret = switchcon_set_report_mode(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
+		goto err_mutex;
+	}
+
+	mutex_unlock(&ctlr->output_mutex);
+
+	ret = switchcon_input_create(ctlr);
+	if (ret) {
+		hid_err(hdev, "Failed to create input device; ret=%d\n", ret);
+		goto err_close;
+	}
+
+	ctlr->ctlr_state = SWITCHCON_CTLR_STATE_READ;
+
+	hid_dbg(hdev, "probe - success\n");
+	return 0;
+
+err_mutex:
+	mutex_unlock(&ctlr->output_mutex);
+err_close:
+	switchcon_ctlr_destroy(ctlr);
+	hid_hw_close(hdev);
+err_stop:
+	hid_hw_stop(hdev);
+err:
+	hid_err(hdev, "probe - fail = %d\n", ret);
+	return ret;
+}
+
+static void switchcon_hid_remove(struct hid_device *hdev)
+{
+	struct switchcon_ctlr *ctlr = hid_get_drvdata(hdev);
+
+	hid_dbg(hdev, "remove\n");
+	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
+	switchcon_ctlr_destroy(ctlr);
+}
+
+static const struct hid_device_id switchcon_hid_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_PROCON) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_PROCON) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_JOYCONL) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_JOYCONR) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, switchcon_hid_devices);
+
+static struct hid_driver switchcon_hid_driver = {
+	.name		= "switchcon",
+	.id_table	= switchcon_hid_devices,
+	.probe		= switchcon_hid_probe,
+	.remove		= switchcon_hid_remove,
+	.raw_event	= switchcon_hid_event,
+};
+module_hid_driver(switchcon_hid_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
+MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");