You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/08/23 12:10:06 UTC

[GitHub] [incubator-nuttx] eenurkka opened a new pull request, #6902: risc-v/mpfs: ihc: update vq ids

eenurkka opened a new pull request, #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902

   Since the commit cf22dd8 (related to OpenAMP update), the notifyid
   is no longer NOTIFY_ALL, but the vq id.
   
   Utilize the vq id now properly as it's being provided. Also swap the
   vring notifyids, they don't match.
   
   Signed-off-by: Eero Nurkkala <ee...@offcode.fi>
   
   ## Summary
   
   OpenAMP update related fix. Currently the IHC/RPMSG doesn't work.
   
   ## Impact
   
   MPFS / Icicle with Linux
   
   ## Testing
   
   Polarfire Icicle kit; NuttX on harts 1 and 2, Linux on harts 3 and 4
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6902: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952928855


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   Yes, this restriction comes from Microchip sw, not the rpmsg itself.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6902: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952893762


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   > Looks like it might create ids 1 & 2,
   
   Because the first id is allocated to fw_rsc_vdev:
   https://github.com/OpenAMP/open-amp/blob/main/lib/include/openamp/remoteproc.h#L296
   and next two to fw_rsc_vdev_vring:
   https://github.com/OpenAMP/open-amp/blob/main/lib/include/openamp/remoteproc.h#L253
   
   > but the Linux side expects fixed 0 and 1. And only id 0 is sent to Linux kernel.
   
   I have long time not watch the Linux change, but I never hit this limitation before when I am working with remoteproc/rpmsg several years ago.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6902: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952548534


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   Should be rsc->rpmsg_vring1.notifyid    = 0;  (not vring0, apparent typo).. need to check



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6902: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6902: WIP: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952583795


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   it's better to initialize notifyid to RSC_NOTIFY_ID_ANY, so the framework will allocate the unique id for you automatically:
   https://github.com/OpenAMP/open-amp/blob/main/lib/remoteproc/rsc_table_parser.c#L131-L136



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6902: WIP: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952809023


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   Looks like it might create ids 1 & 2, but the Linux side expects fixed 0 and 1. And only id 0 is sent to Linux kernel.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6902: WIP: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952607678


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   I updated sim's implementation to reflect this fact: https://github.com/apache/incubator-nuttx/pull/6903



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] eenurkka commented on a diff in pull request #6902: WIP: risc-v/mpfs: ihc: update vq ids

Posted by GitBox <gi...@apache.org>.
eenurkka commented on code in PR #6902:
URL: https://github.com/apache/incubator-nuttx/pull/6902#discussion_r952612056


##########
arch/risc-v/src/mpfs/mpfs_ihc.c:
##########
@@ -965,11 +965,11 @@ mpfs_rptun_get_resource(struct rptun_dev_s *dev)
       rsc->rpmsg_vring0.align       = VRING_ALIGN;
       rsc->rpmsg_vring0.num         = VRING_NR;
       rsc->rpmsg_vring0.da          = VRING0_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 0;
+      rsc->rpmsg_vring0.notifyid    = 1;
       rsc->rpmsg_vring1.align       = VRING_ALIGN;
       rsc->rpmsg_vring1.num         = VRING_NR;
       rsc->rpmsg_vring1.da          = VRING1_DESCRIPTORS;
-      rsc->rpmsg_vring0.notifyid    = 1;
+      rsc->rpmsg_vring0.notifyid    = 0;

Review Comment:
   @xiaoxiang781216 Thanks for the input ,I'll try how it goes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org