[v2,4/4] mfd: dln2: add suspend/resume functionality
diff mbox

Message ID 1418745435-2851-5-git-send-email-octavian.purdila@intel.com
State Not Applicable
Headers show

Commit Message

Octavian Purdila Dec. 16, 2014, 3:57 p.m. UTC
Without suspend/resume functionality in the USB driver the USB core
will disconnect and reconnect the DLN2 port and because the GPIO
framework does not yet support removal of an in-use controller a
suspend/resume operation will result in a crash.

This patch provides suspend and resume functions for the DLN2 driver
so that the above scenario is avoided.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/mfd/dln2.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Johan Hovold Jan. 7, 2015, 10:26 a.m. UTC | #1
On Tue, Dec 16, 2014 at 05:57:15PM +0200, Octavian Purdila wrote:
> Without suspend/resume functionality in the USB driver the USB core
> will disconnect and reconnect the DLN2 port and because the GPIO
> framework does not yet support removal of an in-use controller a
> suspend/resume operation will result in a crash.
> 
> This patch provides suspend and resume functions for the DLN2 driver
> so that the above scenario is avoided.
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>

This patch looks good now, but how did you say this device was powered?

If powered by vbus you cannot assume that the device maintains it's
state over a suspend cycle, something which would complicate matters
quite a bit...

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Octavian Purdila Jan. 7, 2015, 1:16 p.m. UTC | #2
On Wed, Jan 7, 2015 at 11:26 PM, Johan Hovold <johan@kernel.org> wrote:
> On Tue, Dec 16, 2014 at 05:57:15PM +0200, Octavian Purdila wrote:
>> Without suspend/resume functionality in the USB driver the USB core
>> will disconnect and reconnect the DLN2 port and because the GPIO
>> framework does not yet support removal of an in-use controller a
>> suspend/resume operation will result in a crash.
>>
>> This patch provides suspend and resume functions for the DLN2 driver
>> so that the above scenario is avoided.
>>
>> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
>
> This patch looks good now, but how did you say this device was powered?
>
> If powered by vbus you cannot assume that the device maintains it's
> state over a suspend cycle, something which would complicate matters
> quite a bit...
>

Yes, the device is powered by VBUS. During my tests, depending on the
host and USB port, VBUS is sometimes preserved - and this is the case
this patch addresses, and sometimes is not, but in that case because
no reset_resume routine is implemented the resume path will go through
the disconnect/reconnect process. This second case is not addressed by
the patch, as I think in this second case fixing the GPIO framework to
support the removal of an in-use device is the best way to go.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hovold Jan. 7, 2015, 2:26 p.m. UTC | #3
On Thu, Jan 08, 2015 at 02:16:15AM +1300, Octavian Purdila wrote:
> On Wed, Jan 7, 2015 at 11:26 PM, Johan Hovold <johan@kernel.org> wrote:
> > On Tue, Dec 16, 2014 at 05:57:15PM +0200, Octavian Purdila wrote:
> >> Without suspend/resume functionality in the USB driver the USB core
> >> will disconnect and reconnect the DLN2 port and because the GPIO
> >> framework does not yet support removal of an in-use controller a
> >> suspend/resume operation will result in a crash.
> >>
> >> This patch provides suspend and resume functions for the DLN2 driver
> >> so that the above scenario is avoided.
> >>
> >> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> >
> > This patch looks good now, but how did you say this device was powered?
> >
> > If powered by vbus you cannot assume that the device maintains it's
> > state over a suspend cycle, something which would complicate matters
> > quite a bit...
> 
> Yes, the device is powered by VBUS. During my tests, depending on the
> host and USB port, VBUS is sometimes preserved - and this is the case
> this patch addresses, and sometimes is not, but in that case because
> no reset_resume routine is implemented the resume path will go through
> the disconnect/reconnect process.

Yes, it depends on the host controller and I believe the common case is
to drop VBUS.

> This second case is not addressed by
> the patch, as I think in this second case fixing the GPIO framework to
> support the removal of an in-use device is the best way to go.

I agree.

But please do mention that this case is not handled in the commit
message.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
index 75358d2..f9c4a0b 100644
--- a/drivers/mfd/dln2.c
+++ b/drivers/mfd/dln2.c
@@ -791,6 +791,24 @@  out_free:
 	return ret;
 }
 
+static int dln2_suspend(struct usb_interface *iface, pm_message_t message)
+{
+	struct dln2_dev *dln2 = usb_get_intfdata(iface);
+
+	dln2_stop(dln2);
+
+	return 0;
+}
+
+static int dln2_resume(struct usb_interface *iface)
+{
+	struct dln2_dev *dln2 = usb_get_intfdata(iface);
+
+	dln2->disconnect = false;
+
+	return dln2_start_rx_urbs(dln2, GFP_NOIO);
+}
+
 static const struct usb_device_id dln2_table[] = {
 	{ USB_DEVICE(0xa257, 0x2013) },
 	{ }
@@ -803,6 +821,8 @@  static struct usb_driver dln2_driver = {
 	.probe = dln2_probe,
 	.disconnect = dln2_disconnect,
 	.id_table = dln2_table,
+	.suspend = dln2_suspend,
+	.resume = dln2_resume,
 };
 
 module_usb_driver(dln2_driver);