Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-07 Thread Baolin Wang 
Hi Jacek,

On 8 May 2018 at 04:13, Jacek Anaszewski  wrote:
> Hi Baolin,
>
> Thank you for the patch. Generally this is a nice and clean driver,
> but I noticed some locking related issues and one detail
> regarding LED naming. Please refer below.
>
>
> On 05/04/2018 12:08 PM, Baolin Wang wrote:
>>
>> From: Xiaotong Lu 
>>
>> This patch adds Spreadtrum SC27xx PMIC series breathing light controller
>> driver, which can support 3 LEDs. Each LED can work at normal PWM mode
>> and breathing mode.
>>
>> Signed-off-by: Xiaotong Lu 
>> Signed-off-by: Baolin Wang 
>> ---
>>   drivers/leds/Kconfig|   11 ++
>>   drivers/leds/Makefile   |1 +
>>   drivers/leds/leds-sc27xx-bltc.c |  369
>> +++
>>   3 files changed, 381 insertions(+)
>>   create mode 100644 drivers/leds/leds-sc27xx-bltc.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 2c896c0..319449b 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -647,6 +647,17 @@ config LEDS_IS31FL32XX
>>   LED controllers. They are I2C devices with multiple
>> constant-current
>>   channels, each with independent 256-level PWM control.
>>   +config LEDS_SC27XX_BLTC
>> +   tristate "LED support for the SC27xx breathing light controller"
>> +   depends on LEDS_CLASS && MFD_SC27XX_PMIC
>> +   depends on OF
>> +   help
>> + Say Y here to include support for the SC27xx breathing light
>> controller
>> + LEDs.
>> +
>> + This driver can also be built as a module. If so the module will
>> be
>> + called leds-sc27xx-bltc.
>> +
>>   comment "LED driver for blink(1) USB RGB LED is under Special HID
>> drivers (HID_THINGM)"
>> config LEDS_BLINKM
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index 91eca81..ff6917e 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
>>   obj-$(CONFIG_LEDS_NIC78BX)+= leds-nic78bx.o
>>   obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
>>   obj-$(CONFIG_LEDS_LM3692X)+= leds-lm3692x.o
>> +obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
>> # LED SPI Drivers
>>   obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
>> diff --git a/drivers/leds/leds-sc27xx-bltc.c
>> b/drivers/leds/leds-sc27xx-bltc.c
>> new file mode 100644
>> index 000..0016a87
>> --- /dev/null
>> +++ b/drivers/leds/leds-sc27xx-bltc.c
>> @@ -0,0 +1,369 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Spreadtrum Communications Inc.
>> + */
>
>
> Please use uniform "//" comment style here.
>
>
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* PMIC global control register definition */
>> +#define SC27XX_MODULE_EN0  0xc08
>> +#define SC27XX_CLK_EN0 0xc18
>> +#define SC27XX_RGB_CTRL0xebc
>> +
>> +#define SC27XX_BLTC_EN BIT(9)
>> +#define SC27XX_RTC_EN  BIT(7)
>> +#define SC27XX_RGB_PD  BIT(0)
>> +
>> +/* Breathing light controller register definition */
>> +#define SC27XX_LEDS_CTRL   0x00
>> +#define SC27XX_LEDS_PRESCALE   0x04
>> +#define SC27XX_LEDS_DUTY   0x08
>> +#define SC27XX_LEDS_CURVE0 0x0c
>> +#define SC27XX_LEDS_CURVE1 0x10
>> +
>> +#define SC27XX_CTRL_SHIFT  4
>> +#define SC27XX_LED_RUN BIT(0)
>> +#define SC27XX_LED_TYPEBIT(1)
>> +
>> +#define SC27XX_DUTY_SHIFT  8
>> +#define SC27XX_DUTY_MASK   GENMASK(15, 0)
>> +#define SC27XX_MOD_MASKGENMASK(7, 0)
>> +
>> +#define SC27XX_CURVE_SHIFT 8
>> +#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
>> +#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
>> +
>> +#define SC27XX_LEDS_OFFSET 0x10
>> +#define SC27XX_LEDS_MAX3
>> +
>> +/*
>> + * The SC27xx breathing light controller can support 3 outputs: red LED,
>> + * green LED and blue LED. Each LED can support normal PWM mode and
>> breathing
>> + * mode.
>> + *
>> + * In breathing mode, the LED output curve includes raise, high, fall and
>> low 4
>> + * stages, and the duration of each stage is configurable.
>> + */
>> +enum sc27xx_led_config {
>> +   SC27XX_RAISE_TIME,
>> +   SC27XX_FALL_TIME,
>> +   SC27XX_HIGH_TIME,
>> +   SC27XX_LOW_TIME,
>> +};
>> +
>> +struct sc27xx_led {
>> +   struct led_classdev ldev;
>> +   struct sc27xx_led_priv *priv;
>> +   enum led_brightness value;
>> +   u8 line;
>> +   bool breath_mode;
>> +   bool active;
>> +};
>> +
>> +struct sc27xx_led_priv {
>> +   struct sc27xx_led leds[SC27XX_LEDS_MAX];
>> +   struct regmap *regmap;
>> +   u32 base;
>> +};
>> +
>> +#define to_sc27xx_led(ldev) \
>> +   container_of(ldev, struct sc27xx_led, ldev)
>> +
>> +static int

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-07 Thread Baolin Wang 
Hi Jacek,

