[rft]suspend/resume support for yealink

[rft]suspend/resume support for yealink

[Oliver Neukum]

This implements basic suspend/resume support for yealink.
Could somebody test it?

	Regards
		Oliver

---

--- linux-2.6.26-sierra/drivers/input/misc/yealink.alt2.c	2008-06-30 14:13:58.000000000 +0200
+++ linux-2.6.26-sierra/drivers/input/misc/yealink.c	2008-06-30 16:02:30.000000000 +0200
@@ -120,6 +120,7 @@ struct yealink_dev {
 	u8 lcdMap[ARRAY_SIZE(lcdMap)];	/* state of LCD, LED ... */
 	int key_code;			/* last reported key	 */
 	int shutdown:1;
+	int open:1;
 
 	int	stat_ix;
 	union {
@@ -128,6 +129,7 @@ struct yealink_dev {
 	} master, copy;
 };
 
+static DECLARE_RWSEM(sysfs_rwsema);
 
 /*******************************************************************************
  * Yealink lcd interface
@@ -530,12 +532,16 @@ static int input_open(struct input_dev *
 	yld->ctl_data->cmd	= CMD_INIT;
 	yld->ctl_data->size	= 10;
 	yld->ctl_data->sum	= 0x100-CMD_INIT-10;
-	if ((ret = usb_submit_urb(yld->urb_ctl, GFP_KERNEL)) != 0) {
+	down_write(&sysfs_rwsema);
+	if ((ret = usb_submit_urb(yld->urb_ctl, GFP_KERNEL)) != 0)
 		dbg("%s - usb_submit_urb failed with result %d",
 		     __FUNCTION__, ret);
-		return ret;
-	}
-	return 0;
+	else
+		yld->open = 1;
+
+	up_write(&sysfs_rwsema);
+
+	return ret;
 }
 
 static void stop_traffic(struct yealink_dev *yld)
@@ -556,6 +562,9 @@ static void input_close(struct input_dev
 {
 	struct yealink_dev *yld = input_get_drvdata(dev);
 
+	down_write(&sysfs_rwsema);
+	yld->open = 0;
+	up_write(&sysfs_rwsema);
 	stop_traffic(yld);
 }
 
@@ -563,8 +572,6 @@ static void input_close(struct input_dev
  * sysfs interface
  ******************************************************************************/
 
-static DECLARE_RWSEM(sysfs_rwsema);
-
 /* Interface to the 7-segments translation table aka. char set.
  */
 static ssize_t show_map(struct device *dev, struct device_attribute *attr,
@@ -856,6 +863,25 @@ static int usb_cleanup(struct yealink_de
 	return err;
 }
 
+static int usb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct yealink_dev *yld = usb_get_intfdata(intf);
+
+	stop_traffic(yld);
+	return 0;
+}
+
+static int usb_resume(struct usb_interface *intf)
+{
+	struct yealink_dev *yld = usb_get_intfdata(intf);
+	int rv = 0;
+
+	down_read(&sysfs_rwsema);
+	if (yld->open)
+		rv = usb_submit_urb(yld->urb_ctl, GFP_NOIO);
+	up_read(&sysfs_rwsema);
+	return rv;
+}
 static void usb_disconnect(struct usb_interface *intf)
 {
 	struct yealink_dev *yld;
@@ -1002,6 +1028,8 @@ static struct usb_driver yealink_driver
 	.name		= "yealink",
 	.probe		= usb_probe,
 	.disconnect	= usb_disconnect,
+	.suspend	= usb_suspend,
+	.resume		= usb_resume,
 	.id_table	= usb_table,
 };
 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rft]suspend/resume support for yealink

[Alfred E. Heggestad]

Oliver Neukum wrote:
> This implements basic suspend/resume support for yealink.
> Could somebody test it?
> 

(copied Thomas)

Hi Thomas, could you please test this patch with your Yealink device?


/alfred

