drivers: misc: grove_lcd_rgb: cleanup driver

- Cleanup list of includes
- Remove unused structs/definitions
- Make init function static (is called by system init)
- Make config/data naming and access consistent
- Remove redundant zero initialization of data

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>

drivers/misc/grove_lcd_rgb/grove_lcd_rgb.c

@@ -7,40 +7,27 @@
 
 #define DT_DRV_COMPAT seeed_grove_lcd_rgb
 
-#include <errno.h>
+#include <device.h>
-#include <kernel.h>
-#include <init.h>
-#include <stdbool.h>
-
 #include <drivers/i2c.h>
 #include <drivers/misc/grove_lcd/grove_lcd.h>
+#include <kernel.h>
 #include <sys/util.h>
 
-#define LOG_LEVEL CONFIG_DISPLAY_LOG_LEVEL
 #include <logging/log.h>
-LOG_MODULE_REGISTER(grove_lcd);
+LOG_MODULE_REGISTER(grove_lcd, CONFIG_DISPLAY_LOG_LEVEL);
 
 #define GROVE_RGB_BACKLIGHT_ADDR	(0x62)
 
-struct command {
-	uint8_t control;
-	uint8_t data;
-};
-
 struct glcd_data {
 	uint8_t input_set;
 	uint8_t display_switch;
 	uint8_t function;
 };
 
