From 1c92fdcc96205dbfb3c2c1cb126e838c816daf21 Mon Sep 17 00:00:00 2001 From: Philippe G Date: Fri, 17 Dec 2021 21:32:03 -0800 Subject: [PATCH] fix pca8575 - release --- components/services/gpio_exp.c | 60 +++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/components/services/gpio_exp.c b/components/services/gpio_exp.c index fa381f91..cb264f8b 100644 --- a/components/services/gpio_exp.c +++ b/components/services/gpio_exp.c @@ -60,24 +60,23 @@ 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 uint32_t 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 uint32_t pca85xx_read(gpio_exp_t* self); +static void pca85xx_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 uint32_t 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 uint32_t mcp23s17_read(gpio_exp_t* self); static void mcp23s17_write(gpio_exp_t* self); static void service_handler(void *arg); @@ -94,7 +93,7 @@ static const struct gpio_exp_model_s { char *model; gpio_int_type_t trigger; esp_err_t (*init)(gpio_exp_t* self); - int (*read)(gpio_exp_t* self); + uint32_t (*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); @@ -106,7 +105,6 @@ static const struct gpio_exp_model_s { .write = pca9535_write, }, { .model = "pca85xx", .trigger = GPIO_INTR_NEGEDGE, - .set_direction = pca85xx_set_direction, .read = pca85xx_read, .write = pca85xx_write, }, { .model = "mcp23017", @@ -215,7 +213,7 @@ gpio_exp_t* gpio_exp_create(const gpio_exp_config_t *config) { gpio_intr_enable(config->intr); } - ESP_LOGI(TAG, "Create GPIO expander %s at base %u with INT %u at @%x on port/host %d/%d", config->model, config->base, config->intr, config->phy.addr, config->phy.port, config->phy.host); + ESP_LOGI(TAG, "Create GPIO expander %s at base %u with INT %d at @%x on port/host %d/%d", config->model, config->base, config->intr, config->phy.addr, config->phy.port, config->phy.host); return expander; } @@ -475,7 +473,7 @@ void service_handler(void *arg) { } // check if we have some other pending requests - if (xQueueReceive(message_queue, &request, 0) == pdTRUE) { + while (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); } @@ -508,7 +506,7 @@ static void pca9535_set_direction(gpio_exp_t* self) { i2c_write(self->phy.port, self->phy.addr, 0x06, self->r_mask, 2); } -static int pca9535_read(gpio_exp_t* self) { +static uint32_t pca9535_read(gpio_exp_t* self) { return i2c_read(self->phy.port, self->phy.addr, 0x00, 2); } @@ -519,18 +517,24 @@ static void pca9535_write(gpio_exp_t* self) { /**************************************************************************************** * PCA85xx family : read and write */ -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(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, 2); +static uint32_t pca85xx_read(gpio_exp_t* self) { + // must return the full set of pins, not just inputs + uint32_t data = i2c_read(self->phy.port, self->phy.addr, 0xff, 2); + return (data & self->r_mask) | (self->shadow & ~self->r_mask); } static void pca85xx_write(gpio_exp_t* self) { - // all input must be set to 1 (open drain) - i2c_write(self->phy.port, self->phy.addr, 0xff, self->shadow | self->r_mask, true); + /* + There is no good option with this chip: normally, unused pin should be set to input + to avoid any conflict but then they float and create tons of suprious. So option 1 is + to le tthem float and option 2 is to set them as output to 0. + In addition, setting an output pin to 1 equals is making it an input and if this is + use to short a led (e.g.) instead of being the sink, the it generates a spurious + */ + // option 1 + // i2c_write(self->phy.port, self->phy.addr, 0xff, (self->shadow & self->w_mask) | ~self->w_mask, 2); + // option 2 + i2c_write(self->phy.port, self->phy.addr, 0xff, (self->shadow & self->w_mask) | self->r_mask, 2); } /**************************************************************************************** @@ -561,7 +565,7 @@ static void mcp23017_set_pull_mode(gpio_exp_t* self) { i2c_write(self->phy.port, self->phy.addr, 0x0c, self->pullup, 2); } -static int mcp23017_read(gpio_exp_t* self) { +static uint32_t mcp23017_read(gpio_exp_t* self) { // read the pins value, not the stored one @interrupt return i2c_read(self->phy.port, self->phy.addr, 0x12, 2); } @@ -600,7 +604,7 @@ static void mcp23s17_set_pull_mode(gpio_exp_t* self) { spi_write(self->spi_handle, self->phy.addr, 0x0c, self->pullup, 2); } -static int mcp23s17_read(gpio_exp_t* self) { +static uint32_t 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); } @@ -623,7 +627,7 @@ static esp_err_t i2c_write(uint8_t port, uint8_t addr, uint8_t reg, uint32_t dat 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 + // works with our 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); @@ -647,16 +651,18 @@ static uint32_t i2c_read(uint8_t port, uint8_t addr, uint8_t reg, int len) { i2c_cmd_handle_t cmd = i2c_cmd_link_create(); i2c_master_start(cmd); - 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, (addr << 1) | I2C_MASTER_WRITE, I2C_MASTER_NACK); i2c_master_write_byte(cmd, reg, I2C_MASTER_NACK); i2c_master_start(cmd); i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_READ, I2C_MASTER_NACK); + } else { + i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_READ, I2C_MASTER_NACK); } - // works with out endianness + // works with our 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);