Skip to content

Commit 52a1154

Browse files
dhalbertdhalbert
committed
fix OneWire timing and DigitalInOut.switch_to_input()
1 parent 6908eed commit 52a1154

7 files changed

Lines changed: 74 additions & 25 deletions

File tree

‎ports/atmel-samd/common-hal/digitalio/DigitalInOut.c‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ void common_hal_digitalio_digitalinout_deinit(digitalio_digitalinout_obj_t* self
6060
void common_hal_digitalio_digitalinout_switch_to_input(
6161
digitalio_digitalinout_obj_t* self, digitalio_pull_t pull) {
6262
self->output = false;
63-
63+
// This also sets direction to input.
6464
common_hal_digitalio_digitalinout_set_pull(self, pull);
6565
}
6666

@@ -69,13 +69,13 @@ void common_hal_digitalio_digitalinout_switch_to_output(
6969
digitalio_drive_mode_t drive_mode) {
7070
const uint8_t pin = self->pin->pin;
7171
gpio_set_pin_pull_mode(pin, GPIO_PULL_OFF);
72-
gpio_set_pin_direction(pin, GPIO_DIRECTION_OUT);
73-
7472
// Turn on "strong" pin driving (more current available). See DRVSTR doc in datasheet.
7573
hri_port_set_PINCFG_DRVSTR_bit(PORT, (enum gpio_port)GPIO_PORT(pin), GPIO_PIN(pin));
7674

7775
self->output = true;
7876
self->open_drain = drive_mode == DRIVE_MODE_OPEN_DRAIN;
77+
78+
// Direction is set in set_value. We don't need to do it here.
7979
common_hal_digitalio_digitalinout_set_value(self, value);
8080
}
8181

@@ -86,16 +86,22 @@ digitalio_direction_t common_hal_digitalio_digitalinout_get_direction(
8686

8787
void common_hal_digitalio_digitalinout_set_value(
8888
digitalio_digitalinout_obj_t* self, bool value) {
89+
const uint8_t pin = self->pin->pin;
90+
const uint8_t port = GPIO_PORT(pin);
91+
const uint32_t pin_mask = 1U << GPIO_PIN(pin);
8992
if (value) {
9093
if (self->open_drain) {
91-
gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_IN);
94+
// Assertion: pull is off, so the pin is floating in this case.
95+
// We do open-drain high output (no sinking of current)
96+
// by changing the direction to input with no pulls.
97+
hri_port_clear_DIR_DIR_bf(PORT, port, pin_mask);
9298
} else {
93-
gpio_set_pin_level(self->pin->pin, true);
94-
gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_OUT);
99+
hri_port_set_DIR_DIR_bf(PORT, port, pin_mask);
100+
hri_port_set_OUT_OUT_bf(PORT, port, pin_mask);
95101
}
96102
} else {
97-
gpio_set_pin_level(self->pin->pin, false);
98-
gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_OUT);
103+
hri_port_set_DIR_DIR_bf(PORT, port, pin_mask);
104+
hri_port_clear_OUT_OUT_bf(PORT,port, pin_mask);
99105
}
100106
}
101107

@@ -105,10 +111,10 @@ bool common_hal_digitalio_digitalinout_get_value(
105111
if (!self->output) {
106112
return gpio_get_pin_level(pin);
107113
} else {
108-
if (self->open_drain && hri_port_get_DIR_reg(PORT, (enum gpio_port)GPIO_PORT(pin), 1U << GPIO_PIN(pin)) == 0) {
114+
if (self->open_drain && hri_port_get_DIR_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin)) == 0) {
109115
return true;
110116
} else {
111-
return hri_port_get_OUT_reg(PORT, (enum gpio_port)GPIO_PORT(pin), 1U << GPIO_PIN(pin));
117+
return hri_port_get_OUT_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin));
112118
}
113119
}
114120
}
@@ -119,7 +125,7 @@ void common_hal_digitalio_digitalinout_set_drive_mode(
119125
bool value = common_hal_digitalio_digitalinout_get_value(self);
120126
self->open_drain = drive_mode == DRIVE_MODE_OPEN_DRAIN;
121127
// True is implemented differently between modes so reset the value to make
122-
// sure its correct for the new mode.
128+
// sure it's correct for the new mode.
123129
if (value) {
124130
common_hal_digitalio_digitalinout_set_value(self, value);
125131
}
@@ -148,7 +154,9 @@ void common_hal_digitalio_digitalinout_set_pull(
148154
default:
149155
break;
150156
}
157+
// Set pull first to avoid glitches.
151158
gpio_set_pin_pull_mode(self->pin->pin, asf_pull);
159+
gpio_set_pin_direction(self->pin->pin, GPIO_DIRECTION_IN);
152160
}
153161

154162
digitalio_pull_t common_hal_digitalio_digitalinout_get_pull(
@@ -158,9 +166,9 @@ digitalio_pull_t common_hal_digitalio_digitalinout_get_pull(
158166
mp_raise_AttributeError("Cannot get pull while in output mode");
159167
return PULL_NONE;
160168
} else {
161-
if (hri_port_get_PINCFG_PULLEN_bit(PORT, (enum gpio_port)GPIO_PORT(pin), GPIO_PIN(pin)) == 0) {
169+
if (hri_port_get_PINCFG_PULLEN_bit(PORT, GPIO_PORT(pin), GPIO_PIN(pin)) == 0) {
162170
return PULL_NONE;
163-
} if (hri_port_get_OUT_reg(PORT, (enum gpio_port)GPIO_PORT(pin), 1U << GPIO_PIN(pin)) > 0) {
171+
} if (hri_port_get_OUT_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin)) > 0) {
164172
return PULL_UP;
165173
} else {
166174
return PULL_DOWN;

‎ports/atmel-samd/mphalport.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "hal/include/hal_delay.h"
1515
#include "hal/include/hal_gpio.h"
1616
#include "hal/include/hal_sleep.h"
17+
#include "sam.h"
1718

1819
#include "mpconfigboard.h"
1920
#include "mphalport.h"
@@ -22,6 +23,7 @@
2223
#include "usb.h"
2324

2425
extern struct usart_module usart_instance;
26+
extern uint32_t common_hal_mcu_processor_get_frequency(void);
2527

2628
int mp_hal_stdin_rx_chr(void) {
2729
for (;;) {
@@ -75,6 +77,24 @@ void mp_hal_delay_us(mp_uint_t delay) {
7577
tick_delay(delay);
7678
}
7779

80+
// Do a simple timing loop to wait for a certain number of microseconds.
81+
// Can be used when interrupts are disabled, which makes tick_delay() unreliable.
82+
//
83+
// Testing done at 48 MHz on SAMD21 and 120 MHz on SAMD51, multiplication and division cancel out.
84+
// But get the frequency just in case.
85+
#ifdef SAMD21
86+
#define DELAY_LOOP_ITERATIONS_PER_US ( (10U*48000000U) / common_hal_mcu_processor_get_frequency())
87+
#endif
88+
#ifdef SAMD51
89+
#define DELAY_LOOP_ITERATIONS_PER_US ( (30U*120000000U) / common_hal_mcu_processor_get_frequency())
90+
#endif
91+
92+
void mp_hal_delay_us_loop(uint32_t us) {
93+
for (uint32_t i = us*DELAY_LOOP_ITERATIONS_PER_US; i > 0; i--) {
94+
asm volatile("nop");
95+
}
96+
}
97+
7898
void mp_hal_disable_all_interrupts(void) {
7999
common_hal_mcu_disable_interrupts();
80100
}

‎ports/atmel-samd/mphalport.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ int receive_usb(void);
4848
void mp_hal_set_interrupt_char(int c);
4949

5050
void mp_hal_disable_all_interrupts(void);
51-
5251
void mp_hal_enable_all_interrupts(void);
5352

53+
void mp_hal_delay_us_loop(uint32_t us);
54+
5455
#endif // MICROPY_INCLUDED_ATMEL_SAMD_MPHALPORT_H

‎ports/atmel-samd/tick.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ void tick_init() {
6464
for (uint16_t i = 0; i < PERIPH_COUNT_IRQn; i++) {
6565
NVIC_SetPriority(i, (1UL << __NVIC_PRIO_BITS) - 1UL);
6666
}
67-
// Bump up the systick interrupt.
68-
NVIC_SetPriority(SysTick_IRQn, 1);
67+
// Bump up the systick interrupt so nothing else interferes with timekeeping.
68+
NVIC_SetPriority(SysTick_IRQn, 0);
6969
#ifdef SAMD21
7070
NVIC_SetPriority(USB_IRQn, 1);
7171
#endif

‎ports/nrf/mphalport.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,17 @@ void mp_hal_stdout_tx_strn_cooked(const char *str, mp_uint_t len) {
8383
void mp_hal_stdout_tx_str(const char *str) {
8484
mp_hal_stdout_tx_strn(str, strlen(str));
8585
}
86+
87+
// Do a simple timing loop to wait for a certain number of microseconds.
88+
// Can be used when interrupts are disabled, which makes tick_delay() unreliable.
89+
//
90+
// Testing done at 48 MHz on SAMD21 and 120 MHz on SAMD51 (cache on).
91+
// TODO: Test on NRF. For now, use SAMD51 calibration, even though nRF52 runs slower.
92+
// Fraction should compensate.
93+
#define DELAY_LOOP_ITERATIONS_PER_US (30U*120000000U) / common_hal_mcu_processor_get_frequency())
94+
95+
void mp_hal_delay_us_loop(uint32_t us) {
96+
for (uint32_t i = us*DELAY_LOOP_ITERATIONS_PER_US; i > 0; i--) {
97+
asm volatile("nop");
98+
}
99+
}

‎ports/nrf/mphalport.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ static inline uint32_t hal_tick_fake(void) {
5252

5353
extern const unsigned char mp_hal_status_to_errno_table[4];
5454

55+
5556
NORETURN void mp_hal_raise(HAL_StatusTypeDef status);
5657
void mp_hal_set_interrupt_char(int c); // -1 to disable
5758

@@ -69,11 +70,11 @@ bool mp_hal_stdin_any(void);
6970
#define mp_hal_pin_od_high(p) mp_hal_pin_high(p)
7071
#define mp_hal_pin_open_drain(p) hal_gpio_cfg_pin(p->port, p->pin, HAL_GPIO_MODE_INPUT, HAL_GPIO_PULL_DISABLED)
7172

73+
void mp_hal_delay_us_loop(uint32_t us);
7274

7375
// TODO: empty implementation for now. Used by machine_spi.c:69
7476
#define mp_hal_delay_us_fast(p)
7577
#define mp_hal_ticks_us() (0)
7678
#define mp_hal_ticks_cpu() (0)
7779

7880
#endif
79-

‎shared-module/bitbangio/OneWire.c‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
* THE SOFTWARE.
2525
*/
2626

27+
#include "mphalport.h"
28+
2729
#include "common-hal/microcontroller/Pin.h"
2830
#include "shared-bindings/bitbangio/OneWire.h"
2931
#include "shared-bindings/microcontroller/__init__.h"
@@ -49,29 +51,32 @@ void shared_module_bitbangio_onewire_deinit(bitbangio_onewire_obj_t* self) {
4951
common_hal_digitalio_digitalinout_deinit(&self->pin);
5052
}
5153

54+
// We can't use common_hal_mcu_delay_us() here because it needs interrupts to be accurate
55+
// due to SysTick rollover checking, done by an interrupt.
56+
5257
bool shared_module_bitbangio_onewire_reset(bitbangio_onewire_obj_t* self) {
5358
common_hal_mcu_disable_interrupts();
5459
common_hal_digitalio_digitalinout_switch_to_output(&self->pin, false, DRIVE_MODE_OPEN_DRAIN);
55-
common_hal_mcu_delay_us(480);
60+
mp_hal_delay_us_loop(480);
5661
common_hal_digitalio_digitalinout_switch_to_input(&self->pin, PULL_NONE);
57-
common_hal_mcu_delay_us(70);
62+
mp_hal_delay_us_loop(70);
5863
bool value = common_hal_digitalio_digitalinout_get_value(&self->pin);
59-
common_hal_mcu_delay_us(410);
64+
mp_hal_delay_us_loop(410);
6065
common_hal_mcu_enable_interrupts();
6166
return value;
6267
}
6368

6469
bool shared_module_bitbangio_onewire_read_bit(bitbangio_onewire_obj_t* self) {
6570
common_hal_mcu_disable_interrupts();
6671
common_hal_digitalio_digitalinout_switch_to_output(&self->pin, false, DRIVE_MODE_OPEN_DRAIN);
67-
common_hal_mcu_delay_us(6);
72+
mp_hal_delay_us_loop(6);
6873
common_hal_digitalio_digitalinout_switch_to_input(&self->pin, PULL_NONE);
6974
// TODO(tannewt): Test with more devices and maybe make the delays
7075
// configurable. This should be 9 by the datasheet but all bits read as 1
7176
// then.
72-
common_hal_mcu_delay_us(6);
77+
mp_hal_delay_us_loop(6);
7378
bool value = common_hal_digitalio_digitalinout_get_value(&self->pin);
74-
common_hal_mcu_delay_us(55);
79+
mp_hal_delay_us_loop(55);
7580
common_hal_mcu_enable_interrupts();
7681
return value;
7782
}
@@ -80,8 +85,8 @@ void shared_module_bitbangio_onewire_write_bit(bitbangio_onewire_obj_t* self,
8085
bool bit) {
8186
common_hal_mcu_disable_interrupts();
8287
common_hal_digitalio_digitalinout_switch_to_output(&self->pin, false, DRIVE_MODE_OPEN_DRAIN);
83-
common_hal_mcu_delay_us(bit? 6 : 60);
88+
mp_hal_delay_us_loop(bit? 6 : 60);
8489
common_hal_digitalio_digitalinout_switch_to_input(&self->pin, PULL_NONE);
85-
common_hal_mcu_delay_us(bit? 64 : 10);
90+
mp_hal_delay_us_loop(bit? 64 : 10);
8691
common_hal_mcu_enable_interrupts();
8792
}

0 commit comments

Comments
 (0)