Hi
On Thu, Mar 8, 2018 at 1:09 AM, Matt Votava <matt@xxxxxxxxxx> wrote:
> Some wii games come with a guitar controller extension that plugs into
> the wii remote controller. This patch adds support for the les paul
> style guitar that comes with Guitar Hero 3 to hid-wiimote.
>
> Signed-off-by: Matt Votava <matt@xxxxxxxxxx>
> ---
> drivers/hid/hid-wiimote-core.c | 7 ++
> drivers/hid/hid-wiimote-modules.c | 200 ++++++++++++++++++++++++++++++++++++++
> drivers/hid/hid-wiimote.h | 1 +
> 3 files changed, 208 insertions(+)
>
> diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
> index 579884ebd94d..8b18d104934f 100644
> --- a/drivers/hid/hid-wiimote-core.c
> +++ b/drivers/hid/hid-wiimote-core.c
> @@ -449,6 +449,9 @@ static __u8 wiimote_cmd_read_ext(struct wiimote_data *wdata, __u8 *rmem)
>
> if (rmem[4] == 0x00 && rmem[5] == 0x00)
> return WIIMOTE_EXT_NUNCHUK;
> + if (rmem[0] == 0x00 && rmem[1] == 0x00 && rmem[2] == 0xa4 &&
> + rmem[3] == 0x20 && rmem[4] == 0x01 && rmem[5] == 0x03)
> + return WIIMOTE_EXT_GUITAR;
I would skip rmem[2] and rmem[3]. I think they simply encode the
extension address. This should be enough:
+ if (rmem[0] == 0x00 && rmem[1] == 0x00 &&
+ rmem[4] == 0x01 && rmem[5] == 0x03)
...
> if (rmem[4] == 0x01 && rmem[5] == 0x01)
> return WIIMOTE_EXT_CLASSIC_CONTROLLER;
> if (rmem[4] == 0x04 && rmem[5] == 0x02)
> @@ -491,6 +494,7 @@ static bool wiimote_cmd_map_mp(struct wiimote_data *wdata, __u8 exttype)
> wmem = 0x07;
> break;
> case WIIMOTE_EXT_NUNCHUK:
> + case WIIMOTE_EXT_GUITAR:
> wmem = 0x05;
I think using 0x07 here is easier. That is, use the CLASSIC_CONTROLLER
pass-through, rather than the nunchuk one. But see below..
> break;
> default:
> @@ -1075,6 +1079,7 @@ static const char *wiimote_exttype_names[WIIMOTE_EXT_NUM] = {
> [WIIMOTE_EXT_CLASSIC_CONTROLLER] = "Nintendo Wii Classic Controller",
> [WIIMOTE_EXT_BALANCE_BOARD] = "Nintendo Wii Balance Board",
> [WIIMOTE_EXT_PRO_CONTROLLER] = "Nintendo Wii U Pro Controller",
> + [WIIMOTE_EXT_GUITAR] = "Guitar Hero 3 Les Paul Controller",
Is that the official name? I always used "Nintendo Wii Guitar Hero
Guitar" to follow the style of the other ones. However, I never owned
such a device, so no clue what they report as name. I am also unsure
whether Nintendo actually produced them, or whether they're just
licensed.
> };
>
> /*
> @@ -1660,6 +1665,8 @@ static ssize_t wiimote_ext_show(struct device *dev,
> return sprintf(buf, "balanceboard\n");
> case WIIMOTE_EXT_PRO_CONTROLLER:
> return sprintf(buf, "procontroller\n");
> + case WIIMOTE_EXT_GUITAR:
> + return sprintf(buf, "guitar\n");
> case WIIMOTE_EXT_UNKNOWN:
> /* fallthrough */
> default:
> diff --git a/drivers/hid/hid-wiimote-modules.c b/drivers/hid/hid-wiimote-modules.c
> index c830ed39348f..129f67ef7bd9 100644
> --- a/drivers/hid/hid-wiimote-modules.c
> +++ b/drivers/hid/hid-wiimote-modules.c
> @@ -2176,6 +2176,205 @@ const struct wiimod_ops wiimod_mp = {
> .in_mp = wiimod_mp_in_mp,
> };
>
> +/*
> + * Guitar Hero 3 Gibson Les Paul Controller
> + * The Les Paul Guitar Controller is used by Guitar Hero 3 and is
> + * compatible with later Guitar Hero Wii games and Rockband games
> + * as well. The controller has 5 fret keys, strum up and down, an
> + * analog whammy bar, analog stick, and plus and minus keys.
> + */
> +
> +enum wiimod_guitar_keys {
> + WIIMOD_GUITAR_KEY_G,
> + WIIMOD_GUITAR_KEY_R,
> + WIIMOD_GUITAR_KEY_Y,
> + WIIMOD_GUITAR_KEY_B,
> + WIIMOD_GUITAR_KEY_O,
> + WIIMOD_GUITAR_KEY_UP,
> + WIIMOD_GUITAR_KEY_DOWN,
> + WIIMOD_GUITAR_KEY_PLUS,
> + WIIMOD_GUITAR_KEY_MINUS,
> + WIIMOD_GUITAR_KEY_NUM,
> +};
> +
> +static const __u16 wiimod_guitar_map[] = {
> + BTN_1, /* WIIMOD_GUITAR_KEY_G */
> + BTN_2, /* WIIMOD_GUITAR_KEY_R */
> + BTN_3, /* WIIMOD_GUITAR_KEY_Y */
> + BTN_4, /* WIIMOD_GUITAR_KEY_B */
> + BTN_5, /* WIIMOD_GUITAR_KEY_O */
> + BTN_DPAD_UP, /* WIIMOD_GUITAR_KEY_UP */
> + BTN_DPAD_DOWN, /* WIIMOD_GUITAR_KEY_DOWN */
> + BTN_START, /* WIIMOD_GUITAR_KEY_PLUS */
> + BTN_SELECT, /* WIIMOD_GUITAR_KEY_MINUS */
This mapping looks ok to me.
> +};
> +
> +static void wiimod_guitar_in_ext(struct wiimote_data *wdata, const __u8 *ext)
> +{
> + __s16 bx, by, bt, bw;
> +
> + /* Byte | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
> + * -----+----+----+----+----+----+----+----+----+
> + * 0 | | Button X |
> + * 1 | | Button Y |
> + * -----+---------+----+------------------------+
> + * 2 | | Button TB |
> + * 3 | | Button WB |
> + * -----+----+----+----+----+----+----+---------+
> + * 4 | | BD | | B- | | B+ | |
> + * -----+----+----+----+----+----+----+----+----+
> + * 5 | BO | BR | BB | BG | BY | | BU |
> + * -----+----+----+----+----+----+---------+----+
> + * Button X/Y is the analog stick. Button TB, WB is the touch bar
> + * on the neck of guitars supporting this feature, and the whammy bar.
> + *
> + * BD and BU are strum button down and strum button up.
> + * B- and B+ are the minus and plus button at the base of the neck.
> + *
> + * BG, BR, BY, BB, BO are buttons green, red, yellow, blue, and orange.
> + * All buttons are low active
You lack MP+ handling here. If you use the passthrough, some bits are
moved. I am not entirely sure whether there are actually devices with
MP and guitars, but I'd prefer being on the safe side. Have a look at
commit 8e22ecb603c8 in the official kernel tree. This contains details
on how to handle the mapping (which used the 0x07 pass-through as
described above). Note that this commit got reverted some years ago.
> + */
> +
> + bx = ext[0];
> + by = ext[1];
> + bx -= 224;
> + by -= 224;
Err, this looks bogus. The upper bits of bx and by are not necessarily
1. They might be 0 for some devices (I think the "World Tour" ones).
Hence, I think you need to clear the 6th and 7th bit and then use '32'
as center-point, rather than 224.
If you can, have a look at the old guitar-support (as I mentioned:
8e22ecb603c8). I would like to have the MP+ handling in there.
Otherwise, this looks really good! Thanks a lot for picking it up!
Please remember to keep Jiri in CC, otherwise he will not pick up HID
patches.
Thanks
David
> +
> + bt = ext[2];
> + bw = ext[3];
> + bw &= 0x1f;
> +
> + bw -= 15;
> +
> + input_report_abs(wdata->extension.input, ABS_X, bx);
> + input_report_abs(wdata->extension.input, ABS_Y, by);
> +
> + input_report_abs(wdata->extension.input, ABS_HAT0X, bt);
> + input_report_abs(wdata->extension.input, ABS_HAT1X, bw);
> +
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_G],
> + !(ext[5] & 0x10));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_R],
> + !(ext[5] & 0x40));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_Y],
> + !(ext[5] & 0x08));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_B],
> + !(ext[5] & 0x20));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_O],
> + !(ext[5] & 0x80));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_UP],
> + !(ext[5] & 0x01));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_DOWN],
> + !(ext[4] & 0x40));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_PLUS],
> + !(ext[4] & 0x04));
> + input_report_key(wdata->extension.input,
> + wiimod_guitar_map[WIIMOD_GUITAR_KEY_MINUS],
> + !(ext[4] & 0x10));
> +
> + input_sync(wdata->extension.input);
> +}
> +
> +static int wiimod_guitar_open(struct input_dev *dev)
> +{
> + struct wiimote_data *wdata = input_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&wdata->state.lock, flags);
> + wdata->state.flags |= WIIPROTO_FLAG_EXT_USED;
> + wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
> + spin_unlock_irqrestore(&wdata->state.lock, flags);
> +
> + return 0;
> +}
> +
> +static void wiimod_guitar_close(struct input_dev *dev)
> +{
> + struct wiimote_data *wdata = input_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&wdata->state.lock, flags);
> + wdata->state.flags &= ~WIIPROTO_FLAG_EXT_USED;
> + wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL);
> + spin_unlock_irqrestore(&wdata->state.lock, flags);
> +}
> +
> +static int wiimod_guitar_probe(const struct wiimod_ops *ops,
> + struct wiimote_data *wdata)
> +{
> + int ret, i;
> +
> + wdata->extension.input = input_allocate_device();
> + if (!wdata->extension.input)
> + return -ENOMEM;
> +
> + input_set_drvdata(wdata->extension.input, wdata);
> + wdata->extension.input->open = wiimod_guitar_open;
> + wdata->extension.input->close = wiimod_guitar_close;
> + wdata->extension.input->dev.parent = &wdata->hdev->dev;
> + wdata->extension.input->id.bustype = wdata->hdev->bus;
> + wdata->extension.input->id.vendor = wdata->hdev->vendor;
> + wdata->extension.input->id.product = wdata->hdev->product;
> + wdata->extension.input->id.version = wdata->hdev->version;
> + wdata->extension.input->name = WIIMOTE_NAME " Guitar";
> +
> + set_bit(EV_KEY, wdata->extension.input->evbit);
> + for (i = 0; i < WIIMOD_GUITAR_KEY_NUM; ++i)
> + set_bit(wiimod_guitar_map[i],
> + wdata->extension.input->keybit);
> +
> + set_bit(EV_ABS, wdata->extension.input->evbit);
> + set_bit(ABS_X, wdata->extension.input->absbit);
> + set_bit(ABS_Y, wdata->extension.input->absbit);
> + set_bit(ABS_HAT0X, wdata->extension.input->absbit);
> + set_bit(ABS_HAT1X, wdata->extension.input->absbit);
> + input_set_abs_params(wdata->extension.input,
> + ABS_X, -30, 30, 2, 4);
> + input_set_abs_params(wdata->extension.input,
> + ABS_Y, -30, 30, 2, 4);
> + input_set_abs_params(wdata->extension.input,
> + ABS_HAT0X, -120, 120, 2, 4);
> + input_set_abs_params(wdata->extension.input,
> + ABS_HAT1X, 0, 12, 2, 4);
> +
> + ret = input_register_device(wdata->extension.input);
> + if (ret)
> + goto err_free;
> +
> + return 0;
> +
> +err_free:
> + input_free_device(wdata->extension.input);
> + wdata->extension.input = NULL;
> + return ret;
> +}
> +
> +static void wiimod_guitar_remove(const struct wiimod_ops *ops,
> + struct wiimote_data *wdata)
> +{
> + if (!wdata->extension.input)
> + return;
> +
> + input_unregister_device(wdata->extension.input);
> + wdata->extension.input = NULL;
> +}
> +
> +static const struct wiimod_ops wiimod_guitar = {
> + .flags = 0,
> + .arg = 0,
> + .probe = wiimod_guitar_probe,
> + .remove = wiimod_guitar_remove,
> + .in_ext = wiimod_guitar_in_ext,
> +};
> +
> /* module table */
>
> static const struct wiimod_ops wiimod_dummy;
> @@ -2201,4 +2400,5 @@ const struct wiimod_ops *wiimod_ext_table[WIIMOTE_EXT_NUM] = {
> [WIIMOTE_EXT_CLASSIC_CONTROLLER] = &wiimod_classic,
> [WIIMOTE_EXT_BALANCE_BOARD] = &wiimod_bboard,
> [WIIMOTE_EXT_PRO_CONTROLLER] = &wiimod_pro,
> + [WIIMOTE_EXT_GUITAR] = &wiimod_guitar,
> };
> diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h
> index 510ca77fe14e..c55c034281f0 100644
> --- a/drivers/hid/hid-wiimote.h
> +++ b/drivers/hid/hid-wiimote.h
> @@ -89,6 +89,7 @@ enum wiimote_exttype {
> WIIMOTE_EXT_CLASSIC_CONTROLLER,
> WIIMOTE_EXT_BALANCE_BOARD,
> WIIMOTE_EXT_PRO_CONTROLLER,
> + WIIMOTE_EXT_GUITAR,
> WIIMOTE_EXT_NUM,
> };
>
> --
> 2.16.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
