Hi Milas,
On Thu, Mar 28, 2024 at 03:06:51AM +0100, Milas Robin wrote:
> Add support for the Wii U / Nintendo Switch gamecube controller adapter
Thank you for the driver, a few comments below.
>
> Signed-off-by: Milas Robin <milas.robin@xxxxxxx>
> ---
> drivers/input/joystick/Kconfig | 20 +
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/gamecube-adapter.c | 607 ++++++++++++++++++++++
> 3 files changed, 628 insertions(+)
> create mode 100644 drivers/input/joystick/gamecube-adapter.c
>
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 7755e5b454d2..18ab1f893ed0 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -422,4 +422,24 @@ config JOYSTICK_SEESAW
> To compile this driver as a module, choose M here: the module will be
> called adafruit-seesaw.
>
> +config JOYSTICK_NGC
> + tristate "Nintendo GameCube adapter support"
> + depends on USB_ARCH_HAS_HCD
> + select USB
> + help
> + Say Y here if you want to use Nintendo GameCube adapter with
> + your computer.
> + Make sure to say Y to "Joystick support" (CONFIG_INPUT_JOYDEV)
> + and/or "Event interface support" (CONFIG_INPUT_EVDEV) as well.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called gamecube_adapter.
> +
> +config JOYSTICK_NGC_FF
> + bool "Nintendo GameCube adapter rumble support"
> + depends on JOYSTICK_NGC && INPUT
> + select INPUT_FF_MEMLESS
> + help
> + Say Y here if you want to take advantage of GameCube controller rumble features.
> +
> endif
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 9976f596a920..db0f137ba57f 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o
> obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o
> obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
> obj-$(CONFIG_JOYSTICK_N64) += n64joy.o
> +obj-$(CONFIG_JOYSTICK_NGC) += gamecube-adapter.o
> obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
> obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o
> obj-$(CONFIG_JOYSTICK_QWIIC) += qwiic-joystick.o
> diff --git a/drivers/input/joystick/gamecube-adapter.c b/drivers/input/joystick/gamecube-adapter.c
> new file mode 100644
> index 000000000000..85d69f39d732
> --- /dev/null
> +++ b/drivers/input/joystick/gamecube-adapter.c
> @@ -0,0 +1,607 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Milas Robin
> + *
> + * Based on the work of:
> + * Michael Lelli
> + * Dolphin Emulator project
> + */
> +
> +#include <linux/usb.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/input.h>
> +#include <linux/usb/input.h>
> +
> +/* Did not use usb-hid as it is not an hid driver */
> +#define USB_VENDOR_ID_NINTENDO 0x057e
> +#define USB_DEVICE_ID_NINTENDO_GCADAPTER 0x0337
> +
> +#define EP_IN 0x81
> +#define EP_OUT 0x02
> +
> +#define GCC_OUT_PKT_LEN 5
> +#define GCC_IN_PKT_LEN 37
> +
> +enum gamecube_status {
> + GAMECUBE_NONE,
> + GAMECUBE_WIRED = 0x10,
> + GAMECUBE_WIRELESS = 0x20,
> +};
> +
> +struct gcc_data {
> + struct ngc_data *adapter;
> + struct input_dev *input;
> + u8 no;
> + u8 status;
> + bool enable;
> +};
> +
> +struct ngc_data {
> + char phys[64];
> +
> + struct usb_device *udev;
> + struct usb_interface *intf;
> +
> + struct urb *irq_in;
> + u8 *idata;
> + dma_addr_t idata_dma;
> + spinlock_t idata_lock;
> +
> + struct urb *irq_out;
> + struct usb_anchor irq_out_anchor;
> + u8 *odata;
> + dma_addr_t odata_dma;
> + spinlock_t odata_lock; /* output data */
> + bool irq_out_active; /* we must not use an active URB */
I think all of this starting with irq_out should be under #ifdef
CONFIG_JOYSTICK_NGC_FF.
> +#ifdef CONFIG_JOYSTICK_NGC_FF
> + u8 odata_rumbles[4];
> + bool rumble_changed; /* if rumble need update*/
> +#endif
> +
> + struct gcc_data controllers[4];
Would be nice to have a #define for 4.
> + struct work_struct work; /* create/delete controller input files */
> +};
> +
> +#ifdef CONFIG_JOYSTICK_NGC_FF
> +/* Callers must hold gdata->odata_lock spinlock */
Then please use lockdep_assert_held().
> +static int ngc_queue_rumble(struct ngc_data *gdata)
> +{
> + int error;
> +
Please check irq_out_active flag here and also provide a stub for
!CONFIG_JOYSTICK_NGC_FF so you do not need to sprinkle #ifdef checks..
> + memcpy(gdata->odata + 1, gdata->odata_rumbles,
> + sizeof(gdata->odata_rumbles));
> + gdata->irq_out_active = true;
> + gdata->rumble_changed = false;
> + gdata->odata[0] = 0x11;
> + gdata->irq_out->transfer_buffer_length = 5;
> +
> + usb_anchor_urb(gdata->irq_out, &gdata->irq_out_anchor);
> + error = usb_submit_urb(gdata->irq_out, GFP_ATOMIC);
> + if (error) {
> + dev_err(&gdata->intf->dev,
> + "%s - usb_submit_urb failed with result %d\n",
> + __func__, error);
> + usb_unanchor_urb(gdata->irq_out);
> + error = -EIO;
> + }
> + return error;
> +}
> +
> +static int ngc_set_rumble_value(struct ngc_data *gdata, u8 controller, u8 value)
Since you want 0 or 1 for the value make it a "bool value".
> +{
> + unsigned long flags;
> + int error;
/
> +
> + value = !!value;
> + if (controller > 4)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&gdata->odata_lock, flags);
Let's start using guard construct:
guard(spinlock_irqsave)(&gdata->odata_lock);
if (gdata->odata_rumbles[controller] == value)
return 0;
gdata->odata_rumbles[controller] = value;
gdata->rumble_changed = true;
return ngc_queue_rumble(gdata);
> + if (gdata->odata_rumbles[controller] == value) {
> + spin_unlock_irqrestore(&gdata->odata_lock, flags);
> + return 0;
> + }
> + gdata->odata_rumbles[controller] = value;
> + gdata->rumble_changed = true;
> + if (!gdata->irq_out_active)
> + error = ngc_queue_rumble(gdata);
> + spin_unlock_irqrestore(&gdata->odata_lock, flags);
> + return error;
> +}
> +
> +static int ngc_rumble_play(struct input_dev *dev, void *data,
> + struct ff_effect *eff)
> +{
> + struct gcc_data *gccdata = input_get_drvdata(dev);
> + u8 value;
> +
> + /*
> + * The gamecube controller supports only a single rumble motor so if any
> + * magnitude is set to non-zero then we start the rumble motor. If both are
> + * set to zero, we stop the rumble motor.
> + */
> +
> + if (eff->u.rumble.strong_magnitude || eff->u.rumble.weak_magnitude)
> + value = 1;
> + else
> + value = 0;
> + return ngc_set_rumble_value(gccdata->adapter, gccdata->no, value);
If value is a bool you can simply say
return ngc_set_rumble_value(gccdata->adapter, gccdata->no,
eff->u.rumble.strong_magnitude ||
eff->u.rumble.weak_magnitude);
> +}
> +static int ngc_init_ff(struct gcc_data *gccdev)
> +{
> + input_set_capability(gccdev->input, EV_FF, FF_RUMBLE);
> +
> + return input_ff_create_memless(gccdev->input, NULL, ngc_rumble_play);
> +}
> +#else
> +static int ngc_init_ff(struct gcc_data *gccdev) { return 0; }
> +#endif
> +
> +static void ngc_irq_out(struct urb *urb)
> +{
> + struct ngc_data *gdata = urb->context;
> + struct device *dev = &gdata->intf->dev;
> + int status = urb->status;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&gdata->odata_lock, flags);
guard();
> +
> + switch (status) {
> + case 0:
> + /* success */
> +#ifdef CONFIG_JOYSTICK_NGC_FF
> + gdata->irq_out_active = gdata->rumble_changed;
> +#else
> + gdata->irq_out_active = false;
> +#endif
> + break;
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + /* this urb is terminated, clean up */
> + dev_dbg(dev, "%s - urb shutting down with status: %d\n",
> + __func__, status);
> + gdata->irq_out_active = false;
Do you really want to send out URB again in this case?
> + break;
> +
> + default:
> + dev_dbg(dev, "%s - nonzero urb status received: %d\n",
> + __func__, status);
And here?
> + break;
> + }
> +#ifdef CONFIG_JOYSTICK_NGC_FF
> + if (gdata->irq_out_active)
> + ngc_queue_rumble(gdata);
> +#endif
> + spin_unlock_irqrestore(&gdata->odata_lock, flags);
> +}
> +
> +static int ngc_init_output(struct ngc_data *gdata,
> + struct usb_endpoint_descriptor *irq)
> +{
> + int error = -ENOMEM;
> +
> + init_usb_anchor(&gdata->irq_out_anchor);
> +
> + gdata->odata = usb_alloc_coherent(gdata->udev, GCC_OUT_PKT_LEN, GFP_KERNEL,
> + &gdata->odata_dma);
> + if (!gdata->odata)
> + return error;
> +
> + spin_lock_init(&gdata->odata_lock);
> +
> + gdata->irq_out = usb_alloc_urb(0, GFP_KERNEL);
> +
> + if (!gdata->irq_out)
> + goto err_free_coherent;
> +
> + usb_fill_int_urb(gdata->irq_out, gdata->udev,
> + usb_sndintpipe(gdata->udev, irq->bEndpointAddress),
> + gdata->odata, GCC_OUT_PKT_LEN, ngc_irq_out, gdata,
> + irq->bInterval);
> + gdata->irq_out->transfer_dma = gdata->odata_dma;
> + gdata->irq_out->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + return 0;
> +
> +err_free_coherent:
> + usb_free_coherent(gdata->udev, GCC_OUT_PKT_LEN, gdata->odata,
> + gdata->odata_dma);
> + return error;
> +}
> +
> +static void ngc_deinit_output(struct ngc_data *gdata)
> +{
> + usb_free_urb(gdata->irq_out);
> + usb_free_coherent(gdata->udev, GCC_OUT_PKT_LEN, gdata->odata,
> + gdata->odata_dma);
> +}
> +
> +static void gcc_input(struct gcc_data *gccdata, const u8 *keys)
> +{
> + input_report_key(gccdata->input, BTN_A, !!(keys[1] & BIT(0)));
> + input_report_key(gccdata->input, BTN_B, !!(keys[1] & BIT(1)));
> + input_report_key(gccdata->input, BTN_X, !!(keys[1] & BIT(2)));
> + input_report_key(gccdata->input, BTN_Y, !!(keys[1] & BIT(3)));
> + input_report_key(gccdata->input, BTN_DPAD_LEFT, !!(keys[1] & BIT(4)));
> + input_report_key(gccdata->input, BTN_DPAD_RIGHT, !!(keys[1] & BIT(5)));
> + input_report_key(gccdata->input, BTN_DPAD_DOWN, !!(keys[1] & BIT(6)));
> + input_report_key(gccdata->input, BTN_DPAD_UP, !!(keys[1] & BIT(7)));
> +
> + input_report_key(gccdata->input, BTN_START, !!(keys[2] & BIT(0)));
> + input_report_key(gccdata->input, BTN_TR2, !!(keys[2] & BIT(1)));
> + input_report_key(gccdata->input, BTN_TR, !!(keys[2] & BIT(2)));
> + input_report_key(gccdata->input, BTN_TL, !!(keys[2] & BIT(3)));
Can we maybe have a list of mapping bits to events and loop over them.
You also do not need to normalize value for input_report_key(), it does
it for you.
> +
> + input_report_abs(gccdata->input, ABS_X, keys[3]);
> + input_report_abs(gccdata->input, ABS_Y, keys[4] ^ 0xFF);
> + input_report_abs(gccdata->input, ABS_RX, keys[5]);
> + input_report_abs(gccdata->input, ABS_RY, keys[6] ^ 0xFF);
> + input_report_abs(gccdata->input, ABS_Z, keys[7]);
> + input_report_abs(gccdata->input, ABS_RZ, keys[8]);
> +
> + input_sync(gccdata->input);
> +}
> +
> +
> +static u8 ngc_connected_type(u8 status)
> +{
> + u8 type = status & (GAMECUBE_WIRED | GAMECUBE_WIRELESS);
> +
> + switch (type) {
> + case GAMECUBE_WIRED:
> + case GAMECUBE_WIRELESS:
> + return type;
> + default:
> + return 0;
> + }
> +}
> +
> +static int ngc_controller_init(struct gcc_data *gccdev, u8 status)
> +{
> + int error;
> +
> + gccdev->input = input_allocate_device();
> + if (!gccdev->input)
> + return -ENOMEM;
> +
> + input_set_drvdata(gccdev->input, gccdev);
> + usb_to_input_id(gccdev->adapter->udev, &gccdev->input->id);
> + gccdev->input->name = "Nintendo GameCube Controller";
> + gccdev->input->phys = gccdev->adapter->phys;
> +
> + set_bit(EV_KEY, gccdev->input->evbit);
> +
> + set_bit(BTN_A, gccdev->input->keybit);
> + set_bit(BTN_B, gccdev->input->keybit);
> + set_bit(BTN_X, gccdev->input->keybit);
> + set_bit(BTN_Y, gccdev->input->keybit);
> + set_bit(BTN_DPAD_LEFT, gccdev->input->keybit);
> + set_bit(BTN_DPAD_RIGHT, gccdev->input->keybit);
> + set_bit(BTN_DPAD_DOWN, gccdev->input->keybit);
> + set_bit(BTN_DPAD_UP, gccdev->input->keybit);
> + set_bit(BTN_START, gccdev->input->keybit);
> + set_bit(BTN_TR2, gccdev->input->keybit);
> + set_bit(BTN_TR, gccdev->input->keybit);
> + set_bit(BTN_TL, gccdev->input->keybit);
> +
> + set_bit(EV_ABS, gccdev->input->evbit);
> +
> + set_bit(ABS_X, gccdev->input->absbit);
> + set_bit(ABS_Y, gccdev->input->absbit);
> + set_bit(ABS_RX, gccdev->input->absbit);
> + set_bit(ABS_RY, gccdev->input->absbit);
> + set_bit(ABS_Z, gccdev->input->absbit);
> + set_bit(ABS_RZ, gccdev->input->absbit);
These bits will be set by input_set_abs_params() below.
> +
> + input_set_abs_params(gccdev->input, ABS_X, 0, 255, 16, 16);
> + input_set_abs_params(gccdev->input, ABS_Y, 0, 255, 16, 16);
> + input_set_abs_params(gccdev->input, ABS_RX, 0, 255, 16, 16);
> + input_set_abs_params(gccdev->input, ABS_RY, 0, 255, 16, 16);
> + input_set_abs_params(gccdev->input, ABS_Z, 0, 255, 16, 0);
> + input_set_abs_params(gccdev->input, ABS_RZ, 0, 255, 16, 0);
> + error = ngc_init_ff(gccdev);
> + if (error) {
> + dev_warn(&gccdev->input->dev, "Could not create ff (skipped)");
> + goto ngc_deinit_controller;
> + }
> + error = input_register_device(gccdev->input);
> + if (error)
> + goto ngc_deinit_controller_ff;
> + gccdev->enable = true;
> + return 0;
> +ngc_deinit_controller_ff:
> + input_ff_destroy(gccdev->input);
> +ngc_deinit_controller:
> + input_free_device(gccdev->input);
> + return error;
> +}
> +
> +static void ngc_controller_update_work(struct work_struct *work)
> +{
> + int i;
> + u8 status[4];
> + bool enable[4];
> + unsigned long flags;
> + struct ngc_data *gdata = container_of(work, struct ngc_data, work);
> +
> + for (i = 0; i < 4; i++) {
> + status[i] = gdata->controllers[i].status;
> + enable[i] = ngc_connected_type(status[i]) != 0;
> + }
> +
> + for (i = 0; i < 4; i++) {
> + if (enable[i] && !gdata->controllers[i].enable) {
> + if (ngc_controller_init(&gdata->controllers[i], status[i]) != 0)
> + enable[i] = false;
> + }
> + }
> +
> + spin_lock_irqsave(&gdata->idata_lock, flags);
> + for (i = 0; i < 4; i++)
> + swap(gdata->controllers[i].enable, enable[i]);
> + spin_unlock_irqrestore(&gdata->idata_lock, flags);
> +
> + for (i = 0; i < 4; i++) {
> + if (enable[i] && !gdata->controllers[i].enable)
> + input_unregister_device(gdata->controllers[i].input);
> + }
> +}
> +
> +static void ngc_input(struct ngc_data *gdata)
> +{
> + int i;
> + unsigned long flags;
> + bool updated = false;
> +
> + for (i = 0; i < 4; i++) {
> + updated = updated ||
> + gdata->idata[1 + 9 * i] != gdata->controllers[i].status;
> + gdata->controllers[i].status = gdata->idata[1 + 9 * i];
> + }
> + if (updated)
> + schedule_work(&gdata->work);
> + spin_lock_irqsave(&gdata->idata_lock, flags);
> + for (i = 0; i < 4; i++) {
> + if (gdata->controllers[i].enable)
> + gcc_input(&gdata->controllers[i], &gdata->idata[1 + 9 * i]);
> + }
> + spin_unlock_irqrestore(&gdata->idata_lock, flags);
> +}
> +
> +static void ngc_irq_in(struct urb *urb)
> +{
> + struct ngc_data *gdata = urb->context;
> + struct usb_interface *intf = gdata->intf;
> + int error;
> +
> + switch (urb->status) {
> + case 0:
> + break;
> + case -EOVERFLOW:
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + dev_dbg(&intf->dev, "controller urb shutting down: %d\n",
> + urb->status);
> + return;
> + default:
> + dev_dbg(&intf->dev, "controller urb status: %d\n", urb->status);
> + goto exit;
> + }
> + if (gdata->irq_in->actual_length != GCC_IN_PKT_LEN)
> + dev_warn(&intf->dev, "Bad sized packet\n");
> + else if (gdata->idata[0] != 0x21)
> + dev_warn(&intf->dev, "Unknown opcode %d\n", gdata->idata[0]);
> + else
> + ngc_input(gdata);
> +exit:
> + error = usb_submit_urb(gdata->irq_in, GFP_ATOMIC);
> + if (error)
> + dev_err(&intf->dev, "controller urb failed: %d\n", error);
> +}
> +
> +static int ngc_init_input(struct ngc_data *gdata,
> + struct usb_endpoint_descriptor *irq)
> +{
> + int error = -ENOMEM;
> +
> + gdata->idata = usb_alloc_coherent(gdata->udev, GCC_IN_PKT_LEN, GFP_KERNEL,
> + &gdata->idata_dma);
> + if (!gdata->idata)
> + return error;
> +
> + gdata->irq_in = usb_alloc_urb(0, GFP_KERNEL);
> + if (!gdata->irq_in)
> + goto err_free_coherent;
> +
> + usb_fill_int_urb(gdata->irq_in, gdata->udev,
> + usb_rcvintpipe(gdata->udev, irq->bEndpointAddress),
> + gdata->idata, GCC_IN_PKT_LEN, ngc_irq_in, gdata,
> + irq->bInterval);
> + gdata->irq_in->transfer_dma = gdata->idata_dma;
> + gdata->irq_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> + spin_lock_init(&gdata->idata_lock);
> + INIT_WORK(&gdata->work, ngc_controller_update_work);
> +
> + return 0;
> +
> +err_free_coherent:
> + usb_free_coherent(gdata->udev, GCC_IN_PKT_LEN, gdata->idata,
> + gdata->idata_dma);
> + return error;
> +
> +}
> +
> +
> +static void ngc_deinit_input(struct ngc_data *gdata)
> +{
> + usb_free_urb(gdata->irq_in);
> + usb_free_coherent(gdata->udev, GCC_IN_PKT_LEN, gdata->idata,
> + gdata->idata_dma);
> +}
> +
> +
> +static int ngc_init_irq(struct ngc_data *gdata)
> +{
> + struct usb_endpoint_descriptor *eps[] = { NULL, NULL };
> + int error;
> +
> + error = usb_find_common_endpoints(gdata->intf->cur_altsetting, NULL, NULL,
> + &eps[0], &eps[1]);
> + if (error)
> + return -ENODEV;
> + error = ngc_init_output(gdata, eps[1]);
> + if (error)
> + return error;
> + error = ngc_init_input(gdata, eps[0]);
> + if (error)
> + goto err_deinit_out;
> +#ifdef CONFIG_JOYSTICK_NGC_FF
> + memset(gdata->odata_rumbles, 0, 4);
> + gdata->rumble_changed = false;
Don't you allocate zeroed out memory?
> +#endif
> + gdata->irq_out_active = true;
> + gdata->odata[0] = 0x13;
> + gdata->irq_out->transfer_buffer_length = 1;
> +
> + error = usb_submit_urb(gdata->irq_in, GFP_KERNEL);
> + if (error)
> + goto err_deinit_in;
> +
> + usb_anchor_urb(gdata->irq_out, &gdata->irq_out_anchor);
> + error = usb_submit_urb(gdata->irq_out, GFP_ATOMIC);
> + if (error) {
> + dev_err(&gdata->intf->dev,
> + "%s - usb_submit_urb failed with result %d\n",
> + __func__, error);
> + usb_unanchor_urb(gdata->irq_out);
> + error = -EIO;
> + goto err_kill_in_urb;
> + }
> +
> + return 0;
> +err_kill_in_urb:
> + usb_kill_urb(gdata->irq_in);
> +err_deinit_in:
> + ngc_deinit_input(gdata);
> +err_deinit_out:
> + ngc_deinit_output(gdata);
> + return error;
> +}
> +
> +static void ngc_deinit_irq(struct ngc_data *gdata)
> +{
> + if (!usb_wait_anchor_empty_timeout(&gdata->irq_out_anchor, 5000)) {
> + dev_warn(&gdata->intf->dev,
> + "timed out waiting for output URB to complete, killing\n");
> + usb_kill_anchored_urbs(&gdata->irq_out_anchor);
Why can't we simply kill out URB and not dean with anchoring?
> + }
> + usb_kill_urb(gdata->irq_in);
> + /* Make sure we are done with presence work if it was scheduled */
> + flush_work(&gdata->work);
> +
> + ngc_deinit_input(gdata);
> + ngc_deinit_output(gdata);
> +}
> +
> +static void ngc_init_controllers(struct ngc_data *gdata)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(gdata->controllers); i++) {
> + gdata->controllers[i].adapter = gdata;
> + gdata->controllers[i].no = i;
> + gdata->controllers[i].status = GAMECUBE_NONE;
> + gdata->controllers[i].enable = false;
> + }
> +}
> +
> +static struct attribute *ngc_attrs[] = {
> + NULL,
> +};
> +
> +static const struct attribute_group ngc_attr_group = {
> + .attrs = ngc_attrs,
> +};
> +
> +static int ngc_init_attr(struct ngc_data *gdata)
> +{
> + return sysfs_create_group(&gdata->intf->dev.kobj, &ngc_attr_group);
> +}
> +
> +static void ngc_deinit_attr(struct ngc_data *gdata)
> +{
> + sysfs_remove_group(&gdata->intf->dev.kobj, &ngc_attr_group);
> +}
What is all this for?
> +
> +
> +static int ngc_usb_probe(struct usb_interface *iface, const struct usb_device_id *id)
> +{
> + struct usb_device *udev = interface_to_usbdev(iface);
> + struct ngc_data *gdata;
> + int error;
> +
> + gdata = kzalloc(sizeof(struct ngc_data), GFP_KERNEL);
> + if (!gdata)
> + return -ENOMEM;
> + usb_set_intfdata(iface, gdata);
> + gdata->udev = udev;
> + gdata->intf = iface;
> +
> + usb_make_path(udev, gdata->phys, sizeof(gdata->phys));
> + strlcat(gdata->phys, "/input0", sizeof(gdata->phys));
> +
> + ngc_init_controllers(gdata);
> + error = ngc_init_irq(gdata);
> + if (error)
> + goto err_free_devs;
> + error = ngc_init_attr(gdata);
> + if (error)
> + goto err_deinit_endpoints;
> + dev_info(&iface->dev, "New device registered\n");
> + return 0;
> +err_deinit_endpoints:
> + ngc_deinit_irq(gdata);
> +err_free_devs:
> + usb_set_intfdata(iface, NULL);
> + kfree(gdata);
> + return error;
> +}
> +
> +static void ngc_usb_disconnect(struct usb_interface *iface)
> +{
> + int i;
> + struct ngc_data *gdata = usb_get_intfdata(iface);
Make this first line.
> +
> + for (i = 0; i < 4; i++) {
> + if (gdata->controllers[i].enable)
> + input_unregister_device(gdata->controllers[i].input);
> + }
> + ngc_deinit_attr(gdata);
> + ngc_deinit_irq(gdata);
> + usb_set_intfdata(iface, NULL);
> + kfree(gdata);
> +}
> +
> +static const struct usb_device_id ngc_usb_devices[] = {
> + { USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_GCADAPTER) },
> + {}
> +};
> +
Drop empty line.
> +MODULE_DEVICE_TABLE(usb, ngc_usb_devices);
> +
> +static struct usb_driver ngc_usb_driver = {
> + .name = "gamecube_adapter",
> + .id_table = ngc_usb_devices,
> + .probe = ngc_usb_probe,
> + .disconnect = ngc_usb_disconnect,
> +};
> +
> +module_usb_driver(ngc_usb_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Robin Milas <milas.robin@xxxxxxx>");
> +MODULE_DESCRIPTION("Driver for GameCube adapter");
> --
> 2.44.0
>
Thanks.
--
Dmitry
