pkarashchenko commented on code in PR #6298:
URL: https://github.com/apache/incubator-nuttx/pull/6298#discussion_r876919879
##########
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:
Yeah, I wasn't able to find any description as well. but my points are next:
1. The application (for example `examples/pwm`) should be portable from
board to board and across the chips (again this is purely my understanding) so
it has to operate with "virtual" channels like it does now:
```
config EXAMPLES_PWM_CHANNEL1
int "First PWM channel number"
default 1
range 1 6
---help---
The first PWM channel number. Default: 1
```
2. Each specific board can implement PWM "virtual" channel based on specific
HW details (map to HW channel 3 or 4 or even 567).
It is like with `/dev/timerX`, so `/dev/time1` does not mean that HW timer 1
is used.
But anyway let's move this discussion to the mailing thread.
--
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
