linux-kernel - Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ea3f7e6ea7a464fa4bb59b94857a755@XBOX01.axis.com>
Date:   Thu, 25 Mar 2021 06:04:43 +0000
From:   Hermes Zhang <Hermes.Zhang@...s.com>
To:     Marek Behun <marek.behun@....cz>,
        Hermes Zhang <Hermes.Zhang@...s.com>
CC:     "pavel@....cz" <pavel@....cz>, "dmurphy@...com" <dmurphy@...com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "lkml@...s.com" <lkml@...s.com>, kernel <kernel@...s.com>
Subject: Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

Hi Marek,

On 3/24/21 5:34 PM, Marek Behun wrote:
>> +#include <linux/err.h>
>> +#include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/kernel.h>
>> +#include <linux/leds.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/property.h>
>> +#include <linux/slab.h>
> Why do you include slab.h?
Yeah, I will clean it in next commit.
>> +
>> +
>> +static void multi_gpio_led_set(struct led_classdev *led_cdev,
>> +	enum led_brightness value)
>> +{
>> +	struct multi_gpio_led_priv *priv;
>> +	int idx;
>> +
>> +	DECLARE_BITMAP(values, MAX_GPIO_NUM);
>> +
>> +	priv = container_of(led_cdev, struct multi_gpio_led_priv, cdev);
>> +
>> +	idx = (value - LED_OFF) * priv->nr_states / (LED_FULL + 1);
> LED_FULL / LED_OFF are deprecated, don't use them.

Then could I use just 0 (instead LED_OFF) and led_cdev->max_brightness

(instead of LED_FULL) here? The idea here is map the states defined in dts

to the full brightness range.

> +
> +	priv->nr_states = ret;
> +	priv->states = devm_kzalloc(dev, sizeof(*priv->states) * priv->nr_states, GFP_KERNEL);
> +	if (!priv->states)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u8_array(node, "led-states", priv->states, priv->nr_states);
> +	if (ret)
> +		return ret;
> +
> +	priv->cdev.max_brightness = LED_FULL;
> ???? max_brightness is not 255 (= LED_FULL). max_brightness must be
> derived from the led-states property.

Yeah, I will fix this. the max-brightness should for the whole LED,
right? So

it will at same level with led-states.