Skip to content

ports/machine_pwm: Set the PWM duty_u16() back to 65535. #16153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 18, 2024
Merged

ports/machine_pwm: Set the PWM duty_u16() back to 65535. #16153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
898407d
ports: Make PWM duty_u16 have an upper value of 65535 across all ports.
robert-hh Nov 4, 2024
e2532e0
mimxrt/machine_pwm: Fix a few inconsistencies with PWM output.
robert-hh Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/mimxrt/quickref.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ PWM Constructor

- *freq* should be an integer which sets the frequency in Hz for the
PWM cycle. The valid frequency range is 15 Hz resp. 18Hz resp. 24Hz up to > 1 MHz.
- *duty_u16* sets the duty cycle as a ratio ``duty_u16 / 65536``.
- *duty_u16* sets the duty cycle as a ratio ``duty_u16 / 65535``.
The duty cycle of a X channel can only be changed, if the A and B channel
of the respective submodule is not used. Otherwise the duty_16 value of the
X channel is 32768 (50%).
Expand Down Expand Up @@ -231,7 +231,7 @@ is created by dividing the pwm_clk signal by an integral factor, according to th

f = pwm_clk / (2**n * m)

with n being in the range of 0..7, and m in the range of 2..65536. pmw_clk is 125Mhz
with n being in the range of 0..7, and m in the range of 2..65535. pmw_clk is 125Mhz
for MIMXRT1010/1015/1020, 150 MHz for MIMXRT1050/1060/1064 and 160MHz for MIMXRT1170.
The lowest frequency is pwm_clk/2**23 (15, 18, 20Hz). The highest frequency with
U16 resolution is pwm_clk/2**16 (1907, 2288, 2441 Hz), the highest frequency
Expand All @@ -255,7 +255,7 @@ Use the :ref:`machine.ADC <machine.ADC>` class::
from machine import ADC

adc = ADC(Pin('A2')) # create ADC object on ADC pin
adc.read_u16() # read value, 0-65536 across voltage range 0.0v - 3.3v
adc.read_u16() # read value, 0-65535 across voltage range 0.0v - 3.3v

The resolution of the ADC is 12 bit with 10 to 11 bit accuracy, irrespective of the
value returned by read_u16(). If you need a higher resolution or better accuracy, use
Expand Down
4 changes: 2 additions & 2 deletions docs/samd/quickref.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ PWM Constructor

- *freq* should be an integer which sets the frequency in Hz for the
PWM cycle. The valid frequency range is 1 Hz to 24 MHz.
- *duty_u16* sets the duty cycle as a ratio ``duty_u16 / 65536``.
- *duty_u16* sets the duty cycle as a ratio ``duty_u16 / 65535``.
- *duty_ns* sets the pulse width in nanoseconds. The limitation for X channels
apply as well.
- *invert*\=True|False. Setting a bit inverts the respective output.
Expand Down Expand Up @@ -246,7 +246,7 @@ Use the :ref:`machine.ADC <machine.ADC>` class::
from machine import ADC

adc0 = ADC(Pin('A0')) # create ADC object on ADC pin, average=16
adc0.read_u16() # read value, 0-65536 across voltage range 0.0v - 3.3v
adc0.read_u16() # read value, 0-65535 across voltage range 0.0v - 3.3v
adc1 = ADC(Pin('A1'), average=1) # create ADC object on ADC pin, average=1

