Add MCP23s17 + further optimizations

components/services/accessors.c

@@ -491,7 +491,7 @@ const gpio_exp_config_t* config_gpio_exp_get(int index) {
 
 	// re-initialize config every time
 	memset(&config, 0, sizeof(config));
-	config.intr = -1; config.count = 16; config.base = GPIO_NUM_MAX; config.phy.port = i2c_system_port;
+	config.intr = -1; config.count = 16; config.base = GPIO_NUM_MAX; config.phy.port = i2c_system_port; config.phy.host = spi_system_host;
 
 	nvs_item = config_alloc_get(NVS_TYPE_STR, "gpio_exp_config");
 	if (!nvs_item || !*nvs_item) return NULL;
@@ -505,6 +505,8 @@ const gpio_exp_config_t* config_gpio_exp_get(int index) {
 	}
 
 	if ((p = strcasestr(item, "addr")) != NULL) config.phy.addr = atoi(strchr(p, '=') + 1);
+	if ((p = strcasestr(item, "cs_pin")) != NULL) config.phy.cs_pin = atoi(strchr(p, '=') + 1);
+	if ((p = strcasestr(item, "speed")) != NULL) config.phy.speed = atoi(strchr(p, '=') + 1);
 	if ((p = strcasestr(item, "intr")) != NULL) config.intr = atoi(strchr(p, '=') + 1);
 	if ((p = strcasestr(item, "base")) != NULL) config.base = atoi(strchr(p, '=') + 1);
 	if ((p = strcasestr(item, "count")) != NULL) config.count = atoi(strchr(p, '=') + 1);

components/services/gpio_exp.c

@@ -16,6 +16,7 @@
 #include "esp_log.h"
 #include "driver/gpio.h"
 #include "driver/i2c.h"
+#include "driver/spi_master.h"
 #include "gpio_exp.h"
 
 #define GPIO_EXP_INTR	0x100
@@ -28,7 +29,10 @@
  
 typedef struct gpio_exp_s {
 	uint32_t first, last;
-	union gpio_exp_phy_u phy;
+	struct  {
+		struct gpio_exp_phy_s phy;
+		spi_device_handle_t spi_handle;
+	};
 	uint32_t shadow, pending;
 	TickType_t age;
 	SemaphoreHandle_t mutex;
@@ -54,35 +58,44 @@ static const char TAG[] = "gpio expander";
 static void   IRAM_ATTR intr_isr_handler(void* arg);
 static gpio_exp_t* find_expander(gpio_exp_t *expander, int *gpio);
 
-static void   pca9535_set_direction(gpio_exp_t* self);
-static int    pca9535_read(gpio_exp_t* self);
-static void   pca9535_write(gpio_exp_t* self);
+static void pca9535_set_direction(gpio_exp_t* self);
+static int  pca9535_read(gpio_exp_t* self);
+static void pca9535_write(gpio_exp_t* self);
 
-static void   pca85xx_set_direction(gpio_exp_t* self);
-static int    pca85xx_read(gpio_exp_t* self);
-static void   pca85xx_write(gpio_exp_t* self);
+static void pca85xx_set_direction(gpio_exp_t* self);
+static int  pca85xx_read(gpio_exp_t* self);
+static void pca85xx_write(gpio_exp_t* self);
 
-static void   mcp23017_init(gpio_exp_t* self);
-static void   mcp23017_set_pull_mode(gpio_exp_t* self);
-static void   mcp23017_set_direction(gpio_exp_t* self);
-static int    mcp23017_read(gpio_exp_t* self);
-static void   mcp23017_write(gpio_exp_t* self);
+static esp_err_t mcp23017_init(gpio_exp_t* self);
+static void      mcp23017_set_pull_mode(gpio_exp_t* self);
+static void      mcp23017_set_direction(gpio_exp_t* self);
+static int       mcp23017_read(gpio_exp_t* self);
+static void      mcp23017_write(gpio_exp_t* self);
+
+static esp_err_t mcp23s17_init(gpio_exp_t* self);
+static void      mcp23s17_set_pull_mode(gpio_exp_t* self);
+static void      mcp23s17_set_direction(gpio_exp_t* self);
+static int       mcp23s17_read(gpio_exp_t* self);
+static void      mcp23s17_write(gpio_exp_t* self);
 
 static void   service_handler(void *arg);
 static void   debounce_handler( TimerHandle_t xTimer );
 
-static esp_err_t i2c_write_byte(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg, uint8_t val);
-static uint16_t  i2c_read(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg, bool word);
-static esp_err_t i2c_write_word(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg, uint16_t data);
+static esp_err_t i2c_write(uint8_t port, uint8_t addr, uint8_t reg, uint32_t data, int len);
+static uint32_t  i2c_read(uint8_t port, uint8_t addr, uint8_t reg, int len);
+
+static spi_device_handle_t spi_config(struct gpio_exp_phy_s *phy);
+static esp_err_t           spi_write(spi_device_handle_t handle, uint8_t addr, uint8_t reg, uint32_t data, int len);
+static uint32_t            spi_read(spi_device_handle_t handle, uint8_t addr, uint8_t reg, int len);
 
 static const struct gpio_exp_model_s {
 	char *model;
 	gpio_int_type_t trigger;
-	void (*init)(gpio_exp_t* self);
-	int  (*read)(gpio_exp_t* self);
-	void (*write)(gpio_exp_t* self);
-	void (*set_direction)(gpio_exp_t* self);
-	void (*set_pull_mode)(gpio_exp_t* self);
+	esp_err_t (*init)(gpio_exp_t* self);
+	int       (*read)(gpio_exp_t* self);
+	void      (*write)(gpio_exp_t* self);
+	void      (*set_direction)(gpio_exp_t* self);
+	void      (*set_pull_mode)(gpio_exp_t* self);
 } registered[] = {
 	{ .model = "pca9535",
 	  .trigger = GPIO_INTR_NEGEDGE, 
@@ -101,6 +114,13 @@ static const struct gpio_exp_model_s {
 	  .set_pull_mode = mcp23017_set_pull_mode,
 	  .read = mcp23017_read,
 	  .write = mcp23017_write, },
+	{ .model = "mcp23s17",
+	  .trigger = GPIO_INTR_NEGEDGE, 
+	  .init = mcp23s17_init,
+	  .set_direction = mcp23s17_set_direction,
+	  .set_pull_mode = mcp23s17_set_pull_mode,
+	  .read = mcp23s17_read,
+	  .write = mcp23s17_write, },
 };
 
 static EXT_RAM_ATTR uint8_t n_expanders;
@@ -150,7 +170,7 @@ gpio_exp_t* gpio_exp_create(const gpio_exp_config_t *config) {
 	expander->last = config->base + config->count - 1;
 	expander->mutex = xSemaphoreCreateMutex();
 
-	memcpy(&expander->phy, &config->phy, sizeof(union gpio_exp_phy_u));
+	memcpy(&expander->phy, &config->phy, sizeof(struct gpio_exp_phy_s));
 	if (expander->model->init) expander->model->init(expander);
 
 	// create a task to handle asynchronous requests (only write at this time)
@@ -372,6 +392,72 @@ esp_err_t gpio_isr_handler_remove_x(int gpio) {
 	return gpio_exp_isr_handler_remove(gpio, NULL);
 }
 
+
+/****************************************************************************************
+ * INTR low-level handler
+ */
+static void IRAM_ATTR intr_isr_handler(void* arg) {
+	BaseType_t woken = pdFALSE;
+	
+	xTaskNotifyFromISR(service_task, GPIO_EXP_INTR, eSetValueWithOverwrite, &woken);
+	if (woken) portYIELD_FROM_ISR();
+
+	ESP_EARLY_LOGD(TAG, "INTR for expander base %d", gpio_exp_get_base(arg));
+}
+
+/****************************************************************************************
+ * INTR debounce handler
+ */
+static void debounce_handler( TimerHandle_t xTimer ) {
+	struct gpio_exp_isr_s *isr = (struct gpio_exp_isr_s*) pvTimerGetTimerID (xTimer);
+	isr->handler(isr->arg);
+}
+
+/****************************************************************************************
+ * Service task
+ */
+void service_handler(void *arg) {
+	while (1) {
+		queue_request_t request;
+		uint32_t notif = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+
+		// we have been notified of an interrupt
+		if (notif == GPIO_EXP_INTR) {
+			/* If we want a smarter bitmap of expanders with a pending interrupt
+			   we'll have to disable interrupts while clearing that bitmap. For 
+			   now, a loop will do */
+			for (int i = 0; i < n_expanders; i++) {
+				gpio_exp_t *expander = expanders + i;
+				
+				xSemaphoreTake(expander->mutex, pdMS_TO_TICKS(50));
+
+				// read GPIOs and clear all pending status
+				uint32_t value = expander->model->read(expander);
+				uint32_t pending = expander->pending | ((expander->shadow ^ value) & expander->r_mask);
+				expander->shadow = value;
+				expander->pending = 0;
+				expander->age = xTaskGetTickCount();
+
+				xSemaphoreGive(expander->mutex);
+				ESP_LOGD(TAG, "Handling GPIO %d reads 0x%04x and has 0x%04x pending", expander->first, expander->shadow, pending);
+				
+				for (int gpio = 31, clz; pending; pending <<= (clz + 1)) {
+					clz = __builtin_clz(pending);
+					gpio -= clz;
+					if (expander->isr[gpio].timer) xTimerReset(expander->isr[gpio].timer, 1);	// todo 0
+					else if (expander->isr[gpio].handler) expander->isr[gpio].handler(expander->isr[gpio].arg);
+				}
+			}
+		}
+
+		// check if we have some other pending requests
+		if (xQueueReceive(message_queue, &request, 0) == pdTRUE) {
+			esp_err_t err = gpio_exp_set_level(request.gpio, request.level, true, request.expander);
+			if (err != ESP_OK) ESP_LOGW(TAG, "Can't execute async GPIO %d write request (%d)", request.gpio, err);  
+		}
+	}
+}
+
 /****************************************************************************************
  * Find the expander related to base
  */
@@ -387,19 +473,23 @@ static gpio_exp_t* find_expander(gpio_exp_t *expander, int *gpio) {
 	return expander;
 }
 
+/****************************************************************************************
+                                        DRIVERS                                       
+****************************************************************************************/
+
 /****************************************************************************************
  * PCA9535 family : direction, read and write
  */
 static void pca9535_set_direction(gpio_exp_t* self) {
-	i2c_write_word(self->phy.port, self->phy.addr, 0x06, self->r_mask);
+	i2c_write(self->phy.port, self->phy.addr, 0x06, self->r_mask, 2);
 }
 
 static int pca9535_read(gpio_exp_t* self) {
-	return i2c_read(self->phy.port, self->phy.addr, 0x00, true);
+	return i2c_read(self->phy.port, self->phy.addr, 0x00, 2);
 }
 
 static void pca9535_write(gpio_exp_t* self) {
-	i2c_write_word(self->phy.port, self->phy.addr, 0x02, self->shadow);
+	i2c_write(self->phy.port, self->phy.addr, 0x02, self->shadow, 2);
 }
 
 /****************************************************************************************
@@ -407,134 +497,117 @@ static void pca9535_write(gpio_exp_t* self) {
  */
 static void pca85xx_set_direction(gpio_exp_t* self) {
 	// all inputs must be set to 1 (open drain) and output are left open as well
-	i2c_write_word(self->phy.port, self->phy.addr, 0xff, self->r_mask | self->w_mask);
+	i2c_write(self->phy.port, self->phy.addr, 0xff, self->r_mask | self->w_mask, true);
 }
 
 static int pca85xx_read(gpio_exp_t* self) {
-	return i2c_read(self->phy.port, self->phy.addr, 0xff, true);
+	return i2c_read(self->phy.port, self->phy.addr, 0xff, 2);
 }
 
 static void pca85xx_write(gpio_exp_t* self) {
 	// all input must be set to 1 (open drain)
-	i2c_write_word(self->phy.port, self->phy.addr, 0xff, self->shadow | self->r_mask);
+	i2c_write(self->phy.port, self->phy.addr, 0xff, self->shadow | self->r_mask, true);
 }
 
 /****************************************************************************************
  * MCP23017 family : init, direction, read and write
  */
-static void mcp23017_init(gpio_exp_t* self) {
+static esp_err_t mcp23017_init(gpio_exp_t* self) {
 	/*
 	0111 x10x = same bank, mirrot single int, no sequentµial, open drain, active low
 	not sure about this funny change of mapping of the control register itself, really?
 	*/
-	i2c_write_byte(self->phy.port, self->phy.addr, 0x05, 0x74);
-	i2c_write_byte(self->phy.port, self->phy.addr, 0x0a, 0x74);
+	esp_err_t err = i2c_write(self->phy.port, self->phy.addr, 0x05, 0x74, 1);
+	err |= i2c_write(self->phy.port, self->phy.addr, 0x0a, 0x74, 1);
 
 	// no interrupt on comparison or on change
-	i2c_write_word(self->phy.port, self->phy.addr, 0x04, 0x00);
-	i2c_write_word(self->phy.port, self->phy.addr, 0x08, 0x00);
+	err |= i2c_write(self->phy.port, self->phy.addr, 0x04, 0x00, 2);
+	err |= i2c_write(self->phy.port, self->phy.addr, 0x08, 0x00, 2);
+
+	return err;
 }
 
 static void mcp23017_set_direction(gpio_exp_t* self) {
 	// default to input and set real input to generate interrupt
-	i2c_write_word(self->phy.port, self->phy.addr, 0x00, ~self->w_mask);
-	i2c_write_word(self->phy.port, self->phy.addr, 0x04, self->r_mask);
+	i2c_write(self->phy.port, self->phy.addr, 0x00, ~self->w_mask, 2);
+	i2c_write(self->phy.port, self->phy.addr, 0x04, self->r_mask, 2);
 }
 
 static void mcp23017_set_pull_mode(gpio_exp_t* self) {
-	i2c_write_word(self->phy.port, self->phy.addr, 0x0c, self->pullup);
+	i2c_write(self->phy.port, self->phy.addr, 0x0c, self->pullup, 2);
 }
 
 static int mcp23017_read(gpio_exp_t* self) {
-	// read the pin value, not the stored one @interrupt
-	return i2c_read(self->phy.port, self->phy.addr, 0x12, true);
+	// read the pins value, not the stored one @interrupt
+	return i2c_read(self->phy.port, self->phy.addr, 0x12, 2);
 }
 
 static void mcp23017_write(gpio_exp_t* self) {
-	i2c_write_word(self->phy.port, self->phy.addr, 0x12, self->shadow);
+	i2c_write(self->phy.port, self->phy.addr, 0x12, self->shadow, 2);
 }
 
 /****************************************************************************************
- * INTR low-level handler
+ * MCP23s17 family : init, direction, read and write
  */
-static void IRAM_ATTR intr_isr_handler(void* arg) {
-	BaseType_t woken = pdFALSE;
+static esp_err_t mcp23s17_init(gpio_exp_t* self) {
+	self->spi_handle = spi_config(&self->phy);
 	
-	xTaskNotifyFromISR(service_task, GPIO_EXP_INTR, eSetValueWithOverwrite, &woken);
-	if (woken) portYIELD_FROM_ISR();
+	/*
+	0111 x10x = same bank, mirrot single int, no sequentµial, open drain, active low
+	not sure about this funny change of mapping of the control register itself, really?
+	*/
+	esp_err_t err = spi_write(self->spi_handle, self->phy.addr, 0x05, 0x74, 1);
+	err |= spi_write(self->spi_handle, self->phy.addr, 0x0a, 0x74, 1);
 
-	ESP_EARLY_LOGD(TAG, "INTR for expander base %d", gpio_exp_get_base(arg));
-}
+	// no interrupt on comparison or on change
+	err |= spi_write(self->spi_handle, self->phy.addr, 0x04, 0x00, 2);
+	err |= spi_write(self->spi_handle, self->phy.addr, 0x08, 0x00, 2);
 
-/****************************************************************************************
- * INTR debounce handler
- */
-static void debounce_handler( TimerHandle_t xTimer ) {
-	struct gpio_exp_isr_s *isr = (struct gpio_exp_isr_s*) pvTimerGetTimerID (xTimer);
-	isr->handler(isr->arg);
+	return err;
 }
 
-/****************************************************************************************
- * Service task
- */
-void service_handler(void *arg) {
-	while (1) {
-		queue_request_t request;
-		uint32_t notif = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
-
-		// we have been notified of an interrupt
-		if (notif == GPIO_EXP_INTR) {
-			/* If we want a smarter bitmap of expanders with a pending interrupt
-			   we'll have to disable interrupts while clearing that bitmap. For 
-			   now, a loop will do */
-			for (int i = 0; i < n_expanders; i++) {
-				gpio_exp_t *expander = expanders + i;
-				
-				xSemaphoreTake(expander->mutex, pdMS_TO_TICKS(50));
+static void mcp23s17_set_direction(gpio_exp_t* self) {
+	// default to input and set real input to generate interrupt
+	spi_write(self->spi_handle, self->phy.addr, 0x00, ~self->w_mask, 2);
+	spi_write(self->spi_handle, self->phy.addr, 0x04, self->r_mask, 2);
+}
 
-				// read GPIOs and clear all pending status
-				uint32_t value = expander->model->read(expander);
-				uint32_t pending = expander->pending | ((expander->shadow ^ value) & expander->r_mask);
-				expander->shadow = value;
-				expander->pending = 0;
-				expander->age = xTaskGetTickCount();
+static void mcp23s17_set_pull_mode(gpio_exp_t* self) {
+	spi_write(self->spi_handle, self->phy.addr, 0x0c, self->pullup, 2);
+}
 
-				xSemaphoreGive(expander->mutex);
-				ESP_LOGD(TAG, "Handling GPIO %d reads 0x%04x and has 0x%04x pending", expander->first, expander->shadow, pending);
-				
-				for (int gpio = 31, clz; pending; pending <<= (clz + 1)) {
-					clz = __builtin_clz(pending);
-					gpio -= clz;
-					if (expander->isr[gpio].timer) xTimerReset(expander->isr[gpio].timer, 1);	// todo 0
-					else if (expander->isr[gpio].handler) expander->isr[gpio].handler(expander->isr[gpio].arg);
-				}
-			}
-		}
+static int mcp23s17_read(gpio_exp_t* self) {
+	// read the pins value, not the stored one @interrupt
+	return spi_read(self->spi_handle, self->phy.addr, 0x12, 2);
+}
 
-		// check if we have some other pending requests
-		if (xQueueReceive(message_queue, &request, 0) == pdTRUE) {
-			esp_err_t err = gpio_exp_set_level(request.gpio, request.level, true, request.expander);
-			if (err != ESP_OK) ESP_LOGW(TAG, "Can't execute async GPIO %d write request (%d)", request.gpio, err);  
-		}
-	}
+static void mcp23s17_write(gpio_exp_t* self) {
+	spi_write(self->spi_handle, self->phy.addr, 0x12, self->shadow, 2);
 }
 
+/***************************************************************************************
+                                     I2C low level                                   
+***************************************************************************************/
+
 /****************************************************************************************
- * 
+ * I2C write up to 32 bits
  */
-static esp_err_t i2c_write_byte(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg, uint8_t val) {
-    i2c_cmd_handle_t cmd = i2c_cmd_link_create();
+static esp_err_t i2c_write(uint8_t port, uint8_t addr, uint8_t reg, uint32_t data, int len) {
+	i2c_cmd_handle_t cmd = i2c_cmd_link_create();
     i2c_master_start(cmd);
 	
-	i2c_master_write_byte(cmd, (i2c_addr << 1) | I2C_MASTER_WRITE, I2C_MASTER_NACK);
-	i2c_master_write_byte(cmd, reg, I2C_MASTER_NACK);
-	i2c_master_write_byte(cmd, val, I2C_MASTER_NACK);
-	
+	i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_WRITE, I2C_MASTER_NACK);
+	if (reg != 0xff) i2c_master_write_byte(cmd, reg, I2C_MASTER_NACK);
+
+	// works with out endianness
+	if (len > 1) i2c_master_write(cmd, (uint8_t*) &data, len, I2C_MASTER_NACK);
+	else i2c_master_write_byte(cmd, data, I2C_MASTER_NACK);
+    
 	i2c_master_stop(cmd);
-    esp_err_t ret = i2c_master_cmd_begin(i2c_port, cmd, 100 / portTICK_RATE_MS);
+	esp_err_t ret = i2c_master_cmd_begin(port, cmd, 100 / portTICK_RATE_MS);
     i2c_cmd_link_delete(cmd);
 	
-	if (ret != ESP_OK) {
+	if (ret != ESP_OK) {		
 		ESP_LOGW(TAG, "I2C write failed");
 	}
 	
@@ -542,59 +615,96 @@ static esp_err_t i2c_write_byte(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg,
 }
 
 /****************************************************************************************
- * I2C read 8 or 16 bits word
+ * I2C read up to 32 bits
  */
-static uint16_t i2c_read(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg, bool word) {
-	uint8_t data[2];
+static uint32_t i2c_read(uint8_t port, uint8_t addr, uint8_t reg, int len) {
+	uint32_t data = 0;
 	
 	i2c_cmd_handle_t cmd = i2c_cmd_link_create();
 
     i2c_master_start(cmd);
-    i2c_master_write_byte(cmd, (i2c_addr << 1) | I2C_MASTER_WRITE, I2C_MASTER_NACK);
+    i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_WRITE, I2C_MASTER_NACK);
 
 	// when using a register, write it's value then the device address again
 	if (reg != 0xff) {
 		i2c_master_write_byte(cmd, reg, I2C_MASTER_NACK);
 		i2c_master_start(cmd);
-		i2c_master_write_byte(cmd, (i2c_addr << 1) | I2C_MASTER_READ, I2C_MASTER_NACK);
-	}
-
-	if (word) {
-		i2c_master_read_byte(cmd, data, I2C_MASTER_ACK);
-		i2c_master_read_byte(cmd, data + 1, I2C_MASTER_NACK);
-	} else {
-		i2c_master_read_byte(cmd, data, I2C_MASTER_NACK);
+		i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_READ, I2C_MASTER_NACK);
 	}
 	
+	// works with out endianness
+	if (len > 1) i2c_master_read(cmd, (uint8_t*) &data, len, I2C_MASTER_LAST_NACK);
+	else i2c_master_read_byte(cmd, (uint8_t*) &data, I2C_MASTER_NACK);
+		
     i2c_master_stop(cmd);
-    esp_err_t ret = i2c_master_cmd_begin(i2c_port, cmd, 100 / portTICK_RATE_MS);
+    esp_err_t ret = i2c_master_cmd_begin(port, cmd, 100 / portTICK_RATE_MS);
     i2c_cmd_link_delete(cmd);
 	
 	if (ret != ESP_OK) {
 		ESP_LOGW(TAG, "I2C read failed");
 	}
 
-	return *(uint16_t*) data;
+	return data;
 }
 
+/***************************************************************************************
+                                     SPI low level                                   
+***************************************************************************************/
+
 /****************************************************************************************
- * I2C write 16 bits word
+ * SPI device addition
  */
-static esp_err_t i2c_write_word(uint8_t i2c_port, uint8_t i2c_addr, uint8_t reg, uint16_t data) {
-	i2c_cmd_handle_t cmd = i2c_cmd_link_create();
-    i2c_master_start(cmd);
-	
-	i2c_master_write_byte(cmd, (i2c_addr << 1) | I2C_MASTER_WRITE, I2C_MASTER_NACK);
-	if (reg != 0xff) i2c_master_write_byte(cmd, reg, I2C_MASTER_NACK);
-	i2c_master_write(cmd, (uint8_t*) &data, 2, I2C_MASTER_NACK);
-    
-	i2c_master_stop(cmd);
-	esp_err_t ret = i2c_master_cmd_begin(i2c_port, cmd, 100 / portTICK_RATE_MS);
-    i2c_cmd_link_delete(cmd);
-	
-	if (ret != ESP_OK) {		
-		ESP_LOGW(TAG, "I2C write failed");
-	}
+static spi_device_handle_t spi_config(struct gpio_exp_phy_s *phy) {
+    spi_device_interface_config_t config;
+    spi_device_handle_t handle = NULL;
+
+	// initialize ChipSelect (CS)
+	gpio_set_direction(phy->cs_pin, GPIO_MODE_OUTPUT );
+	gpio_set_level(phy->cs_pin, 0 );
 	
-    return ret;
+    memset( &config, 0, sizeof( spi_device_interface_config_t ) );
+
+	config.command_bits = config.address_bits = 8;
+    config.clock_speed_hz = phy->speed ? phy->speed : SPI_MASTER_FREQ_8M;
+    config.spics_io_num = phy->cs_pin;
+    config.queue_size = 1;
+	config.flags = SPI_DEVICE_NO_DUMMY;
+
+    spi_bus_add_device( phy->host, &config, &handle );
+
+	return handle;
+}
+
+/****************************************************************************************
+ * SPI write up to 32 bits
+ */
+static esp_err_t spi_write(spi_device_handle_t handle, uint8_t addr, uint8_t reg, uint32_t data, int len) {
+    spi_transaction_t transaction = { };
+
+	// rx_buffer is NULL, nothing to receive
+	transaction.flags = SPI_TRANS_USE_TXDATA;
+	transaction.cmd = addr << 1;
+	transaction.addr = reg;
+	transaction.tx_data[1] = data; transaction.tx_data[2] = data >> 8;
+	transaction.length = len * 8;
+
+	// only do polling as we don't have contention on SPI (otherwise DMA for transfers > 16 bytes)		
+	return spi_device_polling_transmit(handle, &transaction);
+}
+
+/****************************************************************************************
+ * SPI read up to 32 bits
+ */
+static uint32_t spi_read(spi_device_handle_t handle, uint8_t addr, uint8_t reg, int len) {
+	spi_transaction_t transaction = { };
+
+	// tx_buffer is NULL, nothing to transmit except cmd/addr
+	transaction.flags = SPI_TRANS_USE_RXDATA;
+	transaction.cmd = (addr << 1) | 1;
+	transaction.addr = reg;
+	transaction.length = len * 8;
+
+	// only do polling as we don't have contention on SPI (otherwise DMA for transfers > 16 bytes)		
+	spi_device_polling_transmit(handle, &transaction);
+	return *(uint32_t*) transaction.rx_data;
 }

components/services/gpio_exp.h

@@ -19,12 +19,15 @@ typedef struct {
 	uint8_t intr;
 	uint8_t count;
 	uint32_t base;
-	union gpio_exp_phy_u {
-		struct {
-			uint8_t addr, port;
+	struct gpio_exp_phy_s {
+		uint8_t addr;
+		struct {				// for I2C
+			uint8_t port;
 		};
-		struct {
-			uint8_t cs_pin;
+		struct {				// for SPI
+			uint32_t speed;	
+			uint8_t host;	
+			uint8_t cs_pin; 		
 		};
 	} phy;	
 } gpio_exp_config_t;

components/services/services.c

@@ -39,20 +39,40 @@ static const char *TAG = "services";
 /****************************************************************************************
  * 
  */
-void set_power_gpio(int gpio, char *value) {
+void set_chip_power_gpio(int gpio, char *value) {
 	bool parsed = true;
+
+	// we only parse on-chip GPIOs
+	if (gpio >= GPIO_NUM_MAX) return;
 	
 	if (!strcasecmp(value, "vcc") ) {
-		gpio_pad_select_gpio_x(gpio);
-		gpio_set_direction_x(gpio, GPIO_MODE_OUTPUT);
-		gpio_set_level_x(gpio, 1);
+		gpio_pad_select_gpio(gpio);
+		gpio_set_direction(gpio, GPIO_MODE_OUTPUT);
+		gpio_set_level(gpio, 1);
 	} else if (!strcasecmp(value, "gnd")) {
-		gpio_pad_select_gpio_x(gpio);
-		gpio_set_direction_x(gpio, GPIO_MODE_OUTPUT);
-		gpio_set_level_x(gpio, 0);
+		gpio_pad_select_gpio(gpio);
+		gpio_set_direction(gpio, GPIO_MODE_OUTPUT);
+		gpio_set_level(gpio, 0);
 	} else parsed = false;
 	
 	if (parsed) ESP_LOGI(TAG, "set GPIO %u to %s", gpio, value);
+}	
+
+void set_exp_power_gpio(int gpio, char *value) {
+	bool parsed = true;
+
+	// we only parse on-chip GPIOs
+	if (gpio < GPIO_NUM_MAX) return;
+	
+	if (!strcasecmp(value, "vcc") ) {
+		gpio_exp_set_direction(gpio, GPIO_MODE_OUTPUT, NULL);
+		gpio_exp_set_level(gpio, 1, true, NULL);
+	} else if (!strcasecmp(value, "gnd")) {
+		gpio_exp_set_direction(gpio, GPIO_MODE_OUTPUT, NULL);
+		gpio_exp_set_level(gpio, 0, true, NULL);
+	} else parsed = false;
+	
+	if (parsed) ESP_LOGI(TAG, "set expanded GPIO %u to %s", gpio, value);
  }	
  
 
@@ -70,6 +90,9 @@ void services_init(void) {
 	}
 #endif
 
+	// set potential power GPIO on chip first in case expanders are power using these
+	parse_set_GPIO(set_chip_power_gpio);
+
 	// shared I2C bus 
 	const i2c_config_t * i2c_config = config_i2c_get(&i2c_system_port);
 	ESP_LOGI(TAG,"Configuring I2C sda:%d scl:%d port:%u speed:%u", i2c_config->sda_io_num, i2c_config->scl_io_num, i2c_system_port, i2c_config->master.clk_speed);
@@ -82,13 +105,6 @@ void services_init(void) {
 		ESP_LOGW(TAG, "no I2C configured");
 	}	
 
-	// create GPIO expanders
-	const gpio_exp_config_t* gpio_exp_config;
-	for (int count = 0; (gpio_exp_config = config_gpio_exp_get(count)); count++) gpio_exp_create(gpio_exp_config);
-
-	// set potential power GPIO (a GPIO-powered expander might be an issue)
-	parse_set_GPIO(set_power_gpio);
-
 	const spi_bus_config_t * spi_config = config_spi_get((spi_host_device_t*) &spi_system_host);
 	ESP_LOGI(TAG,"Configuring SPI data:%d clk:%d host:%u dc:%d", spi_config->mosi_io_num, spi_config->sclk_io_num, spi_system_host, spi_system_dc_gpio);
 	
@@ -105,6 +121,13 @@ void services_init(void) {
 		ESP_LOGW(TAG, "no SPI configured");
 	}	
 
+	// create GPIO expanders
+	const gpio_exp_config_t* gpio_exp_config;
+	for (int count = 0; (gpio_exp_config = config_gpio_exp_get(count)); count++) gpio_exp_create(gpio_exp_config);
+
+	// now set potential power GPIO on expander 
+	parse_set_GPIO(set_exp_power_gpio);
+
 	// system-wide PWM timer configuration
 	ledc_timer_config_t pwm_timer = {
 		.duty_resolution = LEDC_TIMER_13_BIT,