diff mbox series

[3/3] leds: Add Broadchip BCT3024 LED driver

Message ID 20230727160525.23312-4-matuszpd@gmail.com
State New
Headers show
Series leds: Add Broadchip BCT3024 LED driver | expand

Commit Message

Matus Gajdos July 27, 2023, 4:05 p.m. UTC 
The BCT3024 chip is an I2C LED driver with independent 24 output
channels. Each channel supports 256 levels.

Signed-off-by: Matus Gajdos <matuszpd@gmail.com>
---
 drivers/leds/Kconfig        |   9 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-bct3024.c | 564 ++++++++++++++++++++++++++++++++++++
 3 files changed, 574 insertions(+)
 create mode 100644 drivers/leds/leds-bct3024.c

Comments

kernel test robot July 28, 2023, 4:20 a.m. UTC | #1 
Hi Matus,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.5-rc3 next-20230727]
[cannot apply to pavel-leds/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matus-Gajdos/dt-bindings-Add-vendor-prefix-for-Broadchip-Technology-Group-Co-Ltd/20230728-001607
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20230727160525.23312-4-matuszpd%40gmail.com
patch subject: [PATCH 3/3] leds: Add Broadchip BCT3024 LED driver
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20230728/202307281212.NIwa5Cab-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230728/202307281212.NIwa5Cab-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307281212.NIwa5Cab-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/device.h:25,
                    from include/linux/acpi.h:14,
                    from include/linux/i2c.h:13,
                    from drivers/leds/leds-bct3024.c:8:
>> drivers/leds/leds-bct3024.c:538:28: error: 'bct3024_runtime_suspend' undeclared here (not in a function); did you mean 'pm_runtime_suspend'?
     538 |         SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
         |                            ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:337:28: note: in definition of macro 'RUNTIME_PM_OPS'
     337 |         .runtime_suspend = suspend_fn, \
         |                            ^~~~~~~~~~
   drivers/leds/leds-bct3024.c:538:9: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
     538 |         SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
         |         ^~~~~~~~~~~~~~~~~~
>> drivers/leds/leds-bct3024.c:538:53: error: 'bct3024_runtime_resume' undeclared here (not in a function); did you mean 'pm_runtime_resume'?
     538 |         SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
         |                                                     ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:338:27: note: in definition of macro 'RUNTIME_PM_OPS'
     338 |         .runtime_resume = resume_fn, \
         |                           ^~~~~~~~~
   drivers/leds/leds-bct3024.c:538:9: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
     538 |         SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
         |         ^~~~~~~~~~~~~~~~~~
>> drivers/leds/leds-bct3024.c:539:28: error: 'bct3024_runtime_idle' undeclared here (not in a function); did you mean 'pm_runtime_idle'?
     539 |                            bct3024_runtime_idle)
         |                            ^~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:339:25: note: in definition of macro 'RUNTIME_PM_OPS'
     339 |         .runtime_idle = idle_fn,
         |                         ^~~~~~~
   drivers/leds/leds-bct3024.c:538:9: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
     538 |         SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
         |         ^~~~~~~~~~~~~~~~~~


vim +538 drivers/leds/leds-bct3024.c

   536	
   537	static const struct dev_pm_ops bct3024_pm_ops = {
 > 538		SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
 > 539				   bct3024_runtime_idle)
   540		SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
   541	};
   542
Krzysztof Kozlowski July 28, 2023, 7:41 a.m. UTC | #2 
On 27/07/2023 18:05, Matus Gajdos wrote:
> The BCT3024 chip is an I2C LED driver with independent 24 output
> channels. Each channel supports 256 levels.
> 
> Signed-off-by: Matus Gajdos <matuszpd@gmail.com>
> ---
>  drivers/leds/Kconfig        |   9 +
>  drivers/leds/Makefile       |   1 +
>  drivers/leds/leds-bct3024.c | 564 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 574 insertions(+)
>  create mode 100644 drivers/leds/leds-bct3024.c

Thank you for your patch. There is something to discuss/improve.


> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 6046dfeca16f..75059db201e2 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -135,6 +135,15 @@ config LEDS_BCM6358
>  	  This option enables support for LEDs connected to the BCM6358
>  	  LED HW controller accessed via