The resolution of the ADC is 12 bit with 12 bit accuracy, irrespective of the
Expand Down
6 changes: 3 additions & 3 deletions ports/esp8266/machine_pwm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static void mp_machine_pwm_init_helper(machine_pwm_obj_t *self, size_t n_args, c
pwm_set_duty(args[ARG_duty].u_int, self->channel);
}
if (args[ARG_duty_u16].u_int != -1) {
pwm_set_duty(args[ARG_duty_u16].u_int * 1000 / 65536, self->channel);
pwm_set_duty(args[ARG_duty_u16].u_int * 1000 / 65535, self->channel);
}
if (args[ARG_duty_ns].u_int != -1) {
uint32_t freq = pwm_get_freq(0);
Expand Down Expand Up @@ -164,13 +164,13 @@ static void mp_machine_pwm_duty_set(machine_pwm_obj_t *self, mp_int_t duty) {

static mp_obj_t mp_machine_pwm_duty_get_u16(machine_pwm_obj_t *self) {
set_active(self, true);
return MP_OBJ_NEW_SMALL_INT(pwm_get_duty(self->channel) * 65536 / 1024);
return MP_OBJ_NEW_SMALL_INT(pwm_get_duty(self->channel) * 65535 / 1024);
}

static void mp_machine_pwm_duty_set_u16(machine_pwm_obj_t *self, mp_int_t duty) {
set_active(self, false);
self->duty_ns = -1;
pwm_set_duty(duty * 1024 / 65536, self->channel);
pwm_set_duty(duty * 1024 / 65535, self->channel);
pwm_start();
}

Expand Down
14 changes: 5 additions & 9 deletions ports/mimxrt/hal/pwm_backport.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ void PWM_UpdatePwmDutycycle_u16(

// Setup the PWM dutycycle of channel A or B
if (pwmSignal == kPWM_PwmA) {
if (dutyCycle >= 65536) {
if (dutyCycle >= PWM_FULL_SCALE) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant is still defined as 65536 in pwm_backport.h. Does it need changing there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Behavior still fine. At duty_u16(32768) the difference between low and high phases with a Teensy 4.x are 700ps (flexpwm) and 400ps (QTMR).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the hardware have even better symmetry with 50% duty cycle, if you choose the right upper limit for the counter? If so, it may be possible to get this working with machine.PWM by using 65536 for the hardware full scale, but 65535 for the duty_u16 max, and then scaling it via x * 65536 / 65535. I think this is how the rp2 port works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference between the high/low phase is 10% of the PWM clock or about 50% of the CPU clock. It hardly can get much better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right... picoseconds are very small! Great work testing it to that degree.

base->SM[subModule].VAL2 = 0;
base->SM[subModule].VAL3 = pulseCnt;
} else {
base->SM[subModule].VAL2 = center - pwmHighPulse / 2;
base->SM[subModule].VAL3 = base->SM[subModule].VAL2 + pwmHighPulse;
}
} else {
if (dutyCycle >= 65536) {
if (dutyCycle >= PWM_FULL_SCALE) {
base->SM[subModule].VAL4 = 0;
base->SM[subModule].VAL5 = pulseCnt;
} else {
Expand Down Expand Up @@ -110,12 +110,8 @@ void PWM_SetupPwmx_u16(PWM_Type *base, pwm_submodule_t subModule,

base->SM[subModule].OCTRL = (base->SM[subModule].OCTRL & ~PWM_OCTRL_POLX_MASK) | PWM_OCTRL_POLX(!invert);

// Switch the output on or off.
if (duty_cycle == 0) {
base->OUTEN &= ~(1U << subModule);
} else {
base->OUTEN |= (1U << subModule);
}
// Enable PWM output
base->OUTEN |= (1U << subModule);
}

#ifdef FSL_FEATURE_SOC_TMR_COUNT
Expand Down Expand Up @@ -160,7 +156,7 @@ status_t QTMR_SetupPwm_u16(TMR_Type *base, qtmr_channel_selection_t channel, uin
if (dutyCycleU16 == 0) {
// Clear the output at the next compare
reg |= (TMR_CTRL_LENGTH_MASK | TMR_CTRL_OUTMODE(kQTMR_ClearOnCompare));
} else if (dutyCycleU16 >= 65536) {
} else if (dutyCycleU16 >= PWM_FULL_SCALE) {
// Set the output at the next compare
reg |= (TMR_CTRL_LENGTH_MASK | TMR_CTRL_OUTMODE(kQTMR_SetOnCompare));
} else {
Expand Down
2 changes: 1 addition & 1 deletion ports/mimxrt/hal/pwm_backport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ typedef struct _pwm_signal_param_u16
uint16_t deadtimeValue; // The deadtime value; only used if channel pair is operating in complementary mode
} pwm_signal_param_u16_t;

#define PWM_FULL_SCALE (65536UL)
#define PWM_FULL_SCALE (65535UL)

void PWM_UpdatePwmDutycycle_u16(PWM_Type *base, pwm_submodule_t subModule,
pwm_channels_t pwmSignal, uint32_t dutyCycle, uint16_t center);
Expand Down
29 changes: 26 additions & 3 deletions ports/mimxrt/machine_pwm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
typedef struct _machine_pwm_obj_t {
mp_obj_base_t base;
PWM_Type *instance;
const machine_pin_obj_t *pwm_pin;
const machine_pin_af_obj_t *pwm_pin_af_obj;
bool is_flexpwm;
uint8_t complementary;
uint8_t module;
Expand Down Expand Up @@ -255,6 +257,8 @@ static void configure_flexpwm(machine_pwm_obj_t *self) {
PWM_SetupFaultDisableMap(self->instance, self->submodule, self->channel2, kPWM_faultchannel_1, 0);
}

// clear the load okay bit for the submodules in case there is a pending load
PWM_SetPwmLdok(self->instance, 1 << self->submodule, false);
if (self->channel1 != kPWM_PwmX) { // Only for A/B channels
// Initialize the channel parameters
pwmSignal.pwmChannel = self->channel1;
Expand Down Expand Up @@ -283,6 +287,17 @@ static void configure_flexpwm(machine_pwm_obj_t *self) {
self->instance->SM[self->submodule].CTRL &= ~(PWM_CTRL_DBLEN_MASK | PWM_CTRL_SPLIT_MASK);
}
} else {
if (self->duty_u16 == 0) {
// For duty_u16 == 0 just set the output to GPIO mode
if (self->invert) {
mp_hal_pin_high(self->pwm_pin);
} else {
mp_hal_pin_low(self->pwm_pin);
}
IOMUXC_SetPinMux(self->pwm_pin->muxRegister, PIN_AF_MODE_ALT5, 0, 0, 0, 0U);
} else {
IOMUXC_SetPinMux(self->pwm_pin->muxRegister, self->pwm_pin_af_obj->af_mode, 0, 0, 0, 0U);
}
PWM_SetupPwmx_u16(self->instance, self->submodule, self->freq, self->duty_u16,
self->invert, pwmSourceClockInHz);
if (self->xor) {
Expand Down Expand Up @@ -408,12 +423,16 @@ static void mp_machine_pwm_init_helper(machine_pwm_obj_t *self,
}
self->center = center;
} else { // Use alignment setting shortcut
if (args[ARG_align].u_int >= 0) {
uint32_t duty = self->duty_u16;
if (duty == VALUE_NOT_SET && self->duty_ns != VALUE_NOT_SET) {
duty = duty_ns_to_duty_u16(self->freq, self->duty_ns);
}
if (args[ARG_align].u_int >= 0 && duty != VALUE_NOT_SET && self->freq != VALUE_NOT_SET) {
uint8_t align = args[ARG_align].u_int & 3; // limit to 0..3
if (align == PWM_BEGIN) {
self->center = self->duty_u16 / 2;
self->center = duty / 2;
} else if (align == PWM_END) {
self->center = PWM_FULL_SCALE - self->duty_u16 / 2;
self->center = PWM_FULL_SCALE - duty / 2;
} else {
self->center = 32768; // Default value: mid.
}
Expand Down Expand Up @@ -515,6 +534,8 @@ static mp_obj_t mp_machine_pwm_make_new(const mp_obj_type_t *type, size_t n_args

// Create and populate the PWM object.
machine_pwm_obj_t *self = mp_obj_malloc(machine_pwm_obj_t, &machine_pwm_type);
self->pwm_pin = pin1;
self->pwm_pin_af_obj = af_obj1;
self->is_flexpwm = is_flexpwm;
self->instance = af_obj1->instance;
self->module = module;
Expand All @@ -534,6 +555,8 @@ static mp_obj_t mp_machine_pwm_make_new(const mp_obj_type_t *type, size_t n_args

// Initialize the Pin(s).
CLOCK_EnableClock(kCLOCK_Iomuxc); // just in case it was not set yet
// Configure PWMX channels to pin output mode to be prepared for duty_u16 == 0.
mp_hal_pin_output(pin1);
IOMUXC_SetPinMux(pin1->muxRegister, af_obj1->af_mode, af_obj1->input_register, af_obj1->input_daisy,
pin1->configRegister, 0U);
IOMUXC_SetPinConfig(pin1->muxRegister, af_obj1->af_mode, af_obj1->input_register, af_obj1->input_daisy,
Expand Down
6 changes: 3 additions & 3 deletions ports/nrf/modules/machine/pwm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ static mp_obj_t mp_machine_pwm_duty_get(machine_pwm_obj_t *self) {
if (self->p_config->duty_mode[self->channel] == DUTY_PERCENT) {
return MP_OBJ_NEW_SMALL_INT(self->p_config->duty[self->channel]);
} else if (self->p_config->duty_mode[self->channel] == DUTY_U16) {
return MP_OBJ_NEW_SMALL_INT(self->p_config->duty[self->channel] * 100 / 65536);
return MP_OBJ_NEW_SMALL_INT(self->p_config->duty[self->channel] * 100 / 65535);
} else {
return MP_OBJ_NEW_SMALL_INT(-1);
}
Expand All @@ -301,7 +301,7 @@ static mp_obj_t mp_machine_pwm_duty_get_u16(machine_pwm_obj_t *self) {
if (self->p_config->duty_mode[self->channel] == DUTY_U16) {
return MP_OBJ_NEW_SMALL_INT(self->p_config->duty[self->channel]);
} else if (self->p_config->duty_mode[self->channel] == DUTY_PERCENT) {
return MP_OBJ_NEW_SMALL_INT(self->p_config->duty[self->channel] * 65536 / 100);
return MP_OBJ_NEW_SMALL_INT(self->p_config->duty[self->channel] * 65535 / 100);
} else {
return MP_OBJ_NEW_SMALL_INT(-1);
}
Expand Down Expand Up @@ -365,7 +365,7 @@ static void machine_hard_pwm_start(const machine_pwm_obj_t *self) {
if (self->p_config->duty_mode[i] == DUTY_PERCENT) {
pulse_width = ((period * self->p_config->duty[i]) / 100);
} else if (self->p_config->duty_mode[i] == DUTY_U16) {
pulse_width = ((period * self->p_config->duty[i]) / 65536);
pulse_width = ((period * self->p_config->duty[i]) / 65535);
} else if (self->p_config->duty_mode[i] == DUTY_NS) {
pulse_width = (uint64_t)self->p_config->duty[i] * tick_freq / 1000000000ULL;
}
Expand Down
2 changes: 1 addition & 1 deletion ports/samd/machine_pwm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ typedef struct _machine_pwm_obj_t {
#define PWM_CLK_READY (1)
#define PWM_TCC_ENABLED (2)
#define PWM_MASTER_CLK (get_peripheral_freq())
#define PWM_FULL_SCALE (65536)
#define PWM_FULL_SCALE (65535)
#define PWM_UPDATE_TIMEOUT (2000)

#define VALUE_NOT_SET (-1)
Expand Down
Loading