linux-remoteproc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] rpmsg: core: Add support for name extension
@ 2020-05-15 20:56 Mathieu Poirier
  2020-05-15 20:56 ` [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service Mathieu Poirier
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Mathieu Poirier @ 2020-05-15 20:56 UTC (permalink / raw)
  To: ohad, bjorn.andersson
  Cc: arnaud.pouliquen, s-anna, linux-remoteproc, linux-kernel

This patchset adds the capability to supplement the base definition
published by an rpmsg_driver with a postfix description so that it
is easy to differentiate entities that use the same name service.

Applies cleanly on rpmsg-next (4f05fc33bebd).

New for V6:
- Added example on how to use the new API.

Thanks,
Mathieu


Mathieu Poirier (3):
  rpmsg: core: Add wildcard match for name service
  rpmsg: core: Add support to retrieve name extension
  sample/rpmsg: Print out RPMSG device name extension

 drivers/rpmsg/rpmsg_core.c          | 115 +++++++++++++++++++++++++++-
 include/linux/rpmsg.h               |  13 ++++
 samples/rpmsg/rpmsg_client_sample.c |   5 ++
 3 files changed, 132 insertions(+), 1 deletion(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service
  2020-05-15 20:56 [PATCH v6 0/3] rpmsg: core: Add support for name extension Mathieu Poirier
@ 2020-05-15 20:56 ` Mathieu Poirier
  2020-06-08 21:11   ` Suman Anna
  2020-05-15 20:56 ` [PATCH v6 2/3] rpmsg: core: Add support to retrieve name extension Mathieu Poirier
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Mathieu Poirier @ 2020-05-15 20:56 UTC (permalink / raw)
  To: ohad, bjorn.andersson
  Cc: arnaud.pouliquen, s-anna, linux-remoteproc, linux-kernel