On 8 May 2018 at 04:13, Jacek Anaszewski  wrote:
> Hi Baolin,
>
> Thank you for the patch. Generally this is a nice and clean driver,
> but I noticed some locking related issues and one detail
> regarding LED naming. Please refer below.
>
>
> On 05/04/2018 12:08 PM, Baolin Wang wrote:
>>
>> From: Xiaotong Lu 
>>
>> This patch adds Spreadtrum SC27xx PMIC series breathing light controller
>> driver, which can support 3 LEDs. Each LED can work at normal PWM mode
>> and breathing mode.
>>
>> Signed-off-by: Xiaotong Lu 
>> Signed-off-by: Baolin Wang 
>> ---
>>   drivers/leds/Kconfig|   11 ++
>>   drivers/leds/Makefile   |1 +
>>   drivers/leds/leds-sc27xx-bltc.c |  369
>> +++
>>   3 files changed, 381 insertions(+)
>>   create mode 100644 drivers/leds/leds-sc27xx-bltc.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 2c896c0..319449b 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -647,6 +647,17 @@ config LEDS_IS31FL32XX
>>   LED controllers. They are I2C devices with multiple
>> constant-current
>>   channels, each with independent 256-level PWM control.
>>   +config LEDS_SC27XX_BLTC
>> +   tristate "LED support for the SC27xx breathing light controller"
>> +   depends on LEDS_CLASS && MFD_SC27XX_PMIC
>> +   depends on OF
>> +   help
>> + Say Y here to include support for the SC27xx breathing light
>> controller
>> + LEDs.
>> +
>> + This driver can also be built as a module. If so the module will
>> be
>> + called leds-sc27xx-bltc.
>> +
>>   comment "LED driver for blink(1) USB RGB LED is under Special HID
>> drivers (HID_THINGM)"
>> config LEDS_BLINKM
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index 91eca81..ff6917e 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
>>   obj-$(CONFIG_LEDS_NIC78BX)+= leds-nic78bx.o
>>   obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
>>   obj-$(CONFIG_LEDS_LM3692X)+= leds-lm3692x.o
>> +obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
>> # LED SPI Drivers
>>   obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
>> diff --git a/drivers/leds/leds-sc27xx-bltc.c
>> b/drivers/leds/leds-sc27xx-bltc.c
>> new file mode 100644
>> index 000..0016a87
>> --- /dev/null
>> +++ b/drivers/leds/leds-sc27xx-bltc.c
>> @@ -0,0 +1,369 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Spreadtrum Communications Inc.
>> + */
>
>
> Please use uniform "//" comment style here.
>
>
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* PMIC global control register definition */
>> +#define SC27XX_MODULE_EN0  0xc08
>> +#define SC27XX_CLK_EN0 0xc18
>> +#define SC27XX_RGB_CTRL0xebc
>> +
>> +#define SC27XX_BLTC_EN BIT(9)
>> +#define SC27XX_RTC_EN  BIT(7)
>> +#define SC27XX_RGB_PD  BIT(0)
>> +
>> +/* Breathing light controller register definition */
>> +#define SC27XX_LEDS_CTRL   0x00
>> +#define SC27XX_LEDS_PRESCALE   0x04
>> +#define SC27XX_LEDS_DUTY   0x08
>> +#define SC27XX_LEDS_CURVE0 0x0c
>> +#define SC27XX_LEDS_CURVE1 0x10
>> +
>> +#define SC27XX_CTRL_SHIFT  4
>> +#define SC27XX_LED_RUN BIT(0)
>> +#define SC27XX_LED_TYPEBIT(1)
>> +
>> +#define SC27XX_DUTY_SHIFT  8
>> +#define SC27XX_DUTY_MASK   GENMASK(15, 0)
>> +#define SC27XX_MOD_MASKGENMASK(7, 0)
>> +
>> +#define SC27XX_CURVE_SHIFT 8
>> +#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
>> +#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
>> +
>> +#define SC27XX_LEDS_OFFSET 0x10
>> +#define SC27XX_LEDS_MAX3
>> +
>> +/*
>> + * The SC27xx breathing light controller can support 3 outputs: red LED,
>> + * green LED and blue LED. Each LED can support normal PWM mode and
>> breathing
>> + * mode.
>> + *
>> + * In breathing mode, the LED output curve includes raise, high, fall and
>> low 4
>> + * stages, and the duration of each stage is configurable.
>> + */
>> +enum sc27xx_led_config {
>> +   SC27XX_RAISE_TIME,
>> +   SC27XX_FALL_TIME,
>> +   SC27XX_HIGH_TIME,
>> +   SC27XX_LOW_TIME,
>> +};
>> +
>> +struct sc27xx_led {
>> +   struct led_classdev ldev;
>> +   struct sc27xx_led_priv *priv;
>> +   enum led_brightness value;
>> +   u8 line;
>> +   bool breath_mode;
>> +   bool active;
>> +};
>> +
>> +struct sc27xx_led_priv {
>> +   struct sc27xx_led leds[SC27XX_LEDS_MAX];
>> +   struct regmap *regmap;
>> +   u32 base;
>> +};
>> +
>> +#define to_sc27xx_led(ldev) \
>> +   container_of(ldev, struct sc27xx_led, ldev)
>> +
>> +static int sc27xx_led_init(struct regmap *regmap)
>> +{
>> +   int err;
>> +
>> +   err = regmap_update_bits(regmap,

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-07 Thread Jacek Anaszewski 

Hi Baolin,

Thank you for the patch. Generally this is a nice and clean driver,
but I noticed some locking related issues and one detail
regarding LED naming. Please refer below.

On 05/04/2018 12:08 PM, Baolin Wang wrote:

From: Xiaotong Lu 

This patch adds Spreadtrum SC27xx PMIC series breathing light controller
driver, which can support 3 LEDs. Each LED can work at normal PWM mode
and breathing mode.

Signed-off-by: Xiaotong Lu 
Signed-off-by: Baolin Wang 
---
  drivers/leds/Kconfig|   11 ++
  drivers/leds/Makefile   |1 +
  drivers/leds/leds-sc27xx-bltc.c |  369 +++
  3 files changed, 381 insertions(+)
  create mode 100644 drivers/leds/leds-sc27xx-bltc.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 2c896c0..319449b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -647,6 +647,17 @@ config LEDS_IS31FL32XX
  LED controllers. They are I2C devices with multiple constant-current
  channels, each with independent 256-level PWM control.
  
+config LEDS_SC27XX_BLTC

+   tristate "LED support for the SC27xx breathing light controller"
+   depends on LEDS_CLASS && MFD_SC27XX_PMIC
+   depends on OF
+   help
+ Say Y here to include support for the SC27xx breathing light 
controller
+ LEDs.
+
+ This driver can also be built as a module. If so the module will be
+ called leds-sc27xx-bltc.
+
  comment "LED driver for blink(1) USB RGB LED is under Special HID drivers 
(HID_THINGM)"
  
  config LEDS_BLINKM

diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 91eca81..ff6917e 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
  obj-$(CONFIG_LEDS_NIC78BX)+= leds-nic78bx.o
  obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
  obj-$(CONFIG_LEDS_LM3692X)+= leds-lm3692x.o
+obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
  
  # LED SPI Drivers

  obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
new file mode 100644
index 000..0016a87
--- /dev/null
+++ b/drivers/leds/leds-sc27xx-bltc.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ */


Please use uniform "//" comment style here.


+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* PMIC global control register definition */
+#define SC27XX_MODULE_EN0  0xc08
+#define SC27XX_CLK_EN0 0xc18
+#define SC27XX_RGB_CTRL0xebc
+
+#define SC27XX_BLTC_EN BIT(9)
+#define SC27XX_RTC_EN  BIT(7)
+#define SC27XX_RGB_PD  BIT(0)
+
+/* Breathing light controller register definition */
+#define SC27XX_LEDS_CTRL   0x00
+#define SC27XX_LEDS_PRESCALE   0x04
+#define SC27XX_LEDS_DUTY   0x08
+#define SC27XX_LEDS_CURVE0 0x0c
+#define SC27XX_LEDS_CURVE1 0x10
+
+#define SC27XX_CTRL_SHIFT  4
+#define SC27XX_LED_RUN BIT(0)
+#define SC27XX_LED_TYPEBIT(1)
+
+#define SC27XX_DUTY_SHIFT  8
+#define SC27XX_DUTY_MASK   GENMASK(15, 0)
+#define SC27XX_MOD_MASKGENMASK(7, 0)
+
+#define SC27XX_CURVE_SHIFT 8
+#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
+#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
+
+#define SC27XX_LEDS_OFFSET 0x10
+#define SC27XX_LEDS_MAX3
+
+/*
+ * The SC27xx breathing light controller can support 3 outputs: red LED,
+ * green LED and blue LED. Each LED can support normal PWM mode and breathing
+ * mode.
+ *
+ * In breathing mode, the LED output curve includes raise, high, fall and low 4
+ * stages, and the duration of each stage is configurable.
+ */
+enum sc27xx_led_config {
+   SC27XX_RAISE_TIME,
+   SC27XX_FALL_TIME,
+   SC27XX_HIGH_TIME,
+   SC27XX_LOW_TIME,
+};
+
+struct sc27xx_led {
+   struct led_classdev ldev;
+   struct sc27xx_led_priv *priv;
+   enum led_brightness value;
+   u8 line;
+   bool breath_mode;
+   bool active;
+};
+
+struct sc27xx_led_priv {
+   struct sc27xx_led leds[SC27XX_LEDS_MAX];
+   struct regmap *regmap;
+   u32 base;
+};
+
+#define to_sc27xx_led(ldev) \
+   container_of(ldev, struct sc27xx_led, ldev)
+
+static int sc27xx_led_init(struct regmap *regmap)
+{
+   int err;
+
+   err = regmap_update_bits(regmap, SC27XX_MODULE_EN0, SC27XX_BLTC_EN,
+SC27XX_BLTC_EN);
+   if (err)
+   return err;
+
+   err = regmap_update_bits(regmap, SC27XX_CLK_EN0, SC27XX_RTC_EN,
+SC27XX_RTC_EN);
+   if (err)
+   return err;
+
+   return regmap_update_bits(regmap, SC27XX_RGB_CTRL, SC27XX_RGB_PD, 0);
+}
+
+static u32 sc27xx_led_get_offset(struct

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-07 Thread Jacek Anaszewski 

Hi Baolin,

Thank you for the patch. Generally this is a nice and clean driver,
but I noticed some locking related issues and one detail
regarding LED naming. Please refer below.

On 05/04/2018 12:08 PM, Baolin Wang wrote:

From: Xiaotong Lu 

This patch adds Spreadtrum SC27xx PMIC series breathing light controller
driver, which can support 3 LEDs. Each LED can work at normal PWM mode
and breathing mode.

Signed-off-by: Xiaotong Lu 
Signed-off-by: Baolin Wang 
---
  drivers/leds/Kconfig|   11 ++
  drivers/leds/Makefile   |1 +
  drivers/leds/leds-sc27xx-bltc.c |  369 +++
  3 files changed, 381 insertions(+)
  create mode 100644 drivers/leds/leds-sc27xx-bltc.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 2c896c0..319449b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -647,6 +647,17 @@ config LEDS_IS31FL32XX
  LED controllers. They are I2C devices with multiple constant-current
  channels, each with independent 256-level PWM control.
  
+config LEDS_SC27XX_BLTC

+   tristate "LED support for the SC27xx breathing light controller"
+   depends on LEDS_CLASS && MFD_SC27XX_PMIC
+   depends on OF
+   help
+ Say Y here to include support for the SC27xx breathing light 
controller
+ LEDs.
+
+ This driver can also be built as a module. If so the module will be
+ called leds-sc27xx-bltc.
+
  comment "LED driver for blink(1) USB RGB LED is under Special HID drivers 
(HID_THINGM)"
  
  config LEDS_BLINKM

diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 91eca81..ff6917e 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
  obj-$(CONFIG_LEDS_NIC78BX)+= leds-nic78bx.o
  obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
  obj-$(CONFIG_LEDS_LM3692X)+= leds-lm3692x.o
+obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
  
  # LED SPI Drivers

  obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
new file mode 100644
index 000..0016a87
--- /dev/null
+++ b/drivers/leds/leds-sc27xx-bltc.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ */


Please use uniform "//" comment style here.


+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* PMIC global control register definition */
+#define SC27XX_MODULE_EN0  0xc08
+#define SC27XX_CLK_EN0 0xc18
+#define SC27XX_RGB_CTRL0xebc
+
+#define SC27XX_BLTC_EN BIT(9)
+#define SC27XX_RTC_EN  BIT(7)
+#define SC27XX_RGB_PD  BIT(0)
+
+/* Breathing light controller register definition */
+#define SC27XX_LEDS_CTRL   0x00
+#define SC27XX_LEDS_PRESCALE   0x04
+#define SC27XX_LEDS_DUTY   0x08
+#define SC27XX_LEDS_CURVE0 0x0c
+#define SC27XX_LEDS_CURVE1 0x10
+
+#define SC27XX_CTRL_SHIFT  4
+#define SC27XX_LED_RUN BIT(0)
+#define SC27XX_LED_TYPEBIT(1)
+
+#define SC27XX_DUTY_SHIFT  8
+#define SC27XX_DUTY_MASK   GENMASK(15, 0)
+#define SC27XX_MOD_MASKGENMASK(7, 0)
+
+#define SC27XX_CURVE_SHIFT 8
+#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
+#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
+
+#define SC27XX_LEDS_OFFSET 0x10
+#define SC27XX_LEDS_MAX3
+
+/*
+ * The SC27xx breathing light controller can support 3 outputs: red LED,
+ * green LED and blue LED. Each LED can support normal PWM mode and breathing
+ * mode.
+ *
+ * In breathing mode, the LED output curve includes raise, high, fall and low 4
+ * stages, and the duration of each stage is configurable.
+ */
+enum sc27xx_led_config {
+   SC27XX_RAISE_TIME,
+   SC27XX_FALL_TIME,
+   SC27XX_HIGH_TIME,
+   SC27XX_LOW_TIME,
+};
+
+struct sc27xx_led {
+   struct led_classdev ldev;
+   struct sc27xx_led_priv *priv;
+   enum led_brightness value;
+   u8 line;
+   bool breath_mode;
+   bool active;
+};
+
+struct sc27xx_led_priv {
+   struct sc27xx_led leds[SC27XX_LEDS_MAX];
+   struct regmap *regmap;
+   u32 base;
+};
+
+#define to_sc27xx_led(ldev) \
+   container_of(ldev, struct sc27xx_led, ldev)
+
+static int sc27xx_led_init(struct regmap *regmap)
+{
+   int err;
+
+   err = regmap_update_bits(regmap, SC27XX_MODULE_EN0, SC27XX_BLTC_EN,
+SC27XX_BLTC_EN);
+   if (err)
+   return err;
+
+   err = regmap_update_bits(regmap, SC27XX_CLK_EN0, SC27XX_RTC_EN,
+SC27XX_RTC_EN);
+   if (err)
+   return err;
+
+   return regmap_update_bits(regmap, SC27XX_RGB_CTRL, SC27XX_RGB_PD, 0);
+}
+
+static u32 sc27xx_led_get_offset(struct sc27xx_led *leds)
+{
+   return leds->priv->base + SC27XX_LEDS_OFFSET *

RE: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-05 Thread 卢小通 
Hi lkp,
Thanks for your advise. I will improve in next version.

Best Regards,
卢小通 Xiaotong Lu
CPSD Dept.
Unigroup Spreadtrum  Technologies Co.,Ltd.
Tel:  86-21-20360600 Ext. 2496
Fax: 86-21-20360700
E-mail: xiaotong...@unisoc.com
Http://www.spreadtrum.com 


-Original Message-
From: kbuild test robot [mailto:l...@intel.com] 
Sent: Saturday, May 05, 2018 1:27 PM
To: Baolin Wang
Cc: kbuild-...@01.org; jacek.anaszew...@gmail.com; pa...@ucw.cz; 
robh...@kernel.org; mark.rutl...@arm.com; Xiaotong Lu (卢小通); 
baolin.w...@linaro.org; broo...@kernel.org; linux-l...@vger.kernel.org; 
devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller 
driver

Hi Xiaotong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next] [also build test WARNING on 
v4.17-rc3 next-20180504] [cannot apply to j.anaszewski-leds/for-next] [if your 
patch is applied to the wrong git tree, please drop us a note to help improve 
the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dt-bindings-leds-Add-SC27xx-breathing-light-controller-documentation/20180504-200830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

smatch warnings:
drivers/leds/leds-sc27xx-bltc.c:337 sc27xx_led_probe() warn: unsigned 'reg' is 
never less than zero.

vim +/reg +337 drivers/leds/leds-sc27xx-bltc.c

   299  
   300  static int sc27xx_led_probe(struct platform_device *pdev)
   301  {
   302  struct device *dev = >dev;
   303  struct device_node *np = dev->of_node, *child;
   304  struct sc27xx_led_priv *priv;
   305  u32 base, count, reg;
   306  int err;
   307  
   308  count = of_get_child_count(np);
   309  if (!count || count > SC27XX_LEDS_MAX)
   310  return -EINVAL;
   311  
   312  err = of_property_read_u32(np, "reg", );
   313  if (err) {
   314  dev_err(dev, "fail to get reg of property\n");
   315  return err;
   316  }
   317  
   318  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   319  if (!priv)
   320  return -ENOMEM;
   321  
   322  priv->base = base;
   323  priv->regmap = dev_get_regmap(dev->parent, NULL);
   324  if (IS_ERR(priv->regmap)) {
   325  err = PTR_ERR(priv->regmap);
   326  dev_err(dev, "failed to get regmap: %d\n", err);
   327  return err;
   328  }
   329  
   330  for_each_child_of_node(np, child) {
   331  err = of_property_read_u32(child, "reg", );
   332  if (err) {
   333  of_node_put(child);
   334  return err;
   335  }
   336  
 > 337  if (reg < 0 || reg >= SC27XX_LEDS_MAX
   338  || priv->leds[reg].active) {
   339  of_node_put(child);
   340  return -EINVAL;
   341  }
   342  
   343  priv->leds[reg].active = true;
   344  priv->leds[reg].ldev.name =
   345  of_get_property(child, "label", NULL) ? : 
child->name;
   346  }
   347  
   348  return sc27xx_led_register(dev, priv);
   349  }
   350  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

RE: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-05 Thread 卢小通 
Hi lkp,
Thanks for your advise. I will improve in next version.

Best Regards,
卢小通 Xiaotong Lu
CPSD Dept.
Unigroup Spreadtrum  Technologies Co.,Ltd.
Tel:  86-21-20360600 Ext. 2496
Fax: 86-21-20360700
E-mail: xiaotong...@unisoc.com
Http://www.spreadtrum.com 


-Original Message-
From: kbuild test robot [mailto:l...@intel.com] 
Sent: Saturday, May 05, 2018 1:27 PM
To: Baolin Wang
Cc: kbuild-...@01.org; jacek.anaszew...@gmail.com; pa...@ucw.cz; 
robh...@kernel.org; mark.rutl...@arm.com; Xiaotong Lu (卢小通); 
baolin.w...@linaro.org; broo...@kernel.org; linux-l...@vger.kernel.org; 
devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller 
driver

Hi Xiaotong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next] [also build test WARNING on 
v4.17-rc3 next-20180504] [cannot apply to j.anaszewski-leds/for-next] [if your 
patch is applied to the wrong git tree, please drop us a note to help improve 
the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dt-bindings-leds-Add-SC27xx-breathing-light-controller-documentation/20180504-200830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

smatch warnings:
drivers/leds/leds-sc27xx-bltc.c:337 sc27xx_led_probe() warn: unsigned 'reg' is 
never less than zero.

vim +/reg +337 drivers/leds/leds-sc27xx-bltc.c

   299  
   300  static int sc27xx_led_probe(struct platform_device *pdev)
   301  {
   302  struct device *dev = >dev;
   303  struct device_node *np = dev->of_node, *child;
   304  struct sc27xx_led_priv *priv;
   305  u32 base, count, reg;
   306  int err;
   307  
   308  count = of_get_child_count(np);
   309  if (!count || count > SC27XX_LEDS_MAX)
   310  return -EINVAL;
   311  
   312  err = of_property_read_u32(np, "reg", );
   313  if (err) {
   314  dev_err(dev, "fail to get reg of property\n");
   315  return err;
   316  }
   317  
   318  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   319  if (!priv)
   320  return -ENOMEM;
   321  
   322  priv->base = base;
   323  priv->regmap = dev_get_regmap(dev->parent, NULL);
   324  if (IS_ERR(priv->regmap)) {
   325  err = PTR_ERR(priv->regmap);
   326  dev_err(dev, "failed to get regmap: %d\n", err);
   327  return err;
   328  }
   329  
   330  for_each_child_of_node(np, child) {
   331  err = of_property_read_u32(child, "reg", );
   332  if (err) {
   333  of_node_put(child);
   334  return err;
   335  }
   336  
 > 337  if (reg < 0 || reg >= SC27XX_LEDS_MAX
   338  || priv->leds[reg].active) {
   339  of_node_put(child);
   340  return -EINVAL;
   341  }
   342  
   343  priv->leds[reg].active = true;
   344  priv->leds[reg].ldev.name =
   345  of_get_property(child, "label", NULL) ? : 
child->name;
   346  }
   347  
   348  return sc27xx_led_register(dev, priv);
   349  }
   350  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-04 Thread kbuild test robot 
Hi Xiaotong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc3 next-20180504]
[cannot apply to j.anaszewski-leds/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dt-bindings-leds-Add-SC27xx-breathing-light-controller-documentation/20180504-200830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

smatch warnings:
drivers/leds/leds-sc27xx-bltc.c:337 sc27xx_led_probe() warn: unsigned 'reg' is 
never less than zero.

vim +/reg +337 drivers/leds/leds-sc27xx-bltc.c

   299  
   300  static int sc27xx_led_probe(struct platform_device *pdev)
   301  {
   302  struct device *dev = >dev;
   303  struct device_node *np = dev->of_node, *child;
   304  struct sc27xx_led_priv *priv;
   305  u32 base, count, reg;
   306  int err;
   307  
   308  count = of_get_child_count(np);
   309  if (!count || count > SC27XX_LEDS_MAX)
   310  return -EINVAL;
   311  
   312  err = of_property_read_u32(np, "reg", );
   313  if (err) {
   314  dev_err(dev, "fail to get reg of property\n");
   315  return err;
   316  }
   317  
   318  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   319  if (!priv)
   320  return -ENOMEM;
   321  
   322  priv->base = base;
   323  priv->regmap = dev_get_regmap(dev->parent, NULL);
   324  if (IS_ERR(priv->regmap)) {
   325  err = PTR_ERR(priv->regmap);
   326  dev_err(dev, "failed to get regmap: %d\n", err);
   327  return err;
   328  }
   329  
   330  for_each_child_of_node(np, child) {
   331  err = of_property_read_u32(child, "reg", );
   332  if (err) {
   333  of_node_put(child);
   334  return err;
   335  }
   336  
 > 337  if (reg < 0 || reg >= SC27XX_LEDS_MAX
   338  || priv->leds[reg].active) {
   339  of_node_put(child);
   340  return -EINVAL;
   341  }
   342  
   343  priv->leds[reg].active = true;
   344  priv->leds[reg].ldev.name =
   345  of_get_property(child, "label", NULL) ? : 
child->name;
   346  }
   347  
   348  return sc27xx_led_register(dev, priv);
   349  }
   350  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-04 Thread kbuild test robot 
Hi Xiaotong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc3 next-20180504]
[cannot apply to j.anaszewski-leds/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dt-bindings-leds-Add-SC27xx-breathing-light-controller-documentation/20180504-200830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

smatch warnings:
drivers/leds/leds-sc27xx-bltc.c:337 sc27xx_led_probe() warn: unsigned 'reg' is 
never less than zero.

vim +/reg +337 drivers/leds/leds-sc27xx-bltc.c

   299  
   300  static int sc27xx_led_probe(struct platform_device *pdev)
   301  {
   302  struct device *dev = >dev;
   303  struct device_node *np = dev->of_node, *child;
   304  struct sc27xx_led_priv *priv;
   305  u32 base, count, reg;
   306  int err;
   307  
   308  count = of_get_child_count(np);
   309  if (!count || count > SC27XX_LEDS_MAX)
   310  return -EINVAL;
   311  
   312  err = of_property_read_u32(np, "reg", );
   313  if (err) {
   314  dev_err(dev, "fail to get reg of property\n");
   315  return err;
   316  }
   317  
   318  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   319  if (!priv)
   320  return -ENOMEM;
   321  
   322  priv->base = base;
   323  priv->regmap = dev_get_regmap(dev->parent, NULL);
   324  if (IS_ERR(priv->regmap)) {
   325  err = PTR_ERR(priv->regmap);
   326  dev_err(dev, "failed to get regmap: %d\n", err);
   327  return err;
   328  }
   329  
   330  for_each_child_of_node(np, child) {
   331  err = of_property_read_u32(child, "reg", );
   332  if (err) {
   333  of_node_put(child);
   334  return err;
   335  }
   336  
 > 337  if (reg < 0 || reg >= SC27XX_LEDS_MAX
   338  || priv->leds[reg].active) {
   339  of_node_put(child);
   340  return -EINVAL;
   341  }
   342  
   343  priv->leds[reg].active = true;
   344  priv->leds[reg].ldev.name =
   345  of_get_property(child, "label", NULL) ? : 
child->name;
   346  }
   347  
   348  return sc27xx_led_register(dev, priv);
   349  }
   350  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

[PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-04 Thread Baolin Wang 
From: Xiaotong Lu 

This patch adds Spreadtrum SC27xx PMIC series breathing light controller
driver, which can support 3 LEDs. Each LED can work at normal PWM mode
and breathing mode.

Signed-off-by: Xiaotong Lu 
Signed-off-by: Baolin Wang 
---
 drivers/leds/Kconfig|   11 ++
 drivers/leds/Makefile   |1 +
 drivers/leds/leds-sc27xx-bltc.c |  369 +++
 3 files changed, 381 insertions(+)
 create mode 100644 drivers/leds/leds-sc27xx-bltc.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 2c896c0..319449b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -647,6 +647,17 @@ config LEDS_IS31FL32XX
  LED controllers. They are I2C devices with multiple constant-current
  channels, each with independent 256-level PWM control.
 
+config LEDS_SC27XX_BLTC
+   tristate "LED support for the SC27xx breathing light controller"
+   depends on LEDS_CLASS && MFD_SC27XX_PMIC
+   depends on OF
+   help
+ Say Y here to include support for the SC27xx breathing light 
controller
+ LEDs.
+
+ This driver can also be built as a module. If so the module will be
+ called leds-sc27xx-bltc.
+
 comment "LED driver for blink(1) USB RGB LED is under Special HID drivers 
(HID_THINGM)"
 
 config LEDS_BLINKM
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 91eca81..ff6917e 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
 obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
 obj-$(CONFIG_LEDS_MT6323)  += leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o
+obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_DAC124S085)  += leds-dac124s085.o
diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
new file mode 100644
index 000..0016a87
--- /dev/null
+++ b/drivers/leds/leds-sc27xx-bltc.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* PMIC global control register definition */
+#define SC27XX_MODULE_EN0  0xc08
+#define SC27XX_CLK_EN0 0xc18
+#define SC27XX_RGB_CTRL0xebc
+
+#define SC27XX_BLTC_EN BIT(9)
+#define SC27XX_RTC_EN  BIT(7)
+#define SC27XX_RGB_PD  BIT(0)
+
+/* Breathing light controller register definition */
+#define SC27XX_LEDS_CTRL   0x00
+#define SC27XX_LEDS_PRESCALE   0x04
+#define SC27XX_LEDS_DUTY   0x08
+#define SC27XX_LEDS_CURVE0 0x0c
+#define SC27XX_LEDS_CURVE1 0x10
+
+#define SC27XX_CTRL_SHIFT  4
+#define SC27XX_LED_RUN BIT(0)
+#define SC27XX_LED_TYPEBIT(1)
+
+#define SC27XX_DUTY_SHIFT  8
+#define SC27XX_DUTY_MASK   GENMASK(15, 0)
+#define SC27XX_MOD_MASKGENMASK(7, 0)
+
+#define SC27XX_CURVE_SHIFT 8
+#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
+#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
+
+#define SC27XX_LEDS_OFFSET 0x10
+#define SC27XX_LEDS_MAX3
+
+/*
+ * The SC27xx breathing light controller can support 3 outputs: red LED,
+ * green LED and blue LED. Each LED can support normal PWM mode and breathing
+ * mode.
+ *
+ * In breathing mode, the LED output curve includes raise, high, fall and low 4
+ * stages, and the duration of each stage is configurable.
+ */
+enum sc27xx_led_config {
+   SC27XX_RAISE_TIME,
+   SC27XX_FALL_TIME,
+   SC27XX_HIGH_TIME,
+   SC27XX_LOW_TIME,
+};
+
+struct sc27xx_led {
+   struct led_classdev ldev;
+   struct sc27xx_led_priv *priv;
+   enum led_brightness value;
+   u8 line;
+   bool breath_mode;
+   bool active;
+};
+
+struct sc27xx_led_priv {
+   struct sc27xx_led leds[SC27XX_LEDS_MAX];
+   struct regmap *regmap;
+   u32 base;
+};
+
+#define to_sc27xx_led(ldev) \
+   container_of(ldev, struct sc27xx_led, ldev)
+
+static int sc27xx_led_init(struct regmap *regmap)
+{
+   int err;
+
+   err = regmap_update_bits(regmap, SC27XX_MODULE_EN0, SC27XX_BLTC_EN,
+SC27XX_BLTC_EN);
+   if (err)
+   return err;
+
+   err = regmap_update_bits(regmap, SC27XX_CLK_EN0, SC27XX_RTC_EN,
+SC27XX_RTC_EN);
+   if (err)
+   return err;
+
+   return regmap_update_bits(regmap, SC27XX_RGB_CTRL, SC27XX_RGB_PD, 0);
+}
+
+static u32 sc27xx_led_get_offset(struct sc27xx_led *leds)
+{
+   return leds->priv->base + SC27XX_LEDS_OFFSET * leds->line;
+}
+
+static int sc27xx_led_enable(struct sc27xx_led *leds)
+{
+   u32 base = sc27xx_led_get_offset(leds);
+   u32 ctrl_base = leds->priv->base + SC27XX_LEDS_CTRL;
+   u8 ctrl_shift =

[PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

2018-05-04 Thread Baolin Wang 
From: Xiaotong Lu 

This patch adds Spreadtrum SC27xx PMIC series breathing light controller
driver, which can support 3 LEDs. Each LED can work at normal PWM mode
and breathing mode.

Signed-off-by: Xiaotong Lu 
Signed-off-by: Baolin Wang 
---
 drivers/leds/Kconfig|   11 ++
 drivers/leds/Makefile   |1 +
 drivers/leds/leds-sc27xx-bltc.c |  369 +++
 3 files changed, 381 insertions(+)
 create mode 100644 drivers/leds/leds-sc27xx-bltc.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 2c896c0..319449b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -647,6 +647,17 @@ config LEDS_IS31FL32XX
  LED controllers. They are I2C devices with multiple constant-current
  channels, each with independent 256-level PWM control.
 
+config LEDS_SC27XX_BLTC
+   tristate "LED support for the SC27xx breathing light controller"
+   depends on LEDS_CLASS && MFD_SC27XX_PMIC
+   depends on OF
+   help
+ Say Y here to include support for the SC27xx breathing light 
controller
+ LEDs.
+
+ This driver can also be built as a module. If so the module will be
+ called leds-sc27xx-bltc.
+
 comment "LED driver for blink(1) USB RGB LED is under Special HID drivers 
(HID_THINGM)"
 
 config LEDS_BLINKM
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 91eca81..ff6917e 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
 obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
 obj-$(CONFIG_LEDS_MT6323)  += leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o
+obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_DAC124S085)  += leds-dac124s085.o
diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
new file mode 100644
index 000..0016a87
--- /dev/null
+++ b/drivers/leds/leds-sc27xx-bltc.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* PMIC global control register definition */
+#define SC27XX_MODULE_EN0  0xc08
+#define SC27XX_CLK_EN0 0xc18
+#define SC27XX_RGB_CTRL0xebc
+
+#define SC27XX_BLTC_EN BIT(9)
+#define SC27XX_RTC_EN  BIT(7)
+#define SC27XX_RGB_PD  BIT(0)
+
+/* Breathing light controller register definition */
+#define SC27XX_LEDS_CTRL   0x00
+#define SC27XX_LEDS_PRESCALE   0x04
+#define SC27XX_LEDS_DUTY   0x08
+#define SC27XX_LEDS_CURVE0 0x0c
+#define SC27XX_LEDS_CURVE1 0x10
+
+#define SC27XX_CTRL_SHIFT  4
+#define SC27XX_LED_RUN BIT(0)
+#define SC27XX_LED_TYPEBIT(1)
+
+#define SC27XX_DUTY_SHIFT  8
+#define SC27XX_DUTY_MASK   GENMASK(15, 0)
+#define SC27XX_MOD_MASKGENMASK(7, 0)
+
+#define SC27XX_CURVE_SHIFT 8
+#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
+#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
+
+#define SC27XX_LEDS_OFFSET 0x10
+#define SC27XX_LEDS_MAX3
+
+/*
+ * The SC27xx breathing light controller can support 3 outputs: red LED,
+ * green LED and blue LED. Each LED can support normal PWM mode and breathing
+ * mode.
+ *
+ * In breathing mode, the LED output curve includes raise, high, fall and low 4
+ * stages, and the duration of each stage is configurable.
+ */
+enum sc27xx_led_config {
+   SC27XX_RAISE_TIME,
+   SC27XX_FALL_TIME,
+   SC27XX_HIGH_TIME,
+   SC27XX_LOW_TIME,
+};
+
+struct sc27xx_led {
+   struct led_classdev ldev;
+   struct sc27xx_led_priv *priv;
+   enum led_brightness value;
+   u8 line;
+   bool breath_mode;
+   bool active;
+};
+
+struct sc27xx_led_priv {
+   struct sc27xx_led leds[SC27XX_LEDS_MAX];
+   struct regmap *regmap;
+   u32 base;
+};
+
+#define to_sc27xx_led(ldev) \
+   container_of(ldev, struct sc27xx_led, ldev)
+
+static int sc27xx_led_init(struct regmap *regmap)
+{
+   int err;
+
+   err = regmap_update_bits(regmap, SC27XX_MODULE_EN0, SC27XX_BLTC_EN,
+SC27XX_BLTC_EN);
+   if (err)
+   return err;
+
+   err = regmap_update_bits(regmap, SC27XX_CLK_EN0, SC27XX_RTC_EN,
+SC27XX_RTC_EN);
+   if (err)
+   return err;
+
+   return regmap_update_bits(regmap, SC27XX_RGB_CTRL, SC27XX_RGB_PD, 0);
+}
+
+static u32 sc27xx_led_get_offset(struct sc27xx_led *leds)
+{
+   return leds->priv->base + SC27XX_LEDS_OFFSET * leds->line;
+}
+
+static int sc27xx_led_enable(struct sc27xx_led *leds)
+{
+   u32 base = sc27xx_led_get_offset(leds);
+   u32 ctrl_base = leds->priv->base + SC27XX_LEDS_CTRL;
+   u8 ctrl_shift = SC27XX_CTRL_SHIFT * leds->line;
+   struct regmap *regmap = leds->priv->regmap;