[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

The F81232 can do remote wakeup via RX/RI pin with pulse.
This patch will use device_set_wakeup_enable to enable this
feature.

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index bdd7f337cd5f..dadf024ae494 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -742,6 +742,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
 	port->port.drain_delay = 256;
 	priv->port = port;
 
+	device_set_wakeup_enable(&port->serial->dev->dev, true);
 	return 0;
 }
 
@@ -752,6 +753,7 @@ static int f81232_port_remove(struct usb_serial_port *port)
 	priv = usb_get_serial_port_data(port);
 	kfree(priv);
 
+	device_set_wakeup_enable(&port->serial->dev->dev, false);
 	return 0;
 }
 

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Johan Hovold]

On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The F81232 can do remote wakeup via RX/RI pin with pulse.
> This patch will use device_set_wakeup_enable to enable this
> feature.

This is a policy decision that should be made by user space by setting
the power/wakeup attribute, and not something that something that
drivers should enable directly themselves.

Perhaps you really wanted to use device_set_wakeup_capable()? But then
you also need to honour the current setting in suspend() as well.

How have you tested this feature?

Johan
---
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

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The F81232 can do remote wakeup via RX/RI pin with pulse.
>> This patch will use device_set_wakeup_enable to enable this
>> feature.
> 
> This is a policy decision that should be made by user space by setting
> the power/wakeup attribute, and not something that something that
> drivers should enable directly themselves.
> 
> Perhaps you really wanted to use device_set_wakeup_capable()? But then
> you also need to honour the current setting in suspend() as well.
> 
> How have you tested this feature?
> 

Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
other peripherals and we had tested it by following procedure with
device_set_wakeup_enable() enabled:
     1. Using pm-suspend to S3
     2. Trigger a pulse to RI/RX to wake up system.

In our test, we can do remote wakeup only with
device_set_wakeup_enable() enabled.

Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
like following link??
https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476

Thanks

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Johan Hovold]

On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
> 
> Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> > On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The F81232 can do remote wakeup via RX/RI pin with pulse.
> >> This patch will use device_set_wakeup_enable to enable this
> >> feature.
> > 
> > This is a policy decision that should be made by user space by setting
> > the power/wakeup attribute, and not something that something that
> > drivers should enable directly themselves.
> > 
> > Perhaps you really wanted to use device_set_wakeup_capable()? But then
> > you also need to honour the current setting in suspend() as well.
> > 
> > How have you tested this feature?
> > 
> 
> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
> other peripherals and we had tested it by following procedure with
> device_set_wakeup_enable() enabled:
>      1. Using pm-suspend to S3
>      2. Trigger a pulse to RI/RX to wake up system.
> 
> In our test, we can do remote wakeup only with
> device_set_wakeup_enable() enabled.

Yeah, but you need to enable it though sysfs. Not every device should be
able to wake the system up. That's a decision left for user space.

> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
> like following link??
> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476

No, your driver should not call device_set_wakeup_enable() itself. Just
set the wakeup capable flag in probe. And if you can disable the wake up
feature, this needs to be done at suspend depending on what user space
has requested.

Johan
---
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

[3/5] USB: serial: f81232: enable remote wakeup via RX/RI pin

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2018/2/4 上午 09:46 寫道:
> On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
>> other peripherals and we had tested it by following procedure with
>> device_set_wakeup_enable() enabled:
>>       1. Using pm-suspend to S3
>>       2. Trigger a pulse to RI/RX to wake up system.
>>
>> In our test, we can do remote wakeup only with
>> device_set_wakeup_enable() enabled.
> 
> Yeah, but you need to enable it though sysfs. Not every device should be
> able to wake the system up. That's a decision left for user space.
> 
>> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
>> like following link??
>> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476
> 
> No, your driver should not call device_set_wakeup_enable() itself. Just
> set the wakeup capable flag in probe. And if you can disable the wake up
> feature, this needs to be done at suspend depending on what user space
> has requested.

We'll change to device_set_wakeup_capable() and send the v2 patch when
test OK.

Thanks