Adding the capability to supplement the base definition published
by an rpmsg_driver with a postfix description so that it is possible
for several entity to use the same service.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/rpmsg/rpmsg_core.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index a6361cad608b..5e01e8dede6b 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -399,7 +399,25 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
 static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
 				  const struct rpmsg_device_id *id)
 {
-	return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
+	size_t len = min_t(size_t, strlen(id->name), RPMSG_NAME_SIZE);
+
+	/*
+	 * Allow for wildcard matches.  For example if rpmsg_driver::id_table
+	 * is:
+	 *
+	 * static struct rpmsg_device_id rpmsg_driver_sample_id_table[] = {
+	 *      { .name = "rpmsg-client-sample" },
+	 *      { },
+	 * }
+	 *
+	 * Then it is possible to support "rpmsg-client-sample*", i.e:
+	 *	rpmsg-client-sample
+	 *	rpmsg-client-sample_instance0
+	 *	rpmsg-client-sample_instance1
+	 *	...
+	 *	rpmsg-client-sample_instanceX
+	 */
+	return strncmp(id->name, rpdev->id.name, len) == 0;
 }
 
 /* match rpmsg channel and rpmsg driver */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 2/3] rpmsg: core: Add support to retrieve name extension
  2020-05-15 20:56 [PATCH v6 0/3] rpmsg: core: Add support for name extension Mathieu Poirier
  2020-05-15 20:56 ` [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service Mathieu Poirier
@ 2020-05-15 20:56 ` Mathieu Poirier
  2020-06-08 22:47   ` Suman Anna
  2020-05-15 20:56 ` [PATCH v6 3/3] sample: rpmsg: Print out RPMSG device " Mathieu Poirier
  2020-05-15 21:09 ` [PATCH v6 0/3] rpmsg: core: Add support for " Bjorn Andersson
  3 siblings, 1 reply; 9+ messages in thread
From: Mathieu Poirier @ 2020-05-15 20:56 UTC (permalink / raw)
  To: ohad, bjorn.andersson
  Cc: arnaud.pouliquen, s-anna, linux-remoteproc, linux-kernel

After adding support for rpmsg device name extension, this patch
provides a function that returns the extension portion of an rpmsg
device name.  That way users of the name extension functionality don't
have to write the same boiler plate code to extract the information.

Suggested-by: Suman Anna <s-anna@ti.com>;
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/rpmsg/rpmsg_core.c | 95 ++++++++++++++++++++++++++++++++++++++
 include/linux/rpmsg.h      | 13 ++++++
 2 files changed, 108 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 5e01e8dede6b..9583eb936607 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -439,6 +439,101 @@ static int rpmsg_dev_match(struct device *dev, struct device_driver *drv)
 	return of_driver_match_device(dev, drv);
 }
 
+/**
+ * rpmsg_device_get_name_extension() - get the name extension of a rpmsg device
+ * @rpdev: the rpmsg device to work with
+ * @skip: how many characters in the extension should be skipped over
+ *
+ * With function rpmsg_id_match() allowing for extension of the base driver name
+ * in order to differentiate services, this function returns the extension part
+ * of an rpmsg device name.  As such and with the following rpmsg driver device
+ * id table and rpmsg device names:
+ *
+ * static struct rpmsg_device_id rpmsg_driver_sample_id_table[] = {
+ *      { .name = "rpmsg-client-sample" },
+ *      { },
+ * }
+ *
+ * rpdev1->id.name == "rpmsg-client-sample";
+ * rpdev2->id.name == "rpmsg-client-sample_instance0";
+ *
+ * Calling rpmsg_device_get_name_extension() will yields the following:
+ *
+ * rpmsg_device_get_name_extension(rpdev1, 0) == NULL;
+ * rpmsg_device_get_name_extension(rpdev2, 0) == "_instance0";
+ * rpmsg_device_get_name_extension(rpdev2, 1) == "instance0";
+ *
+ *
+ * Return: The name extension if found, NULL if the name of the RPMSG device
+ *	   equals the name of the RPMSG driver and an error if no match is
+ *	   found or a validation problem has occurred.
+ */
+const char *rpmsg_device_get_name_extension(struct rpmsg_device *rpdev,
+					    unsigned int skip)
+{
+	const char *drv_name, *dev_name, *extension;
+	const struct rpmsg_device_id *ids;
+	struct device *dev = &rpdev->dev;
+	struct rpmsg_driver *rpdrv;
+	bool match = false;
+	unsigned int i;
+
+	if (!dev->driver)
+		return ERR_PTR(-EINVAL);
+
+	rpdrv = to_rpmsg_driver(dev->driver);
+
+	/*
+	 * No point in going further if the device doesn't have name or
+	 * the driver doesn't have a table to work with.
+	 */
+	if (!rpdev->id.name[0] || !rpdrv->id_table)
+		return ERR_PTR(-EINVAL);
+
+	ids = rpdrv->id_table;
+	dev_name = rpdev->id.name;
+
+	/*
+	 * See if any name in the driver's table match the beginning
+	 * of the rpmsg device's name.
+	 */
+	for (i = 0; ids[i].name[0]; i++) {
+		drv_name = ids[i].name;
+		if (strncmp(drv_name,
+			    dev_name, strlen(drv_name)) == 0) {
+			match = true;
+			break;
+		}
+	}
+
+	/*
+	 * A match was not found, return an error to differentiate with cases
+	 * where a match was found but the name has no extension (see below).
+	 */
+	if (!match)
+		return ERR_PTR(-ENOENT);
+
+	 /* No name extension to return if device and driver are the same */
+	if (strlen(dev_name) == strlen(drv_name))
+		return NULL;
+
+	/*
+	 * Make sure we were not requested to skip past the end
+	 * of the device name.
+	 */
+	if (strlen(drv_name) + skip >= strlen(dev_name))
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * Move past the base name published by the driver and
+	 * skip any extra characters if needed.
+	 */
+	extension = dev_name + strlen(drv_name) + skip;
+
+	return extension;
+}
+EXPORT_SYMBOL(rpmsg_device_get_name_extension);
+
 static int rpmsg_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 9fe156d1c018..9537b95ad30a 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -135,6 +135,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);
 
+const char *rpmsg_device_get_name_extension(struct rpmsg_device *dev,
+					    unsigned int skip);
+
 #else
 
 static inline int register_rpmsg_device(struct rpmsg_device *dev)
@@ -242,6 +245,16 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
 	return 0;
 }
 
+static inline
+const char *rpmsg_device_get_name_extension(struct rpmsg_device *dev,
+					    unsigned int skip)
+{
+	/* This shouldn't be possible */
+	WARN_ON(1);
+
+	return NULL;
+}
+
 #endif /* IS_ENABLED(CONFIG_RPMSG) */
 
 /* use a macro to avoid include chaining to get THIS_MODULE */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 3/3] sample: rpmsg: Print out RPMSG device name extension
  2020-05-15 20:56 [PATCH v6 0/3] rpmsg: core: Add support for name extension Mathieu Poirier
  2020-05-15 20:56 ` [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service Mathieu Poirier
  2020-05-15 20:56 ` [PATCH v6 2/3] rpmsg: core: Add support to retrieve name extension Mathieu Poirier
@ 2020-05-15 20:56 ` Mathieu Poirier
  2020-05-15 21:09 ` [PATCH v6 0/3] rpmsg: core: Add support for " Bjorn Andersson
  3 siblings, 0 replies; 9+ messages in thread
From: Mathieu Poirier @ 2020-05-15 20:56 UTC (permalink / raw)
  To: ohad, bjorn.andersson
  Cc: arnaud.pouliquen, s-anna, linux-remoteproc, linux-kernel

Use the new rpmsg_device_get_name_extension() API to print the
RPMSG name extension if used by a RPMSG device name.

Suggested-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 samples/rpmsg/rpmsg_client_sample.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/samples/rpmsg/rpmsg_client_sample.c b/samples/rpmsg/rpmsg_client_sample.c
index ae5081662283..2c5dc65ed844 100644
--- a/samples/rpmsg/rpmsg_client_sample.c
+++ b/samples/rpmsg/rpmsg_client_sample.c
@@ -51,8 +51,13 @@ static int rpmsg_sample_cb(struct rpmsg_device *rpdev, void *data, int len,
 static int rpmsg_sample_probe(struct rpmsg_device *rpdev)
 {
 	int ret;
+	const char *extension;
 	struct instance_data *idata;
 
+	extension = rpmsg_device_get_name_extension(rpdev, 1);
+	if (!IS_ERR_OR_NULL(extension))
+		dev_info(&rpdev->dev, "extension: %s\n", extension);
+
 	dev_info(&rpdev->dev, "new channel: 0x%x -> 0x%x!\n",
 					rpdev->src, rpdev->dst);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 0/3] rpmsg: core: Add support for name extension
  2020-05-15 20:56 [PATCH v6 0/3] rpmsg: core: Add support for name extension Mathieu Poirier
                   ` (2 preceding siblings ...)
  2020-05-15 20:56 ` [PATCH v6 3/3] sample: rpmsg: Print out RPMSG device " Mathieu Poirier
@ 2020-05-15 21:09 ` Bjorn Andersson
  2020-05-20 14:49   ` Arnaud POULIQUEN
  3 siblings, 1 reply; 9+ messages in thread
From: Bjorn Andersson @ 2020-05-15 21:09 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, arnaud.pouliquen, s-anna, linux-remoteproc, linux-kernel

On Fri 15 May 13:56 PDT 2020, Mathieu Poirier wrote:

> This patchset adds the capability to supplement the base definition
> published by an rpmsg_driver with a postfix description so that it
> is easy to differentiate entities that use the same name service.
> 
> Applies cleanly on rpmsg-next (4f05fc33bebd).
> 

Thanks Mathieu, this series does look good.


But before merging this, can someone show me a real example where this
is being/would be used? What are some real channel names and extensions?

Regards,
Bjorn

> New for V6:
> - Added example on how to use the new API.
> 
> Thanks,
> Mathieu
> 
> 
> Mathieu Poirier (3):
>   rpmsg: core: Add wildcard match for name service
>   rpmsg: core: Add support to retrieve name extension
>   sample/rpmsg: Print out RPMSG device name extension
> 
>  drivers/rpmsg/rpmsg_core.c          | 115 +++++++++++++++++++++++++++-
>  include/linux/rpmsg.h               |  13 ++++
>  samples/rpmsg/rpmsg_client_sample.c |   5 ++
>  3 files changed, 132 insertions(+), 1 deletion(-)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 0/3] rpmsg: core: Add support for name extension
  2020-05-15 21:09 ` [PATCH v6 0/3] rpmsg: core: Add support for " Bjorn Andersson
@ 2020-05-20 14:49   ` Arnaud POULIQUEN
  2020-06-08 22:33     ` Suman Anna
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaud POULIQUEN @ 2020-05-20 14:49 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier
  Cc: ohad, s-anna, linux-remoteproc, linux-kernel, Xiang Xiao

Hi Bjorn,

On 5/15/20 11:09 PM, Bjorn Andersson wrote:
> On Fri 15 May 13:56 PDT 2020, Mathieu Poirier wrote:
> 
>> This patchset adds the capability to supplement the base definition
>> published by an rpmsg_driver with a postfix description so that it
>> is easy to differentiate entities that use the same name service.
>>
>> Applies cleanly on rpmsg-next (4f05fc33bebd).
>>
> 
> Thanks Mathieu, this series does look good.
> 
> 
> But before merging this, can someone show me a real example where this
> is being/would be used? What are some real channel names and extensions?

On ST side, This is something we plan to integrate in the TTY over RPMSG support.
The use case is the support of multi-instances. We already provided to our
customer a TTY service supporting it but without name extension. 
Some feed-backs are: how can we know which TTY instances to use to communicate
to the expected remote application in case of multi-instance.
A concrete example would be one instance to control a remote processor
application, the other instance to get the remote system logs.

Then in rpmsg TTY proposed for upstream the extension could also been used to
differentiate the data from the control channels, as discussed with Mathieu
during reviews: https://lkml.org/lkml/2020/4/3/964. 
Means the service is the TTY, the sub-services are the data and the control.

An other usecase i have in mind is the management of the rpmsg flow control for
the QOS.
This could be reused to create a core flow control manager based on the
service extension, which could be quite smooth in term of legacy support.

Suman and Xiang(added in CC) have probably also some usecases as they 
proposed similar patches...

Regards,
Arnaud

> 
> Regards,
> Bjorn
> 
>> New for V6:
>> - Added example on how to use the new API.
>>
>> Thanks,
>> Mathieu
>>
>>
>> Mathieu Poirier (3):
>>   rpmsg: core: Add wildcard match for name service
>>   rpmsg: core: Add support to retrieve name extension
>>   sample/rpmsg: Print out RPMSG device name extension
>>
>>  drivers/rpmsg/rpmsg_core.c          | 115 +++++++++++++++++++++++++++-
>>  include/linux/rpmsg.h               |  13 ++++
>>  samples/rpmsg/rpmsg_client_sample.c |   5 ++
>>  3 files changed, 132 insertions(+), 1 deletion(-)
>>
>> -- 
>> 2.20.1
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service
  2020-05-15 20:56 ` [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service Mathieu Poirier
@ 2020-06-08 21:11   ` Suman Anna
  0 siblings, 0 replies; 9+ messages in thread
From: Suman Anna @ 2020-06-08 21:11 UTC (permalink / raw)
  To: Mathieu Poirier, ohad, bjorn.andersson
  Cc: arnaud.pouliquen, linux-remoteproc, linux-kernel

On 5/15/20 3:56 PM, Mathieu Poirier wrote:
> Adding the capability to supplement the base definition published
> by an rpmsg_driver with a postfix description so that it is possible
> for several entity to use the same service.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Acked-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>

Going back to my review and testing done back on v2,

Acked-by: Suman Anna <s-anna@ti.com>

regards
Suman

> ---
>   drivers/rpmsg/rpmsg_core.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index a6361cad608b..5e01e8dede6b 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -399,7 +399,25 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
>   static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
>   				  const struct rpmsg_device_id *id)
>   {
> -	return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> +	size_t len = min_t(size_t, strlen(id->name), RPMSG_NAME_SIZE);
> +
> +	/*
> +	 * Allow for wildcard matches.  For example if rpmsg_driver::id_table
> +	 * is:
> +	 *
> +	 * static struct rpmsg_device_id rpmsg_driver_sample_id_table[] = {
> +	 *      { .name = "rpmsg-client-sample" },
> +	 *      { },
> +	 * }
> +	 *
> +	 * Then it is possible to support "rpmsg-client-sample*", i.e:
> +	 *	rpmsg-client-sample
> +	 *	rpmsg-client-sample_instance0
> +	 *	rpmsg-client-sample_instance1
> +	 *	...
> +	 *	rpmsg-client-sample_instanceX
> +	 */
> +	return strncmp(id->name, rpdev->id.name, len) == 0;
>   }
>   
>   /* match rpmsg channel and rpmsg driver */
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 0/3] rpmsg: core: Add support for name extension
  2020-05-20 14:49   ` Arnaud POULIQUEN
@ 2020-06-08 22:33     ` Suman Anna
  0 siblings, 0 replies; 9+ messages in thread
