From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95130C77B73 for ; Mon, 24 Apr 2023 12:50:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231716AbjDXMuD (ORCPT ); Mon, 24 Apr 2023 08:50:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231799AbjDXMuB (ORCPT ); Mon, 24 Apr 2023 08:50:01 -0400 Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 11E8B44A4; Mon, 24 Apr 2023 05:49:53 -0700 (PDT) Received: from pps.filterd (m0288072.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 33O9Nvji012062; Mon, 24 Apr 2023 14:49:38 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=n+onXhQc/Z9zA1qo+XmGpT4jO0zjGOwnR5I8Oi1IVS8=; b=AQcDXT5WlQetPs5EXyPbKvxWAtU686ZmV/UELOOTYvJLi3Y7wnXAJJd8ETAnk1pt++As MuU6HfyS87Hb1dXbfh6SiZIbscr1j2qh4ptSkQ35DVmot4Pqg3x0YF4w/ORo+tDl+N2V 39bl3Is/pQEl8uweWQC5hY7OUcPM2nJZLS4SCUn8xB0kjsV/GIu5U9NAK9/V9k0pegng DQKxMaSEwzMo7Am8i5iRVYEGlytrhjD9/zRH0xkodX4vs977FQoz/QN0240YiRTAQJj1 eRfXgNqENGFuZ9SbOYFr6fLO3OTIxo1bYca9xL1p0fq2VOK4eHsP3sPyGD34M2XSxup7 EQ== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3q48dwbf7y-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 24 Apr 2023 14:49:38 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id C067510002A; Mon, 24 Apr 2023 14:49:32 +0200 (CEST) Received: from Webmail-eu.st.com (shfdag1node2.st.com [10.75.129.70]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id B4C6D22D17C; Mon, 24 Apr 2023 14:49:32 +0200 (CEST) Received: from [10.252.21.25] (10.252.21.25) by SHFDAG1NODE2.st.com (10.75.129.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Mon, 24 Apr 2023 14:49:29 +0200 Message-ID: Date: Mon, 24 Apr 2023 14:49:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH V7 1/3] rpmsg: core: Add signal API support Content-Language: en-US To: Sarannya S , , , , CC: , , , Deepak Kumar Singh , Bjorn Andersson References: <1682160127-18103-1-git-send-email-quic_sarannya@quicinc.com> <1682160127-18103-2-git-send-email-quic_sarannya@quicinc.com> From: Arnaud POULIQUEN Organization: STMicroelectronics In-Reply-To: <1682160127-18103-2-git-send-email-quic_sarannya@quicinc.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.252.21.25] X-ClientProxiedBy: SHFCAS1NODE1.st.com (10.75.129.72) To SHFDAG1NODE2.st.com (10.75.129.70) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-04-24_07,2023-04-21_01,2023-02-09_01 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Hello, On 4/22/23 12:42, Sarannya S wrote: > From: Deepak Kumar Singh > > 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 > Signed-off-by: Sarannya S > --- > drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++ > drivers/rpmsg/rpmsg_internal.h | 2 ++ > include/linux/rpmsg.h | 15 +++++++++++++++ > 3 files changed, 38 insertions(+) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index a2207c0..e8bbe05 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -331,6 +331,25 @@ 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 > + * @dst: destination address of the endpoint > + * > + * Return: 0 on success and an appropriate error value on failure. > + */ > +int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable, u32 dst) > +{ > + if (WARN_ON(!ept)) > + return -EINVAL; > + if (!ept->ops->set_flow_control) > + return -ENXIO; Here we return an error if the backend does not implement the ops. But the set_flow_control ops is optional. Should we return 0 instead with a debug message? > + > + return ept->ops->set_flow_control(ept, enable, dst); > +} > +EXPORT_SYMBOL_GPL(rpmsg_set_flow_control); > + > +/** > * rpmsg_get_mtu() - get maximum transmission buffer size for sending message. > * @ept: the rpmsg endpoint > * > @@ -539,6 +558,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..b6efd3e 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, u32 dst); > ssize_t (*get_mtu)(struct rpmsg_endpoint *ept); > }; > > diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h > index 523c98b..a0e9d38 100644 > --- a/include/linux/rpmsg.h > +++ b/include/linux/rpmsg.h > @@ -64,12 +64,14 @@ struct rpmsg_device { > }; > > typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32); > +typedef int (*rpmsg_flowcontrol_cb_t)(struct rpmsg_device *, void *, bool); > > /** > * struct rpmsg_endpoint - binds a local rpmsg address to its user > * @rpdev: rpmsg channel device > * @refcount: when this drops to zero, the ept is deallocated > * @cb: rx callback handler > + * @flow_cb: remote flow control callback handler > * @cb_lock: must be taken before accessing/changing @cb > * @addr: local rpmsg address > * @priv: private data for the driver's use > @@ -92,6 +94,7 @@ struct rpmsg_endpoint { > struct rpmsg_device *rpdev; > struct kref refcount; > rpmsg_rx_cb_t cb; > + rpmsg_flowcontrol_cb_t flow_cb; > struct mutex cb_lock; > u32 addr; > void *priv; > @@ -106,6 +109,7 @@ struct rpmsg_endpoint { > * @probe: invoked when a matching rpmsg channel (i.e. device) is found > * @remove: invoked when the rpmsg channel is removed > * @callback: invoked when an inbound message is received on the channel > + * @flowcontrol: invoked when remote side flow control status is received > */ > struct rpmsg_driver { > struct device_driver drv; > @@ -113,6 +117,7 @@ struct rpmsg_driver { > int (*probe)(struct rpmsg_device *dev); > void (*remove)(struct rpmsg_device *dev); > int (*callback)(struct rpmsg_device *, void *, int, void *, u32); > + int (*flowcontrol)(struct rpmsg_device *, void *, bool); I wonder here if we should also add a parameter for the remote source address. This parameter is already exist for the int (*callback)(struct rpmsg_device *, void *, int, void *, u32) useful in case of point to multi point communication... Regards, Arnaud > }; > > static inline u16 rpmsg16_to_cpu(struct rpmsg_device *rpdev, __rpmsg16 val) > @@ -192,6 +197,8 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp, > > ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept); > > +int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable, u32 dst); > + > #else > > static inline int rpmsg_register_device_override(struct rpmsg_device *rpdev, > @@ -316,6 +323,14 @@ static inline ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept) > return -ENXIO; > } > > +static inline int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable, u32 dst) > +{ > + /* This shouldn't be possible */ > + WARN_ON(1); > + > + return -ENXIO; > +} > + > #endif /* IS_ENABLED(CONFIG_RPMSG) */ > > /* use a macro to avoid include chaining to get THIS_MODULE */