| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <17c9dab2-c636-84a4-2b56-e83e3ea9fcc6@quicinc.com>
Date: Thu, 1 Dec 2022 16:57:09 +0530
From: Sarannya S <quic_sarannya@...cinc.com>
To: Mathieu Poirier <mathieu.poirier@...aro.org>,
Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
CC: <quic_bjorande@...cinc.com>, <swboyd@...omium.org>,
<quic_clew@...cinc.com>, <linux-kernel@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>,
<linux-remoteproc@...r.kernel.org>,
"Deepak Kumar Singh" <quic_deesin@...cinc.com>,
Bjorn Andersson <andersson@...nel.org>
Subject: Re: [PATCH V4 1/3] rpmsg: core: Add signal API support
On 11/30/2022 11:53 PM, Mathieu Poirier wrote:
> On Tue, 29 Nov 2022 at 02:29, Arnaud POULIQUEN
> <arnaud.pouliquen@...s.st.com> wrote:
>>
>> Hi Sarannya,
>>
>> On 11/28/22 19:02, Sarannya S wrote:
>>> 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@...cinc.com>
>>> Signed-off-by: Sarannya S <quic_sarannya@...cinc.com>
>>> ---
>>> drivers/rpmsg/rpmsg_core.c | 20 ++++++++++++++++++++
>>> drivers/rpmsg/rpmsg_internal.h | 2 ++
>>> include/linux/rpmsg.h | 15 +++++++++++++++
>>> 3 files changed, 37 insertions(+)
>>>
>>> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
>>> index d6dde00e..0c5bf67 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: enable or disable serial flow control
>>> + *
>>> + * Return: 0 on success and an appropriate error value on failure.
>>> + */
>>> +int rpmsg_set_flow_control(struct rpmsg_endpoint *ept, bool enable)
>>
>> Seems that you did not take into account comment in your V3 asking you to
>> add the destination address of the endpoint as parameter
>
> I will not review this patchset until Arnaud's comment is addressed or
> a reason for the omission is provided.
>
Arnaud, Mathieu,
Sorry, I did not find this comment in V3 patch.
I will update the patch by adding destination address as parameter.
Regards,
Sarannya
>>
>> 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);
>>> };
>>>
>>> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
>>> index 523c98b..cc7a917 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);
>>> };
>>>
>>> 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);
>>> +
>>> #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)
>>> +{
>>> + /* 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 */
Powered by blists - more mailing lists