...

> +}
> +
> +static int bct3024_brightness_set(struct led_classdev *ldev,
> +				  enum led_brightness brightness)
> +{
> +	struct bct3024_led *led = container_of(ldev, struct bct3024_led, ldev);
> +	struct bct3024_data *priv = led->priv;
> +	struct device *dev = priv->dev;
> +	int ret;
> +	unsigned int ctrl, bright;
> +
> +	if (priv->state == BCT3024_STATE_INIT)
> +		return -EIO;
> +
> +	if (brightness == 0) {
> +		ctrl = 0x00;
> +		bright = 0;
> +	} else if (brightness < 256) {
> +		ctrl = 0x01;
> +		bright = brightness;
> +	}
> +
> +	mutex_lock(&priv->lock);

What do you protect with lock? This must be documented in lock's
definition in your struct.

> +
> +	if (bct3024_any_active(priv) && (priv->state == BCT3024_STATE_IDLE ||
> +	    priv->state == BCT3024_STATE_SHUTDOWN)) {
> +		pm_runtime_get_sync(dev);
> +		priv->state = BCT3024_STATE_ACTIVE;

I don't understand why you added state machine for handling state. You
are basically duplicating runtime PM...

> +	}
> +
> +	for (; led; led = led->next) {
> +		ret = bct3024_write(priv, BCT3024_REG_CONTROL(led->idx), ctrl);
> +		if (ret < 0)
> +			goto exit;
> +		ret = bct3024_write(priv, BCT3024_REG_BRIGHTNESS(led->idx), bright);
> +		if (ret < 0)
> +			goto exit;
> +	}
> +
> +	ret = bct3024_write(priv, BCT3024_REG_UPDATE, 0);
> +	if (ret < 0)
> +		goto exit;
> +
> +	ret = bct3024_set_shutdown_reg(priv, false);
> +	if (ret < 0)
> +		goto exit;
> +
> +	if (!ret && priv->state == BCT3024_STATE_ACTIVE) {
> +		priv->state = BCT3024_STATE_IDLE;
> +		pm_runtime_mark_last_busy(dev);
> +		pm_runtime_put_autosuspend(dev);
> +		/* Activate autosuspend */
> +		pm_runtime_idle(dev);
> +	}
> +
> +	ret = 0;
> +
> +exit:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int bct3024_setup_led(struct bct3024_data *priv, u32 reg,
> +			     struct bct3024_led **prev, struct led_init_data *idata)
> +{
> +	struct device *dev = priv->dev;
> +	struct bct3024_led *led;
> +	int ret;
> +
> +	if (reg >= BCT3024_LED_COUNT) {
> +		ret = -EINVAL;
> +		dev_err_probe(dev, ret, "invalid reg value: %u\n", reg);
> +		return ret;

That's not correct syntax.

return dev_err_probe
> +	}
> +
> +	led = &priv->leds[reg];
> +
> +	if (led->active) {
> +		ret = -EINVAL;
> +		dev_err_probe(dev, ret, "reg redeclared: %u\n", reg);
> +		return ret;

Ditto

> +	}
> +
> +	led->active = true;
> +	led->priv = priv;
> +	led->idx = reg;
> +
> +	if (!*prev) {
> +		led->ldev.max_brightness = 255;
> +		led->ldev.brightness_set_blocking = bct3024_brightness_set;
> +
> +		ret = devm_led_classdev_register_ext(dev, &led->ldev, idata);
> +		if (ret < 0) {
> +			dev_err_probe(dev, ret, "failed to register led %u\n", reg);
> +			return ret;

Ditto

> +		}
> +	} else
> +		(*prev)->next = led;
> +
> +	*prev = led;
> +
> +	return 0;
> +}
> +
> +static int bct3024_of_parse(struct i2c_client *client)
> +{
> +	struct bct3024_data *priv = i2c_get_clientdata(client);
> +	struct device *dev = priv->dev;
> +	struct device_node *np;
> +	int ret;
> +
> +	ret = of_get_child_count(dev->of_node);
> +	if (!ret || ret > ARRAY_SIZE(priv->leds)) {
> +		dev_err_probe(dev, -EINVAL, "invalid nodes count: %d\n", ret);
> +		return -EINVAL;

Ditto

> +	}
> +
> +	for_each_child_of_node(dev->of_node, np) {
> +		u32 regs[BCT3024_LED_COUNT];
> +		struct led_init_data idata = {
> +			.fwnode = of_fwnode_handle(np),
> +			.devicename = client->name,
> +		};
> +		struct bct3024_led *led;
> +		int count, i;
> +
> +		count = of_property_read_variable_u32_array(np, "reg", regs, 1,
> +							    BCT3024_LED_COUNT);
> +		if (count < 0) {
> +			ret = count;
> +			dev_err_probe(dev, ret, "failed to read node reg\n");
> +			goto fail;

Ditto

> +		}
> +
> +		for (i = 0, led = NULL; i < count; i++) {
> +			ret = bct3024_setup_led(priv, regs[i], &led, &idata);
> +			if (ret < 0)
> +				goto fail;
> +		}
> +	}
> +
> +	return 0;
> +
> +fail:
> +	of_node_put(np);
> +
> +	return ret;
> +}
> +
> +static int bct3024_i2c_probe(struct i2c_client *client)
> +{
> +	struct bct3024_data *priv;
> +	struct device *dev = &client->dev;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +
> +	priv->supply = devm_regulator_get_optional(dev, "vdd");
> +	if (IS_ERR(priv->supply)) {
> +		ret = PTR_ERR(priv->supply);
> +		if (ret != -ENODEV) {
> +			dev_err_probe(dev, ret, "failed to get supply\n");
> +			return ret;

Ditto

> +		}
> +		priv->supply = NULL;
> +	}
> +
> +	priv->shutdown_gpiod = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->shutdown_gpiod)) {
> +		ret = PTR_ERR(priv->shutdown_gpiod);
> +		dev_err_probe(dev, ret, "failed to get shutdown gpio\n");
> +		return ret;

Everywhere...

> +	}
> +
> +	priv->regmap = devm_regmap_init_i2c(client, &bct3024_regmap);
> +	if (IS_ERR(priv->regmap)) {
> +		ret = PTR_ERR(priv->regmap);
> +		return ret;


It's return PTR_ERR....

> +	}
> +
> +	mutex_init(&priv->lock);
> +	i2c_set_clientdata(client, priv);
> +
> +	pm_runtime_set_autosuspend_delay(dev, 2000);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_enable(dev);
> +	if (!pm_runtime_enabled(dev)) {
> +		ret = bct3024_shutdown(priv, false);

This should go to error handling path... Although why? There was no
power on code between devm_regmap_init_i2c() and here.

> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0)
> +		goto fail;
> +
> +	ret = bct3024_of_parse(client);
> +	if (ret < 0)
> +		goto fail;
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return 0;
> +
> +fail:
> +	dev_err_probe(dev, ret, "probe failed\n");

