drivers: led: is31fl319x - redo color to channel

Removed the driver knowing anything about what
color each channel is.  When you output to
an LED with a buffer it simply writes the
channels out with buffer.

Also removed the assumptions that the leds
are either a) RGB  or b) 3 channels and
instead use them the way they are defined.

That is for example if we have 4 channels, we
could define an LED with just one color in it
and we could define another with three.

The user can define 4 leds each with color yellow.

For each LED it counts how many buffers were used
by previous LEDs and starts there and outputs N
channels.

Decided it looked cleaner to use helper function
write channels, so implemented it and added it
to the call table.

Updated example sketch:
It manually computes which channel maps to red, green and blue and then
uses those indexes to map the table of colors to the right channels.
Signed-off-by: Kurt Eckhardt <[email protected]>

sample changes

Author: KurtE

drivers/led/is31fl319x.c

@@ -104,45 +104,35 @@ static int is31fl319x_get_info(const struct device *dev,
 	return 0;
 }
 
-static int is31fl319x_set_color(const struct device *dev, uint32_t led, uint8_t num_colors,
-				const uint8_t *color)
+static uint8_t is31fl319x_map_led_to_start_channel(const struct is31fl319x_config *config,
+						   uint32_t led)
+{
+	/* It is assumed that led has been validated before calling this */
+	const struct led_info *info = config->led_infos;
+	uint8_t channel = 0;
+
+	while (led) {
+		channel += info->num_colors;
+		led--;
+		info++;
+	}
+	return channel;
+}
+
+static int is31fl319x_write_channels(const struct device *dev, uint32_t start_channel,
+				     uint32_t num_channels, const uint8_t *buf)
 {
 	const struct is31fl319x_config *config = dev->config;
-	const struct led_info *info = is31fl319x_led_to_info(config, led);
 	const struct is31f1319x_model *regs = config->regs;
 	int ret;
 
-	if (info == NULL) {
-		return -ENODEV;
-	}
-
-	if (info->num_colors > config->channel_count) {
+	if ((start_channel + num_channels) > config->channel_count) {
 		return -ENOTSUP;
 	}
 
-	if (num_colors > config->channel_count) {
-		return -ENOTSUP;
-	}
-
-	for (int i = 0; i < num_colors; i++) {
-		uint8_t value;
-
-		switch (info->color_mapping[i]) {
-		case LED_COLOR_ID_RED:
-			value = color[0];
-			break;
-		case LED_COLOR_ID_GREEN:
-			value = color[1];
-			break;
-		case LED_COLOR_ID_BLUE:
-			value = color[2];
-			break;
-		default:
-			/* unreachable: mapping already tested in is31fl319x_check_config */
-			return -EINVAL;
-		}
-
-		ret = i2c_reg_write_byte_dt(&config->bus, regs->led_channels[i], value);
+	for (int i = 0; i < num_channels; i++) {
+		ret = i2c_reg_write_byte_dt(&config->bus, regs->led_channels[i + start_channel],
+					    buf[i]);
 		if (ret != 0) {
 			break;
 		}
@@ -161,6 +151,25 @@ static int is31fl319x_set_color(const struct device *dev, uint32_t led, uint8_t
 	return ret;
 }
 
+static int is31fl319x_set_color(const struct device *dev, uint32_t led, uint8_t num_colors,
+				const uint8_t *color)
+{
+	const struct is31fl319x_config *config = dev->config;
+	const struct led_info *info = is31fl319x_led_to_info(config, led);
+
+	if (info == NULL) {
+		return -ENODEV;
+	}
+
+	uint8_t channel_start = is31fl319x_map_led_to_start_channel(config, led);
+
+	if (info->num_colors > config->channel_count) {
+		return -ENOTSUP;
+	}
+
+	return is31fl319x_write_channels(dev, channel_start, num_colors, color);
+}
+
 static int is31fl319x_set_brightness(const struct device *dev, uint32_t led, uint8_t value)
 {
 	const struct is31fl319x_config *config = dev->config;
@@ -180,7 +189,9 @@ static int is31fl319x_set_brightness(const struct device *dev, uint32_t led, uin
 	/* Rescale 0..100 to 0..255 */
 	value = value * 255 / LED_BRIGHTNESS_MAX;
 
-	ret = i2c_reg_write_byte_dt(&config->bus, regs->led_channels[led], value);
+	uint8_t channel_start = is31fl319x_map_led_to_start_channel(config, led);
+
+	ret = i2c_reg_write_byte_dt(&config->bus, regs->led_channels[channel_start], value);
 	if (ret == 0) {
 		ret = i2c_reg_write_byte_dt(&config->bus,
 					    regs->update_reg,
@@ -194,81 +205,20 @@ static int is31fl319x_set_brightness(const struct device *dev, uint32_t led, uin
 	return ret;
 }
 
-/*
- * Counts red, green, blue channels; returns true if color_id is valid
- * and no more than one channel maps to the same color
- */
-static bool is31fl319x_count_colors(const struct device *dev,
-				    uint8_t color_id, uint8_t *rgb_counts)
-{
-	bool ret = false;
-
-	switch (color_id) {
-	case LED_COLOR_ID_RED:
-		ret = (++rgb_counts[0] == 1);
-		break;
-	case LED_COLOR_ID_GREEN:
-		ret = (++rgb_counts[1] == 1);
-		break;
-	case LED_COLOR_ID_BLUE:
-		ret = (++rgb_counts[2] == 1);
-		break;
-	}
-
-	if (!ret) {
-		LOG_ERR("%s: invalid color %d (duplicate or not RGB)",
-			dev->name, color_id);
-	}
-
-	return ret;
-}
-
 static int is31fl319x_check_config(const struct device *dev)
 {
 	const struct is31fl319x_config *config = dev->config;
 	const struct led_info *info;
-	uint8_t rgb_counts[3] = { 0 };
+	uint8_t rgb_count = 0;
 	uint8_t i;
 
-	switch (config->num_leds) {
-	case 1:
-		/* check that it is a three-channel LED */
-		info = &config->led_infos[0];
-
-		if (info->num_colors != 3) {
-			LOG_ERR("%s: invalid number of colors %d "
-				"(must be 3 for RGB LED)",
-				dev->name, info->num_colors);
-			return -EINVAL;
-		}
-
-		for (i = 0; i < 3; i++) {
-			if (!is31fl319x_count_colors(dev, info->color_mapping[i], rgb_counts)) {
-				return -EINVAL;
-			}
-
-		}
-		break;
-	case 3:
-		/* check that each LED is single-color */
-		for (i = 0; i < 3; i++) {
-			info = &config->led_infos[i];
-
-			if (info->num_colors != 1) {
-				LOG_ERR("%s: invalid number of colors %d "
-					"(must be 1 when defining multiple LEDs)",
-					dev->name, info->num_colors);
-				return -EINVAL;
-			}
+	/* verify that number of leds defined is not > number of channels */
+	for (i = 0; i < config->num_leds; i++) {
+		info = &config->led_infos[i];
+		rgb_count += info->num_colors;
+	}
 
-			if (!is31fl319x_count_colors(dev, info->color_mapping[0], rgb_counts)) {
-				return -EINVAL;
-			}
-		}
-		break;
-	default:
-		LOG_ERR("%s: invalid number of LEDs %d (must be 1 or 3)",
-			dev->name, config->num_leds);
+	if (rgb_count > config->channel_count) {
 		return -EINVAL;
 	}
 
@@ -280,8 +230,8 @@ static int is31fl319x_init(const struct device *dev)
 	const struct is31fl319x_config *config = dev->config;
 	const struct led_info *info = NULL;
 	const struct is31f1319x_model *regs = config->regs;
-	int i, ret;
-	uint8_t prod_id, band;
+	int i, j, ret;
+	uint8_t prod_id, band, channel;
 	uint8_t current_reg = 0;
 
 	ret = is31fl319x_check_config(dev);
@@ -307,27 +257,24 @@ static int is31fl319x_init(const struct device *dev)
 	}
 
 	/* calc current limit register value */
-	info = &config->led_infos[0];
-	if (info->num_colors == IS31FL3194_CHANNEL_COUNT) {
-		/* one RGB LED: set all channels to the same current limit */
-		band = (config->current_limits[0] / 10) - 1;
-		for (i = 0; i < IS31FL3194_CHANNEL_COUNT; i++) {
-			current_reg |= band << (2 * i);
-		}
-	} else {
-		/* single-channel LEDs: independent limits */
+	if (regs->current_reg != REG_NOT_DEFINED) {
+		channel = 0;
 		for (i = 0; i < config->num_leds; i++) {
+			info = &config->led_infos[i];
 			band = (config->current_limits[i] / 10) - 1;
-			current_reg |= band << (2 * i);
+
+			for (j = 0; j < info->num_colors; j++) {
+				current_reg |= band << (2 * channel);
+				channel++;
+			}
 		}
-	}
 
-	ret = i2c_reg_write_byte_dt(&config->bus, IS31FL3194_CURRENT_REG, current_reg);
-	if (ret != 0) {
-		LOG_ERR("%s: failed to set current limit", dev->name);
-		return ret;
+		ret = i2c_reg_write_byte_dt(&config->bus, regs->current_reg, current_reg);
+		if (ret != 0) {
+			LOG_ERR("%s: failed to set current limit", dev->name);
+			return ret;
+		}
 	}
-
 	/* enable device */
 	return i2c_reg_write_byte_dt(&config->bus, regs->conf_reg,
 				     regs->conf_enable);
@@ -337,6 +284,7 @@ static DEVICE_API(led, is31fl319x_led_api) = {
 	.set_brightness = is31fl319x_set_brightness,
 	.get_info = is31fl319x_get_info,
 	.set_color = is31fl319x_set_color,
+	.write_channels = is31fl319x_write_channels,
 };
 
 #define COLOR_MAPPING(led_node_id)						\

samples/drivers/led/is31fl319x/README.rst

@@ -2,12 +2,23 @@
    :name: IS31FL319x RGB LED
    :relevant-api: led_interface
 
-   Cycle colors on an RGB LED connected to the IS31FL3194 using the LED API.
+   Cycle colors on an RGB LED connected to the IS31FL3194 or IS31FL3197
+   using the LED API.
 
 Overview
 ********
 
-This sample cycles several colors on an RGB LED forever using the LED API.
+This sample first looks through the color table to map which channels
+map to Red, Green and Blue.
+
+It then shows the mapping, but setting one color on and fading it
+off.  First Red, then Green and finally Blue.
+
+It then forever cycles through several colors on an RGB LED.  It uses a
+helper function, that maps the RGB colors into the correct order
+for the actual hardware LED, as some hardware LEDs will be defined in
+RGB order whereas others may be in another order such as BGR.
+Once mapped it calls off to the led library to display the color.
 
 Building and Running
 ********************
@@ -18,11 +29,18 @@ any board where the devicetree has an I2C device node with compatible
 controller node also being enabled.
 
 .. zephyr-app-commands::
-   :zephyr-app: samples/drivers/led/is31fl3194
+   :zephyr-app: samples/drivers/led/is31fl319x
    :board: arduino_nicla_sense_me
    :goals: build flash
    :compact:
 
+.. zephyr-app-commands::
+   :zephyr-app: samples/drivers/led/is31fl319x
+   :board: arduino_giga_r1//m7
+   :shield: arduino_giga_display_shield
+   :goals: build flash
+   :compact:
+
 After flashing, the LED starts to switch colors and messages with the current
 LED color are printed on the console. If a runtime error occurs, the sample
 exits without printing to the console.