Skip to contentSkip to conversation

leds: Add Broadchip BCT3024 LED driver

PATCH In review Matus Gajdos 3 participants 8 messages Jul 27, 2023 – Jul 28, 2023

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover leds: Add Broadchip BCT3024 LED driver

Hi,

I'm sending a series of patches to add support for the Broadchip BCT3024 LED driver. The first adds Broadchip to the DT vendor prefixes, the second is the DT bindings documentation and the third is the driver itself.

The Broadchip BCT3024 is an I2C LED driver with 24 independent channels, each with 256 brightness levels. It is targeted mainly for mobile phones and other hand-held devices.

As we use the chip in a device that needs to keep its power consumption to a minimum, I added support for the runtime PM to let the kernel disable the chip and its power supply when brightness is set to zero.

Any comments are welcome.

Matus Gajdos (3):
dt-bindings: Add vendor prefix for Broadchip Technology Group Co.,
Ltd.
dt-bindings: leds: Add binding for Broadchip BCT3024 LED driver
leds: Add Broadchip BCT3024 LED driver

 .../bindings/leds/broadchip,bct3024.yaml      |  89 +++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/leds/Kconfig                          |   9 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-bct3024.c                   | 564 ++++++++++++++++++
 5 files changed, 665 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml
 create mode 100644 drivers/leds/leds-bct3024.c
Signature · 1 line
2.25.1
#1 lore raw

PATCH 1/3 dt-bindings: Add vendor prefix for Broadchip Technology Group Co., Ltd.

Website: http://www.broadchip.com

---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
index af60bf1a6664..dc1ed3dc0c9f 100644
@@ -202,6 +202,8 @@ patternProperties:
     description: Bosch Sensortec GmbH
   "^boundary,.*":
     description: Boundary Devices Inc.
+  "^broadchip,.*":
+    description: Broadchip Technology Group Co., Ltd.
   "^brcm,.*":
     description: Broadcom Corporation
   "^broadmobi,.*":
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH 1/3] dt-bindings: Add vendor prefix for Broadchip Technology Group Co., Ltd.

Quoted 12 lines · Matus Gajdos

Website: http://www.broadchip.com

---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/vendor-prefixes.yaml +0 -0
index af60bf1a6664..dc1ed3dc0c9f 100644

Documentation/devicetree/bindings/vendor-prefixes.yaml patternProperties: · line 207

Quoted 6 lines
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
@@ -202,6 +202,8 @@ patternProperties:
     description: Bosch Sensortec GmbH
   "^boundary,.*":
     description: Boundary Devices Inc.
+  "^broadchip,.*":
+    description: Broadchip Technology Group Co., Ltd.
   "^brcm,.*":

Does not look ordered. 'c' is before 'o'.

Best regards,
Krzysztof

#3 lore raw

PATCH 2/3 dt-bindings: leds: Add binding for Broadchip BCT3024 LED driver

In reply to Matus Gajdos

The BCT3024 chip is an I2C LED driver with independent 24 output channels. Each channel supports 256 levels.

---
 .../bindings/leds/broadchip,bct3024.yaml      | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml
Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml new file +89 -0
new file mode 100644
index 000000000000..0d622894e79c
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/broadchip,bct3024.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadchip BCT3024 LED Driver
+
+maintainers:
+  - Matus Gajdos <[email protected]>
+
+description: |
+  The BCT3024 is an I2C LED driver with independent 24 output channels. Each
+  channel supports 256 levels and its output current can be scaled by a factor
+  of 1, 1/2, 1/3 and 1/4.
+
+properties:
+  compatible:
+    const: broadchip,bct3024
+
+  reg:
+    description: I2C slave address of the driver.
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator providing power to the VDD pin.
+
+  shutdown-gpios:
+    maxItems: 1
+    description: GPIO attached to the SDB pin.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^led@[0-9a-f]+$":
+    type: object
+    $ref: common.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        description: Index of the LED channel.
+        minimum: 0
+        maximum: 23
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@3c {
+            compatible = "broadchip,bct3024";
+            reg = <0x3c>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            vdd-supply = <&reg_3v3d>;
+
+            led@0 {
+                reg = <0x00>;
+                function = LED_FUNCTION_INDICATOR;
+                color = <LED_COLOR_ID_RED>;
+            };
+
+            led@1 {
+                reg = <0x01>;
+                function = LED_FUNCTION_INDICATOR;
+                color = <LED_COLOR_ID_GREEN>;
+            };
+        };
+    };
+
+...
Signature · 1 line
2.25.1
#4 lore raw

Re: [PATCH 2/3] dt-bindings: leds: Add binding for Broadchip BCT3024 LED driver

Quoted 4 lines · Matus Gajdos

The BCT3024 chip is an I2C LED driver with independent 24 output channels. Each channel supports 256 levels.

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

A nit, subject: drop second/last, redundant "binding for". The "dt-bindings" prefix is already stating that these are bindings.