-struct glcd_driver {
+struct glcd_config {
 	struct i2c_dt_spec bus;
 };
 
-
-#define ON	0x1
-#define OFF	0x0
-
 /********************************************
  *  LCD FUNCTIONS
  *******************************************/
@@ -67,7 +54,6 @@ struct glcd_driver {
  *  RGB FUNCTIONS
  *******************************************/
 
-#define REGISTER_POWER  0x08
 #define REGISTER_R	0x04
 #define REGISTER_G	0x03
 #define REGISTER_B	0x02
@@ -93,24 +79,22 @@ static void rgb_reg_set(const struct device *i2c, uint8_t addr, uint8_t dta)
 /********************************************
  *  PUBLIC FUNCTIONS
  *******************************************/
-void glcd_print(const struct device *port, char *data, uint32_t size)
+void glcd_print(const struct device *dev, char *data, uint32_t size)
 {
-	const struct glcd_driver * const rom = (const struct glcd_driver *)
+	const struct glcd_config *config = dev->config;
-						port->config;
 	uint8_t buf[] = { GLCD_CMD_SET_CGRAM_ADDR, 0 };
 	int i;
 
 	for (i = 0; i < size; i++) {
 		buf[1] = data[i];
-		i2c_write_dt(&rom->bus, buf, sizeof(buf));
+		i2c_write_dt(&config->bus, buf, sizeof(buf));
 	}
 }
 
 
-void glcd_cursor_pos_set(const struct device *port, uint8_t col, uint8_t row)
+void glcd_cursor_pos_set(const struct device *dev, uint8_t col, uint8_t row)
 {
-	const struct glcd_driver * const rom = (const struct glcd_driver *)
+	const struct glcd_config *config = dev->config;
-						port->config;
 
 	unsigned char data[2];
 
@@ -123,129 +107,121 @@ void glcd_cursor_pos_set(const struct device *port, uint8_t col, uint8_t row)
 	data[0] = GLCD_CMD_SET_DDRAM_ADDR;
 	data[1] = col;
 
-	i2c_write_dt(&rom->bus, data, 2);
+	i2c_write_dt(&config->bus, data, 2);
 }
 
 
-void glcd_clear(const struct device *port)
+void glcd_clear(const struct device *dev)
 {
-	const struct glcd_driver * const rom = (const struct glcd_driver *)
+	const struct glcd_config *config = dev->config;
-						port->config;
 	uint8_t clear[] = { 0, GLCD_CMD_SCREEN_CLEAR };
 
-	i2c_write_dt(&rom->bus, clear, sizeof(clear));
+	i2c_write_dt(&config->bus, clear, sizeof(clear));
 	LOG_DBG("clear, delay 20 ms");
 	k_sleep(K_MSEC(20));
 }
 
 
-void glcd_display_state_set(const struct device *port, uint8_t opt)
+void glcd_display_state_set(const struct device *dev, uint8_t opt)
 {
-	const struct glcd_driver * const rom = (const struct glcd_driver *)
+	const struct glcd_config *config = dev->config;
-						port->config;
+	struct glcd_data *data = dev->data;
-	struct glcd_data *dev = port->data;
+	uint8_t buf[] = { 0, 0 };
-	uint8_t data[] = { 0, 0 };
 
-	dev->display_switch = opt;
+	data->display_switch = opt;
-	data[1] = (opt | GLCD_CMD_DISPLAY_SWITCH);
+	buf[1] = (opt | GLCD_CMD_DISPLAY_SWITCH);
 
-	i2c_write_dt(&rom->bus, data, sizeof(data));
+	i2c_write_dt(&config->bus, buf, sizeof(buf));
 
 	LOG_DBG("set display_state options, delay 5 ms");
 	k_sleep(K_MSEC(5));
 }
 
 
-uint8_t glcd_display_state_get(const struct device *port)
+uint8_t glcd_display_state_get(const struct device *dev)
 {
-	struct glcd_data *dev = port->data;
+	struct glcd_data *data = dev->data;
 
-	return dev->display_switch;
+	return data->display_switch;
 }
 
 
-void glcd_input_state_set(const struct device *port, uint8_t opt)
+void glcd_input_state_set(const struct device *dev, uint8_t opt)
 {
-	const struct glcd_driver * const rom = port->config;
+	const struct glcd_config *config = dev->config;
-	struct glcd_data *dev = port->data;
+	struct glcd_data *data = dev->data;
-	uint8_t data[] = { 0, 0 };
+	uint8_t buf[] = { 0, 0 };
 
-	dev->input_set = opt;
+	data->input_set = opt;
-	data[1] = (opt | GLCD_CMD_INPUT_SET);
+	buf[1] = (opt | GLCD_CMD_INPUT_SET);
 
-	i2c_write_dt(&rom->bus, &dev->input_set, sizeof(dev->input_set));
+	i2c_write_dt(&config->bus, buf, sizeof(buf));
 	LOG_DBG("set the input_set, no delay");
 }
 
 
-uint8_t glcd_input_state_get(const struct device *port)
+uint8_t glcd_input_state_get(const struct device *dev)
 {
-	struct glcd_data *dev = port->data;
+	struct glcd_data *data = dev->data;
 
-	return dev->input_set;
+	return data->input_set;
 }
 
 
-void glcd_color_select(const struct device *port, uint8_t color)
+void glcd_color_select(const struct device *dev, uint8_t color)
 {
 	if (color > 3) {
 		LOG_WRN("selected color is too high a value");
 		return;
 	}
-	glcd_color_set(port, color_define[color][0],
+	glcd_color_set(dev, color_define[color][0], color_define[color][1],
-			     color_define[color][1],
+		       color_define[color][2]);
-			     color_define[color][2]);
 }
 
 
-void glcd_color_set(const struct device *port, uint8_t r, uint8_t g,
+void glcd_color_set(const struct device *dev, uint8_t r, uint8_t g,
 		    uint8_t b)
 {
-	const struct glcd_driver * const rom = port->config;
+	const struct glcd_config *config = dev->config;
 
-	rgb_reg_set(rom->bus.bus, REGISTER_R, r);
+	rgb_reg_set(config->bus.bus, REGISTER_R, r);
-	rgb_reg_set(rom->bus.bus, REGISTER_G, g);
+	rgb_reg_set(config->bus.bus, REGISTER_G, g);
-	rgb_reg_set(rom->bus.bus, REGISTER_B, b);
+	rgb_reg_set(config->bus.bus, REGISTER_B, b);
 }
 
 
-void glcd_function_set(const struct device *port, uint8_t opt)
+void glcd_function_set(const struct device *dev, uint8_t opt)
 {
-	const struct glcd_driver * const rom = port->config;
+	const struct glcd_config *config = dev->config;
-	struct glcd_data *dev = port->data;
+	struct glcd_data *data = dev->data;
-	uint8_t data[] = { 0, 0 };
+	uint8_t buf[] = { 0, 0 };
 
-	dev->function = opt;
+	data->function = opt;
-	data[1] = (opt | GLCD_CMD_FUNCTION_SET);
+	buf[1] = (opt | GLCD_CMD_FUNCTION_SET);
 
-	i2c_write_dt(&rom->bus, data, sizeof(data));
+	i2c_write_dt(&config->bus, buf, sizeof(buf));
 
 	LOG_DBG("set function options, delay 5 ms");
 	k_sleep(K_MSEC(5));
 }
 
 
-uint8_t glcd_function_get(const struct device *port)
+uint8_t glcd_function_get(const struct device *dev)
 {
-	struct glcd_data *dev = port->data;
+	struct glcd_data *data = dev->data;
 
-	return dev->function;
+	return data->function;
 }
 
 
-int glcd_initialize(const struct device *port)
+static int glcd_initialize(const struct device *dev)
 {
-	const struct glcd_driver * const rom = port->config;
+	const struct glcd_config *config = dev->config;
-	struct glcd_data *dev = port->data;
 	uint8_t cmd;
 
 	LOG_DBG("initialize called");
 
-	dev->input_set = 0U;
+	if (!device_is_ready(config->bus.bus)) {
-	dev->display_switch = 0U;
-	dev->function = 0U;
-
-	if (!device_is_ready(rom->bus.bus)) {
 		return -ENODEV;
 	}
 
@@ -273,46 +249,42 @@ int glcd_initialize(const struct device *port)
 
 	/* Configure everything for the display function first */
 	cmd = GLCD_CMD_FUNCTION_SET | GLCD_FS_ROWS_2;
-	glcd_function_set(port, cmd);
+	glcd_function_set(dev, cmd);
 
 	/* turn the display on - by default no cursor and no blinking */
 	cmd = GLCD_DS_DISPLAY_ON | GLCD_DS_CURSOR_OFF | GLCD_DS_BLINK_OFF;
 
-	glcd_display_state_set(port, cmd);
+	glcd_display_state_set(dev, cmd);
 
 	/* Clear the screen */
-	glcd_clear(port);
+	glcd_clear(dev);
 
 	/* Initialize to the default text direction for romance languages */
 	cmd = GLCD_IS_ENTRY_LEFT | GLCD_IS_SHIFT_DECREMENT;
 
-	glcd_input_state_set(port, cmd);
+	glcd_input_state_set(dev, cmd);
 
 	/* Now power on the background RGB control */
 	LOG_INF("configuring the RGB background");
-	rgb_reg_set(rom->bus.bus, 0x00, 0x00);
+	rgb_reg_set(config->bus.bus, 0x00, 0x00);
-	rgb_reg_set(rom->bus.bus, 0x01, 0x05);
+	rgb_reg_set(config->bus.bus, 0x01, 0x05);
-	rgb_reg_set(rom->bus.bus, 0x08, 0xAA);
+	rgb_reg_set(config->bus.bus, 0x08, 0xAA);
 
 	/* Now set the background color to white */
 	LOG_DBG("background set to white");
-	rgb_reg_set(rom->bus.bus, REGISTER_R, color_define[GROVE_RGB_WHITE][0]);
+	rgb_reg_set(config->bus.bus, REGISTER_R, color_define[GROVE_RGB_WHITE][0]);
-	rgb_reg_set(rom->bus.bus, REGISTER_G, color_define[GROVE_RGB_WHITE][1]);
+	rgb_reg_set(config->bus.bus, REGISTER_G, color_define[GROVE_RGB_WHITE][1]);
-	rgb_reg_set(rom->bus.bus, REGISTER_B, color_define[GROVE_RGB_WHITE][2]);
+	rgb_reg_set(config->bus.bus, REGISTER_B, color_define[GROVE_RGB_WHITE][2]);
 
 	return 0;
 }
 
-static const struct glcd_driver grove_lcd_config = {
+static const struct glcd_config grove_lcd_config = {
 	.bus = I2C_DT_SPEC_INST_GET(0),
 };
 
-static struct glcd_data grove_lcd_driver = {
+static struct glcd_data grove_lcd_data;
-	.input_set = 0,
-	.display_switch = 0,
-	.function = 0,
-};
 
-DEVICE_DT_INST_DEFINE(0, glcd_initialize, NULL, &grove_lcd_driver,
+DEVICE_DT_INST_DEFINE(0, glcd_initialize, NULL, &grove_lcd_data,
 		      &grove_lcd_config, POST_KERNEL,
 		      CONFIG_KERNEL_INIT_PRIORITY_DEVICE, NULL);

include/drivers/misc/grove_lcd/grove_lcd.h

@@ -7,6 +7,10 @@
 #ifndef ZEPHYR_INCLUDE_DISPLAY_GROVE_LCD_H_
 #define ZEPHYR_INCLUDE_DISPLAY_GROVE_LCD_H_
 
+#include <stdint.h>
+
+#include <device.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -28,28 +32,28 @@ extern "C" {
 /**
  *  @brief Send text to the screen
  *
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param data the ASCII text to display
  *  @param size the length of the text in bytes
  */
-void glcd_print(const struct device *port, char *data, uint32_t size);
+void glcd_print(const struct device *dev, char *data, uint32_t size);
 
 
 /**
  *  @brief Set text cursor position for next additions
  *
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param col the column for the cursor to be moved to (0-15)
  *  @param row the row it should be moved to (0 or 1)
  */
-void glcd_cursor_pos_set(const struct device *port, uint8_t col, uint8_t row);
+void glcd_cursor_pos_set(const struct device *dev, uint8_t col, uint8_t row);
 
 /**
  *  @brief Clear the current display
  *
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  */
-void glcd_clear(const struct device *port);
+void glcd_clear(const struct device *dev);
 
 /* Defines for the GLCD_CMD_DISPLAY_SWITCH options */
 #define GLCD_DS_DISPLAY_ON		(1 << 2)
@@ -65,20 +69,20 @@ void glcd_clear(const struct device *port);
  *  the screen, the option to display the cursor or not, and the ability to
  *  blink the cursor.
  *
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param opt An 8bit bitmask of GLCD_DS_* options.
  *
  */
-void glcd_display_state_set(const struct device *port, uint8_t opt);
+void glcd_display_state_set(const struct device *dev, uint8_t opt);
 
 /**
  * @brief return the display feature set associated with the device
  *
- * @param port the Grove LCD to get the display features set
+ * @param dev the Grove LCD to get the display features set
  *
  * @return the display feature set associated with the device.
  */
-uint8_t glcd_display_state_get(const struct device *port);
+uint8_t glcd_display_state_get(const struct device *dev);
 
 /* Defines for the GLCD_CMD_INPUT_SET to change text direction */
 #define GLCD_IS_SHIFT_INCREMENT	(1 << 1)
@@ -91,20 +95,20 @@ uint8_t glcd_display_state_get(const struct device *port);
  *  of the text input. Controlling things like text entry from the left or
  *  right side, and how far to increment on new text
  *
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param opt A bitmask of GLCD_IS_* options
  *
  */
-void glcd_input_state_set(const struct device *port, uint8_t opt);
+void glcd_input_state_set(const struct device *dev, uint8_t opt);
 
 /**
  * @brief return the input set associated with the device
  *
- * @param port the Grove LCD to get the input features set
+ * @param dev the Grove LCD to get the input features set
  *
  * @return the input set associated with the device.
  */
-uint8_t glcd_input_state_get(const struct device *port);
+uint8_t glcd_input_state_get(const struct device *dev);
 
 /* Defines for the LCD_FUNCTION_SET */
 #define GLCD_FS_8BIT_MODE	(1 << 4)
@@ -116,23 +120,23 @@ uint8_t glcd_input_state_get(const struct device *port);
 
 /**
  *  @brief Function to set the functional state of the display.
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param opt A bitmask of GLCD_FS_* options
  *
  *  @details This function provides the user the ability to change the state
  *  of the display as per needed.  Controlling things like the number of rows,
  *  dot size, and text display quality.
  */
-void glcd_function_set(const struct device *port, uint8_t opt);
+void glcd_function_set(const struct device *dev, uint8_t opt);
 
 /**
  * @brief return the function set associated with the device
  *
- * @param port the Grove LCD to get the functions set
+ * @param dev the Grove LCD to get the functions set
  *
  * @return the function features set associated with the device.
  */
-uint8_t glcd_function_get(const struct device *port);
+uint8_t glcd_function_get(const struct device *dev);
 
 
 /* Available color selections */
@@ -142,34 +146,24 @@ uint8_t glcd_function_get(const struct device *port);
 #define GROVE_RGB_BLUE		3
 /**
  *  @brief Set LCD background to a predefined color
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param color One of the predefined color options
  */
-void glcd_color_select(const struct device *port, uint8_t color);
+void glcd_color_select(const struct device *dev, uint8_t color);
 
 
 /**
  *  @brief Set LCD background to custom RGB color value
  *
- *  @param port Pointer to device structure for driver instance.
+ *  @param dev Pointer to device structure for driver instance.
  *  @param r A numeric value for the red color (max is 255)
  *  @param g A numeric value for the green color (max is 255)
  *  @param b A numeric value for the blue color (max is 255)
  */
-void glcd_color_set(const struct device *port, uint8_t r, uint8_t g,
+void glcd_color_set(const struct device *dev, uint8_t r, uint8_t g,
 		    uint8_t b);
 
 
-/**
- *  @brief Initialize the Grove LCD panel
- *
- *  @param port Pointer to device structure for driver instance.
- *
- *  @return Returns 0 if all passes
- */
-int glcd_initialize(const struct device *port);
-
-
 /**
  * @}
  */