From 5fe4c3bec90435a036083aea6e4a347333876fb5 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 5 Dec 2021 21:16:46 -0500 Subject: [PATCH] fix mistaken use of PWM channel for slice --- .../common-hal/audiopwmio/PWMAudioOut.c | 2 +- .../raspberrypi/common-hal/countio/Counter.c | 11 ++-- ports/raspberrypi/common-hal/pwmio/PWMOut.c | 52 +++++++++---------- ports/raspberrypi/common-hal/pwmio/PWMOut.h | 24 ++++----- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c b/ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c index 0654037d66..4b4c365385 100644 --- a/ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c +++ b/ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c @@ -207,7 +207,7 @@ void common_hal_audiopwmio_pwmaudioout_play(audiopwmio_pwmaudioout_obj_t *self, uint32_t tx_register = (uint32_t)&pwm_hw->slice[self->left_pwm.slice].cc; if (self->stereo) { // Shift the destination if we are outputting to both PWM channels. - tx_register += self->left_pwm.channel * sizeof(uint16_t); + tx_register += self->left_pwm.ab_channel * sizeof(uint16_t); } self->pacing_timer = pacing_timer; diff --git a/ports/raspberrypi/common-hal/countio/Counter.c b/ports/raspberrypi/common-hal/countio/Counter.c index bbf71f996e..fe7dd545f8 100644 --- a/ports/raspberrypi/common-hal/countio/Counter.c +++ b/ports/raspberrypi/common-hal/countio/Counter.c @@ -25,8 +25,8 @@ void common_hal_countio_counter_construct(countio_counter_obj_t *self, mp_raise_RuntimeError(translate("PWM slice already in use")); } - uint8_t channel = pwm_gpio_to_channel(self->pin_a); - if (!pwmio_claim_slice_channels(self->slice_num)) { + uint8_t ab_channel = pwm_gpio_to_channel(self->pin_a); + if (!pwmio_claim_slice_ab_channels(self->slice_num)) { mp_raise_RuntimeError(translate("PWM slice channel A already in use")); } @@ -69,7 +69,7 @@ void common_hal_countio_counter_deinit(countio_counter_obj_t *self) { pwm_set_enabled(self->slice_num, false); pwm_set_irq_enabled(self->slice_num, false); - pwmio_release_slice_channels(self->slice_num); + pwmio_release_slice_ab_channels(self->slice_num); reset_pin_number(self->pin_a); @@ -98,13 +98,14 @@ void common_hal_countio_counter_reset(countio_counter_obj_t *self) { void counter_interrupt_handler(void) { uint32_t mask = pwm_get_irq_status_mask(); - uint8_t i = 1, pos = 1; + uint8_t i = 1; + uint8_t pos = 0; while (!(i & mask)) { i = i << 1; ++pos; } - countio_counter_obj_t *self = MP_STATE_PORT(counting)[pos - 1]; + countio_counter_obj_t *self = MP_STATE_PORT(counting)[pos]; if (self != NULL) { pwm_clear_irq(self->slice_num); self->count += 65536; diff --git a/ports/raspberrypi/common-hal/pwmio/PWMOut.c b/ports/raspberrypi/common-hal/pwmio/PWMOut.c index 27e9bd4d03..3ef4fb57f3 100644 --- a/ports/raspberrypi/common-hal/pwmio/PWMOut.c +++ b/ports/raspberrypi/common-hal/pwmio/PWMOut.c @@ -42,7 +42,7 @@ uint32_t target_slice_frequencies[NUM_PWM_SLICES]; uint32_t slice_variable_frequency; -#define CHANNELS_PER_SLICE 2 +#define AB_CHANNELS_PER_SLICE 2 static uint32_t channel_use; static uint32_t never_reset_channel; @@ -58,11 +58,11 @@ static uint32_t never_reset_channel; // So 65534 should be the maximum top value, and we'll set CC to be TOP+1 as appropriate. #define MAX_TOP 65534 -static uint32_t _mask(uint8_t slice, uint8_t channel) { - return 1 << (slice * CHANNELS_PER_SLICE + channel); +static uint32_t _mask(uint8_t slice, uint8_t ab_channel) { + return 1 << (slice * AB_CHANNELS_PER_SLICE + ab_channel); } -bool pwmio_claim_slice_channels(uint8_t slice) { +bool pwmio_claim_slice_ab_channels(uint8_t slice) { uint32_t channel_use_mask_a = _mask(slice, 0); uint32_t channel_use_mask_b = _mask(slice, 1); @@ -78,37 +78,37 @@ bool pwmio_claim_slice_channels(uint8_t slice) { return true; } -void pwmio_release_slice_channels(uint8_t slice) { +void pwmio_release_slice_ab_channels(uint8_t slice) { uint32_t channel_mask = _mask(slice, 0); channel_use &= ~channel_mask; channel_mask = _mask(slice, 1); channel_use &= ~channel_mask; } -void pwmout_never_reset(uint8_t slice, uint8_t channel) { - never_reset_channel |= _mask(slice, channel); +void pwmout_never_reset(uint8_t slice, uint8_t ab_channel) { + never_reset_channel |= _mask(slice, ab_channel); } -void pwmout_reset_ok(uint8_t slice, uint8_t channel) { - never_reset_channel &= ~_mask(slice, channel); +void pwmout_reset_ok(uint8_t slice, uint8_t ab_channel) { + never_reset_channel &= ~_mask(slice, ab_channel); } void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { - pwmout_never_reset(self->slice, self->channel); + pwmout_never_reset(self->slice, self->ab_channel); never_reset_pin_number(self->pin->number); } void common_hal_pwmio_pwmout_reset_ok(pwmio_pwmout_obj_t *self) { - pwmout_reset_ok(self->slice, self->channel); + pwmout_reset_ok(self->slice, self->ab_channel); } void pwmout_reset(void) { // Reset all slices for (size_t slice = 0; slice < NUM_PWM_SLICES; slice++) { bool reset = true; - for (size_t channel = 0; channel < CHANNELS_PER_SLICE; channel++) { - uint32_t channel_use_mask = _mask(slice, channel); + for (size_t ab_channel = 0; ab_channel < AB_CHANNELS_PER_SLICE; ab_channel++) { + uint32_t channel_use_mask = _mask(slice, ab_channel); if ((never_reset_channel & channel_use_mask) != 0) { reset = false; continue; @@ -124,8 +124,8 @@ void pwmout_reset(void) { } } -pwmout_result_t pwmout_allocate(uint8_t slice, uint8_t channel, bool variable_frequency, uint32_t frequency) { - uint32_t channel_use_mask = _mask(slice, channel); +pwmout_result_t pwmout_allocate(uint8_t slice, uint8_t ab_channel, bool variable_frequency, uint32_t frequency) { + uint32_t channel_use_mask = _mask(slice, ab_channel); // Check the channel first. if ((channel_use & channel_use_mask) != 0) { @@ -171,15 +171,15 @@ pwmout_result_t common_hal_pwmio_pwmout_construct(pwmio_pwmout_obj_t *self, } uint8_t slice = pwm_gpio_to_slice_num(pin->number); - uint8_t channel = pwm_gpio_to_channel(pin->number); + uint8_t ab_channel = pwm_gpio_to_channel(pin->number); - int r = pwmout_allocate(slice, channel, variable_frequency, frequency); + int r = pwmout_allocate(slice, ab_channel, variable_frequency, frequency); if (r != PWMOUT_OK) { return r; } self->slice = slice; - self->channel = channel; + self->ab_channel = ab_channel; if (target_slice_frequencies[slice] != frequency) { // Reset the counter and compare values. @@ -202,11 +202,11 @@ bool common_hal_pwmio_pwmout_deinited(pwmio_pwmout_obj_t *self) { return self->pin == NULL; } -void pwmout_free(uint8_t slice, uint8_t channel) { - uint32_t channel_mask = _mask(slice, channel); +void pwmout_free(uint8_t slice, uint8_t ab_channel) { + uint32_t channel_mask = _mask(slice, ab_channel); channel_use &= ~channel_mask; never_reset_channel &= ~channel_mask; - uint32_t slice_mask = ((1 << CHANNELS_PER_SLICE) - 1) << (slice * CHANNELS_PER_SLICE); + uint32_t slice_mask = ((1 << AB_CHANNELS_PER_SLICE) - 1) << (slice * AB_CHANNELS_PER_SLICE); if ((channel_use & slice_mask) == 0) { target_slice_frequencies[slice] = 0; slice_variable_frequency &= ~(1 << slice); @@ -218,7 +218,7 @@ void common_hal_pwmio_pwmout_deinit(pwmio_pwmout_obj_t *self) { if (common_hal_pwmio_pwmout_deinited(self)) { return; } - pwmout_free(self->slice, self->channel); + pwmout_free(self->slice, self->ab_channel); reset_pin_number(self->pin->number); self->pin = NULL; } @@ -235,13 +235,13 @@ extern void common_hal_pwmio_pwmout_set_duty_cycle(pwmio_pwmout_obj_t *self, uin compare_count = ((uint32_t)duty * self->top + MAX_TOP / 2) / MAX_TOP; } // compare_count is the CC register value, which should be TOP+1 for 100% duty cycle. - pwm_set_chan_level(self->slice, self->channel, compare_count); + pwm_set_chan_level(self->slice, self->ab_channel, compare_count); // Wait for wrap so that we know our new cc value has been applied. Clear // the internal interrupt and then wait for it to be set. Worst case, we // wait a full cycle. - pwm_hw->intr = 1 << self->channel; - while ((pwm_hw->en & (1 << self->channel)) != 0 && - (pwm_hw->intr & (1 << self->channel)) == 0 && + pwm_hw->intr = 1 << self->slice; + while ((pwm_hw->en & (1 << self->slice)) != 0 && + (pwm_hw->intr & (1 << self->slice)) == 0 && !mp_hal_is_interrupted()) { } } diff --git a/ports/raspberrypi/common-hal/pwmio/PWMOut.h b/ports/raspberrypi/common-hal/pwmio/PWMOut.h index c7707762e4..0d179934d0 100644 --- a/ports/raspberrypi/common-hal/pwmio/PWMOut.h +++ b/ports/raspberrypi/common-hal/pwmio/PWMOut.h @@ -24,8 +24,8 @@ * THE SOFTWARE. */ -#ifndef MICROPY_INCLUDED_ATMEL_SAMD_COMMON_HAL_PWMIO_PWMOUT_H -#define MICROPY_INCLUDED_ATMEL_SAMD_COMMON_HAL_PWMIO_PWMOUT_H +#ifndef MICROPY_INCLUDED_RASPBERRY_PI_COMMON_HAL_PWMIO_PWMOUT_H +#define MICROPY_INCLUDED_RASPBERRY_PI_COMMON_HAL_PWMIO_PWMOUT_H #include "common-hal/microcontroller/Pin.h" @@ -34,8 +34,8 @@ typedef struct { mp_obj_base_t base; const mcu_pin_obj_t *pin; - uint8_t slice; - uint8_t channel; + uint8_t slice; // 0-7 + uint8_t ab_channel; // 0-1: A or B slice channel bool variable_frequency; uint16_t duty_cycle; uint32_t actual_frequency; @@ -46,13 +46,13 @@ void pwmout_reset(void); // Private API for AudioPWMOut. void pwmio_pwmout_set_top(pwmio_pwmout_obj_t *self, uint16_t top); // Private APIs for RGBMatrix -enum pwmout_result_t pwmout_allocate(uint8_t slice, uint8_t channel, bool variable_frequency, uint32_t frequency); -void pwmout_free(uint8_t slice, uint8_t channel); -void pwmout_never_reset(uint8_t slice, uint8_t channel); -void pwmout_reset_ok(uint8_t slice, uint8_t channel); +enum pwmout_result_t pwmout_allocate(uint8_t slice, uint8_t ab_channel, bool variable_frequency, uint32_t frequency); +void pwmout_free(uint8_t slice, uint8_t ab_channel); +void pwmout_never_reset(uint8_t slice, uint8_t ab_channel); +void pwmout_reset_ok(uint8_t slice, uint8_t ab_channel); -// Private API for countio to claim both channels on a slice -bool pwmio_claim_slice_channels(uint8_t slice); -void pwmio_release_slice_channels(uint8_t slice); +// Private API for countio to claim both ab_channels on a slice +bool pwmio_claim_slice_ab_channels(uint8_t slice); +void pwmio_release_slice_ab_channels(uint8_t slice); -#endif // MICROPY_INCLUDED_ATMEL_SAMD_COMMON_HAL_PWMIO_PWMOUT_H +#endif // MICROPY_INCLUDED_RASPBERRY_PI_COMMON_HAL_PWMIO_PWMOUT_H