2020-09-02 16:28:08

by Deepak Kumar Singh

[permalink] [raw]
Subject: [PATCH V7 0/4] Signaling api support in glink/rpmsg clients

Change from version 6
In last series(v6) i had put wrong version(v5) for cover note.
Which led to confusion for patch set series.

In this series i have updated the label for cover letter(v7).
There is no change in patches. Only cover note label is updated.

Change from version 5
[V6,4/4] rpmsg: char: Add signal callback and POLLPRI support
Updated for sparse warning. Replaced POLLPRI => EPOLLPRI to fix
warning.

Change from version 4
I am taking over these patches from [email protected]
Fixed all the trivial review comments.

Signal conversion to and from native signal as done in patch V4,2/4
is intentional.

Arun Kumar Neelakantam (3):
rpmsg: glink: Add support to handle signals command
rpmsg: char: Add TIOCMGET/TIOCMSET ioctl support
rpmsg: char: Add signal callback and POLLPRI support

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

drivers/rpmsg/qcom_glink_native.c | 125 ++++++++++++++++++++++++++++++++++++++
drivers/rpmsg/rpmsg_char.c | 76 ++++++++++++++++++++++-
drivers/rpmsg/rpmsg_core.c | 40 ++++++++++++
drivers/rpmsg/rpmsg_internal.h | 5 ++
include/linux/rpmsg.h | 27 ++++++++
5 files changed, 270 insertions(+), 3 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-09-02 16:28:38

by Deepak Kumar Singh

[permalink] [raw]
Subject: [PATCH V7 1/4] rpmsg: core: Add signal API support

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: Chris Lew <[email protected]>
Signed-off-by: Deepak Kumar Singh <[email protected]>
Signed-off-by: Arun Kumar Neelakantam <[email protected]>
---
drivers/rpmsg/rpmsg_core.c | 40 ++++++++++++++++++++++++++++++++++++++++
drivers/rpmsg/rpmsg_internal.h | 5 +++++
include/linux/rpmsg.h | 27 +++++++++++++++++++++++++++
3 files changed, 72 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index a6361ca..06733a6 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -283,6 +283,42 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
}
EXPORT_SYMBOL(rpmsg_trysend_offchannel);

+/**
+ * rpmsg_get_signals() - get the signals for this endpoint
+ * @ept: the rpmsg endpoint
+ *
+ * Returns signal bits on success and an appropriate error value on failure.
+ */
+int rpmsg_get_signals(struct rpmsg_endpoint *ept)
+{
+ if (WARN_ON(!ept))
+ return -EINVAL;
+ if (!ept->ops->get_signals)
+ return -ENXIO;
+
+ return ept->ops->get_signals(ept);
+}
+EXPORT_SYMBOL(rpmsg_get_signals);
+
+/**
+ * rpmsg_set_signals() - set the remote signals for this endpoint
+ * @ept: the rpmsg endpoint
+ * @set: set mask for signals
+ * @clear: clear mask for signals
+ *
+ * Returns 0 on success and an appropriate error value on failure.
+ */
+int rpmsg_set_signals(struct rpmsg_endpoint *ept, u32 set, u32 clear)
+{
+ if (WARN_ON(!ept))
+ return -EINVAL;
+ if (!ept->ops->set_signals)
+ return -ENXIO;
+
+ return ept->ops->set_signals(ept, set, clear);
+}
+EXPORT_SYMBOL(rpmsg_set_signals);
+
/*
* match a rpmsg channel with a channel info struct.
* this is used to make sure we're not creating rpmsg devices for channels
@@ -468,6 +504,10 @@ 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 3fc83cd..8958d6c 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -2,6 +2,7 @@
/*
* remote processor messaging bus internals
*
+ * Copyright (c) 2018, The Linux Foundation.
* Copyright (C) 2011 Texas Instruments, Inc.
* Copyright (C) 2011 Google, Inc.
*
@@ -47,6 +48,8 @@ struct rpmsg_device_ops {
* @trysendto: see @rpmsg_trysendto(), optional
* @trysend_offchannel: see @rpmsg_trysend_offchannel(), optional
* @poll: see @rpmsg_poll(), optional
+ * @get_signals: see @rpmsg_get_signals(), optional
+ * @set_signals: see @rpmsg_set_signals(), optional
*
* Indirection table for the operations that a rpmsg backend should implement.
* In addition to @destroy_ept, the backend must at least implement @send and
@@ -66,6 +69,8 @@ struct rpmsg_endpoint_ops {
void *data, int len);
__poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp,
poll_table *wait);
+ int (*get_signals)(struct rpmsg_endpoint *ept);
+ int (*set_signals)(struct rpmsg_endpoint *ept, u32 set, u32 clear);
};

int rpmsg_register_device(struct rpmsg_device *rpdev);
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 9fe156d..c4dbb47 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -2,6 +2,7 @@
/*
* Remote processor messaging
*
+ * Copyright (c) 2018 The Linux Foundation.
* Copyright (C) 2011 Texas Instruments, Inc.
* Copyright (C) 2011 Google, Inc.
* All rights reserved.
@@ -60,6 +61,7 @@ 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, u32);

/**
* struct rpmsg_endpoint - binds a local rpmsg address to its user
@@ -67,6 +69,7 @@ typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32);
* @refcount: when this drops to zero, the ept is deallocated
* @cb: rx callback handler
* @cb_lock: must be taken before accessing/changing @cb
+ * @sig_cb: rx serial signal handler
* @addr: local rpmsg address
* @priv: private data for the driver's use
*
@@ -89,6 +92,7 @@ struct rpmsg_endpoint {
struct kref refcount;
rpmsg_rx_cb_t cb;
struct mutex cb_lock;
+ rpmsg_rx_sig_t sig_cb;
u32 addr;
void *priv;

@@ -102,6 +106,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
+ * @signals: invoked when a serial signal change is received on the channel
*/
struct rpmsg_driver {
struct device_driver drv;
@@ -109,6 +114,8 @@ 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 old, u32 new);
};

