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



##########
File path: drivers/input/ajoystick.c
##########
@@ -526,12 +461,7 @@ static ssize_t ajoy_read(FAR struct file *filep, FAR char 
*buffer,
 
   /* Get exclusive access to the driver structure */
 
-  ret = ajoy_takesem(&priv->au_exclsem);
-  if (ret < 0)
-    {
-      ierr("ERROR: ajoy_takesem failed: %d\n", ret);
-      return ret;
-    }
+  flags = enter_critical_section();

Review comment:
       Is `lower->al_sample(lower, (FAR struct ajoy_sample_s *)buffer);` -> 
`al_sample` implementation ready that now it is called from inside the critical 
section?

##########
File path: drivers/input/ajoystick.c
##########
@@ -181,12 +162,6 @@ static void ajoy_enable(FAR struct ajoy_upperhalf_s *priv)
   lower = priv->au_lower;
   DEBUGASSERT(lower);
 
-  /* This routine is called both task level and interrupt level, so
-   * interrupts must be disabled.
-   */
-
-  flags = enter_critical_section();

Review comment:
       Probably need to remove `irqstate_t flags;` as well

##########
File path: drivers/input/ajoystick.c
##########
@@ -686,7 +612,8 @@ static int ajoy_poll(FAR struct file *filep, FAR struct 
pollfd *fds,
   FAR struct inode *inode;
   FAR struct ajoy_upperhalf_s *priv;
   FAR struct ajoy_open_s *opriv;
-  int ret;
+  irqstate_t flags;
+  int ret = 0;

Review comment:
       ```suggestion
     int ret = OK;
   ```

##########
File path: drivers/input/djoystick.c
##########
@@ -524,12 +459,7 @@ static ssize_t djoy_read(FAR struct file *filep, FAR char 
*buffer,
 
   /* Get exclusive access to the driver structure */
 
-  ret = djoy_takesem(&priv->du_exclsem);
-  if (ret < 0)
-    {
-      ierr("ERROR: djoy_takesem failed: %d\n", ret);
-      return ret;
-    }
+  flags = enter_critical_section();

Review comment:
       Is `dl_sample` implementation ready that now it is called from inside 
the critical section?

##########
File path: drivers/input/djoystick.c
##########
@@ -181,12 +162,6 @@ static void djoy_enable(FAR struct djoy_upperhalf_s *priv)
   lower = priv->du_lower;
   DEBUGASSERT(lower);
 
-  /* This routine is called both task level and interrupt level, so
-   * interrupts must be disabled.
-   */
-
-  flags = enter_critical_section();

Review comment:
       Probably need to remove `irqstate_t flags;` as well




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to