michallenc commented on code in PR #6298:
URL: https://github.com/apache/incubator-nuttx/pull/6298#discussion_r876999019


##########
arch/arm/src/samv7/sam_pwm.c:
##########
@@ -493,71 +449,39 @@ static int pwm_start(struct pwm_lowerhalf_s *dev,
                      const struct pwm_info_s *info)
 {
   struct sam_pwm_s *priv = (struct sam_pwm_s *)dev;
-  int ret = OK;
 
-  /* Change frequency only if it is needed */
-
-  if (info->frequency != priv->frequency)
-    {
+#ifdef CONFIG_PWM_MULTICHAN
       for (int i = 0; i < PWM_NCHANNELS; i++)
         {
-#ifdef CONFIG_PWM_MULTICHAN
+          int8_t index = info->channels[i].channel;
+
           /* Break the loop if all following channels are not configured */
 
-          if (info->channels[i].channel == -1)
+          if (index == -1)
             {
               break;
             }
 
           /* Configure the module freq only if is set to be used */
 
-          if (info->channels[i].channel != 0)
+          if (index != 0)
             {
-              ret = pwm_change_freq(dev, info, i);
-            }
-#else
-          ret = pwm_change_freq(dev, info, i);
-#endif
-        }
-
-      /* Save current frequency */
-
-      if (ret == OK)
-        {
-          priv->frequency = info->frequency;
-        }
-    }
+              /* Set the frequency and enable PWM output for each channel */
 
-#ifdef CONFIG_PWM_MULTICHAN
-  for (int i = 0; ret == OK && i < PWM_NCHANNELS; i++)
-    {
-      /* Break the loop if all following channels are not configured */
-
-      if (info->channels[i].channel == -1)
-        {
-          break;
-        }
-
-      /* Enable PWM output for each channel */
-
-      if (info->channels[i].channel != 0)
-        {
-          ret = pwm_set_output(dev, info->channels[i].channel - 1,
-                                    info->channels[i].duty);
+              pwm_set_freq(dev, priv->channels[index - 1].channel,

Review Comment:
   I agree with your point about board to board (chip to chip) portability 
which is better with virtual channels. I tested your branch and managed to get 
both single channel and multiple channels output (using virtual channels in 
`example/pwm` app).
   
   Yes, we can move the discussion regarding channel indexes to mailing list. I 
am now inclining towards virtual channels but we should document it properly 
somewhere.



-- 
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-unsubscr...@nuttx.apache.org

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

Reply via email to