| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e1de10a6-49ad-c7f2-9246-5bee29f58c80@ti.com>
Date: Wed, 18 Sep 2019 20:07:04 -0500
From: Dan Murphy <dmurphy@...com>
To: Jacek Anaszewski <jacek.anaszewski@...il.com>, <pavel@....cz>
CC: <linux-leds@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 6/9] leds: multicolor: Introduce a multicolor class
definition
Jacek
On 9/18/19 4:27 PM, Jacek Anaszewski wrote:
> Hi Dan,
>
> I think Greg's guidance clarified everything nicely -
> we will avoid <color> sub-dirs in favour of prefixes
> to *intensity and *max_intensity.
Yes I will make the change accordingly. It will simplify the code.
>
> Before you will send an update I have some improvement
> ideas regarding the remnants after the previous approach,
> where single color intensity update resulted in updating
> hardware state. Now the update will happen only on write to
> brightness file, so we will not need color_set/color_get ops
> anymore.
I left those call backs in specifically for the LP50xx. Otherwise the
LEDs are only updated when the brightness file is written.
The LP50xx has an engine that performs the intensity computation for the
specific LED. So there is no call back to the MC FW for calculating the
intensity.
The brightness and intensity are written directly to the device and the
MCU in the device does all the computations so you have real time update.
For the LP55xx device the LEDs are only updated when the brightness file
is written.
I think we can leave those call backs in if device driver or product
development teams would like to use them.
Dan
> On 9/17/19 7:59 PM, Dan Murphy wrote:
>> Introduce a multicolor class that groups colored LEDs
>> within a LED node.
>>
>> The framework allows for dynamically setting individual LEDs
>> or setting brightness levels of LEDs and updating them virtually
>> simultaneously.
>>
>> Signed-off-by: Dan Murphy <dmurphy@...com>
>> ---
>>
>> v6 removed color_id and color_mix files, used sysfs_create_groups instead of
>> kobject call for LED color directory, kept kobject_create for the "colors" directory,
>> removed the calculate function, updated the export for the intensity calculations.
>>
>>
>> drivers/leds/Kconfig | 10 +
>> drivers/leds/Makefile | 1 +
>> drivers/leds/led-class-multicolor.c | 306 +++++++++++++++++++++++++++
>> include/linux/led-class-multicolor.h | 90 ++++++++
>> 4 files changed, 407 insertions(+)
>> create mode 100644 drivers/leds/led-class-multicolor.c
>> create mode 100644 include/linux/led-class-multicolor.h
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 1988de1d64c0..71e7fd4f6f15 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -30,6 +30,16 @@ config LEDS_CLASS_FLASH
>> for the flash related features of a LED device. It can be built
>> as a module.
>>
>> +config LEDS_CLASS_MULTI_COLOR
>> + tristate "LED Mulit Color LED Class Support"
>> + depends on LEDS_CLASS
>> + help
>> + This option enables the multicolor LED sysfs class in /sys/class/leds.
>> + It wraps LED class and adds multicolor LED specific sysfs attributes
>> + and kernel internal API to it. You'll need this to provide support
>> + for multicolor LEDs that are grouped together. This class is not
>> + intended for single color LEDs. It can be built as a module.
>> +
>> config LEDS_BRIGHTNESS_HW_CHANGED
>> bool "LED Class brightness_hw_changed attribute support"
>> depends on LEDS_CLASS
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index 41fb073a39c1..897b810257dd 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -4,6 +4,7 @@
>> obj-$(CONFIG_NEW_LEDS) += led-core.o
>> obj-$(CONFIG_LEDS_CLASS) += led-class.o
>> obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o
>> +obj-$(CONFIG_LEDS_CLASS_MULTI_COLOR) += led-class-multicolor.o
>> obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o
>>
>> # LED Platform Drivers
>> diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
>> new file mode 100644
>> index 000000000000..d43bd344ed4c
>> --- /dev/null
>> +++ b/drivers/leds/led-class-multicolor.c
>> @@ -0,0 +1,306 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// LED Multi Color class interface
>> +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>> +
>> +#include <linux/device.h>
>> +#include <linux/init.h>
>> +#include <linux/led-class-multicolor.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +
>> +#include "leds.h"
>> +
>> +struct led_mc_color_entry {
>> + struct led_classdev_mc *mcled_cdev;
>> +
>> + struct device_attribute max_intensity_attr;
>> + struct device_attribute intensity_attr;
>> +
>> + enum led_brightness max_intensity;
>> + enum led_brightness intensity;
>> +
>> + struct list_head list;
>> +
>> + int led_color_id;
>> +};
>> +
>> +void led_mc_calc_brightness(struct led_classdev_mc *mcled_cdev,
>> + enum led_brightness brightness,
>> + int brightness_val[])
>> +{
>> + struct led_classdev_mc_data *data = mcled_cdev->data;
>> + struct led_mc_color_entry *priv;
>> + int i = 0;
>> +
>> + list_for_each_entry(priv, &data->color_list, list) {
>> + brightness_val[i] = brightness *
>> + priv->intensity / priv->max_intensity;
>> + i++;
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(led_mc_calc_brightness);
>> +
>> +static ssize_t intensity_store(struct device *dev,
>> + struct device_attribute *intensity_attr,
>> + const char *buf, size_t size)
>> +{
>> + struct led_mc_color_entry *priv = container_of(intensity_attr,
>> + struct led_mc_color_entry,
>> + intensity_attr);
>> + struct led_classdev_mc_data *data = priv->mcled_cdev->data;
>> + struct led_classdev_mc *mcled_cdev = data->mcled_cdev;
>> + struct led_classdev *led_cdev = priv->mcled_cdev->led_cdev;
>> + unsigned long value;
>> + ssize_t ret;
>> +
>> + mutex_lock(&led_cdev->led_access);
>> +
>> + ret = kstrtoul(buf, 10, &value);
>> + if (ret)
>> + goto unlock;
>> +
>> + if (value > priv->max_intensity) {
>> + ret = -EINVAL;
>> + goto unlock;
>> + }
>> +
>> + priv->intensity = value;
>> +
>> + if (mcled_cdev->ops) {
>> + ret = mcled_cdev->ops->set_color_brightness(mcled_cdev,
>> + priv->led_color_id,
>> + priv->intensity);
> I don't think this is a good idea to update hw here now.
> As I proposed before - let's do the write only in brightness set.
> Otherwise any change of hue requiring alteration of more than one color
> component will be non-atomic w.r.t. hw state change. Just cache the
> intensity in the entry here.
>
> [...]
>
Powered by blists - more mailing lists