Re: [PATCH leds/for-next v2 2/2] leds: turris-omnia: Add support for 256 brightness values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marek, Pavel,

On 4/29/19 11:29 PM, Marek Behún wrote:
The controller supports setting brightness of each channel of the RGB
LEDs to values 0-255. We do not support RGB LED class yet, but we can
use this to be able to have 256 brightness levels for each LED, instead
of just on/off state.

Also add code to set all LEDs color to [255, 255, 255] on driver unbind.

Signed-off-by: Marek Behún <marek.behun@xxxxxx>
---
  drivers/leds/Kconfig             |  4 ++--
  drivers/leds/leds-turris-omnia.c | 21 +++++++++++++++++++--
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 3747cbd0de2c..62d17c2f4878 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -139,8 +139,8 @@ config LEDS_TURRIS_OMNIA
  	  side of CZ.NIC's Turris Omnia router. There are 12 RGB LEDs on the
  	  front panel.
  	  This driver does not currently support setting LED colors, only
-	  on/off state. Also HW triggering is disabled when the controller
-	  is probed by this driver.
+	  brightness. Also HW triggering is disabled when the controller is
+	  probed by this driver.
config LEDS_LM3530
  	tristate "LCD Backlight driver for LM3530"
diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
index dc9fac56b13a..0e805d94f298 100644
--- a/drivers/leds/leds-turris-omnia.c
+++ b/drivers/leds/leds-turris-omnia.c
@@ -54,7 +54,7 @@ static int omnia_led_brightness_set_blocking(struct led_classdev *led,
  	struct omnia_leds *leds = dev_get_drvdata(led->dev->parent);
  	int idx = omnia_led_idx(leds, led);
  	int ret;
-	u8 state;
+	u8 buf[5], state;
if (idx < 0)
  		return idx;
@@ -63,8 +63,16 @@ static int omnia_led_brightness_set_blocking(struct led_classdev *led,
  	if (brightness)
  		state |= CMD_LED_STATE_ON;
+ buf[0] = CMD_LED_COLOR;
+	buf[1] = idx;
+	buf[2] = buf[3] = buf[4] = brightness;
+
  	mutex_lock(&leds->lock);
+
  	ret = i2c_smbus_write_byte_data(leds->client, CMD_LED_STATE, state);
+	if (ret >= 0 && brightness)
+		ret = i2c_master_send(leds->client, buf, 5);
+
  	mutex_unlock(&leds->lock);
return ret;
@@ -97,7 +105,7 @@ static int omnia_led_register(struct omnia_leds *leds,
  	ret = fwnode_property_read_string(node, "label", &str);
  	snprintf(led->name, sizeof(led->name), "omnia::%s", ret ? "" : str);
- led->cdev.max_brightness = 1;
+	led->cdev.max_brightness = 255;
  	led->cdev.brightness_set_blocking = omnia_led_brightness_set_blocking;
  	led->cdev.name = led->name;
@@ -166,10 +174,19 @@ static int omnia_leds_probe(struct i2c_client *client, static int omnia_leds_remove(struct i2c_client *client)
  {
+	u8 buf[5];
+
  	/* put all LEDs into default (HW triggered) mode */
  	i2c_smbus_write_byte_data(client, CMD_LED_MODE,
  				  CMD_LED_MODE_LED(OMNIA_BOARD_LEDS));
+ /* set all LEDs color to [255, 255, 255] */
+	buf[0] = CMD_LED_COLOR;
+	buf[1] = OMNIA_BOARD_LEDS;
+	buf[2] = buf[3] = buf[4] = 255;
+
+	i2c_master_send(client, buf, 5);
+
  	return 0;
  }

I wonder if we're doing right merging this driver in this form.
We break the rule one-led-class-device-per-one-channel.
We don't have LED multi color support yet, so this should support
RGB LEDs in the old manner. Or switch to using LED multi color class.

Once we will have LED multi color class, we will be able to add
the support for it to the driver and make the driver configurable
to be able to expose old interface or the LED multi color one.

Moreover, the bindings should use led-sources property for grouping
three channels under single LED class device. This is certainly
to be fixed.

--
Best regards,
Jacek Anaszewski



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux