Skip to content

fix mistaken use of PWM channel for slice #5667

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.

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
Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ports/raspberrypi/common-hal/audiopwmio/PWMAudioOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 6 additions & 5 deletions ports/raspberrypi/common-hal/countio/Counter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down
52 changes: 26 additions & 26 deletions ports/raspberrypi/common-hal/pwmio/PWMOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);

Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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;
}
Expand All @@ -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()) {
}
}
Expand Down
24 changes: 12 additions & 12 deletions ports/raspberrypi/common-hal/pwmio/PWMOut.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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;
Expand All @@ -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
1 change: 0 additions & 1 deletion tests/extmod/utimeq1.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
def dprint(*v):
print(*v)


else:

def dprint(*v):
Expand Down
1 change: 0 additions & 1 deletion tools/pydfu.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
def get_string(dev, index):
return usb.util.get_string(dev, 255, index)


else:
# PyUSB 1.0.0.b2 dropped the length argument
def get_string(dev, index):
Expand Down