From 8ba38a8a630ae2716eca3ec593bd9fa8b93ce2d9 Mon Sep 17 00:00:00 2001 From: Andrey G Date: Thu, 29 Apr 2021 01:29:35 +0300 Subject: [PATCH] Mc33972 update (#2599) * MC33972: update -per-instance thread -enable pull-ups for enabled pins only * smart-gpio: mc33972: uses 8-bit spi frames 3 bytes are sent to make 24-bit frame --- firmware/hw_layer/drivers/gpio/mc33972.c | 233 ++++++++++++++++------- firmware/hw_layer/smart_gpio.cpp | 4 +- 2 files changed, 169 insertions(+), 68 deletions(-) diff --git a/firmware/hw_layer/drivers/gpio/mc33972.c b/firmware/hw_layer/drivers/gpio/mc33972.c index 2d3fe342dd..d2d7967990 100644 --- a/firmware/hw_layer/drivers/gpio/mc33972.c +++ b/firmware/hw_layer/drivers/gpio/mc33972.c @@ -33,8 +33,6 @@ #define DRIVER_NAME "mc33972" -static bool drv_task_ready = false; - typedef enum { MC33972_DISABLED = 0, MC33972_WAIT_INIT, @@ -42,25 +40,35 @@ typedef enum { MC33972_FAILED } mc33972_drv_state; +#define SP_BANK 0 +#define SG_BANK 1 + /* all commands and reply data are 24 bit */ #define CMD(cmd, data) (((cmd) << 16) | ((data) << 0)) -#define CMD_STATUS CMD(0x00, 0x00) -#define CMD_SETTINGS(mask) CMD(0x01, (mask)) -#define CMD_WAKEUPEN(i, mask) CMD((i) ? 0x03 : 0x02, (mask)) -#define CMD_METALLIC(i, mask) CMD((i) ? 0x05 : 0x04, (mask)) -#define CMD_ANALOG(cur, ch) CMD(0x06, (((curr) & 0x3) << 5) | (((d) & 0x1f) << 0)) -#define CMD_WETTING_TMR(i, mask) CMD((i) ? 0x08 : 0x07, (mask)) -#define CMD_TRI_STATE(i, mask) CMD((i) ? 0x0a : 0x09, (mask)) -#define CMD_CALIB CMD(0x0b, 0x00) -#define CMD_SLEEP(int_t, scan_t) CMD(0x0c, (((int_t) & 0x7) << 3) | (((scan_t) & 0x7) << 0)) -#define CMD_RST CMD(0x7f, 0x00) +#define CMD_STATUS CMD(0x00, 0x00) +#define CMD_SETTINGS(mask) CMD(0x01, (mask)) +#define CMD_WAKEUPEN(i, mask) CMD(0x02 + (i), (mask)) +#define CMD_METALLIC(i, mask) CMD(0x04 + (i), (mask)) +#define CMD_ANALOG(curr, ch) CMD(0x06, (((curr) & 0x3) << 5) | (((ch) & 0x1f) << 0)) +#define CMD_WETTING_TMR(i, mask) CMD(0x07 + (i), (mask)) +#define CMD_TRI_STATE(i, mask) CMD(0x09 + (i), (mask)) +#define CMD_CALIB CMD(0x0b, 0x00) +#define CMD_SLEEP(int_t, scan_t) CMD(0x0c, (((int_t) & 0x7) << 3) | (((scan_t) & 0x7) << 0)) +#define CMD_RST CMD(0x7f, 0x00) + +/* all switch to battery or ground pins mask */ +#define SP_PINS_MASK 0x00ff +#define SP_PINS_EXTRACT(pins) (((pins) >> 14) & SP_PINS_MASK) +/* all switch-to-ground inputs mask */ +#define SG_PINS_MASK 0x3fff +#define SG_PINS_EXTRACT(pins) (((pins) >> 0) & SG_PINS_MASK) /* reply is allways same 24 status bits */ #define FLAG_THERM (1 << 23) #define FLAG_INT (1 << 22) /* from LSB to MSB: SG0..SG13, SP0..SP7 */ -#define FLAG_PIN(pin) (1 << (pin)) +#define PIN_MASK(pin) (BIT(pin)) /*==========================================================================*/ /* Driver exported variables. */ @@ -70,17 +78,21 @@ typedef enum { /* Driver local variables and types. */ /*==========================================================================*/ -/* OS */ -SEMAPHORE_DECL(mc33972_wake, 10 /* or BOARD_MC33972_COUNT ? */); -static THD_WORKING_AREA(mc33972_thread_1_wa, 256); - /* Driver */ struct mc33972_priv { const struct mc33972_config *cfg; + + /* thread stuff */ + thread_t *thread; + THD_WORKING_AREA(thread_wa, 256); + semaphore_t wake; + /* last input state from chip */ uint32_t i_state; /* currently selected analog input */ uint8_t analog_ch; + /* enabled inputs */ + uint32_t en_pins; mc33972_drv_state drv_state; }; @@ -155,6 +167,45 @@ static int mc33972_update_status(struct mc33972_priv *chip) return ret; } +static int mc33972_update_pullups(struct mc33972_priv *chip) +{ + int ret; + + /* enable tri-state for all unused pins */ + ret = mc33972_spi_w(chip, CMD_TRI_STATE(SP_BANK, SP_PINS_EXTRACT(~chip->en_pins))); + if (ret) + return ret; + ret = mc33972_spi_w(chip, CMD_TRI_STATE(SG_BANK, SG_PINS_EXTRACT(~chip->en_pins))); + + return ret; +} + +static int mc33972_comm_test(struct mc33972_priv *chip) +{ + int ret; + + /* After an input has been selected as the analog, + * the corresponding bit in the next SO data stream + * will be logic [0] */ + + /* go throught all inputs, read inputs status and + * check that muxed input bit is zero */ + for (int i = 0; i < MC33972_INPUTS; i++) { + /* indexed starting from 1 */ + ret = mc33972_spi_w(chip, CMD_ANALOG(0, i + 1)); + if (ret) + return ret; + ret = mc33972_update_status(chip); + if (ret) + return ret; + + if (chip->i_state & PIN_MASK(i)) + return -1; + } + + return 0; +} + /** * @brief MC33972 chip init. * @details There is no way to check communication. @@ -165,9 +216,10 @@ static int mc33972_chip_init(struct mc33972_priv *chip) { int ret; + /* reset first */ ret = mc33972_spi_w(chip, CMD_RST); if (ret) - goto err; + return ret; /* is it enought? */ chThdSleepMilliseconds(3); @@ -175,7 +227,7 @@ static int mc33972_chip_init(struct mc33972_priv *chip) /* * Default settings from Power-ON Reset via V PWR or the * Reset Command are as follows: - * * Programmable switch – set to switch to battery + * * Programmable switch - set to switch to battery * * All inputs set as wake-up * * Wetting current on (16 mA) * * Wetting current timer on (20 ms) @@ -183,20 +235,25 @@ static int mc33972_chip_init(struct mc33972_priv *chip) * * Analog select 00000 (no input channel selected) */ - /* disable tri-state */ - ret = mc33972_spi_w(chip, CMD_TRI_STATE(0, 0)); - ret |= mc33972_spi_w(chip, CMD_TRI_STATE(1, 0)); + /* check communication */ + ret = mc33972_comm_test(chip); if (ret) - goto err; + return ret; + + /* disable tri-state for used pins only */ + ret = mc33972_update_pullups(chip); + if (ret) + return ret; /* Set wetting current to 2 mA */ - ret = mc33972_spi_w(chip, CMD_METALLIC(0, 0)); - ret |= mc33972_spi_w(chip, CMD_METALLIC(1, 0)); + ret = mc33972_spi_w(chip, CMD_METALLIC(SP_BANK, 0)); if (ret) - goto err; + return ret; + ret = mc33972_spi_w(chip, CMD_METALLIC(SG_BANK, 0)); + if (ret) + return ret; -err: - return ret; + return 0; } /** @@ -204,16 +261,23 @@ err: * @details Wake up driver. Will cause input and diagnostic * update */ -/* todo: why is this unused? dead code? bug? static int mc33972_wake_driver(struct mc33972_priv *chip) { - (void)chip; - - chSemSignal(&mc33972_wake); + /* Entering a reentrant critical zone.*/ + syssts_t sts = chSysGetStatusAndLockX(); + chSemSignalI(&chip->wake); + if (!port_is_isr_context()) { + /** + * chSemSignalI above requires rescheduling + * interrupt handlers have implicit rescheduling + */ + chSchRescheduleS(); + } + /* Leaving the critical zone.*/ + chSysRestoreStatusX(sts); return 0; } -*/ /*==========================================================================*/ /* Driver thread. */ @@ -221,33 +285,41 @@ static int mc33972_wake_driver(struct mc33972_priv *chip) static THD_FUNCTION(mc33972_driver_thread, p) { - int i; - msg_t msg; - - (void)p; + int ret; + struct mc33972_priv *chip = p; chRegSetThreadName(DRIVER_NAME); + /* repeat init until success */ + do { + ret = mc33972_chip_init(chip); + if (ret) { + chThdSleepMilliseconds(1000); + continue; + } + /* else */ + chip->drv_state = MC33972_READY; + } while (chip->drv_state != MC33972_READY); + while(1) { - msg = chSemWaitTimeout(&mc33972_wake, TIME_MS2I(MC33972_POLL_INTERVAL_MS)); + msg_t msg = chSemWaitTimeout(&chip->wake, TIME_MS2I(MC33972_POLL_INTERVAL_MS)); - /* should we care about msg == MSG_TIMEOUT? */ - (void)msg; - - for (i = 0; i < BOARD_MC33972_COUNT; i++) { - int ret; - struct mc33972_priv *chip; - - chip = &chips[i]; - if ((chip->cfg == NULL) || - (chip->drv_state == MC33972_DISABLED) || - (chip->drv_state == MC33972_FAILED)) - continue; + if ((chip->cfg == NULL) || + (chip->drv_state == MC33972_DISABLED) || + (chip->drv_state == MC33972_FAILED)) + continue; + if (msg == MSG_TIMEOUT) { + /* only input state update */ ret = mc33972_update_status(chip); - if (ret) { - /* set state to MC33972_FAILED? */ - } + } else { + /* someone waked thread and asks us to update pin config */ + /* inputs state is also readed */ + ret = mc33972_update_pullups(chip); + } + + if (ret) { + /* set state to MC33972_FAILED? */ } } } @@ -262,6 +334,31 @@ static THD_FUNCTION(mc33972_driver_thread, p) /* Driver exported functions. */ /*==========================================================================*/ +static int mc33972_setPadMode(void *data, unsigned int pin, iomode_t mode) { + struct mc33972_priv *chip; + + if ((pin >= MC33972_INPUTS) || (data == NULL)) + return -1; + + chip = (struct mc33972_priv *)data; + + /* currently driver doesn't know how to hanlde different modes */ + (void)mode; + + /* NOTE: currently this driver supports only input mode... + * while chip can output by manipulating with pull-up and + * pull-down modes.... + * if this function is called for pin, that means someone + * wants to read this pin and we can enable pull-up + * Also pull down is not supported yet */ + chip->en_pins |= PIN_MASK(pin); + + /* ask for reinit */ + mc33972_wake_driver(chip); + + return 0; +} + static int mc33972_readPad(void *data, unsigned int pin) { struct mc33972_priv *chip; @@ -271,7 +368,7 @@ static int mc33972_readPad(void *data, unsigned int pin) { chip = (struct mc33972_priv *)data; /* convert to some common enum? */ - return !!(chip->i_state & FLAG_PIN(pin)); + return !!(chip->i_state & PIN_MASK(pin)); } static brain_pin_diag_e mc33972_getDiag(void *data, unsigned int pin) { @@ -292,35 +389,39 @@ static brain_pin_diag_e mc33972_getDiag(void *data, unsigned int pin) { static int mc33972_init(void * data) { - int ret; struct mc33972_priv *chip; chip = (struct mc33972_priv *)data; - ret = mc33972_chip_init(chip); - if (ret) - return ret; + /* no pins enabled yet */ + chip->en_pins = 0x0000; - chip->drv_state = MC33972_READY; + /* init semaphore */ + chSemObjectInit(&chip->wake, 0); - if (!drv_task_ready) { - chThdCreateStatic(mc33972_thread_1_wa, sizeof(mc33972_thread_1_wa), - PRIO_GPIOCHIP, mc33972_driver_thread, NULL); - drv_task_ready = true; - } + /* start thread */ + chip->thread = chThdCreateStatic(chip->thread_wa, sizeof(chip->thread_wa), + PRIO_GPIOCHIP, mc33972_driver_thread, chip); return 0; } static int mc33972_deinit(void *data) { - (void)data; + struct mc33972_priv *chip; + + chip = (struct mc33972_priv *)data; + + /* TODO: disable pulls for all pins? */ + + /* stop thread */ + chThdTerminate(chip->thread); - /* TODO: set all pins to inactive state, stop task? */ return 0; } struct gpiochip_ops mc33972_ops = { + .setPadMode = mc33972_setPadMode, .writePad = NULL, /* chip input only */ .readPad = mc33972_readPad, .getDiag = mc33972_getDiag, diff --git a/firmware/hw_layer/smart_gpio.cpp b/firmware/hw_layer/smart_gpio.cpp index 5e7661f513..32c56f394c 100644 --- a/firmware/hw_layer/smart_gpio.cpp +++ b/firmware/hw_layer/smart_gpio.cpp @@ -86,7 +86,7 @@ struct mc33972_config mc33972 = { .ssport = NULL, .sspad = 0, .cr1 = - SPI_CR1_24BIT_MODE | + SPI_CR1_8BIT_MODE | SPI_CR1_SSM | SPI_CR1_SSI | /* SPI_CR1_LSBFIRST | */ @@ -95,7 +95,7 @@ struct mc33972_config mc33972 = { /* SPI_CR1_CPOL | */ /* = 0 */ SPI_CR1_CPHA | /* = 1 */ 0, - .cr2 = SPI_CR2_24BIT_MODE + .cr2 = SPI_CR2_8BIT_MODE }, }; #endif /* (BOARD_MC33972_COUNT > 0) */