Quoted 23 lines
---
 .../bindings/leds/broadchip,bct3024.yaml      | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml
Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml new file +12 -0
new file mode 100644
index 000000000000..0d622894e79c
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/broadchip,bct3024.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadchip BCT3024 LED Driver
+
+maintainers:
+  - Matus Gajdos <[email protected]>
+
+description: |

Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml line 22

Quoted 10 lines
Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml new file +10 -0
+  The BCT3024 is an I2C LED driver with independent 24 output channels. Each
+  channel supports 256 levels and its output current can be scaled by a factor
+  of 1, 1/2, 1/3 and 1/4.
+
+properties:
+  compatible:
+    const: broadchip,bct3024
+
+  reg:
+    description: I2C slave address of the driver.

Drop description, it is obvious.

Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml line 28

Quoted 6 lines
Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml new file +6 -0
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator providing power to the VDD pin.
+
+  shutdown-gpios:

powerdown-gpios instead. See gpio-consumer-common.yaml

Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml line 29

Quoted 1 line
Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml new file +1 -0
+    maxItems: 1

And this won't be needed.

Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml line 37

Quoted 8 lines
Documentation/devicetree/bindings/leds/broadchip,bct3024.yaml new file +8 -0
+    description: GPIO attached to the SDB pin.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+

Best regards,
Krzysztof

#5 lore raw

PATCH 3/3 leds: Add Broadchip BCT3024 LED driver

In reply to Matus Gajdos

The BCT3024 chip is an I2C LED driver with independent 24 output channels. Each channel supports 256 levels.

---
 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
drivers/leds/Kconfig +9 -0
index 6046dfeca16f..75059db201e2 100644
@@ -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
drivers/leds/Makefile +1 -0
index d71f1226540c..f9eaed4c2317 100644
@@ -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
drivers/leds/leds-bct3024.c new file +564 -0

564 lines not on this page · raw message

Signature · 1 line
2.25.1
#6 lore raw
kernel test robot noticed the following build errors:

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/[email protected]/config)
compiler: sh4-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230728/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#7 lore raw
In reply to Matus Gajdos
Quoted 10 lines · Matus Gajdos

The BCT3024 chip is an I2C LED driver with independent 24 output channels. Each channel supports 256 levels.

---
 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.

Quoted 8 lines
drivers/leds/Kconfig +0 -0
index 6046dfeca16f..75059db201e2 100644
@@ -135,6 +135,15 @@ config LEDS_BCM6358
 	  This option enables support for LEDs connected to the BCM6358
 	  LED HW controller accessed via

...

Quoted 13 lines
drivers/leds/Kconfig +13 -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;

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+
+	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.

drivers/leds/Kconfig

Quoted 5 lines
drivers/leds/Kconfig +5 -0
+
+	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...

Quoted 36 lines
drivers/leds/Kconfig +36 -0
+	}
+
+	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,

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+			     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

drivers/leds/Kconfig

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

Ditto

Quoted 4 lines
drivers/leds/Kconfig +4 -0
+	}
+
+	led->active = true;
+	led->priv = priv;

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+	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

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+		}
+	} else
+		(*prev)->next = led;
+
+	*prev = led;
+
+	return 0;
+}
+
+static int bct3024_of_parse(struct i2c_client *client)

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+{
+	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

Quoted 7 lines
drivers/leds/Kconfig +7 -0
+	}
+
+	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,

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+		};
+		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

Quoted 25 lines
drivers/leds/Kconfig +25 -0
+		}
+
+		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)

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+		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

drivers/leds/Kconfig

Quoted 9 lines
drivers/leds/Kconfig +9 -0
+		}
+		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...

drivers/leds/Kconfig

Quoted 6 lines
drivers/leds/Kconfig +6 -0
+	}
+
+	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....

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+	}
+
+	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.

Quoted 9 lines
drivers/leds/Kconfig +9 -0
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		goto fail;
+
+	ret = bct3024_of_parse(client);

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+	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?

drivers/leds/Kconfig

Quoted 2 lines
drivers/leds/Kconfig +2 -0
+	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?

Quoted 15 lines
drivers/leds/Kconfig +15 -0
+	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);

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+
+	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.

drivers/leds/Kconfig

Quoted 9 lines
drivers/leds/Kconfig +9 -0
+
+	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

Quoted 8 lines
drivers/leds/Kconfig +8 -0
+
+	switch (priv->state) {
+	case BCT3024_STATE_INIT:
+	case BCT3024_STATE_SHUTDOWN:
+		return 0;
+	default:
+		break;
+	}

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+
+	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

Quoted 35 lines
drivers/leds/Kconfig +35 -0
+
+	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,
+	},

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+	.probe_new = bct3024_i2c_probe,
+	.shutdown = bct3024_i2c_shutdown,
+	.remove = bct3024_i2c_remove,
+};
+
+module_i2c_driver(bct3024_driver);
+
+MODULE_AUTHOR("Matus Gajdos <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Broadchip BCT3024 LED driver");

Best regards,
Krzysztof