| 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: <Yiu7DPHDY3uwcnLK@builder.lan>
Date: Fri, 11 Mar 2022 15:11:40 -0600
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: Deepak Kumar Singh <quic_deesin@...cinc.com>
Cc: swboyd@...omium.org, quic_clew@...cinc.com,
mathieu.poirier@...aro.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
Ohad Ben-Cohen <ohad@...ery.com>
Subject: Re: [PATCH V2 1/3] rpmsg: core: Add signal API support
On Tue 18 Jan 13:43 CST 2022, Deepak Kumar Singh wrote:
> Some transports like Glink support the state notifications between
> clients using signals similar to serial protocol signals.
> Local glink client drivers can send and receive signals to glink
> clients running on remote processors.
>
> Add APIs to support sending and receiving of signals by rpmsg clients.
>
> Signed-off-by: Deepak Kumar Singh <quic_deesin@...cinc.com>
> ---
> drivers/rpmsg/rpmsg_core.c | 21 +++++++++++++++++++++
> drivers/rpmsg/rpmsg_internal.h | 2 ++
> include/linux/rpmsg.h | 14 ++++++++++++++
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index d3eb600..6712418 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -328,6 +328,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)
This API looks nice and clean and deals with flow control.
> +{
> + 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
> *
> @@ -535,6 +553,9 @@ static int rpmsg_dev_probe(struct device *dev)
>
> rpdev->ept = ept;
> rpdev->src = ept->addr;
> +
> + if (rpdrv->signals)
> + ept->sig_cb = rpdrv->signals;
> }
>
> err = rpdrv->probe(rpdev);
> diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
> index b1245d3..35c2197 100644
> --- a/drivers/rpmsg/rpmsg_internal.h
> +++ b/drivers/rpmsg/rpmsg_internal.h
> @@ -53,6 +53,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.
> @@ -73,6 +74,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 02fa911..06d090c 100644
> --- a/include/linux/rpmsg.h
> +++ b/include/linux/rpmsg.h
> @@ -62,12 +62,14 @@ struct rpmsg_device {
> };
>
> typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
> +typedef int (*rpmsg_rx_sig_t)(struct rpmsg_device *, void *, u32);
This callback however, is still using the original low level tty
signals.
Is there any reason why this can't be "rpmsg_flowcontrol_cb_t" and take
a boolean, so we get a clean interface in both directions?
Regards,
Bjorn
>
> /**
> * 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
> + * @sig_cb: rx serial signal handler
> * @cb_lock: must be taken before accessing/changing @cb
> * @addr: local rpmsg address
> * @priv: private data for the driver's use
> @@ -90,6 +92,7 @@ struct rpmsg_endpoint {
> struct rpmsg_device *rpdev;
> struct kref refcount;
> rpmsg_rx_cb_t cb;
> + rpmsg_rx_sig_t sig_cb;
> struct mutex cb_lock;
> u32 addr;
> void *priv;
> @@ -111,6 +114,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 (*signals)(struct rpmsg_device *rpdev, void *priv, u32);
> };
>
> static inline u16 rpmsg16_to_cpu(struct rpmsg_device *rpdev, __rpmsg16 val)
> @@ -188,6 +192,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(struct rpmsg_device *rpdev)
> @@ -306,6 +312,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 */
> --
> 2.7.4
>
Powered by blists - more mailing lists