[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5230: Simplify the RTC rpmsg driver implementation

[GitBox <gi...@apache.org>]

pkarashchenko commented on a change in pull request #5230:
URL: https://github.com/apache/incubator-nuttx/pull/5230#discussion_r785386203



##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -653,31 +656,31 @@ static void rpmsg_rtc_server_ns_bind(FAR struct rpmsg_device *rdev,
                                      uint32_t dest)
 {
   FAR struct rpmsg_rtc_server_s *server = priv;
-  FAR struct rpmsg_rtc_session_s *session;
+  FAR struct rpmsg_rtc_client_s *client;
 
   if (strcmp(name, RPMSG_RTC_EPT_NAME))
     {
       return;
     }
 
-  session = kmm_zalloc(sizeof(*session));
-  if (!session)
+  client = kmm_zalloc(sizeof(*client));
+  if (!client)

Review comment:
       ```suggestion
     if (client == NULL)
   ```

##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -590,38 +590,41 @@ static int rpmsg_rtc_server_ept_cb(FAR struct rpmsg_endpoint *ept,
     case RPMSG_RTC_GET:
       {
         FAR struct rpmsg_rtc_get_s *msg = data;
-        struct timespec ts;
+        struct rtc_time rtctime;
 
-        header->result = clock_gettime(CLOCK_REALTIME, &ts);
-        msg->sec = ts.tv_sec;
-        msg->nsec = ts.tv_nsec;
+        header->result = server->lower->ops->rdtime(server->lower,
+                                                    &rtctime);
+        msg->sec = timegm((FAR struct tm *)&rtctime);
+        msg->nsec = rtctime.tm_nsec;
         return rpmsg_send(ept, msg, sizeof(*msg));
       }
 
     case RPMSG_RTC_SET:
       {
         FAR struct rpmsg_rtc_set_s *msg = data;
-        struct timespec ts;
+        struct rtc_time rtctime;
+        time_t time = msg->sec;
 
-        ts.tv_sec = msg->sec;
-        ts.tv_nsec = msg->nsec;
-        header->result = clock_settime(CLOCK_REALTIME, &ts);
+        gmtime_r(&time, (FAR struct tm *)rtctime);
+        rtctime.tm_nsec = msg->nsec;
+        header->result = server->lower->ops->settime(server->lower,
+                                                     &rtctime);
         return rpmsg_send(ept, msg, sizeof(*msg));
       }
 
 #ifdef CONFIG_RTC_ALARM
     case RPMSG_RTC_ALARM_SET:
       {
-        FAR struct rpmsg_rtc_session_s *session = container_of(ept,
-                                            struct rpmsg_rtc_session_s, ept);
+        FAR struct rpmsg_rtc_client_s *client = container_of(ept,
+                                            struct rpmsg_rtc_client_s, ept);
         FAR struct rpmsg_rtc_alarm_set_s *msg = data;
         FAR struct rpmsg_rtc_server_s *server = priv;
         time_t time = msg->sec;
         struct lower_setalarm_s alarminfo =
         {
           .id = msg->id,
           .cb = rpmsg_rtc_server_alarm_cb,
-          .priv = session
+          .priv = client

Review comment:
       C89 incompatible

##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -554,28 +554,28 @@ static int rpmsg_rtc_server_cancelperiodic
 
 static void rpmsg_rtc_server_ns_unbind(FAR struct rpmsg_endpoint *ept)
 {
-  FAR struct rpmsg_rtc_session_s *session = container_of(ept,
-                                            struct rpmsg_rtc_session_s, ept);
+  FAR struct rpmsg_rtc_client_s *client = container_of(ept,
+                                            struct rpmsg_rtc_client_s, ept);
   FAR struct rpmsg_rtc_server_s *server = ept->priv;
 
   nxsem_wait_uninterruptible(&server->exclsem);
-  list_delete(&session->node);
+  list_delete(&client->node);
   nxsem_post(&server->exclsem);
-  rpmsg_destroy_ept(&session->ept);
-  kmm_free(session);
+  rpmsg_destroy_ept(&client->ept);
+  kmm_free(client);
 }
 
 #ifdef CONFIG_RTC_ALARM
 static void rpmsg_rtc_server_alarm_cb(FAR void *priv, int alarmid)
 {
-  FAR struct rpmsg_rtc_session_s *session = priv;
+  FAR struct rpmsg_rtc_client_s *client = priv;
   struct rpmsg_rtc_alarm_fire_s msg =
   {
     .header.command = RPMSG_RTC_ALARM_FIRE,
     .id = alarmid,

Review comment:
       C89 incompatible

##########
File path: drivers/timers/rpmsg_rtc.c
##########
@@ -689,31 +692,26 @@ static void rpmsg_rtc_server_ns_bind(FAR struct rpmsg_device *rdev,
  *
  *   Take remote core RTC as external RTC hardware through rpmsg.
  *
- * Input Parameters:
- *   minor  - device minor number
- *
  * Returned Value:
  *   Return the lower half RTC driver instance on success;
  *   A NULL pointer on failure.
  *
  ****************************************************************************/
 
 #ifndef CONFIG_RTC_RPMSG_SERVER
-FAR struct rtc_lowerhalf_s *rpmsg_rtc_initialize(int minor)
+FAR struct rtc_lowerhalf_s *rpmsg_rtc_initialize(void)
 {
   FAR struct rpmsg_rtc_lowerhalf_s *lower;
 
   lower = kmm_zalloc(sizeof(*lower));
   if (lower)

Review comment:
       ```suggestion
     if (lower != NULL)
   ```




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