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



##########
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:
       I agree with buttons and djoystick changes, but first look here leads me 
to the next findings in `boards/arm/stm32/nucleo-f4x1re/src/stm32_ajoystick.c`:
   ```
   static int ajoy_sample(FAR const struct ajoy_lowerhalf_s *lower,
                          FAR struct ajoy_sample_s *sample)
   {
   #ifndef NO_JOYSTICK_ADC
     struct adc_msg_s adcmsg[MAX_ADC_CHANNELS];
     FAR struct adc_msg_s *ptr;
     ssize_t nread;
     ssize_t offset;
     int have;
     int i;
   
     /* Read all of the available samples (handling the case where additional
      * channels are enabled).
      */
   
     nread = file_read(&g_adcfile, adcmsg,
                       MAX_ADC_CHANNELS * sizeof(struct adc_msg_s));
     if (nread < 0)
       {
         if (nread != -EINTR)
           {
             ierr("ERROR: read failed: %d\n", (int)nread);
           }
   
         return nread;
       }
   ...
   ```
   So the question is: Is it safe to call `file_read` from the critical section?




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