Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Hermes Zhang <Hermes.Zhang@axis.com>
To:     Marek Behun <marek.behun@nic.cz>,
        Hermes Zhang <Hermes.Zhang@axis.com>
CC:     "pavel@ucw.cz" <pavel@ucw.cz>, "dmurphy@ti.com" <dmurphy@ti.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "lkml@axis.com" <lkml@axis.com>, kernel <kernel@axis.com>
Subject: Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver
Thread-Topic: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver
Thread-Index: AQHXIINVpz7s49aciUujD1l9rQIjiw==
Date:   Thu, 25 Mar 2021 06:04:43 +0000
Message-ID: <7ea3f7e6ea7a464fa4bb59b94857a755@XBOX01.axis.com>
References: <20210324075631.5004-1-chenhui.zhang@axis.com>
 <20210324075631.5004-2-chenhui.zhang@axis.com>
 <20210324103431.4b945915@thinkpad>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk

Hi Marek,=0A=
=0A=
On 3/24/21 5:34 PM, Marek Behun wrote:=0A=
>> +#include <linux/err.h>=0A=
>> +#include <linux/gpio.h>=0A=
>> +#include <linux/gpio/consumer.h>=0A=
>> +#include <linux/kernel.h>=0A=
>> +#include <linux/leds.h>=0A=
>> +#include <linux/module.h>=0A=
>> +#include <linux/of.h>=0A=
>> +#include <linux/of_gpio.h>=0A=
>> +#include <linux/platform_device.h>=0A=
>> +#include <linux/property.h>=0A=
>> +#include <linux/slab.h>=0A=
> Why do you include slab.h?=0A=
Yeah, I will clean it in next commit.=0A=
>> +=0A=
>> +=0A=
>> +static void multi_gpio_led_set(struct led_classdev *led_cdev,=0A=
>> +	enum led_brightness value)=0A=
>> +{=0A=
>> +	struct multi_gpio_led_priv *priv;=0A=
>> +	int idx;=0A=
>> +=0A=
>> +	DECLARE_BITMAP(values, MAX_GPIO_NUM);=0A=
>> +=0A=
>> +	priv =3D container_of(led_cdev, struct multi_gpio_led_priv, cdev);=0A=
>> +=0A=
>> +	idx =3D (value - LED_OFF) * priv->nr_states / (LED_FULL + 1);=0A=
> LED_FULL / LED_OFF are deprecated, don't use them.=0A=
=0A=
Then could I use just 0 (instead LED_OFF) and led_cdev->max_brightness=0A=
=0A=
(instead of LED_FULL) here? The idea here is map the states defined in dts=
=0A=
=0A=
to the full brightness range.=0A=
=0A=
> +=0A=
> +	priv->nr_states =3D ret;=0A=
> +	priv->states =3D devm_kzalloc(dev, sizeof(*priv->states) * priv->nr_sta=
tes, GFP_KERNEL);=0A=
> +	if (!priv->states)=0A=
> +		return -ENOMEM;=0A=
> +=0A=
> +	ret =3D of_property_read_u8_array(node, "led-states", priv->states, pri=
v->nr_states);=0A=
> +	if (ret)=0A=
> +		return ret;=0A=
> +=0A=
> +	priv->cdev.max_brightness =3D LED_FULL;=0A=
> ???? max_brightness is not 255 (=3D LED_FULL). max_brightness must be=0A=
> derived from the led-states property.=0A=
=0A=
Yeah, I will fix this. the max-brightness should for the whole LED,=0A=
right? So=0A=
=0A=
it will at same level with led-states.=0A=
=0A=
=0A=
=0A=