Re: [PATCH] rpmsg: qcom: glink: support waking up on channel rx

[Bjorn Andersson <andersson@kernel.org>]

On Tue, Jan 17, 2023 at 02:24:13PM +0000, Caleb Connolly wrote:
> Configure all channels as wakeup capable and report a wakeup event
> when data is received.
> 
> This allows userspace to "subscribe" to a particular channel where
> it is useful to wake up to process new data. The expected usecase
> is to allow for handling incoming SMS or phone calls where the only
> notification mechanism is via QRTR on the IPCRTR glink channel.
> 
> As this behaviour is likely undesirable for most users, this patch
> doesn't enable a wakeup_source for any channels by default.
> 

This looks reasonable to me.

One suggestion that came up as we discussed this problem (elsewhere) a
while ago was the idea of using EPOLLWAKEUP to control the wakeup source
on a per-socket basis.

Would that suite your userspace?

Regards,
Bjorn

> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  drivers/rpmsg/qcom_glink_native.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 115c0a1eddb1..1a96a7ae23bb 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -914,6 +914,9 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
>  		channel->buf = NULL;
>  
>  		qcom_glink_rx_done(glink, channel, intent);
> +
> +		pm_wakeup_ws_event(channel->ept.rpdev->dev.power.wakeup, 0,
> +				   true);
>  	}
>  
>  advance_rx:
> @@ -1510,6 +1513,17 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
>  		if (ret)
>  			goto rcid_remove;
>  
> +		/*
> +		 * Declare all channels as wakeup capable, but don't enable
> +		 * waking up by default.
> +		 *
> +		 * Userspace may wish to be woken up for incoming messages on a
> +		 * specific channel, for example to handle incoming calls or SMS
> +		 * messages on the IPCRTR channel. This can be done be enabling
> +		 * the wakeup source via sysfs.
> +		 */
> +		device_set_wakeup_capable(&rpdev->dev, true);
> +
>  		channel->rpdev = rpdev;
>  	}
>  
> -- 
> 2.39.0
>