Re: [PATCH] PS3Remote unpairing and power-saving

[Bastien Nocera <hadess@hadess.net>]

On Fri, 2009-08-28 at 16:42 +0200, Ruslan N. Marchenko wrote:
> PS3 BD Remote driver in fakehid is not able to detect remote unpairing, thus
> not handling it properly.
> Also it keeps permanent BT link with remote controller, which drains batteries.
> With these changes it now utilize IdleTimeout to implicitly unpair device,
> forcing it to enter low power sleep mode.
> IdleTimeout is treated in seconds, in contrast with default - minutes.
> HID device is created on first pairing, and then kept cached till the end of
> daemon life, allowing pairing key-presses to be sent to application with minimum
> delays.
> HID is distinguished by vendor/product codes, which restricts it to only single
> HID for all registered and active PS3 BD Remote controllers.

Please split your patch into 2:
- one part for the disconnect timeout
- one for the PS button disconnecting the remote

Also, don't call it "unpair", it's not paired in the first place, and it
doesn't unpair it, it disconnects it.

<snip>
> +		fake_hid->timeout = iconn->timeout/60;

Have the timeout be either in seconds, or minutes, don't mix them up.
<snip>
> +static gboolean ps3remote_sendkey(int uinput, unsigned int key,
> +				  unsigned int value)
> +{
> +	struct uinput_event event;
> +	memset(&event, 0, sizeof(event));

Add a linefeed after declarations.

<snip>
>  static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
>  				gpointer data)
>  {
> +	static unsigned int lastkey = 0;
> +	static unsigned int lastval = 0;

Don't use statics, add those to a ps3 specific struct.
<snip>
> +	g_timer_start(((struct fake_hid *)fake->priv)->timer);

Why do you start the timer unconditionally, instead of just when using
the HOME key?

<snip>
> -failed:
> +failed: /*
>  	ioctl(fake->uinput, UI_DEV_DESTROY);
>  	close(fake->uinput);
> -	fake->uinput = -1;
> +	fake->uinput = -1;*/

That looks very wrong to me.

Cheers