From 64180f0742a1926162b784c68bafa49b9b58596c Mon Sep 17 00:00:00 2001 From: Damien George Date: Wed, 18 Nov 2020 14:50:43 +1100 Subject: [PATCH] extmod/machine_i2c: Add init protocol method for generic I2C bindings. Hardware I2C implementations must provide a .init() protocol method if they want to support reconfiguration. Otherwise the default is that i2c.init() raises an OSError (currently the case for all ports). mp_machine_soft_i2c_locals_dict is renamed to mp_machine_i2c_locals_dict to match the generic SPI bindings. Fixes issue #6623 (where calling .init() on a HW I2C would crash). Signed-off-by: Damien George --- extmod/machine_i2c.c | 87 +++++++++++++++++++-------------- extmod/machine_i2c.h | 9 ++-- ports/esp32/machine_i2c.c | 2 +- ports/nrf/modules/machine/i2c.c | 2 +- ports/stm32/machine_i2c.c | 2 +- ports/zephyr/machine_i2c.c | 2 +- 6 files changed, 59 insertions(+), 45 deletions(-) diff --git a/extmod/machine_i2c.c b/extmod/machine_i2c.c index 12c9abbcba..44161fbbb9 100644 --- a/extmod/machine_i2c.c +++ b/extmod/machine_i2c.c @@ -300,45 +300,18 @@ STATIC int mp_machine_i2c_writeto(mp_obj_base_t *self, uint16_t addr, const uint } /******************************************************************************/ -// MicroPython bindings for I2C +// MicroPython bindings for generic machine.I2C -STATIC void mp_machine_soft_i2c_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { - mp_machine_soft_i2c_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_printf(print, "SoftI2C(scl=" MP_HAL_PIN_FMT ", sda=" MP_HAL_PIN_FMT ", freq=%u)", - mp_hal_pin_name(self->scl), mp_hal_pin_name(self->sda), 500000 / self->us_delay); -} - -STATIC void machine_i2c_obj_init_helper(machine_i2c_obj_t *self, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { - enum { ARG_scl, ARG_sda, ARG_freq, ARG_timeout }; - static const mp_arg_t allowed_args[] = { - { MP_QSTR_scl, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, - { MP_QSTR_sda, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, - { MP_QSTR_freq, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 400000} }, - { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 255} }, - }; - mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; - mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); - self->scl = mp_hal_get_pin_obj(args[ARG_scl].u_obj); - self->sda = mp_hal_get_pin_obj(args[ARG_sda].u_obj); - self->us_timeout = args[ARG_timeout].u_int; - mp_hal_i2c_init(self, args[ARG_freq].u_int); -} - -STATIC mp_obj_t mp_machine_soft_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { - // create new soft I2C object - machine_i2c_obj_t *self = m_new_obj(machine_i2c_obj_t); - self->base.type = &mp_machine_soft_i2c_type; - mp_map_t kw_args; - mp_map_init_fixed_table(&kw_args, n_kw, args + n_args); - machine_i2c_obj_init_helper(self, n_args, args, &kw_args); - return MP_OBJ_FROM_PTR(self); -} - -STATIC mp_obj_t machine_i2c_obj_init(size_t n_args, const mp_obj_t *args, mp_map_t *kw_args) { - machine_i2c_obj_init_helper(MP_OBJ_TO_PTR(args[0]), n_args - 1, args + 1, kw_args); +STATIC mp_obj_t machine_i2c_init(size_t n_args, const mp_obj_t *args, mp_map_t *kw_args) { + mp_obj_base_t *self = (mp_obj_base_t *)MP_OBJ_TO_PTR(args[0]); + mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t *)self->type->protocol; + if (i2c_p->init == NULL) { + mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("I2C operation not supported")); + } + i2c_p->init(self, n_args - 1, args + 1, kw_args); return mp_const_none; } -MP_DEFINE_CONST_FUN_OBJ_KW(machine_i2c_init_obj, 1, machine_i2c_obj_init); +MP_DEFINE_CONST_FUN_OBJ_KW(machine_i2c_init_obj, 1, machine_i2c_init); STATIC mp_obj_t machine_i2c_scan(mp_obj_t self_in) { mp_obj_base_t *self = MP_OBJ_TO_PTR(self_in); @@ -653,8 +626,45 @@ STATIC const mp_rom_map_elem_t machine_i2c_locals_dict_table[] = { { MP_ROM_QSTR(MP_QSTR_readfrom_mem_into), MP_ROM_PTR(&machine_i2c_readfrom_mem_into_obj) }, { MP_ROM_QSTR(MP_QSTR_writeto_mem), MP_ROM_PTR(&machine_i2c_writeto_mem_obj) }, }; +MP_DEFINE_CONST_DICT(mp_machine_i2c_locals_dict, machine_i2c_locals_dict_table); -MP_DEFINE_CONST_DICT(mp_machine_soft_i2c_locals_dict, machine_i2c_locals_dict_table); +/******************************************************************************/ +// Implementation of soft I2C + +STATIC void mp_machine_soft_i2c_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { + mp_machine_soft_i2c_obj_t *self = MP_OBJ_TO_PTR(self_in); + mp_printf(print, "SoftI2C(scl=" MP_HAL_PIN_FMT ", sda=" MP_HAL_PIN_FMT ", freq=%u)", + mp_hal_pin_name(self->scl), mp_hal_pin_name(self->sda), 500000 / self->us_delay); +} + +STATIC void mp_machine_soft_i2c_init(mp_obj_base_t *self_in, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { + enum { ARG_scl, ARG_sda, ARG_freq, ARG_timeout }; + static const mp_arg_t allowed_args[] = { + { MP_QSTR_scl, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, + { MP_QSTR_sda, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, + { MP_QSTR_freq, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 400000} }, + { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 255} }, + }; + + mp_machine_soft_i2c_obj_t *self = (mp_machine_soft_i2c_obj_t *)self_in; + mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; + mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); + + self->scl = mp_hal_get_pin_obj(args[ARG_scl].u_obj); + self->sda = mp_hal_get_pin_obj(args[ARG_sda].u_obj); + self->us_timeout = args[ARG_timeout].u_int; + mp_hal_i2c_init(self, args[ARG_freq].u_int); +} + +STATIC mp_obj_t mp_machine_soft_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { + // create new soft I2C object + machine_i2c_obj_t *self = m_new_obj(machine_i2c_obj_t); + self->base.type = &mp_machine_soft_i2c_type; + mp_map_t kw_args; + mp_map_init_fixed_table(&kw_args, n_kw, args + n_args); + mp_machine_soft_i2c_init(&self->base, n_args, args, &kw_args); + return MP_OBJ_FROM_PTR(self); +} int mp_machine_soft_i2c_read(mp_obj_base_t *self_in, uint8_t *dest, size_t len, bool nack) { machine_i2c_obj_t *self = (machine_i2c_obj_t *)self_in; @@ -684,6 +694,7 @@ int mp_machine_soft_i2c_write(mp_obj_base_t *self_in, const uint8_t *src, size_t } STATIC const mp_machine_i2c_p_t mp_machine_soft_i2c_p = { + .init = mp_machine_soft_i2c_init, .start = (int (*)(mp_obj_base_t *))mp_hal_i2c_start, .stop = (int (*)(mp_obj_base_t *))mp_hal_i2c_stop, .read = mp_machine_soft_i2c_read, @@ -697,7 +708,7 @@ const mp_obj_type_t mp_machine_soft_i2c_type = { .print = mp_machine_soft_i2c_print, .make_new = mp_machine_soft_i2c_make_new, .protocol = &mp_machine_soft_i2c_p, - .locals_dict = (mp_obj_dict_t *)&mp_machine_soft_i2c_locals_dict, + .locals_dict = (mp_obj_dict_t *)&mp_machine_i2c_locals_dict, }; #endif // MICROPY_PY_MACHINE_I2C diff --git a/extmod/machine_i2c.h b/extmod/machine_i2c.h index e3a87e282a..9e43747323 100644 --- a/extmod/machine_i2c.h +++ b/extmod/machine_i2c.h @@ -51,9 +51,12 @@ typedef struct _mp_machine_i2c_buf_t { } mp_machine_i2c_buf_t; // I2C protocol -// the first 4 methods can be NULL, meaning operation is not supported -// transfer_single only needs to be set if transfer=mp_machine_i2c_transfer_adaptor +// - init must be non-NULL +// - start/stop/read/write can be NULL, meaning operation is not supported +// - transfer must be non-NULL +// - transfer_single only needs to be set if transfer=mp_machine_i2c_transfer_adaptor typedef struct _mp_machine_i2c_p_t { + void (*init)(mp_obj_base_t *obj, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args); int (*start)(mp_obj_base_t *obj); int (*stop)(mp_obj_base_t *obj); int (*read)(mp_obj_base_t *obj, uint8_t *dest, size_t len, bool nack); @@ -71,7 +74,7 @@ typedef struct _mp_machine_soft_i2c_obj_t { } mp_machine_soft_i2c_obj_t; extern const mp_obj_type_t mp_machine_soft_i2c_type; -extern const mp_obj_dict_t mp_machine_soft_i2c_locals_dict; +extern const mp_obj_dict_t mp_machine_i2c_locals_dict; int mp_machine_i2c_transfer_adaptor(mp_obj_base_t *self, uint16_t addr, size_t n, mp_machine_i2c_buf_t *bufs, unsigned int flags); int mp_machine_soft_i2c_transfer(mp_obj_base_t *self, uint16_t addr, size_t n, mp_machine_i2c_buf_t *bufs, unsigned int flags); diff --git a/ports/esp32/machine_i2c.c b/ports/esp32/machine_i2c.c index a1d0ad0f4d..87b08fc9bc 100644 --- a/ports/esp32/machine_i2c.c +++ b/ports/esp32/machine_i2c.c @@ -177,5 +177,5 @@ const mp_obj_type_t machine_hw_i2c_type = { .print = machine_hw_i2c_print, .make_new = machine_hw_i2c_make_new, .protocol = &machine_hw_i2c_p, - .locals_dict = (mp_obj_dict_t *)&mp_machine_soft_i2c_locals_dict, + .locals_dict = (mp_obj_dict_t *)&mp_machine_i2c_locals_dict, }; diff --git a/ports/nrf/modules/machine/i2c.c b/ports/nrf/modules/machine/i2c.c index ab4d516621..aac9320873 100644 --- a/ports/nrf/modules/machine/i2c.c +++ b/ports/nrf/modules/machine/i2c.c @@ -167,7 +167,7 @@ const mp_obj_type_t machine_hard_i2c_type = { .print = machine_hard_i2c_print, .make_new = machine_hard_i2c_make_new, .protocol = &machine_hard_i2c_p, - .locals_dict = (mp_obj_dict_t*)&mp_machine_soft_i2c_locals_dict, + .locals_dict = (mp_obj_dict_t*)&mp_machine_i2c_locals_dict, }; #endif // MICROPY_PY_MACHINE_I2C diff --git a/ports/stm32/machine_i2c.c b/ports/stm32/machine_i2c.c index e0c408c6d0..cfa00b86d8 100644 --- a/ports/stm32/machine_i2c.c +++ b/ports/stm32/machine_i2c.c @@ -273,7 +273,7 @@ const mp_obj_type_t machine_hard_i2c_type = { .print = machine_hard_i2c_print, .make_new = machine_hard_i2c_make_new, .protocol = &machine_hard_i2c_p, - .locals_dict = (mp_obj_dict_t *)&mp_machine_soft_i2c_locals_dict, + .locals_dict = (mp_obj_dict_t *)&mp_machine_i2c_locals_dict, }; #endif // MICROPY_HW_ENABLE_HW_I2C diff --git a/ports/zephyr/machine_i2c.c b/ports/zephyr/machine_i2c.c index ec4b8620a5..efd4bdcb21 100644 --- a/ports/zephyr/machine_i2c.c +++ b/ports/zephyr/machine_i2c.c @@ -134,5 +134,5 @@ const mp_obj_type_t machine_hard_i2c_type = { .print = machine_hard_i2c_print, .make_new = machine_hard_i2c_make_new, .protocol = &machine_hard_i2c_p, - .locals_dict = (mp_obj_dict_t *)&mp_machine_soft_i2c_locals_dict, + .locals_dict = (mp_obj_dict_t *)&mp_machine_i2c_locals_dict, };