Re: patch to autosuspend mice #2

On Wed, 31 Jan 2007, Oliver Neukum wrote:

> In fact it should do this to all devices which are HID, claimed by the 
> input layer and support remote wakeup.

>From a quick look, in addition to Alan's comments, it seems to me that you 
don't handle a case in which the hid driver decides to send an output 
report to the suspended device.

A first example that comes to mind - suppose that you have two keyboards, 
at least one of them being HID. Now when the timer expires, the HID 
keyboard will be suspended. Then when you hit a capslock/numlock/... 
(basically anything corresponding to the EV_LED event) on the second 
(non-suspended) one, you will get errors from hid_submit_ctrl(), as it 
will try to deliver a report to the suspended device and it will of course 
fail to do so.

> +static ssize_t set_idle_time(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);		\
> +	struct hid_device *hh = usb_get_intfdata(intf);
> +	struct usbhid_device *uh = hh->driver_data;
> +	int old = uh->idle_time;
> +	int temp = simple_strtoul(buf, NULL, 10);
> +
> +	spin_lock_irq(&idle_guard);
> +	uh->idle_time = temp;
> +	spin_unlock_irq(&idle_guard);
> +
> +	return count;
> +}

You declare and initialize local old variable here, but never use it.

Thanks,

-- 
Jiri Kosina

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel