| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <200705022255.44746.dtor@insightbb.com>
Date: Wed, 2 May 2007 22:55:43 -0400
From: Dmitry Torokhov <dtor@...ightbb.com>
To: linux-input@...ey.karlin.mff.cuni.cz
Cc: Jan Kratochvil <honza@...os.cz>,
Greg Kroah-Hartman <gregkh@...e.de>,
linux-usb-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, Jiri Kosina <jkosina@...e.cz>
Subject: Re: [PATCH 3/3] xpad.c: Added Xbox360 gamepad rumble support.
On Wednesday 02 May 2007 11:05, Jan Kratochvil wrote:
>
> +config XPAD_FF
> + default n
Please don't default to anything.
>
> +#ifdef CONFIG_XPAD_FF
> +/**
> + * xpad_irq_out
> + */
Comments are welcome when they say something...
> +static void xpad_irq_out(struct urb *urb)
> +{
> + int retval;
> +
> + switch (urb->status) {
> + case 0:
> + /* success */
> + break;
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + /* this urb is terminated, clean up */
> + dbg("%s - urb shutting down with status: %d", __FUNCTION__, urb->status);
> + return;
> + default:
> + dbg("%s - nonzero urb status received: %d", __FUNCTION__, urb->status);
> + goto exit;
> + }
> +
> +exit:
> + retval = usb_submit_urb(urb, GFP_ATOMIC);
> + if (retval)
> + err("%s - usb_submit_urb failed with result %d",
> + __FUNCTION__, retval);
> +}
> +
> +int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
> +{
> + struct usb_xpad *xpad = dev->private;
> + if (effect->type == FF_RUMBLE) {
> + __u16 strong = effect->u.rumble.strong_magnitude;
> + __u16 weak = effect->u.rumble.weak_magnitude;
> + xpad->odata[0] = 0x00;
> + xpad->odata[1] = 0x08;
> + xpad->odata[2] = 0x00;
> + xpad->odata[3] = strong / 256;
> + xpad->odata[4] = weak / 256;
> + xpad->odata[5] = 0x00;
> + xpad->odata[6] = 0x00;
> + xpad->odata[7] = 0x00;
> + usb_submit_urb(xpad->irq_out, GFP_KERNEL);
> + }
> +
> + return 0;
> +}
> +
> +static int xpad_init_ff(struct usb_interface *intf, struct usb_xpad *xpad)
> +{
> + if (xpad->flags & XPAD_FLAGS_XBOX360) {
> + struct usb_endpoint_descriptor *ep_irq_out;
> + int rv;
> +
> + xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
> + GFP_ATOMIC, &xpad->odata_dma );
> + if (!xpad->idata)
> + goto fail1;
> +
> + xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
> + if (!xpad->irq_out)
> + goto fail2;
> +
> +
> + ep_irq_out = &intf->cur_altsetting->endpoint[1].desc;
> + usb_fill_int_urb(xpad->irq_out, xpad->udev,
> + usb_sndintpipe(xpad->udev, ep_irq_out->bEndpointAddress),
> + xpad->odata, XPAD_PKT_LEN,
> + xpad_irq_out, xpad, ep_irq_out->bInterval);
> + xpad->irq_out->transfer_dma = xpad->odata_dma;
> + xpad->irq_out->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> + set_bit( FF_RUMBLE, xpad->dev->ffbit );
> + rv = input_ff_create_memless(xpad->dev, NULL, xpad_play_effect);
> +
Error handling seems to be missing.
> + return 0;
> +
> +fail2: usb_buffer_free(xpad->udev, XPAD_PKT_LEN, xpad->odata, xpad->odata_dma);
> +fail1:
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> +static void xpad_deinit_ff(struct usb_interface *intf, struct usb_xpad *xpad)
> +{
> + if (xpad->flags & XPAD_FLAGS_XBOX360) {
> + usb_kill_urb(xpad->irq_out);
You may want to do that in xpad_close().
> + usb_free_urb(xpad->irq_out);
> + usb_buffer_free(interface_to_usbdev(intf), XPAD_PKT_LEN,
> + xpad->odata, xpad->odata_dma);
> + }
> +}
> +#endif
> +
> static int xpad_open (struct input_dev *dev)
> {
> struct usb_xpad *xpad = dev->private;
> @@ -432,6 +535,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>
> input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
>
> +#ifdef CONFIG_XPAD_FF
> + if (xpad->flags & XPAD_FLAGS_XBOX360)
> + input_dev->evbit[0] |= BIT(EV_FF);
> +#endif
Can this be moved into xpad_init_ff?
> +
> /* set up buttons */
> for (i = 0; xpad_btn[i] >= 0; i++)
> set_bit(xpad_btn[i], input_dev->keybit);
> @@ -449,6 +557,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
> for (i = 0; xpad_abs_pad[i] >= 0; i++)
> xpad_set_up_abs(input_dev, xpad_abs_pad[i]);
>
> +#ifdef CONFIG_XPAD_FF
> + if (xpad_init_ff(intf, xpad))
> + goto fail2;
> +#endif
> +
Normally we define dummy fucntions when corresponding config option is disabled
to avoid littering main code with #ifdefs.
> ep_irq_in = &intf->cur_altsetting->endpoint[0].desc;
> usb_fill_int_urb(xpad->irq_in, udev,
> usb_rcvintpipe(udev, ep_irq_in->bEndpointAddress),
> @@ -476,6 +589,9 @@ static void xpad_disconnect(struct usb_interface *intf)
> usb_set_intfdata(intf, NULL);
> if (xpad) {
> usb_kill_urb(xpad->irq_in);
This is extra - we already did that in xpad_close.
> +#ifdef CONFIG_XPAD_FF
> + xpad_deinit_ff(intf, xpad);
> +#endif
> input_unregister_device(xpad->dev);
> usb_free_urb(xpad->irq_in);
> usb_buffer_free(interface_to_usbdev(intf), XPAD_PKT_LEN,
--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists