[PATCH] bcm5974-0.57 plus hack

Tim Gardner tcanonical at tpi.com
Tue Jul 22 14:44:20 UTC 2008


Henrik Rydberg wrote:
> From: ´╗┐Henrik Rydberg <rydberg at euromail.se>
> 
> The usbhid_modify_dquirk function is not exported in hardy,
> and trying to load the bcm5974 from the hardy-proposed LBM 2.6.24-20 simply
> fails with an undefined-symbol error. This patch upgrades hardy-lbm to bcm5974-0.57,
> and comments out the dynamic quirk addition, awaiting a possible addition of
> EXPORT_SYMBOL(usbhid_modify_dquirk) in usbhid/hid-quirks.c.
> 
> Signed-off-by: Henrik Rydberg <rydberg at euromail.se>
> ---
>  updates/input/mouse/bcm5974.c |  117 ++++++++++++++---------------------------
>  1 files changed, 40 insertions(+), 77 deletions(-)
> 
> diff --git a/updates/input/mouse/bcm5974.c b/updates/input/mouse/bcm5974.c
> index c43fc0c..6bd21fb 100644
> --- a/updates/input/mouse/bcm5974.c
> +++ b/updates/input/mouse/bcm5974.c
> @@ -157,7 +157,6 @@ struct atp {
>  	struct bt_data *bt_data;	/* button transferred data */
>  	struct urb *tp_urb;		/* trackpad usb request block */
>  	struct tp_data *tp_data;	/* trackpad transferred data */
> -	unsigned tp_valid;		/* trackpad sensors valid */
>  };
>  
>  /* logical dimensions */
> @@ -283,10 +282,9 @@ static int report_tp_state(struct atp *dev, int size)
>  #define ATP_WELLSPRING_MODE_WRITE_REQUEST_ID	9
>  #define ATP_WELLSPRING_MODE_REQUEST_VALUE	0x300
>  #define ATP_WELLSPRING_MODE_REQUEST_INDEX	0
> -#define ATP_WELLSPRING_MODE_VENDOR_VALUE_1	0x01
> -#define ATP_WELLSPRING_MODE_VENDOR_VALUE_2	0x05
> +#define ATP_WELLSPRING_MODE_VENDOR_VALUE	0x01
>  
> -static int atp_wellspring_init(struct atp *dev)
> +static int atp_wellspring_mode(struct atp *dev)
>  {
>  	char *data = kmalloc(8, GFP_KERNEL);
>  	int error = 0, size;
> @@ -297,24 +295,6 @@ static int atp_wellspring_init(struct atp *dev)
>  		goto error;
>  	}
>  
> -	/* reset button endpoint */
> -	if (usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> -			USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT,
> -			0, dev->cfg.bt_ep, NULL, 0, 5000)) {
> -		err("bcm5974: could not reset button endpoint");
> -		error = -EIO;
> -		goto error;
> -	}
> -
> -	/* reset trackpad endpoint */
> -	if (usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> -			USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT,
> -			0, dev->cfg.tp_ep, NULL, 0, 5000)) {
> -		err("bcm5974: could not reset trackpad endpoint");
> -		error = -EIO;
> -		goto error;
> -	}
> -
>  	/* read configuration */
>  	size = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
>  		ATP_WELLSPRING_MODE_READ_REQUEST_ID,
> @@ -329,8 +309,7 @@ static int atp_wellspring_init(struct atp *dev)
>  	}
>  
>  	/* apply the mode switch */
> -	data[0] = ATP_WELLSPRING_MODE_VENDOR_VALUE_1;
> -	data[1] = ATP_WELLSPRING_MODE_VENDOR_VALUE_2;
> +	data[0] = ATP_WELLSPRING_MODE_VENDOR_VALUE;
>  
>  	/* write configuration */
>  	size = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
> @@ -345,9 +324,7 @@ static int atp_wellspring_init(struct atp *dev)
>  		goto error;
>  	}
>  
> -	dev->tp_valid = 0;
> -
> -	printk(KERN_INFO "bcm5974: Wellspring mode initialized.\n");
> +	dprintk(2, "bcm5974: switched to wellspring mode.\n");
>  
>  	kfree(data);
>  	return 0;
> @@ -409,11 +386,9 @@ static void irq_trackpad(struct urb *urb)
>  		goto exit;
>  	}
>  
> -	/* first sample data ignored */
> -	if (!dev->tp_valid) {
> -		dev->tp_valid = 1;
> +	/* control response ignored */
> +	if (dev->tp_urb->actual_length == 2)
>  		goto exit;
> -	}
>  
>  	if (report_tp_state(dev, dev->tp_urb->actual_length)) {
>  		dprintk(1, "bcm5974: bad trackpad package, length: %d\n",
> @@ -429,23 +404,45 @@ exit:
>  		err("bcm5974: trackpad urb failed: %d", error);
>  }
>  
> +/*
> + * The Wellspring trackpad, like many recent Apple trackpads, share
> + * the usb device with the keyboard. Since keyboards are usually
> + * handled by the HID system, the device ends up being handled by two
> + * modules. Setting up the device therefore becomes slightly
> + * complicated. To enable multitouch features, a mode switch is
> + * required, which is usually applied via the control interface of the
> + * device.  It can be argued where this switch should take place. In
> + * some drivers, like appletouch, the switch is made during
> + * probe. However, the hid module may also alter the state of the
> + * device, resulting in trackpad malfunction under certain
> + * circumstances. To get around this problem, there is at least one
> + * example that utilizes the USB_QUIRK_RESET_RESUME quirk in order to
> + * recieve a reset_resume request rather than the normal resume. Since
> + * the implementation of reset_resume is equal to mode switch plus
> + * open, it seems easier to always do the switch while opening the
> + * device.
> + */
>  static int atp_open(struct input_dev *input)
>  {
>  	struct atp *dev = input_get_drvdata(input);
>  
>  	if (!dev->open) {
> +		if (atp_wellspring_mode(dev)) {
> +			dprintk(1, "bcm5974: mode switch failed\n");
> +			goto error;
> +		}
>  		if (usb_submit_urb(dev->bt_urb, GFP_KERNEL))
>  			goto error;
>  		if (usb_submit_urb(dev->tp_urb, GFP_KERNEL))
> -			goto err_free_bt_urb;
> +			goto err_kill_bt;
>  	}
>  
>  	dev->open = 1;
>  	dev->suspended = 0;
>  	return 0;
>  
> -err_free_bt_urb:
> -	usb_free_urb(dev->bt_urb);
> +err_kill_bt:
> +	usb_kill_urb(dev->bt_urb);
>  error:
>  	return -EIO;
>  }
> @@ -486,12 +483,6 @@ static int atp_probe(struct usb_interface *iface,
>  	dev->input = input_dev;
>  	dev->cfg = *cfg;
>  
> -	/* switch to raw sensor mode */
> -	if (atp_wellspring_init(dev)) {
> -		error = -EIO;
> -		goto err_free_devs;
> -	}
> -
>  	dev->bt_urb = usb_alloc_urb(0, GFP_KERNEL);
>  	if (!dev->bt_urb) {
>  		error = -ENOMEM;
> @@ -631,57 +622,27 @@ static int atp_resume(struct usb_interface *iface)
>  	return error;
>  }
>  
> -static int atp_reset_resume(struct usb_interface *iface)
> -{
> -	struct atp *dev = usb_get_intfdata(iface);
> -
> -	if (dev && atp_wellspring_init(dev))
> -		printk(KERN_INFO "bcm5974: warning: reset failed\n");
> -
> -	return atp_resume(iface);
> -}
> -
> -static int atp_pre_reset(struct usb_interface *iface)
> -{
> -	return atp_suspend(iface, PMSG_ON);
> -}
> -
> -static int atp_post_reset(struct usb_interface *iface)
> -{
> -	return atp_reset_resume(iface);
> -}
> -
>  static struct usb_driver atp_driver = {
>  	.name = "bcm5974",
>  	.probe = atp_probe,
>  	.disconnect = atp_disconnect,
>  	.suspend = atp_suspend,
>  	.resume = atp_resume,
> -	.reset_resume = atp_reset_resume,
> -	.pre_reset = atp_pre_reset,
> -	.post_reset = atp_post_reset,
> +	.reset_resume = atp_resume,
>  	.id_table = atp_table,
>  };
>  
> -#define USB_VENDOR_ID_APPLE		0x05ac
> -#define USB_DEVICE_ID_APPLE_WELLSPRING_ANSI   0x0223
> -#define USB_DEVICE_ID_APPLE_WELLSPRING_ISO    0x0224
> -#define USB_DEVICE_ID_APPLE_WELLSPRING_JIS    0x0225
> -#define USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI   0x0230
> -#define USB_DEVICE_ID_APPLE_WELLSPRING2_ISO    0x0231
> -#define USB_DEVICE_ID_APPLE_WELLSPRING2_JIS    0x0232
> -
>  static const struct hid_blacklist {
>  	__u16 idVendor;
>  	__u16 idProduct;
>  	__u32 quirks;
>  } hid_blacklist[] = {
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD | HID_QUIRK_IGNORE_MOUSE},
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE},
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE},
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD | HID_QUIRK_IGNORE_MOUSE },
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS, HID_QUIRK_APPLE_HAS_FN  | HID_QUIRK_IGNORE_MOUSE },
> +	{ APPLE_VENDOR_ID, ATP_WELLSPRING_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
> +	{ APPLE_VENDOR_ID, ATP_WELLSPRING_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD | HID_QUIRK_IGNORE_MOUSE},
> +	{ APPLE_VENDOR_ID, ATP_WELLSPRING_JIS, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE},
> +	{ APPLE_VENDOR_ID, ATP_WELLSPRING2_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE},
> +	{ APPLE_VENDOR_ID, ATP_WELLSPRING2_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD | HID_QUIRK_IGNORE_MOUSE },
> +	{ APPLE_VENDOR_ID, ATP_WELLSPRING2_JIS, HID_QUIRK_APPLE_HAS_FN  | HID_QUIRK_IGNORE_MOUSE },
>  	{0,0,0}
>  };
>  
> @@ -689,6 +650,7 @@ static int __init atp_update_quirks(void)
>  {
>  	struct hid_blacklist *bp;
>  	int result=0;
> +#if 0
>  	for (bp=hid_blacklist; bp->quirks; bp++)
>  	{
>  		result = usbhid_modify_dquirk(bp->idVendor,bp->idProduct,bp->quirks);
> @@ -698,6 +660,7 @@ static int __init atp_update_quirks(void)
>  			break;
>  		}
>  	}
> +#endif
>  	return result;
>  }
>  

NACK - one bug, one patch.

I've started SRU Bug #250838, so the next time the Hardy kernel is
uploaded the usbhid_modify_dquirk symbol should be available. I'm
assuming this should also be done for Intrepid since your driver won't
make it upstream until 2.6.27?

rtg
-- 
Tim Gardner tim.gardner at ubuntu.com




More information about the kernel-team mailing list