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

@@ -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_STATUS					CMD(0x00,		0x00)
-#define CMD_SETTINGS(mask)			CMD(0x01, (mask))
+#define CMD_SETTINGS(mask)			CMD(0x01,		(mask))
-#define CMD_WAKEUPEN(i, mask)		CMD((i) ? 0x03 : 0x02, (mask))
+#define CMD_WAKEUPEN(i, mask)		CMD(0x02 + (i),	(mask))
-#define CMD_METALLIC(i, mask)		CMD((i) ? 0x05 : 0x04, (mask))
+#define CMD_METALLIC(i, mask)		CMD(0x04 + (i),	(mask))
-#define CMD_ANALOG(cur, ch)			CMD(0x06, (((curr) & 0x3) << 5) | (((d) & 0x1f) << 0))
+#define CMD_ANALOG(curr, ch)		CMD(0x06,		(((curr) & 0x3) << 5) | (((ch) & 0x1f) << 0))
-#define CMD_WETTING_TMR(i, mask)	CMD((i) ? 0x08 : 0x07, (mask))
+#define CMD_WETTING_TMR(i, mask)	CMD(0x07 + (i),	(mask))
-#define CMD_TRI_STATE(i, mask)		CMD((i) ? 0x0a : 0x09, (mask))
+#define CMD_TRI_STATE(i, mask)		CMD(0x09 + (i),	(mask))
-#define CMD_CALIB					CMD(0x0b, 0x00)
+#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_SLEEP(int_t, scan_t)	CMD(0x0c,		(((int_t) & 0x7) << 3) | (((scan_t) & 0x7) << 0))
-#define CMD_RST						CMD(0x7f, 0x00)
+#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 */
+	/* check communication */
-	ret  = mc33972_spi_w(chip, CMD_TRI_STATE(0, 0));
+	ret = mc33972_comm_test(chip);
-	ret |= mc33972_spi_w(chip, CMD_TRI_STATE(1, 0));
 	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(SP_BANK, 0));
-	ret |= mc33972_spi_w(chip, CMD_METALLIC(1, 0));
 	if (ret)
-		goto err;
+		return ret;
+	ret = mc33972_spi_w(chip, CMD_METALLIC(SG_BANK, 0));
+	if (ret)
+		return ret;
 
-err:
+	return 0;
-	return ret;
 }
 
 /**
@@ -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;
+	/* Entering a reentrant critical zone.*/
-
+	syssts_t sts = chSysGetStatusAndLockX();
-	chSemSignal(&mc33972_wake);
+	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;
+	int ret;
-	msg_t msg;
+	struct mc33972_priv *chip = p;
-
-	(void)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? */
+		if ((chip->cfg == NULL) ||
-		(void)msg;
+			(chip->drv_state == MC33972_DISABLED) ||
-
+			(chip->drv_state == MC33972_FAILED))
-		for (i = 0; i < BOARD_MC33972_COUNT; i++) {
+			continue;
-			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 (msg == MSG_TIMEOUT) {
+			/* only input state update */
 			ret = mc33972_update_status(chip);
-			if (ret) {
+		} else {
-				/* set state to MC33972_FAILED? */
+			/* 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);
+	/* no pins enabled yet */
-	if (ret)
+	chip->en_pins = 0x0000;
-		return ret;
 
-	chip->drv_state = MC33972_READY;
+	/* init semaphore */
+	chSemObjectInit(&chip->wake, 0);
 
-	if (!drv_task_ready) {
+	/* start thread */
-		chThdCreateStatic(mc33972_thread_1_wa, sizeof(mc33972_thread_1_wa),
+	chip->thread = chThdCreateStatic(chip->thread_wa, sizeof(chip->thread_wa),
-						  PRIO_GPIOCHIP, mc33972_driver_thread, NULL);
+									 PRIO_GPIOCHIP, mc33972_driver_thread, chip);
-		drv_task_ready = true;
-	}
 
 	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,

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) */