1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
|
Currently, the rpmsg core automatically creates an endpoint for rpmsg
drivers that specify a receive callback. This works "somewhat" in most
simple cases, but it is often prone to race conditions: The receive
callback can be called as soon and as long as the endpoint is open, so
drivers must be prepared to handle calls to the receive callback:
- Before their probe() function is called
(after the endpoint was created)
- In parallel to their probe() function
- In parallel to their remove() function
- After their remove() function is called
(before the endpoint is destroyed)
It is difficult for drivers to handle this without being able to run code
before endpoint creation and after endpoint destruction. Also, they may
need to hold locks while creating/destroying the endpoint to handle edge
cases reliably.
Drivers can already create endpoints manually by omitting the receive
callback in rpmsg_driver, but for most simple cases where there is only
a single channel, the boilerplate required for that is a bit cumbersome.
Add a rpmsg_dev_open_ept() function that can be called by drivers during
the probe() function. It results in effectively the same that the rpmsg
core would normally do if the receive callback is specified.
announce_create() and announce_destroy() are still handled by the rpmsg
core. During remove(), the drivers can directly call rpmsg_destroy_ept().
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
drivers/rpmsg/rpmsg_core.c | 51 +++++++++++++++++++++++++++-----------
include/linux/rpmsg.h | 12 +++++++++
2 files changed, 49 insertions(+), 14 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 6783d04b591dd1..896fb8a3a4f5f4 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -95,6 +95,14 @@ EXPORT_SYMBOL(rpmsg_release_channel);
* equals to the src address of their rpmsg channel), the driver's handler
* is invoked to process it.
*
+ * Note that the endpoint for simple rpmsg drivers is created before calling
+ * probe() and closed after calling remove(), so special care must be taken
+ * to handle calls to the rx callback before/in parallel of probe() and
+ * after/in parallel of remove(). If more control over the endpoint creation
+ * is required to avoid race conditions, drivers can omit the callback and
+ * explicitly call rpmsg_dev_open_ept() in probe() and rpmsg_destroy_ept() in
+ * remove(), together with locks as needed.
+ *
* That said, more complicated drivers might need to allocate
* additional rpmsg addresses, and bind them to different rx callbacks.
* To accomplish that, those drivers need to call this function.
@@ -451,6 +459,32 @@ static int rpmsg_uevent(const struct device *dev, struct kobj_uevent_env *env)
rpdev->id.name);
}
+struct rpmsg_endpoint *rpmsg_dev_open_ept(struct rpmsg_device *rpdev,
+ rpmsg_rx_cb_t cb, void *priv)
+{
+ struct rpmsg_driver *rpdrv = to_rpmsg_driver(rpdev->dev.driver);
+ struct rpmsg_channel_info chinfo = {
+ .src = rpdev->src,
+ .dst = RPMSG_ADDR_ANY,
+ };
+ struct rpmsg_endpoint *ept;
+
+ strscpy(chinfo.name, rpdev->id.name, sizeof(chinfo.name));
+
+ ept = rpmsg_create_ept(rpdev, cb, priv, chinfo);
+ if (!ept) {
+ dev_err(&rpdev->dev, "failed to create endpoint\n");
+ return NULL;
+ }
+
+ rpdev->ept = ept;
+ rpdev->src = ept->addr;
+
+ ept->flow_cb = rpdrv->flowcontrol;
+ return ept;
+}
+EXPORT_SYMBOL(rpmsg_dev_open_ept);
+
/*
* when an rpmsg driver is probed with a channel, we seamlessly create
* it an endpoint, binding its rx callback to a unique local rpmsg
@@ -463,7 +497,6 @@ static int rpmsg_dev_probe(struct device *dev)
{
struct rpmsg_device *rpdev = to_rpmsg_device(dev);
struct rpmsg_driver *rpdrv = to_rpmsg_driver(rpdev->dev.driver);
- struct rpmsg_channel_info chinfo = {};
struct rpmsg_endpoint *ept = NULL;
int err;
@@ -473,21 +506,11 @@ static int rpmsg_dev_probe(struct device *dev)
goto out;
if (rpdrv->callback) {
- strscpy(chinfo.name, rpdev->id.name, sizeof(chinfo.name));
- chinfo.src = rpdev->src;
- chinfo.dst = RPMSG_ADDR_ANY;
-
- ept = rpmsg_create_ept(rpdev, rpdrv->callback, NULL, chinfo);
+ ept = rpmsg_dev_open_ept(rpdev, rpdrv->callback, NULL);
if (!ept) {
- dev_err(dev, "failed to create endpoint\n");
err = -ENOMEM;
goto out;
}
-
- rpdev->ept = ept;
- rpdev->src = ept->addr;
-
- ept->flow_cb = rpdrv->flowcontrol;
}
err = rpdrv->probe(rpdev);
@@ -496,7 +519,7 @@ static int rpmsg_dev_probe(struct device *dev)
goto destroy_ept;
}
- if (ept && rpdev->ops->announce_create) {
+ if (rpdev->ept && rpdev->ops->announce_create) {
err = rpdev->ops->announce_create(rpdev);
if (err) {
dev_err(dev, "failed to announce creation\n");
@@ -527,7 +550,7 @@ static void rpmsg_dev_remove(struct device *dev)
if (rpdrv->remove)
rpdrv->remove(rpdev);
- if (rpdev->ept)
+ if (rpdrv->callback && rpdev->ept)
rpmsg_destroy_ept(rpdev->ept);
}
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 83266ce1464204..c3719703553c0a 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -181,6 +181,8 @@ void rpmsg_destroy_ept(struct rpmsg_endpoint *);
struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *,
rpmsg_rx_cb_t cb, void *priv,
struct rpmsg_channel_info chinfo);
+struct rpmsg_endpoint *rpmsg_dev_open_ept(struct rpmsg_device *rpdev,
+ rpmsg_rx_cb_t cb, void *priv);
int rpmsg_send(struct rpmsg_endpoint *ept, const void *data, int len);
int rpmsg_sendto(struct rpmsg_endpoint *ept, const void *data, int len, u32 dst);
@@ -249,6 +251,16 @@ static inline struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *rpdev
return NULL;
}
+static inline struct rpmsg_endpoint *rpmsg_dev_open_ept(struct rpmsg_device *rpdev,
+ rpmsg_rx_cb_t cb,
+ void *priv)
+{
+ /* This shouldn't be possible */
+ WARN_ON(1);
+
+ return NULL;
+}
+
static inline int rpmsg_send(struct rpmsg_endpoint *ept, const void *data, int len)
{
/* This shouldn't be possible */
|
