drivers: display: ili9340: improve error handling

Add error handling to multiple areas of the code and improve log
messages.

Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>

drivers/display/display_ili9340.c

@@ -53,10 +53,18 @@ struct ili9340_data {
 #define ILI9340_RGB_SIZE 3U
 #endif
 
-static void ili9340_exit_sleep(const struct device *dev)
+static int ili9340_exit_sleep(const struct device *dev)
 {
-	ili9340_transmit(dev, ILI9340_CMD_EXIT_SLEEP, NULL, 0);
+	int r;
+
+	r = ili9340_transmit(dev, ILI9340_CMD_EXIT_SLEEP, NULL, 0);
+	if (r < 0) {
+		return r;
+	}
+
 	k_sleep(K_MSEC(120));
+
+	return 0;
 }
 
 static int ili9340_init(const struct device *dev)
@@ -64,12 +72,14 @@ static int ili9340_init(const struct device *dev)
 	const struct ili9340_config *config = (struct ili9340_config *)dev->config;
 	struct ili9340_data *data = (struct ili9340_data *)dev->data;
 
+	int r;
+
 	LOG_DBG("Initializing display driver");
 
 	data->spi_dev = device_get_binding(config->spi_name);
 	if (data->spi_dev == NULL) {
-		LOG_ERR("Could not get SPI device for ILI9340");
+		LOG_ERR("Could not get SPI device %s", config->spi_name);
-		return -EPERM;
+		return -ENODEV;
 	}
 
 	data->spi_config.frequency = config->spi_max_freq;
@@ -86,18 +96,26 @@ static int ili9340_init(const struct device *dev)
 
 	data->reset_gpio = device_get_binding(config->reset_label);
 	if (data->reset_gpio) {
-		gpio_pin_configure(data->reset_gpio, config->reset_pin,
+		r = gpio_pin_configure(data->reset_gpio, config->reset_pin,
-				   GPIO_OUTPUT_INACTIVE | config->reset_flags);
+				       GPIO_OUTPUT_INACTIVE | config->reset_flags);
+		if (r < 0) {
+			LOG_ERR("Could not configure reset GPIO (%d)", r);
+			return r;
+		}
 	}
 
 	data->command_data_gpio = device_get_binding(config->cmd_data_label);
 	if (data->command_data_gpio == NULL) {
-		LOG_ERR("Could not get GPIO port for ILI9340 command/data");
+		LOG_ERR("Could not get command/data GPIO port %s", config->cmd_data_label);
-		return -EPERM;
+		return -ENODEV;
 	}
 
-	gpio_pin_configure(data->command_data_gpio, config->cmd_data_pin,
+	r = gpio_pin_configure(data->command_data_gpio, config->cmd_data_pin,
-			   GPIO_OUTPUT | config->cmd_data_flags);
+			       GPIO_OUTPUT | config->cmd_data_flags);
+	if (r < 0) {
+		LOG_ERR("Could not configure command/data GPIO (%d)", r);
+		return r;
+	}
 
 	if (data->reset_gpio) {
 		LOG_DBG("Resetting display driver");
@@ -109,26 +127,43 @@ static int ili9340_init(const struct device *dev)
 	}
 
 	LOG_DBG("Initializing LCD");
-	ili9340_lcd_init(dev);
+	r = ili9340_lcd_init(dev);
+	if (r < 0) {
+		LOG_ERR("Could not initialize LCD (%d)", r);
+		return r;
+	}
 
 	LOG_DBG("Exiting sleep mode");
-	ili9340_exit_sleep(dev);
+	r = ili9340_exit_sleep(dev);
+	if (r < 0) {
+		LOG_ERR("Could not exit sleep mode (%d)", r);
+		return r;
+	}
 
 	return 0;
 }
 
