> Subject: Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO > device > > Hi Peng, > > On 9/21/22 16:07, Arnaud POULIQUEN wrote: > > Hi Peng, > > > > On 9/21/22 10:54, Peng Fan wrote: > >> Hi Arnaud, > >> > >>> Subject: [PATCH v8 0/4] remoteproc: restructure the remoteproc > >>> VirtIO device > >> > >> Sorry to get in at this late time, just try to catch up. > >> Not reviewing comments, just have a question, Does remote core > >> firmware requires changes to use this new feature? > > > > For this series, it is not. > > For the whole work, it should not, but it will probably depend on the > > evolutions related to the reviews and requirements that will come. > > > >> Does your 4 branches listed below still work with linux-6.x? > > > > I have to rebase them. Today my github branches are based on v5.18.rc1 > > I plan to do this end of this week or next week. > > > >> Could the multiple vdev still share same mbox channel? > > > > Yes I'm trying to keep the legacy support of the mailbox in the > > remoteproc platform driver. > > If no mailbox is declared in the virtio subnode it calls the > > rproc->ops->kick > > > >> > >> I not own i.MX remote core firmware development, so if no need > >> firmware change, I would like give a try and see how it works. > > > > Great! That would be nice to have your feedback. > > Mailbox management is one point, I'm also ineterested in having > > feedback on the memory regions management I will ping you when my > work > > will be rebased on 6.0 > > My github branches have been rebased on top of thre > rproc_next(1d7b61c06dc3) > > As a first step you should be able to rebase on my step4-virtio-mailboxes[1] > without any update of your driver. If I did my dev well, I kept the > compatibility with the legacy. Thanks for the quick action. I will give a try on i.MX, and see how it works, but I may not respond quick. Thanks, Peng. > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu > b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio- > mailboxes&data=05%7C01%7Cpeng.fan%40nxp.com%7C50e9c0e75117 > 4c09328808da9d4bab9b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C > 0%7C637995245590228209%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w > LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C > %7C%7C&sdata=Jbwa0aPM4aoR8H1cyTLFKw4lK9VUcCyHwzkleVsnkRc > %3D&reserved=0 > > Regards, > Arnaud > > > > > Thanks, > > Arnaud > > > >> > >> Thanks, > >> Peng. > >> > >>> > >>> 1) Update from V7 [1]: > >>> > >>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc: > >>> imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init") > >>> The updates take into account the integration of the > >>> commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after > >>> rproc_shutdown") > >>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> > >>> according to reviews on V7 > >>> > >>> > >>> [1] > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml. > >>> > org%2Flkml%2F2022%2F7%2F13%2F663&data=05%7C01%7Cpeng.fan% > >>> > 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c > >>> > 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C > >>> > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL > >>> > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=77pEuwAI7Lh61hx1%2B > >>> Hs79Cu0G5KOa6mzQ0PnTC5r8Xk%3D&reserved=0 > >>> [2] > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >>> > t.ke%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C50e9c0e751174c > 093288 > >>> > 08da9d4bab9b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637 > 9952455 > >>> > 90228209%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo > iV2luMzI > >>> > iLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bF > M3T6bY1c > >>> p%2FvyRyZo6mdXGeN7%2BTCMfcmeY3OFpksWA%3D&reserved=0 > >>> > rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fremoteproc%2Flinux.g > >>> it%2Flog%2F%3Fh%3Dfor- > >>> > next&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e7 > >>> > 439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > >>> > C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA > >>> > wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7 > >>> > C%7C&sdata=iWUSzKkN9BpHwqbO62awIcyVf9PXcftcdt2kytWVR78%3D > >>> &reserved=0 > >>> > >>> 2) Patchset description: > >>> > >>> This series is a part of the work initiated a long time ago in the > >>> series > >>> "remoteproc: Decorelate virtio from core"[3] > >>> > >>> Objective of the work: > >>> - Update the remoteproc VirtIO device creation (use platform device) > >>> - Allow to declare remoteproc VirtIO device in DT > >>> - declare resources associated to a remote proc VirtIO > >>> - declare a list of VirtIO supported by the platform. > >>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, > video, ...). > >>> For instance be able to declare a I2C device in a virtio-i2C node. > >>> - Keep the legacy working! > >>> - Try to improve the picture about concerns reported by Christoph > >>> Hellwing [4][5] > >>> > >>> [3] > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml. > >>> > org%2Flkml%2F2020%2F4%2F16%2F1817&data=05%7C01%7Cpeng.fan > >>> %40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3b > c2b4 > >>> > c6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7 > >>> > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw > >>> > iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=oPWSfUweLdhUFK5X9 > >>> 2YcGHem8s%2Bfelcr%2FHx9JAlKG%2BI%3D&reserved=0 > >>> [4] > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml. > >>> > org%2Flkml%2F2021%2F6%2F23%2F607&data=05%7C01%7Cpeng.fan% > >>> > 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c > >>> > 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C > >>> > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL > >>> > CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HPpnlaykes8R1Kz1dEN > >>> nirEHkDNr7JvRs%2FcsaDPuLdI%3D&reserved=0 > >>> [5] > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa > >>> tc > >>> hwork.kernel.org%2Fproject%2Flinux- > >>> > remoteproc%2Fpatch%2FAOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E > >>> %40cp7-web- > >>> > 042.plabs.ch%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e520 > >>> > 0d739a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635% > >>> > 7C0%7C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIj > >>> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C > >>> > 3000%7C%7C%7C&sdata=GtNruefDreOoogL%2BlntAC7GBfk6E1Goq4j% > >>> 2BYXt36RdI%3D&reserved=0 > >>> > >>> In term of device tree this would result in such hierarchy (stm32mp1 > >>> example with 2 virtio RPMSG): > >>> > >>> m4_rproc: m4@10000000 { > >>> compatible = "st,stm32mp1-m4"; > >>> reg = <0x10000000 0x40000>, > >>> <0x30000000 0x40000>, > >>> <0x38000000 0x10000>; > >>> memory-region = <&retram>, <&mcuram>,<&mcuram2>; > >>> mboxes = <&ipcc 2>, <&ipcc 3>; > >>> mbox-names = "shutdown", "detach"; > >>> status = "okay"; > >>> > >>> #address-cells = <1>; > >>> #size-cells = <0>; > >>> > >>> vdev@0 { > >>> compatible = "rproc-virtio"; > >>> reg = <0>; > >>> virtio,id = <7>; /* RPMSG */ > >>> memory-region = <&vdev0vring0>, <&vdev0vring1>, > <&vdev0buffer>; > >>> mboxes = <&ipcc 0>, <&ipcc 1>; > >>> mbox-names = "vq0", "vq1"; > >>> status = "okay"; > >>> }; > >>> > >>> vdev@1 { > >>> compatible = "rproc-virtio"; > >>> reg = <1>; > >>> virtio,id = <7>; /*RPMSG */ > >>> memory-region = <&vdev1vring0>, <&vdev1vring1>, > <&vdev1buffer>; > >>> mboxes = <&ipcc 4>, <&ipcc 5>; > >>> mbox-names = "vq0", "vq1"; > >>> status = "okay"; > >>> }; > >>> }; > >>> > >>> I have divided the work in 4 steps to simplify the review, This > >>> series implements only the step 1: > >>> step 1: Redefine the remoteproc VirtIO device as a platform device > >>> - migrate rvdev management in remoteproc virtio.c, > >>> - create a remotproc virtio config ( can be disabled for platform > >>> that not use VirtIO IPC. > >>> step 2: Add possibility to declare and probe a VirtIO sub node > >>> - VirtIO bindings declaration, > >>> - multi DT VirtIO devices support, > >>> - introduction of a remote proc virtio bind device mechanism , => > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >>> thu > >>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep2-virtio-in- > >>> > DT&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e74 > >>> > 39508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C > >>> > 637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > >>> > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C > >>> %7C&sdata=XtF%2FQnml3QXFL7rgqST1Z2FotUzoj%2FD57WfiuAVM > nr8 > >>> %3D&reserved=0 > >>> step 3: Add memory declaration in VirtIO subnode => > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >>> thu > >>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep3-virtio- > >>> > memories&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739 > >>> > a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 > >>> > C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 > >>> > wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000% > >>> > 7C%7C%7C&sdata=6gq28c6a1TJ%2FdkvokcEjgy6FKQcKTXSz%2BNAbJPo > >>> mjac%3D&reserved=0 > >>> step 4: Add mailbox declaration in VirtIO subnode => > >>> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >>> thu > >>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio- > >>> > mailboxes&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739 > >>> > a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 > >>> > C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 > >>> > wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000% > >>> > 7C%7C%7C&sdata=wfy2euuOPoMmBMIH3BOsGcsEYGSTWsDaRr7ENN > >>> QCK70%3D&reserved=0 > >>> > >>> Arnaud Pouliquen (4): > >>> remoteproc: core: Introduce rproc_rvdev_add_device function > >>> remoteproc: core: Introduce rproc_add_rvdev function > >>> remoteproc: Move rproc_vdev management to remoteproc_virtio.c > >>> remoteproc: virtio: Create platform device for the > >>> remoteproc_virtio > >>> > >>> drivers/remoteproc/remoteproc_core.c | 154 +++--------------- > >>> drivers/remoteproc/remoteproc_internal.h | 23 ++- > >>> drivers/remoteproc/remoteproc_virtio.c | 189 > ++++++++++++++++++++--- > >>> include/linux/remoteproc.h | 6 +- > >>> 4 files changed, 210 insertions(+), 162 deletions(-) > >>> > >>> -- > >>> 2.24.3 > >>