Hi David,
> Occasionally the trackpad in my MacBookPro 8,3 stops working, spewing
> 'bcm5974: bad trackpad package, length: 8'
We have seen this on a few exemplars over the years. It could be a
hardware problem.
> I haven't been able to reproduce this on demand, but it's annoying when
> it does happen. I'm *assuming* that it's somehow forgotten what mode
> it's supposed to be in. The fix for this on suspend/resume was to switch
> it back to Wellspring mode again, so let's try that...
What do you mean by "fix for this on suspend/resume"? The driver
always returns to normal mode at suspend, and sets wellspring mode at
resume.
> Untested, because my trackpad seems to *know* when I'm actually running
> a kernel with this patch, and doesn't ever misbehave. I've played with
> removing the *normal* mode switch in bcm5974_start_traffic() but can't
> get it to produce the 'bad trackpad package' message at all in that
> case, so the rest function doesn't get invoked.
Being random, it really sounds like faulty hardware. As such, perhaps
the problem can be detected in the usb layer?
>
> Signed-off-by: David Woodhouse <David.Woodhouse@xxxxxxxxx>
>
> diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
> index 2baff1b..db0b3ab 100644
> --- a/drivers/input/mouse/bcm5974.c
> +++ b/drivers/input/mouse/bcm5974.c
> @@ -244,6 +244,7 @@ struct bcm5974 {
> struct bt_data *bt_data; /* button transferred data */
> struct urb *tp_urb; /* trackpad usb request block */
> u8 *tp_data; /* trackpad transferred data */
> + struct work_struct reset_work; /* reset to wellspring mode */
> const struct tp_finger *index[MAX_FINGERS]; /* finger index data */
> struct input_mt_pos pos[MAX_FINGERS]; /* position array */
> int slots[MAX_FINGERS]; /* slot assignments */
> @@ -676,9 +677,14 @@ static void bcm5974_irq_trackpad(struct urb *urb)
> if (dev->tp_urb->actual_length == 2)
> goto exit;
>
> - if (report_tp_state(dev, dev->tp_urb->actual_length))
> + if (report_tp_state(dev, dev->tp_urb->actual_length)) {
> dprintk(1, "bcm5974: bad trackpad package, length: %d\n",
> dev->tp_urb->actual_length);
> + if (dev->tp_urb->actual_length == 8) {
> + /* Hm. Make sure it's in wellspring mode... */
> + schedule_work(&dev->reset_work);
> + }
> + }
>
> exit:
> error = usb_submit_urb(dev->tp_urb, GFP_ATOMIC);
> @@ -815,6 +821,14 @@ static int bcm5974_resume(struct usb_interface *iface)
> return error;
> }
>
> +static void bcm5974_mode_workfn(struct work_struct *work)
> +{
> + struct bcm5974 *dev = container_of(work, struct bcm5974, reset_work);
> +
> + dev_info(&dev->intf->dev, "Reset into wellspring mode...\n");
> + bcm5974_wellspring_mode(dev, true);
> +}
> +
This looks racy.
> static int bcm5974_probe(struct usb_interface *iface,
> const struct usb_device_id *id)
> {
> @@ -840,6 +854,7 @@ static int bcm5974_probe(struct usb_interface *iface,
> dev->input = input_dev;
> dev->cfg = *cfg;
> mutex_init(&dev->pm_mutex);
> + INIT_WORK(&dev->reset_work, bcm5974_mode_workfn);
>
> /* setup urbs */
> if (cfg->tp_type == TYPE1) {
> @@ -936,6 +951,7 @@ static void bcm5974_disconnect(struct usb_interface *iface)
> dev->bt_data, dev->bt_urb->transfer_dma);
> usb_free_urb(dev->tp_urb);
> usb_free_urb(dev->bt_urb);
> + cancel_work_sync(&dev->reset_work);
> kfree(dev);
> }
>
In general, It does not really make sense for the transaction mode to
change under our feet without anything in the usb layer knowing about
it. Maybe there is a reset state cycle which does not get handle
properly in the driver?
Thanks,
Henrik
--
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