> 	Regards
> 		Oliver
> 
> ---
> 
> --- linux-2.6.26-sierra/drivers/input/misc/yealink.alt2.c	2008-06-30 14:13:58.000000000 +0200
> +++ linux-2.6.26-sierra/drivers/input/misc/yealink.c	2008-06-30 16:02:30.000000000 +0200
> @@ -120,6 +120,7 @@ struct yealink_dev {
>  	u8 lcdMap[ARRAY_SIZE(lcdMap)];	/* state of LCD, LED ... */
>  	int key_code;			/* last reported key	 */
>  	int shutdown:1;
> +	int open:1;
>  
>  	int	stat_ix;
>  	union {
> @@ -128,6 +129,7 @@ struct yealink_dev {
>  	} master, copy;
>  };
>  
> +static DECLARE_RWSEM(sysfs_rwsema);
>  
>  /*******************************************************************************
>   * Yealink lcd interface
> @@ -530,12 +532,16 @@ static int input_open(struct input_dev *
>  	yld->ctl_data->cmd	= CMD_INIT;
>  	yld->ctl_data->size	= 10;
>  	yld->ctl_data->sum	= 0x100-CMD_INIT-10;
> -	if ((ret = usb_submit_urb(yld->urb_ctl, GFP_KERNEL)) != 0) {
> +	down_write(&sysfs_rwsema);
> +	if ((ret = usb_submit_urb(yld->urb_ctl, GFP_KERNEL)) != 0)
>  		dbg("%s - usb_submit_urb failed with result %d",
>  		     __FUNCTION__, ret);
> -		return ret;
> -	}
> -	return 0;
> +	else
> +		yld->open = 1;
> +
> +	up_write(&sysfs_rwsema);
> +
> +	return ret;
>  }
>  
>  static void stop_traffic(struct yealink_dev *yld)
> @@ -556,6 +562,9 @@ static void input_close(struct input_dev
>  {
>  	struct yealink_dev *yld = input_get_drvdata(dev);
>  
> +	down_write(&sysfs_rwsema);
> +	yld->open = 0;
> +	up_write(&sysfs_rwsema);
>  	stop_traffic(yld);
>  }
>  
> @@ -563,8 +572,6 @@ static void input_close(struct input_dev
>   * sysfs interface
>   ******************************************************************************/
>  
> -static DECLARE_RWSEM(sysfs_rwsema);
> -
>  /* Interface to the 7-segments translation table aka. char set.
>   */
>  static ssize_t show_map(struct device *dev, struct device_attribute *attr,
> @@ -856,6 +863,25 @@ static int usb_cleanup(struct yealink_de
>  	return err;
>  }
>  
> +static int usb_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> +	struct yealink_dev *yld = usb_get_intfdata(intf);
> +
> +	stop_traffic(yld);
> +	return 0;
> +}
> +
> +static int usb_resume(struct usb_interface *intf)
> +{
> +	struct yealink_dev *yld = usb_get_intfdata(intf);
> +	int rv = 0;
> +
> +	down_read(&sysfs_rwsema);
> +	if (yld->open)
> +		rv = usb_submit_urb(yld->urb_ctl, GFP_NOIO);
> +	up_read(&sysfs_rwsema);
> +	return rv;
> +}
>  static void usb_disconnect(struct usb_interface *intf)
>  {
>  	struct yealink_dev *yld;
> @@ -1002,6 +1028,8 @@ static struct usb_driver yealink_driver
>  	.name		= "yealink",
>  	.probe		= usb_probe,
>  	.disconnect	= usb_disconnect,
> +	.suspend	= usb_suspend,
> +	.resume		= usb_resume,
>  	.id_table	= usb_table,
>  };
>  

Re: [rft]suspend/resume support for yealink

[Thomas Reitmayr]

Hi Oliver,
I tested the suspend/resume support with my Yealink USB-P1K doing the
following:

* Suspend to RAM:
  The device suspends properly, I hear the audio subsystem turning off
while the display keeps its current state.
  Apparently the USB root hub gets reset during suspend/resume according
to the kernel log. Then after resume the input events are generated
again and the display can be updated. I noticed however that the display
is reset to its default text, possibly because of the USB reset? Anyway,
I appended a filtered kernel log about this scenario below.

* Suspend the device by "echo suspend > power/level"
  Works too with the difference that the display is not modified during
resume.

Note that during all my tests the input device was opened when the
device got suspended, i.e. I did not try to explicitly open the input
device only after resume. Neglecting this missing test case I would say
your code works fine!

Regards,
-Thomas


PS: Here is some part of the kernel log generated by suspend to RAM +
resume:

> cat yl-suspend-resume.txt  | grep -i 'yealink\|5-2\|usb5'
yealink 5-2:1.3: suspend
usb 5-2:1.2: suspend
usb 5-2:1.1: suspend
usb 5-2:1.0: suspend
usb 5-2: suspend
yealink: urb_ctl_callback - urb status -2
usb usb5: suspend, may wakeup
usb usb5: root hub lost power or was reset
usb usb5: resuming
usb 5-2: resuming
usb 5-2:1.0: resuming
usb 5-2:1.1: resuming
usb 5-2:1.2: resuming
yealink 5-2:1.3: resuming
Restarting tasks ... <6>usb 5-2: USB disconnect, address 4
PM: Removing info for usb:5-2:1.0
PM: Removing info for usb:5-2:1.1
PM: Removing info for usb:5-2:1.2
PM: Removing info for usb:5-2:1.3
PM: Removing info for usb:5-2
usb 5-2: new full speed USB device using uhci_hcd and address 5
usb 5-2: not running at top speed; connect to a high speed hub
PM: Adding info for usb:5-2
usb 5-2: configuration #1 chosen from 1 choice
PM: Adding info for usb:5-2:1.0
PM: Adding info for usb:5-2:1.1
PM: Adding info for usb:5-2:1.2
PM: Adding info for usb:5-2:1.3
input: Yealink usb-p1k
as /devices/pci0000:00/0000:00:1a.2/usb5/5-2/5-2:1.3/input/input8
usb 5-2: New USB device found, idVendor=6993, idProduct=b001
usb 5-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
usb 5-2: Product: VOIP USB Phone           
usb 5-2: Manufacturer: Yealink Network Technology Ltd.



Am Montag, den 30.06.2008, 16:08 +0200 schrieb Oliver Neukum:
> This implements basic suspend/resume support for yealink.
> Could somebody test it?
> 
> 	Regards
> 		Oliver
> 
> ---


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rft]suspend/resume support for yealink