No, no no. Do you see any driver doing it?

> +	if (!pm_runtime_status_suspended(dev))
> +		bct3024_shutdown(priv, 2);

Which call this is reversing? Each step in probe should have its reverse
(when applicable of course). Don't add some new unrelated reverse calls
which do not have a matching enable. If you shutdown here, I expect
there was "bct3024_powerup". Where is it? Looks like you put unrelated
code into the shutdown making it all very difficult to understand.

Where do you reverse PM runtime calls here?

> +	return ret;
> +}
> +
> +static void bct3024_i2c_remove(struct i2c_client *client)
> +{
> +	struct bct3024_data *priv = i2c_get_clientdata(client);
> +
> +	bct3024_shutdown(priv, true);
> +	pm_runtime_disable(priv->dev);
> +	mutex_destroy(&priv->lock);
> +}
> +
> +static void bct3024_i2c_shutdown(struct i2c_client *client)
> +{
> +	struct bct3024_data *priv = i2c_get_clientdata(client);
> +
> +	bct3024_shutdown(priv, true);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int bct3024_runtime_idle(struct device *dev)
> +{
> +	struct bct3024_data *priv = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "%s: %d\n", __func__, priv->state);

No simple debug statements for entry/exit of functions. There is
extensive trace infrastructure for all this.

> +
> +	return priv->state == BCT3024_STATE_ACTIVE ? -EBUSY : 0;
> +}
> +
> +static int bct3024_runtime_suspend(struct device *dev)
> +{
> +	struct bct3024_data *priv = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "%s: %d\n", __func__, priv->state);