-static void ili9340_set_mem_area(const struct device *dev, const uint16_t x,
+static int ili9340_set_mem_area(const struct device *dev, const uint16_t x,
 				 const uint16_t y, const uint16_t w, const uint16_t h)
 {
+	int r;
 	uint16_t spi_data[2];
 
 	spi_data[0] = sys_cpu_to_be16(x);
 	spi_data[1] = sys_cpu_to_be16(x + w - 1);
-	ili9340_transmit(dev, ILI9340_CMD_COLUMN_ADDR, &spi_data[0], 4);
+	r = ili9340_transmit(dev, ILI9340_CMD_COLUMN_ADDR, &spi_data[0], 4);
+	if (r < 0) {
+		return r;
+	}
 
 	spi_data[0] = sys_cpu_to_be16(y);
 	spi_data[1] = sys_cpu_to_be16(y + h - 1);
-	ili9340_transmit(dev, ILI9340_CMD_PAGE_ADDR, &spi_data[0], 4);
+	r = ili9340_transmit(dev, ILI9340_CMD_PAGE_ADDR, &spi_data[0], 4);
+	if (r < 0) {
+		return r;
+	}
+
+	return 0;
 }
 
 static int ili9340_write(const struct device *dev, const uint16_t x,
@@ -137,6 +172,8 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
 			 const void *buf)
 {
 	struct ili9340_data *data = (struct ili9340_data *)dev->data;
+
+	int r;
 	const uint8_t *write_data_start = (uint8_t *) buf;
 	struct spi_buf tx_buf;
 	struct spi_buf_set tx_bufs;
@@ -150,7 +187,10 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
 
 	LOG_DBG("Writing %dx%d (w,h) @ %dx%d (x,y)", desc->width, desc->height,
 			x, y);
-	ili9340_set_mem_area(dev, x, y, desc->width, desc->height);
+	r = ili9340_set_mem_area(dev, x, y, desc->width, desc->height);
+	if (r < 0) {
+		return r;
+	}
 
 	if (desc->pitch > desc->width) {
 		write_h = 1U;
@@ -160,9 +200,12 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
 		nbr_of_writes = 1U;
 	}
 
-	ili9340_transmit(dev, ILI9340_CMD_MEM_WRITE,
+	r = ili9340_transmit(dev, ILI9340_CMD_MEM_WRITE,
-			 (void *) write_data_start,
+			     (void *) write_data_start,
-			 desc->width * ILI9340_RGB_SIZE * write_h);
+			     desc->width * ILI9340_RGB_SIZE * write_h);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_bufs.buffers = &tx_buf;
 	tx_bufs.count = 1;
@@ -171,7 +214,12 @@ static int ili9340_write(const struct device *dev, const uint16_t x,
 	for (write_cnt = 1U; write_cnt < nbr_of_writes; ++write_cnt) {
 		tx_buf.buf = (void *)write_data_start;
 		tx_buf.len = desc->width * ILI9340_RGB_SIZE * write_h;
-		spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
+
+		r = spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
+		if (r < 0) {
+			return r;
+		}
+
 		write_data_start += (desc->pitch * ILI9340_RGB_SIZE);
 	}
 
@@ -196,15 +244,13 @@ static void *ili9340_get_framebuffer(const struct device *dev)
 static int ili9340_display_blanking_off(const struct device *dev)
 {
 	LOG_DBG("Turning display blanking off");
-	ili9340_transmit(dev, ILI9340_CMD_DISPLAY_ON, NULL, 0);
+	return ili9340_transmit(dev, ILI9340_CMD_DISPLAY_ON, NULL, 0);
-	return 0;
 }
 
 static int ili9340_display_blanking_on(const struct device *dev)
 {
 	LOG_DBG("Turning display blanking on");
-	ili9340_transmit(dev, ILI9340_CMD_DISPLAY_OFF, NULL, 0);
+	return ili9340_transmit(dev, ILI9340_CMD_DISPLAY_OFF, NULL, 0);
-	return 0;
 }
 
 static int ili9340_set_brightness(const struct device *dev,
@@ -261,26 +307,35 @@ static void ili9340_get_capabilities(const struct device *dev,
 	capabilities->current_orientation = DISPLAY_ORIENTATION_NORMAL;
 }
 
-void ili9340_transmit(const struct device *dev, uint8_t cmd, void *tx_data,
+int ili9340_transmit(const struct device *dev, uint8_t cmd, void *tx_data,
-		      size_t tx_len)
+		     size_t tx_len)
 {
 	const struct ili9340_config *config = (struct ili9340_config *)dev->config;
 	struct ili9340_data *data = (struct ili9340_data *)dev->data;
 
+	int r;
 	struct spi_buf tx_buf = { .buf = &cmd, .len = 1 };
 	struct spi_buf_set tx_bufs = { .buffers = &tx_buf, .count = 1 };
 
 	gpio_pin_set(data->command_data_gpio, config->cmd_data_pin,
 		     ILI9340_CMD_DATA_PIN_COMMAND);
-	spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
+	r = spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
+	if (r < 0) {
+		return r;
+	}
 
 	if (tx_data != NULL) {
 		tx_buf.buf = tx_data;
 		tx_buf.len = tx_len;
 		gpio_pin_set(data->command_data_gpio, config->cmd_data_pin,
 			     ILI9340_CMD_DATA_PIN_DATA);
-		spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
+		r = spi_write(data->spi_dev, &data->spi_config, &tx_bufs);
+		if (r < 0) {
+			return r;
+		}
 	}
+
+	return 0;
 }
 
 static const struct display_driver_api ili9340_api = {

drivers/display/display_ili9340.h

@@ -60,15 +60,18 @@
  * In case no data should be transmitted pass a NULL pointer
  * @param tx_len Number of bytes in tx_data buffer
  *
+ * @return 0 on success, errno otherwise.
  */
-void ili9340_transmit(const struct device *dev, uint8_t cmd, void *tx_data,
+int ili9340_transmit(const struct device *dev, uint8_t cmd, void *tx_data,
-		      size_t tx_len);
+		     size_t tx_len);
 
 /**
  * Perform LCD specific initialization
  *
  * @param data Device data structure
+ *
+ * @return 0 on success, errno otherwise.
  */
-void ili9340_lcd_init(const struct device *dev);
+int ili9340_lcd_init(const struct device *dev);
 
 #endif /* ZEPHYR_DRIVERS_DISPLAY_DISPLAY_ILI9340_H_ */

drivers/display/display_ili9340_adafruit_1480.c

@@ -6,26 +6,42 @@
 
 #include "display_ili9340.h"
 
-void ili9340_lcd_init(const struct device *dev)
+int ili9340_lcd_init(const struct device *dev)
 {
+	int r;
 	uint8_t tx_data[15];
 
 	tx_data[0] = 0x23;
-	ili9340_transmit(dev, ILI9340_CMD_POWER_CTRL_1, tx_data, 1);
+	r = ili9340_transmit(dev, ILI9340_CMD_POWER_CTRL_1, tx_data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x10;
-	ili9340_transmit(dev, ILI9340_CMD_POWER_CTRL_2, tx_data, 1);
+	r = ili9340_transmit(dev, ILI9340_CMD_POWER_CTRL_2, tx_data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x3e;
 	tx_data[1] = 0x28;
-	ili9340_transmit(dev, ILI9340_CMD_VCOM_CTRL_1, tx_data, 2);
+	r = ili9340_transmit(dev, ILI9340_CMD_VCOM_CTRL_1, tx_data, 2);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x86;
-	ili9340_transmit(dev, ILI9340_CMD_VCOM_CTRL_2, tx_data, 1);
+	r = ili9340_transmit(dev, ILI9340_CMD_VCOM_CTRL_2, tx_data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] =
 	    ILI9340_DATA_MEM_ACCESS_CTRL_MV | ILI9340_DATA_MEM_ACCESS_CTRL_BGR;
-	ili9340_transmit(dev, ILI9340_CMD_MEM_ACCESS_CTRL, tx_data, 1);
+	r = ili9340_transmit(dev, ILI9340_CMD_MEM_ACCESS_CTRL, tx_data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 #ifdef CONFIG_ILI9340_RGB565
 	tx_data[0] = ILI9340_DATA_PIXEL_FORMAT_MCU_16_BIT |
@@ -34,19 +50,31 @@ void ili9340_lcd_init(const struct device *dev)
 	tx_data[0] = ILI9340_DATA_PIXEL_FORMAT_MCU_18_BIT |
 		     ILI9340_DATA_PIXEL_FORMAT_RGB_18_BIT;
 #endif
-	ili9340_transmit(dev, ILI9340_CMD_PIXEL_FORMAT_SET, tx_data, 1);
+	r = ili9340_transmit(dev, ILI9340_CMD_PIXEL_FORMAT_SET, tx_data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x00;
 	tx_data[1] = 0x18;
-	ili9340_transmit(dev, ILI9340_CMD_FRAME_CTRL_NORMAL_MODE, tx_data, 2);
+	r = ili9340_transmit(dev, ILI9340_CMD_FRAME_CTRL_NORMAL_MODE, tx_data, 2);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x08;
 	tx_data[1] = 0x82;
 	tx_data[2] = 0x27;
-	ili9340_transmit(dev, ILI9340_CMD_DISPLAY_FUNCTION_CTRL, tx_data, 3);
+	r = ili9340_transmit(dev, ILI9340_CMD_DISPLAY_FUNCTION_CTRL, tx_data, 3);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x01;
-	ili9340_transmit(dev, ILI9340_CMD_GAMMA_SET, tx_data, 1);
+	r = ili9340_transmit(dev, ILI9340_CMD_GAMMA_SET, tx_data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x0F;
 	tx_data[1] = 0x31;
@@ -63,8 +91,11 @@ void ili9340_lcd_init(const struct device *dev)
 	tx_data[12] = 0x0E;
 	tx_data[13] = 0x09;
 	tx_data[14] = 0x00;
-	ili9340_transmit(dev, ILI9340_CMD_POSITIVE_GAMMA_CORRECTION, tx_data,
+	r = ili9340_transmit(dev, ILI9340_CMD_POSITIVE_GAMMA_CORRECTION, tx_data,
-			 15);
+			     15);
+	if (r < 0) {
+		return r;
+	}
 
 	tx_data[0] = 0x00;
 	tx_data[1] = 0x0E;
@@ -81,6 +112,11 @@ void ili9340_lcd_init(const struct device *dev)
 	tx_data[12] = 0x31;
 	tx_data[13] = 0x36;
 	tx_data[14] = 0x0F;
-	ili9340_transmit(dev, ILI9340_CMD_NEGATIVE_GAMMA_CORRECTION, tx_data,
+	r = ili9340_transmit(dev, ILI9340_CMD_NEGATIVE_GAMMA_CORRECTION, tx_data,
-			 15);
+			     15);
+	if (r < 0) {
+		return r;
+	}
+
+	return 0;
 }

drivers/display/display_ili9340_seeed_tftv2.c

@@ -13,14 +13,18 @@
  * https://github.com/Seeed-Studio/TFT_Touch_Shield_V2
  */
 
-void ili9340_lcd_init(const struct device *dev)
+int ili9340_lcd_init(const struct device *dev)
 {
+	int r;
 	uint8_t cmd;
 	uint8_t data[15];
 
 	/* Software reset */
 	cmd = ILI9340_CMD_SOFTWARE_RESET;
-	ili9340_transmit(dev, cmd, NULL, 0);
+	r = ili9340_transmit(dev, cmd, NULL, 0);
+	if (r < 0) {
+		return r;
+	}
 
 	k_sleep(K_MSEC(5));
 
@@ -28,20 +32,29 @@ void ili9340_lcd_init(const struct device *dev)
 	data[0] = 0x00U;
 	data[1] = 0x8BU;
 	data[2] = 0x30U;
-	ili9340_transmit(dev, cmd, data, 3);
+	r = ili9340_transmit(dev, cmd, data, 3);
+	if (r < 0) {
+		return r;
+	}
 
 	cmd = ILI9341_CMD_POWER_ON_SEQ_CTRL;
 	data[0] = 0x67U;
 	data[1] = 0x03U;
 	data[2] = 0x12U;
 	data[3] = 0x81U;
-	ili9340_transmit(dev, cmd, data, 4);
+	r = ili9340_transmit(dev, cmd, data, 4);
+	if (r < 0) {
+		return r;
+	}
 
 	cmd = ILI9341_CMD_DRVR_TIMING_CTRL_A_I;
 	data[0] = 0x85U;
 	data[1] = 0x10U;
 	data[2] = 0x7AU;
-	ili9340_transmit(dev, cmd, data, 3);
+	r = ili9340_transmit(dev, cmd, data, 3);
+	if (r < 0) {
+		return r;
+	}
 
 	cmd = ILI9341_CMD_POWER_CTRL_A;
 	data[0] = 0x39U;
@@ -49,39 +62,60 @@ void ili9340_lcd_init(const struct device *dev)
 	data[2] = 0x00U;
 	data[3] = 0x34U;
 	data[4] = 0x02U;
-	ili9340_transmit(dev, cmd, data, 5);
+	r = ili9340_transmit(dev, cmd, data, 5);
+	if (r < 0) {
+		return r;
+	}
 
 	cmd = ILI9341_CMD_PUMP_RATIO_CTRL;
 	data[0] = 0x20U;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	cmd = ILI9341_CMD_DRVR_TIMING_CTRL_B;
 	data[0] = 0x00U;
 	data[1] = 0x00U;
-	ili9340_transmit(dev, cmd, data, 2);
+	r = ili9340_transmit(dev, cmd, data, 2);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Power control */
 	/* VRH[5:0] */
 	cmd = ILI9340_CMD_POWER_CTRL_1;
 	data[0] = 0x1BU;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Power control */
 	/* SAP[2:0];BT[3:0] */
 	cmd = ILI9340_CMD_POWER_CTRL_2;
 	data[0] = 0x10U;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* VCM control */
 	cmd = ILI9340_CMD_VCOM_CTRL_1;
 	data[0] = 0x3FU;
 	data[1] = 0x3CU;
-	ili9340_transmit(dev, cmd, data, 2);
+	r = ili9340_transmit(dev, cmd, data, 2);
+	if (r < 0) {
+		return r;
+	}
 
 	/* VCM control2 */
 	cmd = ILI9340_CMD_VCOM_CTRL_2;
 	data[0] = 0xB7U;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Memory Access Control */
 	cmd = ILI9340_CMD_MEM_ACCESS_CTRL;
@@ -89,7 +123,10 @@ void ili9340_lcd_init(const struct device *dev)
 		  ILI9340_DATA_MEM_ACCESS_CTRL_MV |
 		  ILI9340_DATA_MEM_ACCESS_CTRL_ML |
 		  ILI9340_DATA_MEM_ACCESS_CTRL_BGR;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Pixel Format Set */
 	cmd = ILI9340_CMD_PIXEL_FORMAT_SET;
@@ -100,29 +137,44 @@ void ili9340_lcd_init(const struct device *dev)
 	data[0] = ILI9340_DATA_PIXEL_FORMAT_MCU_18_BIT |
 		  ILI9340_DATA_PIXEL_FORMAT_RGB_18_BIT;
 #endif
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Frame Rate */
 	cmd = ILI9340_CMD_FRAME_CTRL_NORMAL_MODE;
 	data[0] = 0x00U;
 	data[1] = 0x1BU;
-	ili9340_transmit(dev, cmd, data, 2);
+	r = ili9340_transmit(dev, cmd, data, 2);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Display Function Control */
 	cmd = ILI9340_CMD_DISPLAY_FUNCTION_CTRL;
 	data[0] = 0x0AU;
 	data[1] = 0xA2U;
-	ili9340_transmit(dev, cmd, data, 2);
+	r = ili9340_transmit(dev, cmd, data, 2);
+	if (r < 0) {
+		return r;
+	}
 
 	/* 3Gamma Function Disable */
 	cmd = ILI9341_CMD_ENABLE_3G;
 	data[0] = 0x00U;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Gamma curve selected */
 	cmd = ILI9340_CMD_GAMMA_SET;
 	data[0] = 0x01U;
-	ili9340_transmit(dev, cmd, data, 1);
+	r = ili9340_transmit(dev, cmd, data, 1);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Positive Gamma Correction */
 	cmd = ILI9340_CMD_POSITIVE_GAMMA_CORRECTION;
@@ -141,7 +193,10 @@ void ili9340_lcd_init(const struct device *dev)
 	data[12] = 0x00U;
 	data[13] = 0x00U;
 	data[14] = 0x00U;
-	ili9340_transmit(dev, cmd, data, 15);
+	r = ili9340_transmit(dev, cmd, data, 15);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Negative Gamma Correction */
 	cmd = ILI9340_CMD_NEGATIVE_GAMMA_CORRECTION;
@@ -160,15 +215,26 @@ void ili9340_lcd_init(const struct device *dev)
 	data[12] = 0x3FU;
 	data[13] = 0x3FU;
 	data[14] = 0x0FU;
-	ili9340_transmit(dev, cmd, data, 15);
+	r = ili9340_transmit(dev, cmd, data, 15);
+	if (r < 0) {
+		return r;
+	}
 
 	/* Sleep Out */
 	cmd = ILI9340_CMD_EXIT_SLEEP;
-	ili9340_transmit(dev, cmd, NULL, 0);
+	r = ili9340_transmit(dev, cmd, NULL, 0);
+	if (r < 0) {
+		return r;
+	}
 
 	k_sleep(K_MSEC(120));
 
 	/* Display Off */
 	cmd = ILI9340_CMD_DISPLAY_OFF;
-	ili9340_transmit(dev, cmd, NULL, 0);
+	r = ili9340_transmit(dev, cmd, NULL, 0);
+	if (r < 0) {
+		return r;
+	}
+
+	return 0;
 }