Marek
On 4/29/19 4: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.
>
I am not seeing where or how this is done in the driver on probe.
> 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;
Magic numbers
>
> 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);
> +
What happens if the LEDs are in HW triggered mode already?
Should this not be checked especially if the driver is removed and re-installed the uP has
this configured as HW mode. Unless you reset the uP as well.
> 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;
How about LED_FULL?
> 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;
> +
LED_FULL for this as above.
Dan
> + i2c_master_send(client, buf, 5);
> +
> return 0;
> }
>
>