Ditto

> +
> +	switch (priv->state) {
> +	case BCT3024_STATE_INIT:
> +	case BCT3024_STATE_SHUTDOWN:
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	return bct3024_shutdown(priv, true);
> +}
> +
> +static int bct3024_runtime_resume(struct device *dev)
> +{
> +	struct bct3024_data *priv = dev_get_drvdata(dev);
> +	int ret;
> +
> +	dev_dbg(dev, "%s: %d\n", __func__, priv->state);

Ditto

> +
> +	switch (priv->state) {
> +	case BCT3024_STATE_INIT:
> +	case BCT3024_STATE_SHUTDOWN:
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	ret = bct3024_shutdown(priv, false);
> +	if (ret < 0)
> +		bct3024_shutdown(priv, 2);
> +
> +	return ret;
> +}
> +#endif
> +
> +static const struct dev_pm_ops bct3024_pm_ops = {
> +	SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
> +			   bct3024_runtime_idle)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> +};
> +
> +static const struct of_device_id bct3024_of_match[] = {
> +	{ .compatible = "broadchip,bct3024" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, bct3024_of_match);
> +
> +static struct i2c_driver bct3024_driver = {
> +	.driver = {
> +		.name = "bct3024",
> +		.of_match_table = bct3024_of_match,
> +		.pm = &bct3024_pm_ops,
> +	},
> +	.probe_new = bct3024_i2c_probe,
> +	.shutdown = bct3024_i2c_shutdown,
> +	.remove = bct3024_i2c_remove,
> +};
> +
> +module_i2c_driver(bct3024_driver);
> +
> +MODULE_AUTHOR("Matus Gajdos <matuszpd@gmail.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Broadchip BCT3024 LED driver");

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6046dfeca16f..75059db201e2 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -135,6 +135,15 @@  config LEDS_BCM6358
 	  This option enables support for LEDs connected to the BCM6358
 	  LED HW controller accessed via MMIO registers.
 
+config LEDS_BCT3024
+	tristate "LED Support for Broadchip BCT3024"
+	depends on LEDS_CLASS
+	depends on I2C
+	depends on OF
+	help
+	  This option enables support for LEDs connected to the BCT3024
+	  LED driver.
+
 config LEDS_CHT_WCOVE
 	tristate "LED support for Intel Cherry Trail Whiskey Cove PMIC"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index d71f1226540c..f9eaed4c2317 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -18,6 +18,7 @@  obj-$(CONFIG_LEDS_AW200XX)		+= leds-aw200xx.o
 obj-$(CONFIG_LEDS_AW2013)		+= leds-aw2013.o
 obj-$(CONFIG_LEDS_BCM6328)		+= leds-bcm6328.o
 obj-$(CONFIG_LEDS_BCM6358)		+= leds-bcm6358.o
+obj-$(CONFIG_LEDS_BCT3024)		+= leds-bct3024.o
 obj-$(CONFIG_LEDS_BD2606MVV)		+= leds-bd2606mvv.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_BLINKM)		+= leds-blinkm.o