From: Suman Anna @ 2020-06-08 22:33 UTC (permalink / raw)
  To: Arnaud POULIQUEN, Bjorn Andersson, Mathieu Poirier
  Cc: ohad, linux-remoteproc, linux-kernel, Xiang Xiao

On 5/20/20 9:49 AM, Arnaud POULIQUEN wrote:
> Hi Bjorn,
> 
> On 5/15/20 11:09 PM, Bjorn Andersson wrote:
>> On Fri 15 May 13:56 PDT 2020, Mathieu Poirier wrote:
>>
>>> This patchset adds the capability to supplement the base definition
>>> published by an rpmsg_driver with a postfix description so that it
>>> is easy to differentiate entities that use the same name service.
>>>
>>> Applies cleanly on rpmsg-next (4f05fc33bebd).
>>>
>>
>> Thanks Mathieu, this series does look good.
>>
>>
>> But before merging this, can someone show me a real example where this
>> is being/would be used? What are some real channel names and extensions?
> 
> On ST side, This is something we plan to integrate in the TTY over RPMSG support.
> The use case is the support of multi-instances. We already provided to our
> customer a TTY service supporting it but without name extension.
> Some feed-backs are: how can we know which TTY instances to use to communicate
> to the expected remote application in case of multi-instance.
> A concrete example would be one instance to control a remote processor
> application, the other instance to get the remote system logs.
> 
> Then in rpmsg TTY proposed for upstream the extension could also been used to
> differentiate the data from the control channels, as discussed with Mathieu
> during reviews: https://lkml.org/lkml/2020/4/3/964.
> Means the service is the TTY, the sub-services are the data and the control.
> 
> An other usecase i have in mind is the management of the rpmsg flow control for
> the QOS.
> This could be reused to create a core flow control manager based on the
> service extension, which could be quite smooth in term of legacy support.
> 
> Suman and Xiang(added in CC) have probably also some usecases as they
> proposed similar patches...