#if IS_ENABLED(CONFIG_RPMSG)
@@ -135,6 +142,9 @@ int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
__poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
poll_table *wait);

+int rpmsg_get_signals(struct rpmsg_endpoint *ept);
+int rpmsg_set_signals(struct rpmsg_endpoint *ept, u32 set, u32 clear);
+
#else

static inline int register_rpmsg_device(struct rpmsg_device *dev)
@@ -242,6 +252,23 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
return 0;
}

+static inline int rpmsg_get_signals(struct rpmsg_endpoint *ept)
+{
+ /* This shouldn't be possible */
+ WARN_ON(1);
+
+ return -ENXIO;
+}
+
+static inline int rpmsg_set_signals(struct rpmsg_endpoint *ept,
+ u32 set, u32 clear)
+{
+ /* 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 */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-07-07 15:57:29

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH V7 0/4] Signaling api support in glink/rpmsg clients

On Tue 06 Jul 10:27 CDT 2021, Lee Jones wrote:

> On Wed, 2 Sept 2020 at 17:25, Deepak Kumar Singh <[email protected]>
> wrote:
>
> > Change from version 6
> > In last series(v6) i had put wrong version(v5) for cover note.
> > Which led to confusion for patch set series.
> >
> > In this series i have updated the label for cover letter(v7).
> > There is no change in patches. Only cover note label is updated.
> >
> > Change from version 5
> > [V6,4/4] rpmsg: char: Add signal callback and POLLPRI support
> > Updated for sparse warning. Replaced POLLPRI => EPOLLPRI to fix
> > warning.
> >
> > Change from version 4
> > I am taking over these patches from [email protected]
> > Fixed all the trivial review comments.
> >
> > Signal conversion to and from native signal as done in patch V4,2/4
> > is intentional.
> >
> > Arun Kumar Neelakantam (3):
> > rpmsg: glink: Add support to handle signals command
> > rpmsg: char: Add TIOCMGET/TIOCMSET ioctl support
> > rpmsg: char: Add signal callback and POLLPRI support
> >
> > Deepak Kumar Singh (1):
> > rpmsg: core: Add signal API support
> >
> > drivers/rpmsg/qcom_glink_native.c | 125
> > ++++++++++++++++++++++++++++++++++++++
> > drivers/rpmsg/rpmsg_char.c | 76 ++++++++++++++++++++++-
> > drivers/rpmsg/rpmsg_core.c | 40 ++++++++++++
> > drivers/rpmsg/rpmsg_internal.h | 5 ++
> > include/linux/rpmsg.h | 27 ++++++++
> > 5 files changed, 270 insertions(+), 3 deletions(-)
> >
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
> >
> Any idea why this died at v7?
>

I had some concerns about the actual users of this and wanted the API to
be more generic. Deepak brought this up again recently and I think we
have a common understanding of how v8 should look like.

Regards,
Bjorn

2021-07-08 11:22:12

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH V7 0/4] Signaling api support in glink/rpmsg clients

On Wed, 07 Jul 2021, Bjorn Andersson wrote:

> On Tue 06 Jul 10:27 CDT 2021, Lee Jones wrote:
>
> > On Wed, 2 Sept 2020 at 17:25, Deepak Kumar Singh <[email protected]>
> > wrote:
> >
> > > Change from version 6
> > > In last series(v6) i had put wrong version(v5) for cover note.
> > > Which led to confusion for patch set series.
> > >
> > > In this series i have updated the label for cover letter(v7).
> > > There is no change in patches. Only cover note label is updated.
> > >
> > > Change from version 5
> > > [V6,4/4] rpmsg: char: Add signal callback and POLLPRI support
> > > Updated for sparse warning. Replaced POLLPRI => EPOLLPRI to fix
> > > warning.
> > >
> > > Change from version 4
> > > I am taking over these patches from [email protected]
> > > Fixed all the trivial review comments.
> > >
> > > Signal conversion to and from native signal as done in patch V4,2/4
> > > is intentional.
> > >
> > > Arun Kumar Neelakantam (3):
> > > rpmsg: glink: Add support to handle signals command
> > > rpmsg: char: Add TIOCMGET/TIOCMSET ioctl support
> > > rpmsg: char: Add signal callback and POLLPRI support
> > >
> > > Deepak Kumar Singh (1):
> > > rpmsg: core: Add signal API support
> > >
> > > drivers/rpmsg/qcom_glink_native.c | 125
> > > ++++++++++++++++++++++++++++++++++++++
> > > drivers/rpmsg/rpmsg_char.c | 76 ++++++++++++++++++++++-
> > > drivers/rpmsg/rpmsg_core.c | 40 ++++++++++++
> > > drivers/rpmsg/rpmsg_internal.h | 5 ++
> > > include/linux/rpmsg.h | 27 ++++++++
> > > 5 files changed, 270 insertions(+), 3 deletions(-)
> > >
> > >
> > >
> > Any idea why this died at v7?
> >
>
> I had some concerns about the actual users of this and wanted the API to
> be more generic. Deepak brought this up again recently and I think we
> have a common understanding of how v8 should look like.

Okay, great.

Deepak, would you be kind enough to Cc me on the next submission please?

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog