From 67c6932ca21942bd1b61dc6e3e13e9da30aac651 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 11 Aug 2021 09:58:31 -0700 Subject: [PATCH 1/2] Reset timers separate from pwmio This prevents timer leakage on builds without pwmio. Fixes #5057 --- ports/atmel-samd/Makefile | 8 +++ ports/atmel-samd/common-hal/pwmio/PWMOut.c | 38 +--------- ports/atmel-samd/shared_timers.c | 84 ++++++++++++++++++++++ ports/atmel-samd/shared_timers.h | 36 ++++++++++ ports/atmel-samd/supervisor/port.c | 4 ++ ports/atmel-samd/timer_handler.h | 4 -- 6 files changed, 133 insertions(+), 41 deletions(-) create mode 100644 ports/atmel-samd/shared_timers.c create mode 100644 ports/atmel-samd/shared_timers.h diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile index 14fc9d43b9..00ce562e9e 100644 --- a/ports/atmel-samd/Makefile +++ b/ports/atmel-samd/Makefile @@ -322,6 +322,14 @@ SRC_C += \ reset.c \ timer_handler.c \ +ifeq ($(CIRCUITPY_PWMIO),1) +SRC_C += shared_timers.c +endif + +ifeq ($(CIRCUITPY_AUDIOIO),1) +SRC_C += shared_timers.c +endif + ifeq ($(CIRCUITPY_SDIOIO),1) SRC_C += ports/atmel-samd/sd_mmc/sd_mmc.c endif diff --git a/ports/atmel-samd/common-hal/pwmio/PWMOut.c b/ports/atmel-samd/common-hal/pwmio/PWMOut.c index 4f6661b55d..3528b55e98 100644 --- a/ports/atmel-samd/common-hal/pwmio/PWMOut.c +++ b/ports/atmel-samd/common-hal/pwmio/PWMOut.c @@ -60,24 +60,6 @@ uint8_t tcc_channels[3]; // Set by pwmout_reset() to {0xf0, 0xfc, 0xfc} initia uint8_t tcc_channels[5]; // Set by pwmout_reset() to {0xc0, 0xf0, 0xf8, 0xfc, 0xfc} initially. #endif -static uint8_t never_reset_tc_or_tcc[TC_INST_NUM + TCC_INST_NUM]; - -STATIC void timer_refcount(int index, bool is_tc, int increment) { - if (is_tc) { - never_reset_tc_or_tcc[index] += increment; - } else { - never_reset_tc_or_tcc[TC_INST_NUM + index] += increment; - } -} - -void timer_never_reset(int index, bool is_tc) { - timer_refcount(index, is_tc, 1); -} - -void timer_reset_ok(int index, bool is_tc) { - timer_refcount(index, is_tc, -1); -} - void common_hal_pwmio_pwmout_never_reset(pwmio_pwmout_obj_t *self) { timer_never_reset(self->timer->index, self->timer->is_tc); @@ -97,32 +79,14 @@ void pwmout_reset(void) { } Tcc *tccs[TCC_INST_NUM] = TCC_INSTS; for (int i = 0; i < TCC_INST_NUM; i++) { - if (never_reset_tc_or_tcc[TC_INST_NUM + i] > 0) { + if (!timer_ok_to_reset(i, false)) { continue; } - // Disable the module before resetting it. - if (tccs[i]->CTRLA.bit.ENABLE == 1) { - tccs[i]->CTRLA.bit.ENABLE = 0; - while (tccs[i]->SYNCBUSY.bit.ENABLE == 1) { - } - } uint8_t mask = 0xff; for (uint8_t j = 0; j < tcc_cc_num[i]; j++) { mask <<= 1; } tcc_channels[i] = mask; - tccs[i]->CTRLA.bit.SWRST = 1; - while (tccs[i]->CTRLA.bit.SWRST == 1) { - } - } - Tc *tcs[TC_INST_NUM] = TC_INSTS; - for (int i = 0; i < TC_INST_NUM; i++) { - if (never_reset_tc_or_tcc[i] > 0) { - continue; - } - tcs[i]->COUNT16.CTRLA.bit.SWRST = 1; - while (tcs[i]->COUNT16.CTRLA.bit.SWRST == 1) { - } } } diff --git a/ports/atmel-samd/shared_timers.c b/ports/atmel-samd/shared_timers.c new file mode 100644 index 0000000000..f07bd372da --- /dev/null +++ b/ports/atmel-samd/shared_timers.c @@ -0,0 +1,84 @@ +/* + * This file is part of the MicroPython project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2017 Scott Shawcroft for Adafruit Industries + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include + +#include "samd/timers.h" + +#include "shared_timers.h" + +static uint8_t never_reset_tc_or_tcc[TC_INST_NUM + TCC_INST_NUM]; + +static void timer_refcount(int index, bool is_tc, int increment) { + if (is_tc) { + never_reset_tc_or_tcc[index] += increment; + } else { + never_reset_tc_or_tcc[TC_INST_NUM + index] += increment; + } +} + +void timer_never_reset(int index, bool is_tc) { + timer_refcount(index, is_tc, 1); +} + +void timer_reset_ok(int index, bool is_tc) { + timer_refcount(index, is_tc, -1); +} + +bool timer_ok_to_reset(int index, bool is_tc) { + if (is_tc) { + return never_reset_tc_or_tcc[index] == 0; + } + return never_reset_tc_or_tcc[TC_INST_NUM + index] == 0; +} + +void reset_timers(void) { + // Reset all timers + Tcc *tccs[TCC_INST_NUM] = TCC_INSTS; + for (int i = 0; i < TCC_INST_NUM; i++) { + if (!timer_ok_to_reset(i, false)) { + continue; + } + // Disable the module before resetting it. + if (tccs[i]->CTRLA.bit.ENABLE == 1) { + tccs[i]->CTRLA.bit.ENABLE = 0; + while (tccs[i]->SYNCBUSY.bit.ENABLE == 1) { + } + } + tccs[i]->CTRLA.bit.SWRST = 1; + while (tccs[i]->CTRLA.bit.SWRST == 1) { + } + } + Tc *tcs[TC_INST_NUM] = TC_INSTS; + for (int i = 0; i < TC_INST_NUM; i++) { + if (!timer_ok_to_reset(i, true)) { + continue; + } + tcs[i]->COUNT16.CTRLA.bit.SWRST = 1; + while (tcs[i]->COUNT16.CTRLA.bit.SWRST == 1) { + } + } +} diff --git a/ports/atmel-samd/shared_timers.h b/ports/atmel-samd/shared_timers.h new file mode 100644 index 0000000000..88edd3aa12 --- /dev/null +++ b/ports/atmel-samd/shared_timers.h @@ -0,0 +1,36 @@ +/* + * This file is part of the MicroPython project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2018 Scott Shawcroft for Adafruit Industries + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#ifndef MICROPY_INCLUDED_ATMEL_SAMD_SHARED_TIMERS_H +#define MICROPY_INCLUDED_ATMEL_SAMD_SHARED_TIMERS_H + +#include + +void timer_never_reset(int index, bool is_tc); +void timer_reset_ok(int index, bool is_tc); +bool timer_ok_to_reset(int index, bool is_tc); +void reset_timers(void); + +#endif // MICROPY_INCLUDED_ATMEL_SAMD_SHARED_TIMERS_H diff --git a/ports/atmel-samd/supervisor/port.c b/ports/atmel-samd/supervisor/port.c index c03b79ce4f..4cd0ee4e73 100644 --- a/ports/atmel-samd/supervisor/port.c +++ b/ports/atmel-samd/supervisor/port.c @@ -77,6 +77,7 @@ #include "samd/dma.h" #include "shared-bindings/microcontroller/__init__.h" #include "shared-bindings/rtc/__init__.h" +#include "shared_timers.h" #include "reset.h" #include "supervisor/shared/safe_mode.h" @@ -347,6 +348,9 @@ void reset_port(void) { #if CIRCUITPY_PWMIO pwmout_reset(); #endif + #if CIRCUITPY_PWMIO || CIRCUITPY_AUDIOIO + reset_timers(); + #endif #if CIRCUITPY_ANALOGIO analogin_reset(); diff --git a/ports/atmel-samd/timer_handler.h b/ports/atmel-samd/timer_handler.h index 34efda4d9e..517ad768ba 100644 --- a/ports/atmel-samd/timer_handler.h +++ b/ports/atmel-samd/timer_handler.h @@ -36,8 +36,4 @@ void set_timer_handler(bool is_tc, uint8_t index, uint8_t timer_handler); void shared_timer_handler(bool is_tc, uint8_t index); -// implementation of these functions is in PWMOut.c -void timer_never_reset(int index, bool is_tc); -void timer_reset_ok(int index, bool is_tc); - #endif // MICROPY_INCLUDED_ATMEL_SAMD_TIMER_HANDLER_H From 7f016ae11e687efe699b019e551b18cfa0b3f291 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 11 Aug 2021 12:10:51 -0700 Subject: [PATCH 2/2] Fix build with filter to do OR --- ports/atmel-samd/Makefile | 8 +++----- ports/atmel-samd/common-hal/pwmio/PWMOut.c | 11 +++-------- ports/atmel-samd/common-hal/rgbmatrix/RGBMatrix.c | 1 + 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile index 00ce562e9e..810d86581b 100644 --- a/ports/atmel-samd/Makefile +++ b/ports/atmel-samd/Makefile @@ -322,11 +322,9 @@ SRC_C += \ reset.c \ timer_handler.c \ -ifeq ($(CIRCUITPY_PWMIO),1) -SRC_C += shared_timers.c -endif - -ifeq ($(CIRCUITPY_AUDIOIO),1) +# This is an OR because it filters to any 1s and then checks to see if it is not +# empty. +ifneq (,$(filter 1,$(CIRCUITPY_PWMIO) $(CIRCUITPY_AUDIOIO) $(CIRCUITPY_RGBMATRIX))) SRC_C += shared_timers.c endif diff --git a/ports/atmel-samd/common-hal/pwmio/PWMOut.c b/ports/atmel-samd/common-hal/pwmio/PWMOut.c index 3528b55e98..fa6a2d3f47 100644 --- a/ports/atmel-samd/common-hal/pwmio/PWMOut.c +++ b/ports/atmel-samd/common-hal/pwmio/PWMOut.c @@ -31,15 +31,15 @@ #include "common-hal/pwmio/PWMOut.h" #include "shared-bindings/pwmio/PWMOut.h" #include "shared-bindings/microcontroller/Processor.h" +#include "shared_timers.h" #include "timer_handler.h" #include "atmel_start_pins.h" #include "hal/utils/include/utils_repeat_macro.h" +#include "samd/pins.h" #include "samd/timers.h" #include "supervisor/shared/translate.h" -#include "samd/pins.h" - #undef ENABLE #define _TCC_SIZE(unused, n) TCC##n##_SIZE, @@ -77,16 +77,11 @@ void pwmout_reset(void) { target_tcc_frequencies[i] = 0; tcc_refcount[i] = 0; } - Tcc *tccs[TCC_INST_NUM] = TCC_INSTS; for (int i = 0; i < TCC_INST_NUM; i++) { if (!timer_ok_to_reset(i, false)) { continue; } - uint8_t mask = 0xff; - for (uint8_t j = 0; j < tcc_cc_num[i]; j++) { - mask <<= 1; - } - tcc_channels[i] = mask; + tcc_channels[i] = 0xff << tcc_cc_num[i]; } } diff --git a/ports/atmel-samd/common-hal/rgbmatrix/RGBMatrix.c b/ports/atmel-samd/common-hal/rgbmatrix/RGBMatrix.c index aa0bd78275..eca3757f20 100644 --- a/ports/atmel-samd/common-hal/rgbmatrix/RGBMatrix.c +++ b/ports/atmel-samd/common-hal/rgbmatrix/RGBMatrix.c @@ -29,6 +29,7 @@ #include "common-hal/rgbmatrix/RGBMatrix.h" #include "samd/timers.h" +#include "shared_timers.h" #include "timer_handler.h" void *common_hal_rgbmatrix_timer_allocate(rgbmatrix_rgbmatrix_obj_t *self) {