Re: [PATCH V5 1/3] rpmsg: core: Add signal API support

[Bjorn Andersson]

On Wed, Apr 19, 2023 at 11:53:49AM +0200, Arnaud POULIQUEN wrote:
> Hi,
> 
> 
> On 4/18/23 10:48, Sarannya S wrote:
> > From: Deepak Kumar Singh <quic_deesin@xxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Sarannya S <quic_sarannya@xxxxxxxxxxx>
> > ---
> >  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
> 

You're right, and I missed that this was gone again. Thanks for pointing
it out. I've backed this series out of rpmsg-next again.

v6 seems to carry the correct argument again...

Regards,
Bjorn

> 
> [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);
> >  };
> >