Good day Guennadi, On Tue, 22 Sep 2020 at 02:09, Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxxxxxxxx> wrote: > > Hi Mathieu, > > Thanks for the patches. I'm trying to understand the concept of > this approach and I'm probably failing at that. It seems to me > that this patch set is making the NS announcement service to a > separate RPMsg device and I don't understand the reasoning for > doing this. As far as I understand namespace announcements > belong to RPMsg devices / channels, they create a dedicated > endpoint on them with a fixed pre-defined address. But they > don't form a separate RPMsg device. I think the current > virtio_rpmsg_bus.c has that correctly: for each rpmsg device / > channel multiple endpoints can be created, where the NS > service is one of them. It's just an endpoing of an rpmsg > device, not a complete separate device. Have I misunderstood > anything? This patchset does not introduce any new features - the end result in terms of functionality is exactly the same. It is also a carbon copy of the work introduced by Arnaud (hence reusing his patches), with the exception that the code is presented in a slightly different order to allow for a complete dissociation of RPMSG name service from the virtIO transport mechanic. To make that happen rpmsg device specific byte conversion operations had to be introduced in struct rpmsg_device_ops and the explicit creation of an rpmsg_device associated with the name service (that wasn't needed when name service was welded to virtIO). But associating a rpmsg_device to the name service doesn't change anything - RPMSG devices are created the same way when name service messages are received from the host or the remote processor. To prove my theory I ran the rpmsg_client_sample.c and it just worked, no changes to client code needed. Let's keep talking, it's the only way we'll get through this. Mathieu > > Thanks > Guennadi > > On Mon, Sep 21, 2020 at 06:09:50PM -0600, Mathieu Poirier wrote: > > Hi all, > > > > After looking at Guennadi[1] and Arnaud's patchsets[2] it became > > clear that we need to go back to a generic rpmsg_ns_msg structure > > if we wanted to make progress. To do that some of the work from > > Arnaud had to be modified in a way that common name service > > functionality was transport agnostic. > > > > This patchset is based on Arnaud's work but also include a patch > > from Guennadi and some input from me. It should serve as a > > foundation for the next revision of [1]. > > > > Applies on rpmsg-next (4e3dda0bc603) and tested on stm32mp157. I > > did not test the modularisation. > > > > Comments and feedback would be greatly appreciated. > > > > Thanks, > > Mathieu > > > > [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=346593 > > [2]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=338335 > > > > Arnaud Pouliquen (5): > > rpmsg: virtio: rename rpmsg_create_channel > > rpmsg: core: Add channel creation internal API > > rpmsg: virtio: Add rpmsg channel device ops > > rpmsg: Turn name service into a stand alone driver > > rpmsg: virtio: use rpmsg ns device for the ns announcement > > > > Guennadi Liakhovetski (1): > > rpmsg: Move common structures and defines to headers > > > > Mathieu Poirier (4): > > rpmsg: virtio: Move virtio RPMSG structures to private header > > rpmsg: core: Add RPMSG byte conversion operations > > rpmsg: virtio: Make endianness conversion virtIO specific > > rpmsg: ns: Make Name service module transport agnostic > > > > drivers/rpmsg/Kconfig | 9 + > > drivers/rpmsg/Makefile | 1 + > > drivers/rpmsg/rpmsg_core.c | 96 +++++++++++ > > drivers/rpmsg/rpmsg_internal.h | 102 +++++++++++ > > drivers/rpmsg/rpmsg_ns.c | 108 ++++++++++++ > > drivers/rpmsg/virtio_rpmsg_bus.c | 284 +++++++++---------------------- > > include/linux/rpmsg_ns.h | 83 +++++++++ > > include/uapi/linux/rpmsg.h | 3 + > > 8 files changed, 487 insertions(+), 199 deletions(-) > > create mode 100644 drivers/rpmsg/rpmsg_ns.c > > create mode 100644 include/linux/rpmsg_ns.h > > > > -- > > 2.25.1 > >