Re: [PATCH v4 8/8] rpmsg: Turn name service into a stand alone driver
On Wed, 4 Nov 2020 at 11:39, Guennadi Liakhovetski wrote: > > On Wed, Nov 04, 2020 at 10:34:24AM -0700, Mathieu Poirier wrote: > > On Wed, Nov 04, 2020 at 03:01:44PM +0100, Guennadi Liakhovetski wrote: > > > Hi Mathieu, Arnaud, > > > > > > I've tried the patch set with my VirtIO / vhost audio implementation, > > > in general it worked quite well, > > > > Very good - it would be nice if you could add your "Tested-by:" tags. > > Sure, I guess it would be better if I reply with both "Tested-by" and > "Reviewed-by" to your next version? > Yes, that will be great. > Thanks > Guennadi > > > > On Tue, Oct 27, 2020 at 11:52:18AM -0600, Mathieu Poirier wrote: > > > > From: Arnaud Pouliquen > > > > > > > > Make the RPMSG name service announcement a stand alone driver so that it > > > > can be reused by other subsystems. It is also the first step in making > > > > the > > > > functionatlity transport independent, i.e that is not tied to virtIO. > > > > > > > > Co-developed-by: Mathieu Poirier > > > > Signed-off-by: Arnaud Pouliquen > > > > --- > > > > > > [snip] > > > > > > > diff --git a/include/linux/rpmsg_ns.h b/include/linux/rpmsg_ns.h > > > > index bb479f430080..42786bb759b5 100644 > > > > --- a/include/linux/rpmsg_ns.h > > > > +++ b/include/linux/rpmsg_ns.h > > > > @@ -39,4 +39,21 @@ enum rpmsg_ns_flags { > > > > /* Address 53 is reserved for advertising remote services */ > > > > #define RPMSG_NS_ADDR(53) > > > > > > > > +/** > > > > + * rpmsg_ns_register_device() - register name service device based on > > > > rpdev > > > > + * @rpdev: prepared rpdev to be used for creating endpoints > > > > + * > > > > + * This function wraps rpmsg_register_device() preparing the rpdev for > > > > use as > > > > + * basis for the rpmsg name service device. > > > > + */ > > > > +static inline int rpmsg_ns_register_device(struct rpmsg_device *rpdev) > > > > +{ > > > > + strcpy(rpdev->id.name, "rpmsg_ns"); > > > > > > you need to > > > > > > #include > > > > Of course yes - I'll simply add the header file. I plan on having another > > revision addressing your comments out by the end of the week or early next > > week. > > > > Thanks, > > Mathieu > > > > > > > > in this file for rpdev definition. Or you could leave this function in > > > rpmsg_ns.c, then it's enough to forward-declare struct rpdev here. > > > > > > Thanks > > > Guennadi > > > > > > > + rpdev->driver_override = "rpmsg_ns"; > > > > + rpdev->src = RPMSG_NS_ADDR; > > > > + rpdev->dst = RPMSG_NS_ADDR; > > > > + > > > > + return rpmsg_register_device(rpdev); > > > > +} > > > > + > > > > #endif > > > > -- > > > > 2.25.1 > > > >
Re: [PATCH v4 8/8] rpmsg: Turn name service into a stand alone driver
On Wed, Nov 04, 2020 at 10:34:24AM -0700, Mathieu Poirier wrote: > On Wed, Nov 04, 2020 at 03:01:44PM +0100, Guennadi Liakhovetski wrote: > > Hi Mathieu, Arnaud, > > > > I've tried the patch set with my VirtIO / vhost audio implementation, > > in general it worked quite well, > > Very good - it would be nice if you could add your "Tested-by:" tags. Sure, I guess it would be better if I reply with both "Tested-by" and "Reviewed-by" to your next version? Thanks Guennadi > > On Tue, Oct 27, 2020 at 11:52:18AM -0600, Mathieu Poirier wrote: > > > From: Arnaud Pouliquen > > > > > > Make the RPMSG name service announcement a stand alone driver so that it > > > can be reused by other subsystems. It is also the first step in making > > > the > > > functionatlity transport independent, i.e that is not tied to virtIO. > > > > > > Co-developed-by: Mathieu Poirier > > > Signed-off-by: Arnaud Pouliquen > > > --- > > > > [snip] > > > > > diff --git a/include/linux/rpmsg_ns.h b/include/linux/rpmsg_ns.h > > > index bb479f430080..42786bb759b5 100644 > > > --- a/include/linux/rpmsg_ns.h > > > +++ b/include/linux/rpmsg_ns.h > > > @@ -39,4 +39,21 @@ enum rpmsg_ns_flags { > > > /* Address 53 is reserved for advertising remote services */ > > > #define RPMSG_NS_ADDR(53) > > > > > > +/** > > > + * rpmsg_ns_register_device() - register name service device based on > > > rpdev > > > + * @rpdev: prepared rpdev to be used for creating endpoints > > > + * > > > + * This function wraps rpmsg_register_device() preparing the rpdev for > > > use as > > > + * basis for the rpmsg name service device. > > > + */ > > > +static inline int rpmsg_ns_register_device(struct rpmsg_device *rpdev) > > > +{ > > > + strcpy(rpdev->id.name, "rpmsg_ns"); > > > > you need to > > > > #include > > Of course yes - I'll simply add the header file. I plan on having another > revision addressing your comments out by the end of the week or early next > week. > > Thanks, > Mathieu > > > > > in this file for rpdev definition. Or you could leave this function in > > rpmsg_ns.c, then it's enough to forward-declare struct rpdev here. > > > > Thanks > > Guennadi > > > > > + rpdev->driver_override = "rpmsg_ns"; > > > + rpdev->src = RPMSG_NS_ADDR; > > > + rpdev->dst = RPMSG_NS_ADDR; > > > + > > > + return rpmsg_register_device(rpdev); > > > +} > > > + > > > #endif > > > -- > > > 2.25.1 > > >
Re: [PATCH v4 8/8] rpmsg: Turn name service into a stand alone driver
On Wed, Nov 04, 2020 at 03:01:44PM +0100, Guennadi Liakhovetski wrote: > Hi Mathieu, Arnaud, > > I've tried the patch set with my VirtIO / vhost audio implementation, > in general it worked quite well, Very good - it would be nice if you could add your "Tested-by:" tags. > > On Tue, Oct 27, 2020 at 11:52:18AM -0600, Mathieu Poirier wrote: > > From: Arnaud Pouliquen > > > > Make the RPMSG name service announcement a stand alone driver so that it > > can be reused by other subsystems. It is also the first step in making the > > functionatlity transport independent, i.e that is not tied to virtIO. > > > > Co-developed-by: Mathieu Poirier > > Signed-off-by: Arnaud Pouliquen > > --- > > [snip] > > > diff --git a/include/linux/rpmsg_ns.h b/include/linux/rpmsg_ns.h > > index bb479f430080..42786bb759b5 100644 > > --- a/include/linux/rpmsg_ns.h > > +++ b/include/linux/rpmsg_ns.h > > @@ -39,4 +39,21 @@ enum rpmsg_ns_flags { > > /* Address 53 is reserved for advertising remote services */ > > #define RPMSG_NS_ADDR (53) > > > > +/** > > + * rpmsg_ns_register_device() - register name service device based on rpdev > > + * @rpdev: prepared rpdev to be used for creating endpoints > > + * > > + * This function wraps rpmsg_register_device() preparing the rpdev for use > > as > > + * basis for the rpmsg name service device. > > + */ > > +static inline int rpmsg_ns_register_device(struct rpmsg_device *rpdev) > > +{ > > + strcpy(rpdev->id.name, "rpmsg_ns"); > > you need to > > #include Of course yes - I'll simply add the header file. I plan on having another revision addressing your comments out by the end of the week or early next week. Thanks, Mathieu > > in this file for rpdev definition. Or you could leave this function in > rpmsg_ns.c, then it's enough to forward-declare struct rpdev here. > > Thanks > Guennadi > > > + rpdev->driver_override = "rpmsg_ns"; > > + rpdev->src = RPMSG_NS_ADDR; > > + rpdev->dst = RPMSG_NS_ADDR; > > + > > + return rpmsg_register_device(rpdev); > > +} > > + > > #endif > > -- > > 2.25.1 > >
Re: [PATCH v4 8/8] rpmsg: Turn name service into a stand alone driver
Hi Mathieu, Arnaud, I've tried the patch set with my VirtIO / vhost audio implementation, in general it worked quite well, but I did need one chamge, which should be improved in this patch: On Tue, Oct 27, 2020 at 11:52:18AM -0600, Mathieu Poirier wrote: > From: Arnaud Pouliquen > > Make the RPMSG name service announcement a stand alone driver so that it > can be reused by other subsystems. It is also the first step in making the > functionatlity transport independent, i.e that is not tied to virtIO. > > Co-developed-by: Mathieu Poirier > Signed-off-by: Arnaud Pouliquen > --- [snip] > diff --git a/include/linux/rpmsg_ns.h b/include/linux/rpmsg_ns.h > index bb479f430080..42786bb759b5 100644 > --- a/include/linux/rpmsg_ns.h > +++ b/include/linux/rpmsg_ns.h > @@ -39,4 +39,21 @@ enum rpmsg_ns_flags { > /* Address 53 is reserved for advertising remote services */ > #define RPMSG_NS_ADDR(53) > > +/** > + * rpmsg_ns_register_device() - register name service device based on rpdev > + * @rpdev: prepared rpdev to be used for creating endpoints > + * > + * This function wraps rpmsg_register_device() preparing the rpdev for use as > + * basis for the rpmsg name service device. > + */ > +static inline int rpmsg_ns_register_device(struct rpmsg_device *rpdev) > +{ > + strcpy(rpdev->id.name, "rpmsg_ns"); you need to #include in this file for rpdev definition. Or you could leave this function in rpmsg_ns.c, then it's enough to forward-declare struct rpdev here. Thanks Guennadi > + rpdev->driver_override = "rpmsg_ns"; > + rpdev->src = RPMSG_NS_ADDR; > + rpdev->dst = RPMSG_NS_ADDR; > + > + return rpmsg_register_device(rpdev); > +} > + > #endif > -- > 2.25.1 >