Yeah, this series is a result of the discussion on those prior patches, 
and maintaining compatibility for both the current in-kernel usage and 
the OpenAMP usage.

My original usecase was with an out-of-tree driver and is explained as 
part of review of those prior solution,
https://patchwork.kernel.org/comment/22850003/

I am also looking at this for future usage with the rpmsg-chrdev driver.

regards
Suman


> 
> Regards,
> Arnaud
> 
>>
>> Regards,
>> Bjorn
>>
>>> New for V6:
>>> - Added example on how to use the new API.
>>>
>>> Thanks,
>>> Mathieu
>>>
>>>
>>> Mathieu Poirier (3):
>>>    rpmsg: core: Add wildcard match for name service
>>>    rpmsg: core: Add support to retrieve name extension
>>>    sample/rpmsg: Print out RPMSG device name extension
>>>
>>>   drivers/rpmsg/rpmsg_core.c          | 115 +++++++++++++++++++++++++++-
>>>   include/linux/rpmsg.h               |  13 ++++
>>>   samples/rpmsg/rpmsg_client_sample.c |   5 ++
>>>   3 files changed, 132 insertions(+), 1 deletion(-)
>>>
>>> -- 
>>> 2.20.1
>>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/3] rpmsg: core: Add support to retrieve name extension
  2020-05-15 20:56 ` [PATCH v6 2/3] rpmsg: core: Add support to retrieve name extension Mathieu Poirier
@ 2020-06-08 22:47   ` Suman Anna
  0 siblings, 0 replies; 9+ messages in thread
