Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
Hi Florian and Pavel,
On 06/27/2016 07:46 AM, Florian Vaussard wrote:
Hi Pavel,
Le 26. 06. 16 à 23:49, Pavel Machek a écrit :
Hi!
+struct ncp5623_led {
+ bool active;
+ unsigned int led_no;
+ struct led_classdev ldev;
+ struct work_struct work;
+ struct ncp5623_priv *priv;
+};
+
+struct ncp5623_priv {
+ struct ncp5623_led leds[NCP5623_MAX_LEDS];
Please allocate memory dynamically, depending on the number
of LEDs defined in a Device Tree.
MAX_LEDs is three. Are you sure overhead of dynamic allocation is
worth it?
And if this is for RGB leds... very probably device will want to use
all 3 channels.
I was about to raise the same question during the v2 of this patch. In addition
to your arguments, this also changes the way this array is indexed.
Currently the LED number is used as index, but with dynamic allocation I have to
use an abstract index. This makes some logic a bit harder, especially to check
if the same LED is declared twice in the device tree (duplicated 'reg'
property).
Fair enough. Please ignore my remark then.
--
Best regards,
Jacek Anaszewski
Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
Hi Pavel,
Le 26. 06. 16 à 23:49, Pavel Machek a écrit :
> Hi!
>
>>> +struct ncp5623_led {
>>> + bool active;
>>> + unsigned int led_no;
>>> + struct led_classdev ldev;
>>> + struct work_struct work;
>>> + struct ncp5623_priv *priv;
>>> +};
>>> +
>>> +struct ncp5623_priv {
>>> + struct ncp5623_led leds[NCP5623_MAX_LEDS];
>>
>> Please allocate memory dynamically, depending on the number
>> of LEDs defined in a Device Tree.
>
> MAX_LEDs is three. Are you sure overhead of dynamic allocation is
> worth it?
>
> And if this is for RGB leds... very probably device will want to use
> all 3 channels.
>
I was about to raise the same question during the v2 of this patch. In addition
to your arguments, this also changes the way this array is indexed.
Currently the LED number is used as index, but with dynamic allocation I have to
use an abstract index. This makes some logic a bit harder, especially to check
if the same LED is declared twice in the device tree (duplicated 'reg'
property).
Best,
Florian
Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
Hi!
> >+struct ncp5623_led {
> >+bool active;
> >+unsigned int led_no;
> >+struct led_classdev ldev;
> >+struct work_struct work;
> >+struct ncp5623_priv *priv;
> >+};
> >+
> >+struct ncp5623_priv {
> >+struct ncp5623_led leds[NCP5623_MAX_LEDS];
>
> Please allocate memory dynamically, depending on the number
> of LEDs defined in a Device Tree.
MAX_LEDs is three. Are you sure overhead of dynamic allocation is
worth it?
And if this is for RGB leds... very probably device will want to use
all 3 channels.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
Hi Florian,
On 06/22/2016 08:08 AM, Florian Vaussard wrote:
Hi Jacek,
Le 21. 06. 16 à 17:29, Jacek Anaszewski a écrit :
Hi Florian,
Thanks for the patch. Please refer to my comments in the code.
Thanks for your review. I completely agree with you, I will address your
comments in V2. I just have one small question below.
On 06/21/2016 09:29 AM, Florian Vaussard wrote:
The NCP5623 is a 3-channel LED driver from On Semiconductor controlled
through I2C. The PWM of each channel can be independently set with 32
distinct levels. In addition, the intensity of the current source can be
globally set using an external bias resistor fixing the reference
current (Iref) and a dedicated register (ILED), following the
relationship:
I = 2400*Iref/(31-ILED)
with Iref = Vref/Rbias, and Vref = 0.6V.
Signed-off-by: Florian Vaussard
---
drivers/leds/Kconfig| 11 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-ncp5623.c | 265
3 files changed, 277 insertions(+)
create mode 100644 drivers/leds/leds-ncp5623.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 5ae2834..6d3e44d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -588,6 +588,17 @@ config LEDS_BLINKM
This option enables support for the BlinkM RGB LED connected
through I2C. Say Y to enable support for the BlinkM LED.
+config LEDS_NCP5623
+tristate "LED Support for NCP5623 I2C chip"
+depends on LEDS_CLASS
+depends on I2C
+help
+ This option enables support for LEDs connected to NCP5623
+ LED driver chips accessed via the I2C bus.
+ Driver supports brightness control.
+
+ Say Y to enable this driver.
+
config LEDS_POWERNV
tristate "LED support for PowerNV Platform"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cb2013d..a2f0e10 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692)+= leds-ktd2692.o
obj-$(CONFIG_LEDS_POWERNV)+= leds-powernv.o
obj-$(CONFIG_LEDS_SEAD3)+= leds-sead3.o
obj-$(CONFIG_LEDS_IS31FL32XX)+= leds-is31fl32xx.o
+obj-$(CONFIG_LEDS_NCP5623)+= leds-ncp5623.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085)+= leds-dac124s085.o
diff --git a/drivers/leds/leds-ncp5623.c b/drivers/leds/leds-ncp5623.c
new file mode 100644
index 000..a341e4a
--- /dev/null
+++ b/drivers/leds/leds-ncp5623.c
@@ -0,0 +1,265 @@
+/*
+ * Copyright 2016 Florian Vaussard
+ *
+ * Based on leds-tlc591xx.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NCP5623_MAX_LEDS3
+#define NCP5623_MAX_STEPS32
+#define NCP5623_MAX_CURRENT31
+#define NCP5623_MAX_CURRENT_UA3
+
+#define NCP5623_CMD_SHIFT5
+#define CMD_SHUTDOWN(0x00 << NCP5623_CMD_SHIFT)
+#define CMD_ILED(0x01 << NCP5623_CMD_SHIFT)
+#define CMD_PWM1(0x02 << NCP5623_CMD_SHIFT)
+#define CMD_PWM2(0x03 << NCP5623_CMD_SHIFT)
+#define CMD_PWM3(0x04 << NCP5623_CMD_SHIFT)
+#define CMD_UPWARD_DIM(0x05 << NCP5623_CMD_SHIFT)
+#define CMD_DOWNWARD_DIM(0x06 << NCP5623_CMD_SHIFT)
+#define CMD_DIM_STEP(0x07 << NCP5623_CMD_SHIFT)
+
+#define NCP5623_DATA_MASKGENMASK(NCP5623_CMD_SHIFT - 1, 0)
+
+#define NCP5623_CMD(cmd, data)(cmd | (data & NCP5623_DATA_MASK))
+
[snip]
+
+static int ncp5623_configure(struct device *dev,
+ struct ncp5623_priv *priv)
+{
+unsigned int i;
+unsigned int n;
+struct ncp5623_led *led;
+int err;
+
+/* Compute the value of ILED register to honor led_max_current */
+n = 2400 * priv->led_iref / priv->led_max_current + 1;
+if (n > NCP5623_MAX_CURRENT)
+n = NCP5623_MAX_CURRENT;
+n = NCP5623_MAX_CURRENT - n;
+
+dev_dbg(dev, "setting maximum current to %u uA\n",
+2400 * priv->led_iref / (NCP5623_MAX_CURRENT - n));
+
+err = ncp5623_send_cmd(priv, CMD_ILED, n);
+if (err < 0) {
+dev_err(dev, "cannot set the current\n");
+return err;
+}
+
+/* Setup each individual LED */
+for (i = 0; i < NCP5623_MAX_LEDS; i++) {
+led = &priv->leds[i];
+
+if (!led->active)
+continue;
+
+led->priv = priv;
+led->led_no = i;
+led->ldev.brightness_set = ncp5623_brightness_set;
+led->ldev.max_brightness = NCP5623_MAX_STEPS - 1;
max_brightness should be asserted according to led-max-microamp
property.
We can set two parameters on this chip:
- The intensity of the current source (common to all LEDs) from 0 to
NCP5623_MAX_CURRENT (31)
- The PWM for each LED independently from 0 to NCP5623_M
Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
Hi Jacek,
Le 21. 06. 16 à 17:29, Jacek Anaszewski a écrit :
> Hi Florian,
>
> Thanks for the patch. Please refer to my comments in the code.
>
Thanks for your review. I completely agree with you, I will address your
comments in V2. I just have one small question below.
> On 06/21/2016 09:29 AM, Florian Vaussard wrote:
>> The NCP5623 is a 3-channel LED driver from On Semiconductor controlled
>> through I2C. The PWM of each channel can be independently set with 32
>> distinct levels. In addition, the intensity of the current source can be
>> globally set using an external bias resistor fixing the reference
>> current (Iref) and a dedicated register (ILED), following the
>> relationship:
>>
>> I = 2400*Iref/(31-ILED)
>>
>> with Iref = Vref/Rbias, and Vref = 0.6V.
>>
>> Signed-off-by: Florian Vaussard
>> ---
>> drivers/leds/Kconfig| 11 ++
>> drivers/leds/Makefile | 1 +
>> drivers/leds/leds-ncp5623.c | 265
>>
>> 3 files changed, 277 insertions(+)
>> create mode 100644 drivers/leds/leds-ncp5623.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 5ae2834..6d3e44d 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -588,6 +588,17 @@ config LEDS_BLINKM
>> This option enables support for the BlinkM RGB LED connected
>> through I2C. Say Y to enable support for the BlinkM LED.
>>
>> +config LEDS_NCP5623
>> +tristate "LED Support for NCP5623 I2C chip"
>> +depends on LEDS_CLASS
>> +depends on I2C
>> +help
>> + This option enables support for LEDs connected to NCP5623
>> + LED driver chips accessed via the I2C bus.
>> + Driver supports brightness control.
>> +
>> + Say Y to enable this driver.
>> +
>> config LEDS_POWERNV
>> tristate "LED support for PowerNV Platform"
>> depends on LEDS_CLASS
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index cb2013d..a2f0e10 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692)+= leds-ktd2692.o
>> obj-$(CONFIG_LEDS_POWERNV)+= leds-powernv.o
>> obj-$(CONFIG_LEDS_SEAD3)+= leds-sead3.o
>> obj-$(CONFIG_LEDS_IS31FL32XX)+= leds-is31fl32xx.o
>> +obj-$(CONFIG_LEDS_NCP5623)+= leds-ncp5623.o
>>
>> # LED SPI Drivers
>> obj-$(CONFIG_LEDS_DAC124S085)+= leds-dac124s085.o
>> diff --git a/drivers/leds/leds-ncp5623.c b/drivers/leds/leds-ncp5623.c
>> new file mode 100644
>> index 000..a341e4a
>> --- /dev/null
>> +++ b/drivers/leds/leds-ncp5623.c
>> @@ -0,0 +1,265 @@
>> +/*
>> + * Copyright 2016 Florian Vaussard
>> + *
>> + * Based on leds-tlc591xx.c
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; version 2 of the License.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define NCP5623_MAX_LEDS3
>> +#define NCP5623_MAX_STEPS32
>> +#define NCP5623_MAX_CURRENT31
>> +#define NCP5623_MAX_CURRENT_UA3
>> +
>> +#define NCP5623_CMD_SHIFT5
>> +#define CMD_SHUTDOWN(0x00 << NCP5623_CMD_SHIFT)
>> +#define CMD_ILED(0x01 << NCP5623_CMD_SHIFT)
>> +#define CMD_PWM1(0x02 << NCP5623_CMD_SHIFT)
>> +#define CMD_PWM2(0x03 << NCP5623_CMD_SHIFT)
>> +#define CMD_PWM3(0x04 << NCP5623_CMD_SHIFT)
>> +#define CMD_UPWARD_DIM(0x05 << NCP5623_CMD_SHIFT)
>> +#define CMD_DOWNWARD_DIM(0x06 << NCP5623_CMD_SHIFT)
>> +#define CMD_DIM_STEP(0x07 << NCP5623_CMD_SHIFT)
>> +
>> +#define NCP5623_DATA_MASKGENMASK(NCP5623_CMD_SHIFT - 1, 0)
>> +
>> +#define NCP5623_CMD(cmd, data)(cmd | (data & NCP5623_DATA_MASK))
>> +
[snip]
>> +
>> +static int ncp5623_configure(struct device *dev,
>> + struct ncp5623_priv *priv)
>> +{
>> +unsigned int i;
>> +unsigned int n;
>> +struct ncp5623_led *led;
>> +int err;
>> +
>> +/* Compute the value of ILED register to honor led_max_current */
>> +n = 2400 * priv->led_iref / priv->led_max_current + 1;
>> +if (n > NCP5623_MAX_CURRENT)
>> +n = NCP5623_MAX_CURRENT;
>> +n = NCP5623_MAX_CURRENT - n;
>> +
>> +dev_dbg(dev, "setting maximum current to %u uA\n",
>> +2400 * priv->led_iref / (NCP5623_MAX_CURRENT - n));
>> +
>> +err = ncp5623_send_cmd(priv, CMD_ILED, n);
>> +if (err < 0) {
>> +dev_err(dev, "cannot set the current\n");
>> +return err;
>> +}
>> +
>> +/* Setup each individual LED */
>> +for (i = 0; i < NCP5623_MAX_LEDS; i++) {
>> +led = &priv->leds[i];
>> +
>> +if (!led->active)
>> +continue;
>> +
>> +led->priv = priv;
>> +led->led_no = i;
>> +led->ldev.brightness_set = ncp5623_brightness_set;
>> +
Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
Hi Florian,
Thanks for the patch. Please refer to my comments in the code.
On 06/21/2016 09:29 AM, Florian Vaussard wrote:
The NCP5623 is a 3-channel LED driver from On Semiconductor controlled
through I2C. The PWM of each channel can be independently set with 32
distinct levels. In addition, the intensity of the current source can be
globally set using an external bias resistor fixing the reference
current (Iref) and a dedicated register (ILED), following the
relationship:
I = 2400*Iref/(31-ILED)
with Iref = Vref/Rbias, and Vref = 0.6V.
Signed-off-by: Florian Vaussard
---
drivers/leds/Kconfig| 11 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-ncp5623.c | 265
3 files changed, 277 insertions(+)
create mode 100644 drivers/leds/leds-ncp5623.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 5ae2834..6d3e44d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -588,6 +588,17 @@ config LEDS_BLINKM
This option enables support for the BlinkM RGB LED connected
through I2C. Say Y to enable support for the BlinkM LED.
+config LEDS_NCP5623
+ tristate "LED Support for NCP5623 I2C chip"
+ depends on LEDS_CLASS
+ depends on I2C
+ help
+ This option enables support for LEDs connected to NCP5623
+ LED driver chips accessed via the I2C bus.
+ Driver supports brightness control.
+
+ Say Y to enable this driver.
+
config LEDS_POWERNV
tristate "LED support for PowerNV Platform"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cb2013d..a2f0e10 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692)+= leds-ktd2692.o
obj-$(CONFIG_LEDS_POWERNV)+= leds-powernv.o
obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
+obj-$(CONFIG_LEDS_NCP5623) += leds-ncp5623.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-ncp5623.c b/drivers/leds/leds-ncp5623.c
new file mode 100644
index 000..a341e4a
--- /dev/null
+++ b/drivers/leds/leds-ncp5623.c
@@ -0,0 +1,265 @@
+/*
+ * Copyright 2016 Florian Vaussard
+ *
+ * Based on leds-tlc591xx.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NCP5623_MAX_LEDS 3
+#define NCP5623_MAX_STEPS 32
+#define NCP5623_MAX_CURRENT31
+#define NCP5623_MAX_CURRENT_UA 3
+
+#define NCP5623_CMD_SHIFT 5
+#define CMD_SHUTDOWN (0x00 << NCP5623_CMD_SHIFT)
+#define CMD_ILED (0x01 << NCP5623_CMD_SHIFT)
+#define CMD_PWM1 (0x02 << NCP5623_CMD_SHIFT)
+#define CMD_PWM2 (0x03 << NCP5623_CMD_SHIFT)
+#define CMD_PWM3 (0x04 << NCP5623_CMD_SHIFT)
+#define CMD_UPWARD_DIM (0x05 << NCP5623_CMD_SHIFT)
+#define CMD_DOWNWARD_DIM (0x06 << NCP5623_CMD_SHIFT)
+#define CMD_DIM_STEP (0x07 << NCP5623_CMD_SHIFT)
+
+#define NCP5623_DATA_MASK GENMASK(NCP5623_CMD_SHIFT - 1, 0)
+
+#define NCP5623_CMD(cmd, data) (cmd | (data & NCP5623_DATA_MASK))
+
+struct ncp5623_led {
+ bool active;
+ unsigned int led_no;
+ struct led_classdev ldev;
+ struct work_struct work;
+ struct ncp5623_priv *priv;
+};
+
+struct ncp5623_priv {
+ struct ncp5623_led leds[NCP5623_MAX_LEDS];
Please allocate memory dynamically, depending on the number
of LEDs defined in a Device Tree.
+ u32 led_iref;
+ u32 led_max_current;
+ struct i2c_client *client;
+};
+
+static struct ncp5623_led *ldev_to_led(struct led_classdev *ldev)
+{
+ return container_of(ldev, struct ncp5623_led, ldev);
+}
+
+static struct ncp5623_led *work_to_led(struct work_struct *work)
+{
+ return container_of(work, struct ncp5623_led, work);
+}
If you registered brightness_set_blocking op instead of brightness_set,
then the workqueue wouldn't be required in the driver.
+static int ncp5623_send_cmd(struct ncp5623_priv *priv, u8 cmd, u8 data)
+{
+ char cmd_data[1] = { NCP5623_CMD(cmd, data) };
+ int err;
+
+ err = i2c_master_send(priv->client, cmd_data, ARRAY_SIZE(cmd_data));
Please add empty line here.
+ return (err < 0 ? err : 0);
+}
+
+static int ncp5623_set_pwm(struct ncp5623_led *led, u8 brightness)
+{
+ struct ncp5623_priv *priv = led->priv;
+ u8 cmd;
+
+ switch (led->led_no) {
+ case 0:
+ cmd = CMD_PWM1;
+ break;
+ case 1:
+ cmd = CMD_PWM2;
+ break;
+ case 2:
+ cmd
[PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver
The NCP5623 is a 3-channel LED driver from On Semiconductor controlled
through I2C. The PWM of each channel can be independently set with 32
distinct levels. In addition, the intensity of the current source can be
globally set using an external bias resistor fixing the reference
current (Iref) and a dedicated register (ILED), following the
relationship:
I = 2400*Iref/(31-ILED)
with Iref = Vref/Rbias, and Vref = 0.6V.
Signed-off-by: Florian Vaussard
---
drivers/leds/Kconfig| 11 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-ncp5623.c | 265
3 files changed, 277 insertions(+)
create mode 100644 drivers/leds/leds-ncp5623.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 5ae2834..6d3e44d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -588,6 +588,17 @@ config LEDS_BLINKM
This option enables support for the BlinkM RGB LED connected
through I2C. Say Y to enable support for the BlinkM LED.
+config LEDS_NCP5623
+ tristate "LED Support for NCP5623 I2C chip"
+ depends on LEDS_CLASS
+ depends on I2C
+ help
+ This option enables support for LEDs connected to NCP5623
+ LED driver chips accessed via the I2C bus.
+ Driver supports brightness control.
+
+ Say Y to enable this driver.
+
config LEDS_POWERNV
tristate "LED support for PowerNV Platform"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cb2013d..a2f0e10 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692)+= leds-ktd2692.o
obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
+obj-$(CONFIG_LEDS_NCP5623) += leds-ncp5623.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-ncp5623.c b/drivers/leds/leds-ncp5623.c
new file mode 100644
index 000..a341e4a
--- /dev/null
+++ b/drivers/leds/leds-ncp5623.c
@@ -0,0 +1,265 @@
+/*
+ * Copyright 2016 Florian Vaussard
+ *
+ * Based on leds-tlc591xx.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NCP5623_MAX_LEDS 3
+#define NCP5623_MAX_STEPS 32
+#define NCP5623_MAX_CURRENT31
+#define NCP5623_MAX_CURRENT_UA 3
+
+#define NCP5623_CMD_SHIFT 5
+#define CMD_SHUTDOWN (0x00 << NCP5623_CMD_SHIFT)
+#define CMD_ILED (0x01 << NCP5623_CMD_SHIFT)
+#define CMD_PWM1 (0x02 << NCP5623_CMD_SHIFT)
+#define CMD_PWM2 (0x03 << NCP5623_CMD_SHIFT)
+#define CMD_PWM3 (0x04 << NCP5623_CMD_SHIFT)
+#define CMD_UPWARD_DIM (0x05 << NCP5623_CMD_SHIFT)
+#define CMD_DOWNWARD_DIM (0x06 << NCP5623_CMD_SHIFT)
+#define CMD_DIM_STEP (0x07 << NCP5623_CMD_SHIFT)
+
+#define NCP5623_DATA_MASK GENMASK(NCP5623_CMD_SHIFT - 1, 0)
+
+#define NCP5623_CMD(cmd, data) (cmd | (data & NCP5623_DATA_MASK))
+
+struct ncp5623_led {
+ bool active;
+ unsigned int led_no;
+ struct led_classdev ldev;
+ struct work_struct work;
+ struct ncp5623_priv *priv;
+};
+
+struct ncp5623_priv {
+ struct ncp5623_led leds[NCP5623_MAX_LEDS];
+ u32 led_iref;
+ u32 led_max_current;
+ struct i2c_client *client;
+};
+
+static struct ncp5623_led *ldev_to_led(struct led_classdev *ldev)
+{
+ return container_of(ldev, struct ncp5623_led, ldev);
+}
+
+static struct ncp5623_led *work_to_led(struct work_struct *work)
+{
+ return container_of(work, struct ncp5623_led, work);
+}
+
+static int ncp5623_send_cmd(struct ncp5623_priv *priv, u8 cmd, u8 data)
+{
+ char cmd_data[1] = { NCP5623_CMD(cmd, data) };
+ int err;
+
+ err = i2c_master_send(priv->client, cmd_data, ARRAY_SIZE(cmd_data));
+ return (err < 0 ? err : 0);
+}
+
+static int ncp5623_set_pwm(struct ncp5623_led *led, u8 brightness)
+{
+ struct ncp5623_priv *priv = led->priv;
+ u8 cmd;
+
+ switch (led->led_no) {
+ case 0:
+ cmd = CMD_PWM1;
+ break;
+ case 1:
+ cmd = CMD_PWM2;
+ break;
+ case 2:
+ cmd = CMD_PWM3;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ncp5623_send_cmd(priv, cmd, brightness);
+}
+
+static void ncp5623_led_work(struct work_struct *work)
+{
+ struct ncp5623_led *led = work_to_led(work);
+ enum led_brightness brightness = led->ldev.brightness;
+ int err;
+
+ err = ncp5623_set_pwm(led, brigh
