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/01/14 16:07:54 UTC

[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #5232: rpmsg_rtc: resolve deadlock when the receive SYNC cmd

GUIDINGLI opened a new pull request #5232:
URL: https://github.com/apache/incubator-nuttx/pull/5232


   ## Summary
   
   rpmsg_rtc: resolve deadlock when the receive SYNC cmd.
   add `tp` param to up_rtc_set_lowerhalf() & clock_synchronize()
   
   Modify these for 2 reasons:
   1. client CPU call `up_rtc_set_lowerhalf(rpmsg_rtc_initialize(0))`;
   up_rtc_set_lowerhalf() -> clock_synchronize -> clock_inittime -> clock_basetime -> up_rtc_getdatetime ->  g_rtc_lower->ops->rdtime  ->  rpmsg
   
   if the server CPU hasn't start, and rptun hasn't setup done, then the rpmsg will be failed.
   So, `up_rtc_set_lowerhalf()` shouldn't call with `rpmsg_rtc_initialize()`
   
   2. rtc rpmsg SYNC packet
   this is means: server set time, then sync the time to all the remote core.
   If the client receive the SYNC packet, then should call clock_synchronize().
   Before this PR, clock_synchronize() will send IPC to server get time again.
   After this PR, SYNC packet with time, and clock_synchronize() don't need do IPC again.
   
   ## Impact
   
   ## Testing
   
   


-- 
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] GUIDINGLI commented on a change in pull request #5232: rpmsg_rtc: resolve deadlock when the receive SYNC cmd

Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #5232:
URL: https://github.com/apache/incubator-nuttx/pull/5232#discussion_r786409489



##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -310,7 +297,16 @@ static int rpmsg_rtc_ept_cb(FAR struct rpmsg_endpoint *ept, FAR void *data,
       break;
 
     case RPMSG_RTC_SYNC:
-      rpmsg_rtc_sync_handler(priv);
+        {
+          struct rpmsg_rtc_set_s *msg = data;
+          struct timespec tp =
+          {
+            .tv_sec  = msg->sec,
+            .tv_nsec = msg->nsec,
+          };

Review comment:
       I have tried add -std=c89 when use `./tools/configure.sh sim:rpserver`, and found lots of the build error.
   
   And if here can't use like this, how about:
   
   drivers/serial/uart_16550.c
    178 static uart_dev_t g_uart0port =
    179 {
    180   .recv     =
    181   {
    182     .size   = CONFIG_16550_UART0_RXBUFSIZE,
    183     .buffer = g_uart0rxbuffer,
    184   },
   
   there are so many places use like this, all of them need change ?
   




-- 
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 #5232: rpmsg_rtc: resolve deadlock when the receive SYNC cmd

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


   


-- 
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] pkarashchenko commented on a change in pull request #5232: rpmsg_rtc: resolve deadlock when the receive SYNC cmd

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5232:
URL: https://github.com/apache/incubator-nuttx/pull/5232#discussion_r785837255



##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -472,18 +468,22 @@ static int rpmsg_rtc_server_settime(FAR struct rtc_lowerhalf_s *lower,
                          (FAR struct rpmsg_rtc_server_s *)lower;
   FAR struct rpmsg_rtc_client_s *client;
   FAR struct list_node *node;
-  struct rpmsg_rtc_header_s header;
   int ret;
+  struct rpmsg_rtc_set_s msg =
+  {
+    .sec  = timegm((FAR struct tm *)rtctime),
+    .nsec = rtctime->tm_nsec,

Review comment:
       C89 incompatible

##########
File path: sched/clock/clock_initialize.c
##########
@@ -155,14 +155,22 @@ int clock_basetime(FAR struct timespec *tp)
  ****************************************************************************/
 
 #ifdef CONFIG_RTC
-static void clock_inittime(void)
+static void clock_inittime(FAR const struct timespec *tp)
 {
   /* (Re-)initialize the time value to match the RTC */
 
 #ifndef CONFIG_CLOCK_TIMEKEEPING
   struct timespec ts;
 
-  clock_basetime(&g_basetime);
+  if (tp)
+    {
+      memcpy(&g_basetime, tp, sizeof(struct timespec));

Review comment:
       Maybe can go with the shorter `g_clock_wall_time = *tp;` as in `clock_timekeeping_set_wall_time` or switch to `memcpy` there. Just for style consistency.

##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -310,7 +297,16 @@ static int rpmsg_rtc_ept_cb(FAR struct rpmsg_endpoint *ept, FAR void *data,
       break;
 
     case RPMSG_RTC_SYNC:
-      rpmsg_rtc_sync_handler(priv);
+        {
+          struct rpmsg_rtc_set_s *msg = data;
+          struct timespec tp =
+          {
+            .tv_sec  = msg->sec,
+            .tv_nsec = msg->nsec,
+          };

Review comment:
       C89 incompatible




-- 
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