mbox series

[V5,0/3] rpmsg signaling/flowcontrol patches

Message ID 1681807721-32343-1-git-send-email-quic_sarannya@quicinc.com
Headers show
Series rpmsg signaling/flowcontrol patches | expand

Message

Sarannya S April 18, 2023, 8:48 a.m. UTC
Added new IOCTLS- RPMSG_GET_SIGNAL_IOCTL and RPMSG_SET_SIGNAL_IOCTL
to get/set the rpmsg char device's flow control signal.
Update the cmd name 'RPM_CMD_SIGNALS' to 'GLINK_CMD_SIGNALS'.
Addressed review comments to change variable names/descriptions. 

Chris Lew (2):
  rpmsg: glink: Add support to handle signals command
  rpmsg: char: Add RPMSG GET/SET SIGNAL IOCTL support

Deepak Kumar Singh (1):
  rpmsg: core: Add signal API support

 drivers/rpmsg/qcom_glink_native.c | 63 +++++++++++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_char.c        | 58 ++++++++++++++++++++++++++++++-----
 drivers/rpmsg/rpmsg_core.c        | 20 +++++++++++++
 drivers/rpmsg/rpmsg_internal.h    |  2 ++
 include/linux/rpmsg.h             | 15 ++++++++++
 include/uapi/linux/rpmsg.h        | 12 +++++++-
 6 files changed, 162 insertions(+), 8 deletions(-)

Comments

Bjorn Andersson April 18, 2023, 4:09 p.m. UTC | #1
On Tue, 18 Apr 2023 14:18:38 +0530, Sarannya S wrote:
> Added new IOCTLS- RPMSG_GET_SIGNAL_IOCTL and RPMSG_SET_SIGNAL_IOCTL
> to get/set the rpmsg char device's flow control signal.
> Update the cmd name 'RPM_CMD_SIGNALS' to 'GLINK_CMD_SIGNALS'.
> Addressed review comments to change variable names/descriptions.
> 
> Chris Lew (2):
>   rpmsg: glink: Add support to handle signals command
>   rpmsg: char: Add RPMSG GET/SET SIGNAL IOCTL support
> 
> [...]

Applied, thanks!

[1/3] rpmsg: core: Add signal API support
      commit: cc7c3bc62c6c74c217ea73e30a135fd2c3affb34
[2/3] rpmsg: glink: Add support to handle signals command
      commit: 96a7a78517751f01c2a3aeef46768159aca370fd
[3/3] rpmsg: char: Add RPMSG GET/SET SIGNAL IOCTL support
      commit: bdae2e497fd6cb1de0f142f3c64d89a38e3576e3

Best regards,
Arnaud POULIQUEN April 19, 2023, 9:53 a.m. UTC | #2
Hi,


On 4/18/23 10:48, Sarannya S wrote:
> From: Deepak Kumar Singh <quic_deesin@quicinc.com>
> 
> Some transports like Glink support the state notifications between
> clients using flow control signals similar to serial protocol signals.
> Local glink client drivers can send and receive flow control status
> to glink clients running on remote processors.
> 
> Add APIs to support sending and receiving of flow control status by
> rpmsg clients.
> 
> Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com>
> Signed-off-by: Sarannya S <quic_sarannya@quicinc.com>
> ---
>  drivers/rpmsg/rpmsg_core.c     | 20 ++++++++++++++++++++
>  drivers/rpmsg/rpmsg_internal.h |  2 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index a2207c0..86b4912 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -331,6 +331,24 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
>  EXPORT_SYMBOL(rpmsg_trysend_offchannel);
>  
>  /**
> + * rpmsg_set_flow_control() - sets/clears serial flow control signals
> + * @ept:	the rpmsg endpoint
> + * @enable:	pause/resume incoming data flow
> + *
> + * Return: 0 on success and an appropriate error value on failure.
> + */
> +int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable)


Regression since V4[1]

In V4 version the function was:

int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable, u32 dst)

Following comments on V3 [2]

Without dst parameter it is not compatible with the rpmsg_virtio backend


[1]https://lkml.org/lkml/2022/12/7/506
[2]https://www.spinics.net/lists/kernel/msg4573082.html

Regards
Arnaud

> +{
> +	if (WARN_ON(!ept))
> +		return -EINVAL;
> +	if (!ept->ops->set_flow_control)
> +		return -ENXIO;
> +
> +	return ept->ops->set_flow_control(ept, enable);
> +}
> +EXPORT_SYMBOL(rpmsg_set_flow_control);
> +
> +/**
>   * rpmsg_get_mtu() - get maximum transmission buffer size for sending message.
>   * @ept: the rpmsg endpoint
>   *
> @@ -539,6 +557,8 @@ static int rpmsg_dev_probe(struct device *dev)
>  
>  		rpdev->ept = ept;
>  		rpdev->src = ept->addr;
> +
> +		ept->flow_cb = rpdrv->flowcontrol;
>  	}
>  
>  	err = rpdrv->probe(rpdev);
> diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
> index 39b646d..4fea45a 100644
> --- a/drivers/rpmsg/rpmsg_internal.h
> +++ b/drivers/rpmsg/rpmsg_internal.h
> @@ -55,6 +55,7 @@ struct rpmsg_device_ops {
>   * @trysendto:		see @rpmsg_trysendto(), optional
>   * @trysend_offchannel:	see @rpmsg_trysend_offchannel(), optional
>   * @poll:		see @rpmsg_poll(), optional
> + * @set_flow_control:	see @rpmsg_set_flow_control(), optional
>   * @get_mtu:		see @rpmsg_get_mtu(), optional
>   *
>   * Indirection table for the operations that a rpmsg backend should implement.
> @@ -75,6 +76,7 @@ struct rpmsg_endpoint_ops {
>  			     void *data, int len);
>  	__poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
>  			     poll_table *wait);
> +	int (*set_flow_control)(struct rpmsg_endpoint *ept, bool enable);
>  	ssize_t (*get_mtu)(struct rpmsg_endpoint *ept);
>  };
>