* [PATCH v4 0/2] pmsg: core: Add support for name extension @ 2020-05-04 21:58 Mathieu Poirier 2020-05-04 21:58 ` [PATCH v4 1/2] rpmsg: core: Add wildcard match for name service Mathieu Poirier 2020-05-04 21:58 ` [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension Mathieu Poirier 0 siblings, 2 replies; 5+ messages in thread From: Mathieu Poirier @ 2020-05-04 21:58 UTC (permalink / raw) To: ohad, bjorn.andersson; +Cc: 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. Offers the same functionality as v3[1] but was tweaked to keep the robot happy[2]. Applies cleanly on rpmsg-next (4a3c6e2327e6). Thanks, Mathieu [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=281005 [2]. https://patchwork.kernel.org/patch/11523237/ Mathieu Poirier (2): rpmsg: core: Add wildcard match for name service rpmsg: core: Add support to retrieve name extension drivers/rpmsg/rpmsg_core.c | 112 ++++++++++++++++++++++++++++++++++++- include/linux/rpmsg.h | 13 +++++ 2 files changed, 124 insertions(+), 1 deletion(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/2] rpmsg: core: Add wildcard match for name service 2020-05-04 21:58 [PATCH v4 0/2] pmsg: core: Add support for name extension Mathieu Poirier @ 2020-05-04 21:58 ` Mathieu Poirier 2020-05-04 21:58 ` [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension Mathieu Poirier 1 sibling, 0 replies; 5+ messages in thread From: Mathieu Poirier @ 2020-05-04 21:58 UTC (permalink / raw) To: ohad, bjorn.andersson; +Cc: 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] 5+ messages in thread
* [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension 2020-05-04 21:58 [PATCH v4 0/2] pmsg: core: Add support for name extension Mathieu Poirier 2020-05-04 21:58 ` [PATCH v4 1/2] rpmsg: core: Add wildcard match for name service Mathieu Poirier @ 2020-05-04 21:58 ` Mathieu Poirier 2020-05-05 8:07 ` Arnaud POULIQUEN 1 sibling, 1 reply; 5+ messages in thread From: Mathieu Poirier @ 2020-05-04 21:58 UTC (permalink / raw) To: ohad, bjorn.andersson; +Cc: 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> --- drivers/rpmsg/rpmsg_core.c | 92 ++++++++++++++++++++++++++++++++++++++ include/linux/rpmsg.h | 13 ++++++ 2 files changed, 105 insertions(+) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index 5e01e8dede6b..dae87c0cb73d 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -439,6 +439,98 @@ 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"; + * + * + * Note: The name extension should be free'd using kfree_const(). + * + * Return: the name extension if found, NULL if not found and an error + * code otherwise. + */ +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 and the driver don't + * have a name or 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; + } + } + + if (!match) + return NULL; + + /* 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 kstrdup_const(extension, GFP_KERNEL); +} +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] 5+ messages in thread
* Re: [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension 2020-05-04 21:58 ` [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension Mathieu Poirier @ 2020-05-05 8:07 ` Arnaud POULIQUEN 2020-05-05 21:30 ` Mathieu Poirier 0 siblings, 1 reply; 5+ messages in thread From: Arnaud POULIQUEN @ 2020-05-05 8:07 UTC (permalink / raw) To: Mathieu Poirier, ohad, bjorn.andersson; +Cc: linux-remoteproc, linux-kernel Hi Mathieu, On 5/4/20 11:58 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. I do not test it yet,but LGTM. I plan to use these patches for the rpmsg_tty. Please find few remarks below. > > Suggested-by: Suman Anna <s-anna@ti.com>; > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/rpmsg/rpmsg_core.c | 92 ++++++++++++++++++++++++++++++++++++++ > include/linux/rpmsg.h | 13 ++++++ > 2 files changed, 105 insertions(+) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index 5e01e8dede6b..dae87c0cb73d 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -439,6 +439,98 @@ 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"; > + * > + * > + * Note: The name extension should be free'd using kfree_const(). > + * > + * Return: the name extension if found, NULL if not found and an error > + * code otherwise. > + */ > +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 and the driver don't > + * have a name or 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; > + } > + } > + > + if (!match) > + return NULL; here i would return an error to differentiate unmatch and name without extension. > + > + /* 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 kstrdup_const(extension, GFP_KERNEL); what about just returning the extension pointer? rpdev->id.name should be valid until device is unregistered. Regards Arnaud > +} > +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] 5+ messages in thread
* Re: [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension 2020-05-05 8:07 ` Arnaud POULIQUEN @ 2020-05-05 21:30 ` Mathieu Poirier 0 siblings, 0 replies; 5+ messages in thread From: Mathieu Poirier @ 2020-05-05 21:30 UTC (permalink / raw) To: Arnaud POULIQUEN Cc: Ohad Ben-Cohen, Bjorn Andersson, linux-remoteproc, Linux Kernel Mailing List On Tue, 5 May 2020 at 02:07, Arnaud POULIQUEN <arnaud.pouliquen@st.com> wrote: > > Hi Mathieu, > > > > On 5/4/20 11:58 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. > I do not test it yet,but LGTM. > I plan to use these patches for the rpmsg_tty. > Please find few remarks below. > > > > > Suggested-by: Suman Anna <s-anna@ti.com>; > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > --- > > drivers/rpmsg/rpmsg_core.c | 92 ++++++++++++++++++++++++++++++++++++++ > > include/linux/rpmsg.h | 13 ++++++ > > 2 files changed, 105 insertions(+) > > > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > > index 5e01e8dede6b..dae87c0cb73d 100644 > > --- a/drivers/rpmsg/rpmsg_core.c > > +++ b/drivers/rpmsg/rpmsg_core.c > > @@ -439,6 +439,98 @@ 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"; > > + * > > + * > > + * Note: The name extension should be free'd using kfree_const(). > > + * > > + * Return: the name extension if found, NULL if not found and an error > > + * code otherwise. > > + */ > > +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 and the driver don't > > + * have a name or 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; > > + } > > + } > > + > > + if (!match) > > + return NULL; > here i would return an error to differentiate unmatch and name without extension. That's a fair point. > > + > > + /* 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 kstrdup_const(extension, GFP_KERNEL); > what about just returning the extension pointer? > rpdev->id.name should be valid until device is unregistered. I grappled with that... I didn't know all the scenarios people would use this for and where the returned string would end up so I decided to play it safe. Does anyone else have an opinion on this? Am I too cautious? > > Regards > Arnaud > > > +} > > +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] 5+ messages in thread
end of thread, other threads:[~2020-05-05 21:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-04 21:58 [PATCH v4 0/2] pmsg: core: Add support for name extension Mathieu Poirier 2020-05-04 21:58 ` [PATCH v4 1/2] rpmsg: core: Add wildcard match for name service Mathieu Poirier 2020-05-04 21:58 ` [PATCH v4 2/2] rpmsg: core: Add support to retrieve name extension Mathieu Poirier 2020-05-05 8:07 ` Arnaud POULIQUEN 2020-05-05 21:30 ` Mathieu Poirier
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).