From: Suman Anna @ 2020-06-08 22:47 UTC (permalink / raw)
  To: Mathieu Poirier, ohad, bjorn.andersson
  Cc: arnaud.pouliquen, linux-remoteproc, linux-kernel

Hi Mathieu,

On 5/15/20 3:56 PM, Mathieu Poirier wrote:
> After adding support for rpmsg device name extension, this patch
> provides a function that returns the extension portion of an rpmsg
> device name.  That way users of the name extension functionality don't
> have to write the same boiler plate code to extract the information.
> 
> Suggested-by: Suman Anna <s-anna@ti.com>;
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Acked-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>

With the below minor nits fixed,

Acked-by: Suman Anna <s-anna@ti.com>

> ---
>   drivers/rpmsg/rpmsg_core.c | 95 ++++++++++++++++++++++++++++++++++++++
>   include/linux/rpmsg.h      | 13 ++++++
>   2 files changed, 108 insertions(+)
> 
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index 5e01e8dede6b..9583eb936607 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -439,6 +439,101 @@ static int rpmsg_dev_match(struct device *dev, struct device_driver *drv)
>   	return of_driver_match_device(dev, drv);
>   }
>   
> +/**
> + * rpmsg_device_get_name_extension() - get the name extension of a rpmsg device
> + * @rpdev: the rpmsg device to work with
> + * @skip: how many characters in the extension should be skipped over
> + *
> + * With function rpmsg_id_match() allowing for extension of the base driver name
> + * in order to differentiate services, this function returns the extension part
> + * of an rpmsg device name.  As such and with the following rpmsg driver device
> + * id table and rpmsg device names:
> + *
> + * static struct rpmsg_device_id rpmsg_driver_sample_id_table[] = {
> + *      { .name = "rpmsg-client-sample" },
> + *      { },
> + * }
> + *
> + * rpdev1->id.name == "rpmsg-client-sample";
> + * rpdev2->id.name == "rpmsg-client-sample_instance0";
> + *
> + * Calling rpmsg_device_get_name_extension() will yields the following:
> + *
> + * rpmsg_device_get_name_extension(rpdev1, 0) == NULL;
> + * rpmsg_device_get_name_extension(rpdev2, 0) == "_instance0";
> + * rpmsg_device_get_name_extension(rpdev2, 1) == "instance0";
> + *
> + *

can drop the additional blank line here,

> + * Return: The name extension if found, NULL if the name of the RPMSG device
> + *	   equals the name of the RPMSG driver and an error if no match is
> + *	   found or a validation problem has occurred.
> + */
> +const char *rpmsg_device_get_name_extension(struct rpmsg_device *rpdev,
> +					    unsigned int skip)
> +{
> +	const char *drv_name, *dev_name, *extension;
> +	const struct rpmsg_device_id *ids;
> +	struct device *dev = &rpdev->dev;
> +	struct rpmsg_driver *rpdrv;
> +	bool match = false;
> +	unsigned int i;
> +
> +	if (!dev->driver)
> +		return ERR_PTR(-EINVAL);
> +
> +	rpdrv = to_rpmsg_driver(dev->driver);
> +
> +	/*
> +	 * No point in going further if the device doesn't have name or
> +	 * the driver doesn't have a table to work with.
> +	 */
> +	if (!rpdev->id.name[0] || !rpdrv->id_table)
> +		return ERR_PTR(-EINVAL);
> +
> +	ids = rpdrv->id_table;
> +	dev_name = rpdev->id.name;
> +
> +	/*
> +	 * See if any name in the driver's table match the beginning
> +	 * of the rpmsg device's name.
> +	 */
> +	for (i = 0; ids[i].name[0]; i++) {
> +		drv_name = ids[i].name;
> +		if (strncmp(drv_name,
> +			    dev_name, strlen(drv_name)) == 0) {

This does fit on the same line, so no need to use multi-lines for this.

regards
Suman

> +			match = true;
> +			break;
> +		}
> +	}
> +
> +	/*
> +	 * A match was not found, return an error to differentiate with cases
> +	 * where a match was found but the name has no extension (see below).
> +	 */
> +	if (!match)
> +		return ERR_PTR(-ENOENT);
> +
> +	 /* No name extension to return if device and driver are the same */
> +	if (strlen(dev_name) == strlen(drv_name))
> +		return NULL;
> +
> +	/*
> +	 * Make sure we were not requested to skip past the end
> +	 * of the device name.
> +	 */
> +	if (strlen(drv_name) + skip >= strlen(dev_name))
> +		return ERR_PTR(-EINVAL);
> +
> +	/*
> +	 * Move past the base name published by the driver and
> +	 * skip any extra characters if needed.
> +	 */
> +	extension = dev_name + strlen(drv_name) + skip;
> +
> +	return extension;
> +}
> +EXPORT_SYMBOL(rpmsg_device_get_name_extension);
> +
>   static int rpmsg_uevent(struct device *dev, struct kobj_uevent_env *env)
>   {
>   	struct rpmsg_device *rpdev = to_rpmsg_device(dev);
> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
> index 9fe156d1c018..9537b95ad30a 100644
> --- a/include/linux/rpmsg.h
> +++ b/include/linux/rpmsg.h
> @@ -135,6 +135,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);
>   
> +const char *rpmsg_device_get_name_extension(struct rpmsg_device *dev,
> +					    unsigned int skip);
> +
>   #else
>   
>   static inline int register_rpmsg_device(struct rpmsg_device *dev)
> @@ -242,6 +245,16 @@ static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
>   	return 0;
>   }
>   
> +static inline
> +const char *rpmsg_device_get_name_extension(struct rpmsg_device *dev,
> +					    unsigned int skip)
> +{
> +	/* This shouldn't be possible */
> +	WARN_ON(1);
> +
> +	return NULL;
> +}
> +
>   #endif /* IS_ENABLED(CONFIG_RPMSG) */
>   
>   /* use a macro to avoid include chaining to get THIS_MODULE */
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-06-08 22:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 20:56 [PATCH v6 0/3] rpmsg: core: Add support for name extension Mathieu Poirier
2020-05-15 20:56 ` [PATCH v6 1/3] rpmsg: core: Add wildcard match for name service Mathieu Poirier
2020-06-08 21:11   ` Suman Anna
2020-05-15 20:56 ` [PATCH v6 2/3] rpmsg: core: Add support to retrieve name extension Mathieu Poirier
2020-06-08 22:47   ` Suman Anna
2020-05-15 20:56 ` [PATCH v6 3/3] sample: rpmsg: Print out RPMSG device " Mathieu Poirier
2020-05-15 21:09 ` [PATCH v6 0/3] rpmsg: core: Add support for " Bjorn Andersson
2020-05-20 14:49   ` Arnaud POULIQUEN
2020-06-08 22:33     ` Suman Anna

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).