[Oliver Neukum]

Am Montag 30 Juni 2008 23:47:08 schrieb Thomas Reitmayr:
> Hi Oliver,
> I tested the suspend/resume support with my Yealink USB-P1K doing the
> following:
> 
> * Suspend to RAM:
>   The device suspends properly, I hear the audio subsystem turning off
> while the display keeps its current state.
>   Apparently the USB root hub gets reset during suspend/resume according
> to the kernel log. Then after resume the input events are generated
> again and the display can be updated. I noticed however that the display
> is reset to its default text, possibly because of the USB reset? Anyway,
> I appended a filtered kernel log about this scenario below.

Yes, the reset triggers a disconnect because the driver doesn't implement
reset_resume().

> * Suspend the device by "echo suspend > power/level"
>   Works too with the difference that the display is not modified during
> resume.

OK, it works. Thank you for testing.

	Regards
		Oliver

Re: [rft]suspend/resume support for yealink

[Oliver Neukum]

Am Montag 30 Juni 2008 23:47:08 schrieb Thomas Reitmayr:
> * Suspend to RAM:
>   The device suspends properly, I hear the audio subsystem turning off
> while the display keeps its current state.
>   Apparently the USB root hub gets reset during suspend/resume according
> to the kernel log. Then after resume the input events are generated

This patch may help.

	Regards
		Oliver

---