diff --git a/drivers/leds/leds-bct3024.c b/drivers/leds/leds-bct3024.c
new file mode 100644
index 000000000000..7732fe022093
--- /dev/null
+++ b/drivers/leds/leds-bct3024.c
@@ -0,0 +1,564 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * LED driver for Broadchip BCT3024, based on leds-syscon.c
+ *
+ * Copyright 2023 Matus Gajdos <matuszpd@gmail.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+#define BCT3024_LED_COUNT		36
+#define BCT3024_REG_SHUTDOWN		(0x00)
+#define BCT3024_REG_BRIGHTNESS(n)	(0x01 + (n))
+#define BCT3024_REG_UPDATE		(0x25)
+#define BCT3024_REG_CONTROL(n)		(0x26 + (n))
+#define BCT3024_REG_GLOBAL_CONTROL	(0x4a)
+#define BCT3024_REG_RESET		(0x4f)
+
+enum bct3024_state {
+	BCT3024_STATE_INIT,
+	BCT3024_STATE_SHUTDOWN,
+	BCT3024_STATE_IDLE,
+	BCT3024_STATE_ACTIVE,
+};
+
+struct bct3024_led {
+	struct bct3024_led *next;
+	bool active;
+	unsigned int idx;
+	struct led_classdev ldev;
+	struct bct3024_data *priv;
+};
+
+struct bct3024_data {
+	enum bct3024_state state;
+	struct device *dev;
+	struct gpio_desc *shutdown_gpiod;
+	struct regulator *supply;
+	struct regmap *regmap;
+	struct mutex lock;
+	struct bct3024_led leds[BCT3024_LED_COUNT];
+};
+
+static const struct reg_default bct3024_defaults[] = {
+	{ BCT3024_REG_SHUTDOWN,		0x00 },
+	{ BCT3024_REG_BRIGHTNESS(0),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(1),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(2),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(3),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(4),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(5),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(6),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(7),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(8),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(9),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(10),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(11),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(12),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(13),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(14),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(15),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(16),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(17),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(18),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(19),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(20),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(21),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(22),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(23),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(24),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(25),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(26),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(27),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(28),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(29),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(30),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(31),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(32),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(33),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(34),	0x00 },
+	{ BCT3024_REG_BRIGHTNESS(35),	0x00 },
+	{ BCT3024_REG_UPDATE,		0x00 },
+	{ BCT3024_REG_CONTROL(0),	0x00 },
+	{ BCT3024_REG_CONTROL(1),	0x00 },
+	{ BCT3024_REG_CONTROL(2),	0x00 },
+	{ BCT3024_REG_CONTROL(3),	0x00 },
+	{ BCT3024_REG_CONTROL(4),	0x00 },
+	{ BCT3024_REG_CONTROL(5),	0x00 },
+	{ BCT3024_REG_CONTROL(6),	0x00 },
+	{ BCT3024_REG_CONTROL(7),	0x00 },
+	{ BCT3024_REG_CONTROL(8),	0x00 },
+	{ BCT3024_REG_CONTROL(9),	0x00 },
+	{ BCT3024_REG_CONTROL(10),	0x00 },
+	{ BCT3024_REG_CONTROL(11),	0x00 },
+	{ BCT3024_REG_CONTROL(12),	0x00 },
+	{ BCT3024_REG_CONTROL(13),	0x00 },
+	{ BCT3024_REG_CONTROL(14),	0x00 },
+	{ BCT3024_REG_CONTROL(15),	0x00 },
+	{ BCT3024_REG_CONTROL(16),	0x00 },
+	{ BCT3024_REG_CONTROL(17),	0x00 },
+	{ BCT3024_REG_CONTROL(18),	0x00 },
+	{ BCT3024_REG_CONTROL(19),	0x00 },
+	{ BCT3024_REG_CONTROL(20),	0x00 },
+	{ BCT3024_REG_CONTROL(21),	0x00 },
+	{ BCT3024_REG_CONTROL(22),	0x00 },
+	{ BCT3024_REG_CONTROL(23),	0x00 },
+	{ BCT3024_REG_CONTROL(24),	0x00 },
+	{ BCT3024_REG_CONTROL(25),	0x00 },
+	{ BCT3024_REG_CONTROL(26),	0x00 },
+	{ BCT3024_REG_CONTROL(27),	0x00 },
+	{ BCT3024_REG_CONTROL(28),	0x00 },
+	{ BCT3024_REG_CONTROL(29),	0x00 },
+	{ BCT3024_REG_CONTROL(30),	0x00 },
+	{ BCT3024_REG_CONTROL(31),	0x00 },
+	{ BCT3024_REG_CONTROL(32),	0x00 },
+	{ BCT3024_REG_CONTROL(33),	0x00 },
+	{ BCT3024_REG_CONTROL(34),	0x00 },
+	{ BCT3024_REG_CONTROL(35),	0x00 },
+	{ BCT3024_REG_GLOBAL_CONTROL,	0x00 },
+	{ BCT3024_REG_RESET,		0x00 },
+};
+
+static bool bct3024_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case BCT3024_REG_UPDATE:
+	case BCT3024_REG_RESET:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool bct3024_readable_reg(struct device *dev, unsigned int reg)
+{
+	/* The chip doesn't support i2c read */
+	return false;
+}
+
+static bool bct3024_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case BCT3024_REG_SHUTDOWN:
+	case BCT3024_REG_BRIGHTNESS(0) ... BCT3024_REG_BRIGHTNESS(35):
+	case BCT3024_REG_UPDATE:
+	case BCT3024_REG_CONTROL(0) ... BCT3024_REG_CONTROL(35):
+	case BCT3024_REG_GLOBAL_CONTROL:
+	case BCT3024_REG_RESET:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config bct3024_regmap = {
+	.reg_bits		= 8,
+	.val_bits		= 8,
+	.max_register		= BCT3024_REG_RESET,
+	.reg_defaults		= bct3024_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(bct3024_defaults),
+	.volatile_reg		= bct3024_volatile_reg,
+	.readable_reg		= bct3024_readable_reg,
+	.writeable_reg		= bct3024_writeable_reg,
+	.cache_type             = REGCACHE_FLAT,
+};
+
+static int bct3024_write(struct bct3024_data *priv, unsigned int reg,
+			 unsigned int val)
+{
+	int ret = regmap_write(priv->regmap, reg, val);
+
+	if (ret < 0)
+		dev_err(priv->dev, "failed to write register 0x%x: %d\n", reg, ret);
+
+	return ret;
+}
+
+static bool bct3024_any_active(struct bct3024_data *priv)
+{
+	int i;
+
+	for (i = ARRAY_SIZE(priv->leds); i--; )
+		if (priv->leds[i].active && priv->leds[i].ldev.brightness)
+			return true;
+
+	return false;
+}
+
+static int bct3024_set_shutdown_reg(struct bct3024_data *priv, bool shutdown)
+{
+	unsigned int val = shutdown || !bct3024_any_active(priv) ? 0 : 1;
+	int ret = bct3024_write(priv, BCT3024_REG_SHUTDOWN, val);
+
+	return ret < 0 ? ret : val;
+}
+
+static int bct3024_shutdown(struct bct3024_data *priv, int active)
+{
+	struct device *dev = priv->dev;
+	int is_off = priv->state == BCT3024_STATE_INIT ||
+		     priv->state == BCT3024_STATE_SHUTDOWN;
+	int ret;
+
+	if (is_off == active) {
+		/* Nothing to do here */
+	} else if (active) {
+		priv->state = BCT3024_STATE_SHUTDOWN;
+
+		bct3024_set_shutdown_reg(priv, true);
+		regcache_cache_only(priv->regmap, true);
+
+		if (priv->shutdown_gpiod)
+			gpiod_set_value_cansleep(priv->shutdown_gpiod, 1);
+
+		if (priv->supply) {
+			ret = regulator_disable(priv->supply);
+			if (ret < 0)
+				dev_err(dev, "failed to disable supply: %d\n", ret);
+		}
+	} else {
+		if (priv->supply) {
+			ret = regulator_enable(priv->supply);
+			if (ret < 0) {
+				dev_err(dev, "failed to enable supply: %d\n", ret);
+				return ret;
+			}
+		}
+
+		if (priv->shutdown_gpiod)
+			gpiod_set_value_cansleep(priv->shutdown_gpiod, 0);
+
+		if (priv->state == BCT3024_STATE_SHUTDOWN) {
+			regcache_cache_only(priv->regmap, false);
+			regcache_mark_dirty(priv->regmap);
+			ret = regcache_sync(priv->regmap);
+			if (ret < 0) {
+				dev_err(dev, "failed to sync regmap: %d\n", ret);
+				return ret;
+			}
+		}
+
+		ret = bct3024_set_shutdown_reg(priv, false);
+		if (ret < 0)
+			return ret;
+
+		priv->state = ret ? BCT3024_STATE_ACTIVE : BCT3024_STATE_IDLE;
+	}
+
+	return 0;
+}
+
+static int bct3024_brightness_set(struct led_classdev *ldev,
+				  enum led_brightness brightness)
+{
+	struct bct3024_led *led = container_of(ldev, struct bct3024_led, ldev);
+	struct bct3024_data *priv = led->priv;
+	struct device *dev = priv->dev;
+	int ret;
+	unsigned int ctrl, bright;
+
+	if (priv->state == BCT3024_STATE_INIT)
+		return -EIO;
+
+	if (brightness == 0) {
+		ctrl = 0x00;
+		bright = 0;
+	} else if (brightness < 256) {
+		ctrl = 0x01;
+		bright = brightness;
+	}
+
+	mutex_lock(&priv->lock);
+
+	if (bct3024_any_active(priv) && (priv->state == BCT3024_STATE_IDLE ||
+	    priv->state == BCT3024_STATE_SHUTDOWN)) {
+		pm_runtime_get_sync(dev);
+		priv->state = BCT3024_STATE_ACTIVE;
+	}
+
+	for (; led; led = led->next) {
+		ret = bct3024_write(priv, BCT3024_REG_CONTROL(led->idx), ctrl);
+		if (ret < 0)
+			goto exit;
+		ret = bct3024_write(priv, BCT3024_REG_BRIGHTNESS(led->idx), bright);
+		if (ret < 0)
+			goto exit;
+	}
+
+	ret = bct3024_write(priv, BCT3024_REG_UPDATE, 0);
+	if (ret < 0)
+		goto exit;
+
+	ret = bct3024_set_shutdown_reg(priv, false);
+	if (ret < 0)
+		goto exit;
+
+	if (!ret && priv->state == BCT3024_STATE_ACTIVE) {
+		priv->state = BCT3024_STATE_IDLE;
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+		/* Activate autosuspend */
+		pm_runtime_idle(dev);
+	}
+
+	ret = 0;
+
+exit:
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int bct3024_setup_led(struct bct3024_data *priv, u32 reg,
+			     struct bct3024_led **prev, struct led_init_data *idata)
+{
+	struct device *dev = priv->dev;
+	struct bct3024_led *led;
+	int ret;
+
+	if (reg >= BCT3024_LED_COUNT) {
+		ret = -EINVAL;
+		dev_err_probe(dev, ret, "invalid reg value: %u\n", reg);
+		return ret;
+	}
+
+	led = &priv->leds[reg];
+
+	if (led->active) {
+		ret = -EINVAL;
+		dev_err_probe(dev, ret, "reg redeclared: %u\n", reg);
+		return ret;
+	}
+
+	led->active = true;
+	led->priv = priv;
+	led->idx = reg;
+
+	if (!*prev) {
+		led->ldev.max_brightness = 255;
+		led->ldev.brightness_set_blocking = bct3024_brightness_set;
+
+		ret = devm_led_classdev_register_ext(dev, &led->ldev, idata);
+		if (ret < 0) {
+			dev_err_probe(dev, ret, "failed to register led %u\n", reg);
+			return ret;
+		}
+	} else
+		(*prev)->next = led;
+
+	*prev = led;
+
+	return 0;
+}
+
+static int bct3024_of_parse(struct i2c_client *client)
+{
+	struct bct3024_data *priv = i2c_get_clientdata(client);
+	struct device *dev = priv->dev;
+	struct device_node *np;
+	int ret;
+
+	ret = of_get_child_count(dev->of_node);
+	if (!ret || ret > ARRAY_SIZE(priv->leds)) {
+		dev_err_probe(dev, -EINVAL, "invalid nodes count: %d\n", ret);
+		return -EINVAL;
+	}
+
+	for_each_child_of_node(dev->of_node, np) {
+		u32 regs[BCT3024_LED_COUNT];
+		struct led_init_data idata = {
+			.fwnode = of_fwnode_handle(np),
+			.devicename = client->name,
+		};
+		struct bct3024_led *led;
+		int count, i;
+
+		count = of_property_read_variable_u32_array(np, "reg", regs, 1,
+							    BCT3024_LED_COUNT);
+		if (count < 0) {
+			ret = count;
+			dev_err_probe(dev, ret, "failed to read node reg\n");
+			goto fail;
+		}
+
+		for (i = 0, led = NULL; i < count; i++) {
+			ret = bct3024_setup_led(priv, regs[i], &led, &idata);
+			if (ret < 0)
+				goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	of_node_put(np);
+
+	return ret;
+}
+
+static int bct3024_i2c_probe(struct i2c_client *client)
+{
+	struct bct3024_data *priv;
+	struct device *dev = &client->dev;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	priv->supply = devm_regulator_get_optional(dev, "vdd");
+	if (IS_ERR(priv->supply)) {
+		ret = PTR_ERR(priv->supply);
+		if (ret != -ENODEV) {
+			dev_err_probe(dev, ret, "failed to get supply\n");
+			return ret;
+		}
+		priv->supply = NULL;
+	}
+
+	priv->shutdown_gpiod = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->shutdown_gpiod)) {
+		ret = PTR_ERR(priv->shutdown_gpiod);
+		dev_err_probe(dev, ret, "failed to get shutdown gpio\n");
+		return ret;
+	}
+
+	priv->regmap = devm_regmap_init_i2c(client, &bct3024_regmap);
+	if (IS_ERR(priv->regmap)) {
+		ret = PTR_ERR(priv->regmap);
+		return ret;
+	}
+
+	mutex_init(&priv->lock);
+	i2c_set_clientdata(client, priv);
+
+	pm_runtime_set_autosuspend_delay(dev, 2000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+	if (!pm_runtime_enabled(dev)) {
+		ret = bct3024_shutdown(priv, false);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		goto fail;
+
+	ret = bct3024_of_parse(client);
+	if (ret < 0)
+		goto fail;
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
+
+fail:
+	dev_err_probe(dev, ret, "probe failed\n");
+	if (!pm_runtime_status_suspended(dev))
+		bct3024_shutdown(priv, 2);
+	return ret;
+}
+
+static void bct3024_i2c_remove(struct i2c_client *client)
+{
+	struct bct3024_data *priv = i2c_get_clientdata(client);
+
+	bct3024_shutdown(priv, true);
+	pm_runtime_disable(priv->dev);
+	mutex_destroy(&priv->lock);
+}
+
+static void bct3024_i2c_shutdown(struct i2c_client *client)
+{
+	struct bct3024_data *priv = i2c_get_clientdata(client);
+
+	bct3024_shutdown(priv, true);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bct3024_runtime_idle(struct device *dev)
+{
+	struct bct3024_data *priv = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "%s: %d\n", __func__, priv->state);
+
+	return priv->state == BCT3024_STATE_ACTIVE ? -EBUSY : 0;
+}
+
+static int bct3024_runtime_suspend(struct device *dev)
+{
+	struct bct3024_data *priv = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "%s: %d\n", __func__, priv->state);
+
+	switch (priv->state) {
+	case BCT3024_STATE_INIT:
+	case BCT3024_STATE_SHUTDOWN:
+		return 0;
+	default:
+		break;
+	}
+
+	return bct3024_shutdown(priv, true);
+}
+
+static int bct3024_runtime_resume(struct device *dev)
+{
+	struct bct3024_data *priv = dev_get_drvdata(dev);
+	int ret;
+
+	dev_dbg(dev, "%s: %d\n", __func__, priv->state);
+
+	switch (priv->state) {
+	case BCT3024_STATE_INIT:
+	case BCT3024_STATE_SHUTDOWN:
+		break;
+	default:
+		return 0;
+	}
+
+	ret = bct3024_shutdown(priv, false);
+	if (ret < 0)
+		bct3024_shutdown(priv, 2);
+
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops bct3024_pm_ops = {
+	SET_RUNTIME_PM_OPS(bct3024_runtime_suspend, bct3024_runtime_resume,
+			   bct3024_runtime_idle)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
+};
+
+static const struct of_device_id bct3024_of_match[] = {
+	{ .compatible = "broadchip,bct3024" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bct3024_of_match);
+
+static struct i2c_driver bct3024_driver = {
+	.driver = {
+		.name = "bct3024",
+		.of_match_table = bct3024_of_match,
+		.pm = &bct3024_pm_ops,
+	},
+	.probe_new = bct3024_i2c_probe,
+	.shutdown = bct3024_i2c_shutdown,
+	.remove = bct3024_i2c_remove,
+};
+
+module_i2c_driver(bct3024_driver);
+
+MODULE_AUTHOR("Matus Gajdos <matuszpd@gmail.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Broadchip BCT3024 LED driver");