diff --git a/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c b/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c index cd0e3f91e5..03bd225b14 100644 --- a/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c +++ b/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c @@ -60,7 +60,7 @@ void common_hal_digitalio_digitalinout_deinit(digitalio_digitalinout_obj_t* self void common_hal_digitalio_digitalinout_switch_to_input( digitalio_digitalinout_obj_t* self, digitalio_pull_t pull) { self->output = false; - + // This also sets direction to input. common_hal_digitalio_digitalinout_set_pull(self, pull); } @@ -69,13 +69,13 @@ void common_hal_digitalio_digitalinout_switch_to_output( digitalio_drive_mode_t drive_mode) { const uint8_t pin = self->pin->pin; gpio_set_pin_pull_mode(pin, GPIO_PULL_OFF); - gpio_set_pin_direction(pin, GPIO_DIRECTION_OUT); - // Turn on "strong" pin driving (more current available). See DRVSTR doc in datasheet. hri_port_set_PINCFG_DRVSTR_bit(PORT, (enum gpio_port)GPIO_PORT(pin), GPIO_PIN(pin)); self->output = true; self->open_drain = drive_mode == DRIVE_MODE_OPEN_DRAIN; + + // Direction is set in set_value. We don't need to do it here. common_hal_digitalio_digitalinout_set_value(self, value); } @@ -86,16 +86,22 @@ digitalio_direction_t common_hal_digitalio_digitalinout_get_direction( void common_hal_digitalio_digitalinout_set_value( digitalio_digitalinout_obj_t* self, bool value) { + const uint8_t pin = self->pin->pin; + const uint8_t port = GPIO_PORT(pin); + const uint32_t pin_mask = 1U << GPIO_PIN(pin); if (value) { if (self->open_drain) { - gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_IN); + // Assertion: pull is off, so the pin is floating in this case. + // We do open-drain high output (no sinking of current) + // by changing the direction to input with no pulls. + hri_port_clear_DIR_DIR_bf(PORT, port, pin_mask); } else { - gpio_set_pin_level(self->pin->pin, true); - gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_OUT); + hri_port_set_DIR_DIR_bf(PORT, port, pin_mask); + hri_port_set_OUT_OUT_bf(PORT, port, pin_mask); } } else { - gpio_set_pin_level(self->pin->pin, false); - gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_OUT); + hri_port_set_DIR_DIR_bf(PORT, port, pin_mask); + hri_port_clear_OUT_OUT_bf(PORT,port, pin_mask); } } @@ -105,10 +111,10 @@ bool common_hal_digitalio_digitalinout_get_value( if (!self->output) { return gpio_get_pin_level(pin); } else { - if (self->open_drain && hri_port_get_DIR_reg(PORT, (enum gpio_port)GPIO_PORT(pin), 1U << GPIO_PIN(pin)) == 0) { + if (self->open_drain && hri_port_get_DIR_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin)) == 0) { return true; } else { - return hri_port_get_OUT_reg(PORT, (enum gpio_port)GPIO_PORT(pin), 1U << GPIO_PIN(pin)); + return hri_port_get_OUT_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin)); } } } @@ -119,7 +125,7 @@ void common_hal_digitalio_digitalinout_set_drive_mode( bool value = common_hal_digitalio_digitalinout_get_value(self); self->open_drain = drive_mode == DRIVE_MODE_OPEN_DRAIN; // True is implemented differently between modes so reset the value to make - // sure its correct for the new mode. + // sure it's correct for the new mode. if (value) { common_hal_digitalio_digitalinout_set_value(self, value); } @@ -148,7 +154,9 @@ void common_hal_digitalio_digitalinout_set_pull( default: break; } + // Set pull first to avoid glitches. gpio_set_pin_pull_mode(self->pin->pin, asf_pull); + gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_IN); } digitalio_pull_t common_hal_digitalio_digitalinout_get_pull( @@ -158,9 +166,9 @@ digitalio_pull_t common_hal_digitalio_digitalinout_get_pull( mp_raise_AttributeError("Cannot get pull while in output mode"); return PULL_NONE; } else { - if (hri_port_get_PINCFG_PULLEN_bit(PORT, (enum gpio_port)GPIO_PORT(pin), GPIO_PIN(pin)) == 0) { + if (hri_port_get_PINCFG_PULLEN_bit(PORT, GPIO_PORT(pin), GPIO_PIN(pin)) == 0) { return PULL_NONE; - } if (hri_port_get_OUT_reg(PORT, (enum gpio_port)GPIO_PORT(pin), 1U << GPIO_PIN(pin)) > 0) { + } if (hri_port_get_OUT_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin)) > 0) { return PULL_UP; } else { return PULL_DOWN; diff --git a/ports/atmel-samd/mphalport.c b/ports/atmel-samd/mphalport.c index c1b8ce5504..dd6a147134 100644 --- a/ports/atmel-samd/mphalport.c +++ b/ports/atmel-samd/mphalport.c @@ -14,6 +14,7 @@ #include "hal/include/hal_delay.h" #include "hal/include/hal_gpio.h" #include "hal/include/hal_sleep.h" +#include "sam.h" #include "mpconfigboard.h" #include "mphalport.h" @@ -22,6 +23,7 @@ #include "usb.h" extern struct usart_module usart_instance; +extern uint32_t common_hal_mcu_processor_get_frequency(void); int mp_hal_stdin_rx_chr(void) { for (;;) { @@ -75,6 +77,24 @@ void mp_hal_delay_us(mp_uint_t delay) { tick_delay(delay); } +// Do a simple timing loop to wait for a certain number of microseconds. +// Can be used when interrupts are disabled, which makes tick_delay() unreliable. +// +// Testing done at 48 MHz on SAMD21 and 120 MHz on SAMD51, multiplication and division cancel out. +// But get the frequency just in case. +#ifdef SAMD21 +#define DELAY_LOOP_ITERATIONS_PER_US ( (10U*48000000U) / common_hal_mcu_processor_get_frequency()) +#endif +#ifdef SAMD51 +#define DELAY_LOOP_ITERATIONS_PER_US ( (30U*120000000U) / common_hal_mcu_processor_get_frequency()) +#endif + +void mp_hal_delay_us_loop(uint32_t us) { + for (uint32_t i = us*DELAY_LOOP_ITERATIONS_PER_US; i > 0; i--) { + asm volatile("nop"); + } +} + void mp_hal_disable_all_interrupts(void) { common_hal_mcu_disable_interrupts(); } diff --git a/ports/atmel-samd/mphalport.h b/ports/atmel-samd/mphalport.h index 3b5955a4ff..31d34c8c39 100644 --- a/ports/atmel-samd/mphalport.h +++ b/ports/atmel-samd/mphalport.h @@ -48,7 +48,8 @@ int receive_usb(void); void mp_hal_set_interrupt_char(int c); void mp_hal_disable_all_interrupts(void); - void mp_hal_enable_all_interrupts(void); +void mp_hal_delay_us_loop(uint32_t us); + #endif // MICROPY_INCLUDED_ATMEL_SAMD_MPHALPORT_H diff --git a/ports/atmel-samd/tick.c b/ports/atmel-samd/tick.c index 9753a1a0c7..1493477933 100644 --- a/ports/atmel-samd/tick.c +++ b/ports/atmel-samd/tick.c @@ -64,8 +64,8 @@ void tick_init() { for (uint16_t i = 0; i < PERIPH_COUNT_IRQn; i++) { NVIC_SetPriority(i, (1UL << __NVIC_PRIO_BITS) - 1UL); } - // Bump up the systick interrupt. - NVIC_SetPriority(SysTick_IRQn, 1); + // Bump up the systick interrupt so nothing else interferes with timekeeping. + NVIC_SetPriority(SysTick_IRQn, 0); #ifdef SAMD21 NVIC_SetPriority(USB_IRQn, 1); #endif diff --git a/ports/nrf/mphalport.c b/ports/nrf/mphalport.c index 957d3535d2..1f732e1889 100644 --- a/ports/nrf/mphalport.c +++ b/ports/nrf/mphalport.c @@ -83,3 +83,17 @@ void mp_hal_stdout_tx_strn_cooked(const char *str, mp_uint_t len) { void mp_hal_stdout_tx_str(const char *str) { mp_hal_stdout_tx_strn(str, strlen(str)); } + +// Do a simple timing loop to wait for a certain number of microseconds. +// Can be used when interrupts are disabled, which makes tick_delay() unreliable. +// +// Testing done at 48 MHz on SAMD21 and 120 MHz on SAMD51 (cache on). +// TODO: Test on NRF. For now, use SAMD51 calibration, even though nRF52 runs slower. +// Fraction should compensate. +#define DELAY_LOOP_ITERATIONS_PER_US (30U*120000000U) / common_hal_mcu_processor_get_frequency()) + +void mp_hal_delay_us_loop(uint32_t us) { + for (uint32_t i = us*DELAY_LOOP_ITERATIONS_PER_US; i > 0; i--) { + asm volatile("nop"); + } +} diff --git a/ports/nrf/mphalport.h b/ports/nrf/mphalport.h index 3334fd7fe9..79e1d2d58e 100644 --- a/ports/nrf/mphalport.h +++ b/ports/nrf/mphalport.h @@ -52,6 +52,7 @@ static inline uint32_t hal_tick_fake(void) { extern const unsigned char mp_hal_status_to_errno_table[4]; + NORETURN void mp_hal_raise(HAL_StatusTypeDef status); void mp_hal_set_interrupt_char(int c); // -1 to disable @@ -69,6 +70,7 @@ bool mp_hal_stdin_any(void); #define mp_hal_pin_od_high(p) mp_hal_pin_high(p) #define mp_hal_pin_open_drain(p) hal_gpio_cfg_pin(p->port, p->pin, HAL_GPIO_MODE_INPUT, HAL_GPIO_PULL_DISABLED) +void mp_hal_delay_us_loop(uint32_t us); // TODO: empty implementation for now. Used by machine_spi.c:69 #define mp_hal_delay_us_fast(p) @@ -76,4 +78,3 @@ bool mp_hal_stdin_any(void); #define mp_hal_ticks_cpu() (0) #endif - diff --git a/shared-module/bitbangio/OneWire.c b/shared-module/bitbangio/OneWire.c index e77bdb3b29..b226b9d784 100644 --- a/shared-module/bitbangio/OneWire.c +++ b/shared-module/bitbangio/OneWire.c @@ -24,6 +24,8 @@ * THE SOFTWARE. */ +#include "mphalport.h" + #include "common-hal/microcontroller/Pin.h" #include "shared-bindings/bitbangio/OneWire.h" #include "shared-bindings/microcontroller/__init__.h" @@ -49,14 +51,17 @@ void shared_module_bitbangio_onewire_deinit(bitbangio_onewire_obj_t* self) { common_hal_digitalio_digitalinout_deinit(&self->pin); } +// We can't use common_hal_mcu_delay_us() here because it needs interrupts to be accurate +// due to SysTick rollover checking, done by an interrupt. + bool shared_module_bitbangio_onewire_reset(bitbangio_onewire_obj_t* self) { common_hal_mcu_disable_interrupts(); common_hal_digitalio_digitalinout_switch_to_output(&self->pin, false, DRIVE_MODE_OPEN_DRAIN); - common_hal_mcu_delay_us(480); + mp_hal_delay_us_loop(480); common_hal_digitalio_digitalinout_switch_to_input(&self->pin, PULL_NONE); - common_hal_mcu_delay_us(70); + mp_hal_delay_us_loop(70); bool value = common_hal_digitalio_digitalinout_get_value(&self->pin); - common_hal_mcu_delay_us(410); + mp_hal_delay_us_loop(410); common_hal_mcu_enable_interrupts(); return value; } @@ -64,14 +69,14 @@ bool shared_module_bitbangio_onewire_reset(bitbangio_onewire_obj_t* self) { bool shared_module_bitbangio_onewire_read_bit(bitbangio_onewire_obj_t* self) { common_hal_mcu_disable_interrupts(); common_hal_digitalio_digitalinout_switch_to_output(&self->pin, false, DRIVE_MODE_OPEN_DRAIN); - common_hal_mcu_delay_us(6); + mp_hal_delay_us_loop(6); common_hal_digitalio_digitalinout_switch_to_input(&self->pin, PULL_NONE); // TODO(tannewt): Test with more devices and maybe make the delays // configurable. This should be 9 by the datasheet but all bits read as 1 // then. - common_hal_mcu_delay_us(6); + mp_hal_delay_us_loop(6); bool value = common_hal_digitalio_digitalinout_get_value(&self->pin); - common_hal_mcu_delay_us(55); + mp_hal_delay_us_loop(55); common_hal_mcu_enable_interrupts(); return value; } @@ -80,8 +85,8 @@ void shared_module_bitbangio_onewire_write_bit(bitbangio_onewire_obj_t* self, bool bit) { common_hal_mcu_disable_interrupts(); common_hal_digitalio_digitalinout_switch_to_output(&self->pin, false, DRIVE_MODE_OPEN_DRAIN); - common_hal_mcu_delay_us(bit? 6 : 60); + mp_hal_delay_us_loop(bit? 6 : 60); common_hal_digitalio_digitalinout_switch_to_input(&self->pin, PULL_NONE); - common_hal_mcu_delay_us(bit? 64 : 10); + mp_hal_delay_us_loop(bit? 64 : 10); common_hal_mcu_enable_interrupts(); }