--- linux-2.6.26-sierra/drivers/input/misc/yealink.alt3.c	2008-07-02 15:42:35.000000000 +0200
+++ linux-2.6.26-sierra/drivers/input/misc/yealink.c	2008-07-02 16:14:57.000000000 +0200
@@ -122,6 +122,9 @@ struct yealink_dev {
 	int shutdown:1;
 	int open:1;
 
+	u8 *old_ringtone;
+	int tonecount;
+
 	int	stat_ix;
 	union {
 		struct yld_status s;
@@ -564,6 +567,8 @@ static void input_close(struct input_dev
 
 	down_write(&sysfs_rwsema);
 	yld->open = 0;
+	kfree(yld->old_ringtone),
+	yld->old_ringtone = NULL;
 	up_write(&sysfs_rwsema);
 	stop_traffic(yld);
 }
@@ -767,6 +772,9 @@ static ssize_t store_ringtone(struct dev
 {
 	struct yealink_dev *yld;
 
+	if (!count)
+		return 0;
+
 	down_write(&sysfs_rwsema);
 	yld = dev_get_drvdata(dev);
 	if (yld == NULL) {
@@ -776,6 +784,14 @@ static ssize_t store_ringtone(struct dev
 
 	/* TODO locking with async usb control interface??? */
 	yealink_set_ringtone(yld, (char *)buf, count);
+	kfree(yld->old_ringtone);
+	yld->old_ringtone = kmalloc(count, GFP_KERNEL);
+	if (!yld->old_ringtone)
+		goto error_bail;
+	memcpy(yld->old_ringtone, buf, count);
+	yld->tonecount = count;
+
+error_bail:
 	up_write(&sysfs_rwsema);
 	return count;
 }
@@ -882,6 +898,30 @@ static int usb_resume(struct usb_interfa
 	up_read(&sysfs_rwsema);
 	return rv;
 }
+
+static int usb_reset_resume(struct usb_interface *intf)
+{
+	struct yealink_dev *yld = usb_get_intfdata(intf);
+	int rv = 0;
+
+	down_read(&sysfs_rwsema);
+	if (!yld->open)
+		goto skip_io;
+	if (yld->old_ringtone)
+		rv = yealink_set_ringtone(yld,
+					  yld->old_ringtone,
+					  yld->tonecount);
+	else
+		rv = yealink_set_ringtone(yld,
+					  default_ringtone,
+					  sizeof(default_ringtone));
+
+skip_io:
+	up_read(&sysfs_rwsema);
+
+	return rv;
+}
+
 static void usb_disconnect(struct usb_interface *intf)
 {
 	struct yealink_dev *yld;
@@ -1030,6 +1070,7 @@ static struct usb_driver yealink_driver
 	.disconnect	= usb_disconnect,
 	.suspend	= usb_suspend,
 	.resume		= usb_resume,
+	.reset_resume	= usb_reset_resume,
 	.id_table	= usb_table,
 };
 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rft]suspend/resume support for yealink

[Thomas Reitmayr]

Hi Oliver,
your patch did improve things, however I had to add the submission of
the control URB to get the update/scan cycle going (same as in
usb_resume). As yealink_set_ringtone uses synchronous USB submissions it
won't invoke the callback function. Also apart from restoring the
ringtone, I forced an update of the remaining device data (display,
LED, ...). Otherwise if during suspend the device is unplugged and
plugged in again, the driver does not know about the changes on the
display (for example) and the driver's data and the data stored in the
device are out of sync.
My test went all well with these additions.

The patch below builds on your latest patch.
Regards,
-Thomas

Signed-off-by: Thomas Reitmayr <treitmayr-lNf3axTzMcOzZXS1Dc/lvw@public.gmane.org>

---

--- linux-2.6.26-rc8/drivers/input/misc/yealink.old.c	2008-07-03 20:10:36.000000000 +0200
+++ linux-2.6.26-rc8/drivers/input/misc/yealink.c	2008-07-03 20:10:26.000000000 +0200
@@ -903,10 +903,18 @@ static int usb_reset_resume(struct usb_i
 {
 	struct yealink_dev *yld = usb_get_intfdata(intf);
 	int rv = 0;
+	int i;
 
 	down_read(&sysfs_rwsema);
 	if (!yld->open)
 		goto skip_io;
+
+	/* force updates to device */
+	for (i = 0; i<sizeof(yld->master); i++)
+		yld->copy.b[i] = ~yld->master.b[i];
+	yld->key_code = -1;	/* no keys pressed */
+
+	/* restore the ringtone */
 	if (yld->old_ringtone)
 		rv = yealink_set_ringtone(yld,
 					  yld->old_ringtone,
@@ -916,6 +924,10 @@ static int usb_reset_resume(struct usb_i
 					  default_ringtone,
 					  sizeof(default_ringtone));
 
+	/* restart updates and key scan */
+	if (rv == 0)
+		rv = usb_submit_urb(yld->urb_ctl, GFP_NOIO);
+
 skip_io:
 	up_read(&sysfs_rwsema);
 
--



Am Mittwoch, den 02.07.2008, 16:26 +0200 schrieb Oliver Neukum:
> Am Montag 30 Juni 2008 23:47:08 schrieb Thomas Reitmayr:
> > * Suspend to RAM:
> >   The device suspends properly, I hear the audio subsystem turning off
> > while the display keeps its current state.
> >   Apparently the USB root hub gets reset during suspend/resume according
> > to the kernel log. Then after resume the input events are generated
> 
> This patch may help.
> 
> 	Regards
> 		Oliver
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rft]suspend/resume support for yealink

[Thomas Reitmayr]

Hi Oliver,
I just wanted to make sure my little patch on top of yours did not get
missed in all the other stuff going on. Does it look ok to you (and
possibly others) to be added?
Regards,
-Thomas

Am Donnerstag, den 03.07.2008, 20:27 +0200 schrieb Thomas Reitmayr:
> Hi Oliver,
> your patch did improve things, however I had to add the submission of
> the control URB to get the update/scan cycle going (same as in
> usb_resume). As yealink_set_ringtone uses synchronous USB submissions it
> won't invoke the callback function. Also apart from restoring the
> ringtone, I forced an update of the remaining device data (display,
> LED, ...). Otherwise if during suspend the device is unplugged and
> plugged in again, the driver does not know about the changes on the
> display (for example) and the driver's data and the data stored in the
> device are out of sync.
> My test went all well with these additions.
> 
> The patch below builds on your latest patch.
> Regards,
> -Thomas
> 
> Signed-off-by: Thomas Reitmayr <treitmayr@devbase.at>
> 
> ---
> 
> --- linux-2.6.26-rc8/drivers/input/misc/yealink.old.c	2008-07-03 20:10:36.000000000 +0200
> +++ linux-2.6.26-rc8/drivers/input/misc/yealink.c	2008-07-03 20:10:26.000000000 +0200
> @@ -903,10 +903,18 @@ static int usb_reset_resume(struct usb_i
>  {
>  	struct yealink_dev *yld = usb_get_intfdata(intf);
>  	int rv = 0;
> +	int i;
>  
>  	down_read(&sysfs_rwsema);
>  	if (!yld->open)
>  		goto skip_io;
> +
> +	/* force updates to device */
> +	for (i = 0; i<sizeof(yld->master); i++)
> +		yld->copy.b[i] = ~yld->master.b[i];
> +	yld->key_code = -1;	/* no keys pressed */
> +
> +	/* restore the ringtone */
>  	if (yld->old_ringtone)
>  		rv = yealink_set_ringtone(yld,
>  					  yld->old_ringtone,
> @@ -916,6 +924,10 @@ static int usb_reset_resume(struct usb_i
>  					  default_ringtone,
>  					  sizeof(default_ringtone));
>  
> +	/* restart updates and key scan */
> +	if (rv == 0)
> +		rv = usb_submit_urb(yld->urb_ctl, GFP_NOIO);
> +
>  skip_io:
>  	up_read(&sysfs_rwsema);
>  
> --
> 
> 
> 
> Am Mittwoch, den 02.07.2008, 16:26 +0200 schrieb Oliver Neukum:
> > Am Montag 30 Juni 2008 23:47:08 schrieb Thomas Reitmayr:
> > > * Suspend to RAM:
> > >   The device suspends properly, I hear the audio subsystem turning off
> > > while the display keeps its current state.
> > >   Apparently the USB root hub gets reset during suspend/resume according
> > > to the kernel log. Then after resume the input events are generated
> > 
> > This patch may help.
> > 
> > 	Regards
> > 		Oliver
> > 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>