[PATCH v2 0/2] Support for Texas Instruments TPS6131X flash LED driver

[PATCH v2 0/2] Support for Texas Instruments TPS6131X flash LED driver

[Matthias Fend]

The TPS61310/TPS61311 is a flash LED driver with I2C interface. Its power
stage is capable of supplying a maximum total current of roughly 1500mA.
The TPS6131x provides three constant-current sinks, capable of sinking up
to 2 × 400mA (LED1 and LED3) and 800mA (LED2) in flash mode. In torch mode
each sink (LED1, LED2, LED3) supports currents up to 175m

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
Changes in v2:
- Bindings: Extend device description
- Bindings: Drop unused address/size cells
- Bindings: Use fallback compatible 
- Bindings: Corrected minimum current for 50mA steps
- Bindings: Drop node label
- Fix name of REGISTER4 INDC shift define
- Save device instead i2c_client in private data
- Add comment for mutex
- Use macro to convert from uA to mA
- Use defines to describe initial register values
- Add safety delay during reset sequence
- Use fixed value enum to set the mode
- Renamed some local variables
- Re-sorted local variables
- Replaced ifdefs for V4L2_FLASH_LED_CLASS
- Improved some error messages
- Link to v1: https://lore.kernel.org/r/20250228-leds-tps6131x-v1-0-d1071d90f9ea@emfend.at

---
Matthias Fend (2):
      dt-bindings: leds: add Texas Instruments TPS6131x flash LED driver
      leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

 .../devicetree/bindings/leds/ti,tps61310.yaml      | 120 ++++
 MAINTAINERS                                        |   7 +
 drivers/leds/flash/Kconfig                         |  11 +
 drivers/leds/flash/Makefile                        |   1 +
 drivers/leds/flash/leds-tps6131x.c                 | 794 +++++++++++++++++++++
 5 files changed, 933 insertions(+)
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250227-leds-tps6131x-a7437883e400

Best regards,
-- 
Matthias Fend <matthias.fend@emfend.at>

[PATCH v2 1/2] dt-bindings: leds: add Texas Instruments TPS6131x flash LED driver

[Matthias Fend]

Document Texas Instruments TPS61310/TPS61311 flash LED driver devicetree
bindings.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
 .../devicetree/bindings/leds/ti,tps61310.yaml      | 120 +++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/ti,tps61310.yaml b/Documentation/devicetree/bindings/leds/ti,tps61310.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..1f631889a06a70185bb7ca2cef531e2618f83765
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ti,tps61310.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/ti,tps61310.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TPS6131X flash LED driver
+
+maintainers:
+  - Matthias Fend <matthias.fend@emfend.at>
+
+description: |
+  The TPS61310/TPS61311 is a flash LED driver with I2C interface.
+  Its power stage is capable of supplying a maximum total current of roughly 1500mA.
+  The TPS6131x provides three constant-current sinks, capable of sinking
+  up to 2 × 400mA (LED1 and LED3) and 800mA (LED2) in flash mode.
+  In torch mode, each sink (LED1, LED2, LED3) supports currents up to 175mA.
+  Since the three current sinks share most of the control components such as
+  flash timer, control logic, safety timer and the operating mode, they cannot
+  be used completely independently of each other. Therefore, only one LED is
+  supported, but the current sinks can be combined accordingly.
+
+  The data sheet can be found at:
+    https://www.ti.com/lit/ds/symlink/tps61310.pdf
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - ti,tps61311
+          - const: ti,tps61310
+      - items:
+          - const: ti,tps61310
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+    description: GPIO connected to NRESET pin
+
+  ti,valley-current-limit:
+    type: boolean
+    description:
+      Reduce the valley peak current limit from 1750mA to 1250mA (TPS61310) or
+      from 2480mA to 1800mA (TPS61311).
+
+  led:
+    type: object
+    $ref: common.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      led-sources:
+        minItems: 1
+        maxItems: 3
+        items:
+          enum: [1, 2, 3]
+
+      led-max-microamp:
+        oneOf:
+          - minimum: 50000
+            maximum: 350000
+            multipleOf: 50000
+          - minimum: 25000
+            maximum: 525000
+            multipleOf: 25000
+
+      flash-max-microamp:
+        oneOf:
+          - minimum: 50000
+            maximum: 800000
+            multipleOf: 50000
+          - minimum: 25000
+            maximum: 1500000
+            multipleOf: 25000
+
+      flash-max-timeout-us:
+        enum: [ 5300, 10700, 16000, 21300, 26600, 32000, 37300, 68200, 71500,
+                102200, 136300, 170400, 204500, 340800, 579300, 852000 ]
+
+    required:
+      - led-sources
+      - led-max-microamp
+      - flash-max-microamp
+      - flash-max-timeout-us
+
+required:
+  - compatible
+  - reg
+  - led
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      led-controller@33 {
+        compatible = "ti,tps61311", "ti,tps61310";
+        reg = <0x33>;
+
+        reset-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
+
+        led {
+          function = LED_FUNCTION_FLASH;
+          color = <LED_COLOR_ID_WHITE>;
+          led-sources = <1>, <2>, <3>;
+          led-max-microamp = <525000>;
+          flash-max-microamp = <1500000>;
+          flash-max-timeout-us = <852000>;
+        };
+      };
+    };

-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: leds: add Texas Instruments TPS6131x flash LED driver

[Krzysztof Kozlowski]

On Tue, Mar 18, 2025 at 08:28:57AM +0100, Matthias Fend wrote:
> Document Texas Instruments TPS61310/TPS61311 flash LED driver devicetree
> bindings.
> 
> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> ---
>  .../devicetree/bindings/leds/ti,tps61310.yaml      | 120 +++++++++++++++++++++
>  1 file changed, 120 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[Matthias Fend]

The TPS61310/TPS61311 is a flash LED driver with I2C interface. Its power
stage is capable of supplying a maximum total current of roughly 1500mA.
The TPS6131x provides three constant-current sinks, capable of sinking up
to 2 × 400mA (LED1 and LED3) and 800mA (LED2) in flash mode. In torch mode
each sink (LED1, LED2, LED3) supports currents up to 175mA.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
 MAINTAINERS                        |   7 +
 drivers/leds/flash/Kconfig         |  11 +
 drivers/leds/flash/Makefile        |   1 +
 drivers/leds/flash/leds-tps6131x.c | 794 +++++++++++++++++++++++++++++++++++++
 4 files changed, 813 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0fa7c5728f1e64d031f4a47b6fce1db484ce0fc2..d2ca840647b566cfee4d8ded1f787e32be4aa163 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23225,6 +23225,13 @@ F:	Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
 F:	Documentation/hwmon/tps23861.rst
 F:	drivers/hwmon/tps23861.c
 
+TEXAS INSTRUMENTS TPS6131X FLASH LED DRIVER
+M:	Matthias Fend <matthias.fend@emfend.at>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/leds/ti,tps6131x.yaml
+F:	drivers/leds/flash/leds-tps6131x.c
+
 TEXAS INSTRUMENTS' DAC7612 DAC DRIVER
 M:	Ricardo Ribalda <ribalda@kernel.org>
 L:	linux-iio@vger.kernel.org
diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
index f39f0bfe6eefcd376405d9d35dc36e323a485002..55ca663ca506ad8be627f58f6d6308368ea2b928 100644
--- a/drivers/leds/flash/Kconfig
+++ b/drivers/leds/flash/Kconfig
@@ -132,4 +132,15 @@ config LEDS_SY7802
 
 	  This driver can be built as a module, it will be called "leds-sy7802".
 
+config LEDS_TPS6131X
+	tristate "LED support for TI TPS6131x flash LED driver"
+	depends on I2C && OF
+	depends on GPIOLIB
+	select REGMAP_I2C
+	help
+	  This option enables support for Texas Instruments TPS61310/TPS61311
+	  flash LED driver.
+
+	  This driver can be built as a module, it will be called "leds-tps6131x".
+
 endif # LEDS_CLASS_FLASH
diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
index 48860eeced79513a0ed303e4af3db9bfe9724b7e..712fb737a428e42747e1aa339058dc4306ade9c8 100644
--- a/drivers/leds/flash/Makefile
+++ b/drivers/leds/flash/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_LEDS_RT4505)	+= leds-rt4505.o
 obj-$(CONFIG_LEDS_RT8515)	+= leds-rt8515.o
 obj-$(CONFIG_LEDS_SGM3140)	+= leds-sgm3140.o
 obj-$(CONFIG_LEDS_SY7802)	+= leds-sy7802.o
+obj-$(CONFIG_LEDS_TPS6131X)	+= leds-tps6131x.o
diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c
new file mode 100644
index 0000000000000000000000000000000000000000..e5005c291d3f048af27ff70ecdce9fc38e114efe
--- /dev/null
+++ b/drivers/leds/flash/leds-tps6131x.c
@@ -0,0 +1,794 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Texas Instruments TPS61310/TPS61311 flash LED driver with I2C interface
+ *
+ * Copyright 2025 Matthias Fend <matthias.fend@emfend.at>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/led-class-flash.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <media/v4l2-flash-led-class.h>
+
+/* Registers */
+
+#define TPS6131X_REG_0				0x00
+#define TPS6131X_REG_0_RESET			BIT(7)
+#define TPS6131X_REG_0_DCLC13			GENMASK(5, 3)
+#define TPS6131X_REG_0_DCLC13_SHIFT		3
+#define TPS6131X_REG_0_DCLC2			GENMASK(2, 0)
+#define TPS6131X_REG_0_DCLC2_SHIFT		0
+
+#define TPS6131X_REG_1				0x01
+#define TPS6131X_REG_1_MODE			GENMASK(7, 6)
+#define TPS6131X_REG_1_MODE_SHIFT		6
+#define TPS6131X_REG_1_FC2			GENMASK(5, 0)
+#define TPS6131X_REG_1_FC2_SHIFT		0
+
+#define TPS6131X_REG_2				0x02
+#define TPS6131X_REG_2_MODE			GENMASK(7, 6)
+#define TPS6131X_REG_2_MODE_SHIFT		6
+#define TPS6131X_REG_2_ENVM			BIT(5)
+#define TPS6131X_REG_2_FC13			GENMASK(4, 0)
+#define TPS6131X_REG_2_FC13_SHIFT		0
+
+#define TPS6131X_REG_3				0x03
+#define TPS6131X_REG_3_STIM			GENMASK(7, 5)
+#define TPS6131X_REG_3_STIM_SHIFT		5
+#define TPS6131X_REG_3_HPFL			BIT(4)
+#define TPS6131X_REG_3_SELSTIM_TO		BIT(3)
+#define TPS6131X_REG_3_STT			BIT(2)
+#define TPS6131X_REG_3_SFT			BIT(1)
+#define TPS6131X_REG_3_TXMASK			BIT(0)
+
+#define TPS6131X_REG_4				0x04
+#define TPS6131X_REG_4_PG			BIT(7)
+#define TPS6131X_REG_4_HOTDIE_HI		BIT(6)
+#define TPS6131X_REG_4_HOTDIE_LO		BIT(5)
+#define TPS6131X_REG_4_ILIM			BIT(4)
+#define TPS6131X_REG_4_INDC			GENMASK(3, 0)
+#define TPS6131X_REG_4_INDC_SHIFT		0
+
+#define TPS6131X_REG_5				0x05
+#define TPS6131X_REG_5_SELFCAL			BIT(7)
+#define TPS6131X_REG_5_ENPSM			BIT(6)
+#define TPS6131X_REG_5_STSTRB1_DIR		BIT(5)
+#define TPS6131X_REG_5_GPIO			BIT(4)
+#define TPS6131X_REG_5_GPIOTYPE			BIT(3)
+#define TPS6131X_REG_5_ENLED3			BIT(2)
+#define TPS6131X_REG_5_ENLED2			BIT(1)
+#define TPS6131X_REG_5_ENLED1			BIT(0)
+
+#define TPS6131X_REG_6				0x06
+#define TPS6131X_REG_6_ENTS			BIT(7)
+#define TPS6131X_REG_6_LEDHOT			BIT(6)
+#define TPS6131X_REG_6_LEDWARN			BIT(5)
+#define TPS6131X_REG_6_LEDHDR			BIT(4)
+#define TPS6131X_REG_6_OV			GENMASK(3, 0)
+#define TPS6131X_REG_6_OV_SHIFT			0
+
+#define TPS6131X_REG_7				0x07
+#define TPS6131X_REG_7_ENBATMON			BIT(7)
+#define TPS6131X_REG_7_BATDROOP			GENMASK(6, 4)
+#define TPS6131X_REG_7_BATDROOP_SHIFT		4
+#define TPS6131X_REG_7_REVID			GENMASK(2, 0)
+#define TPS6131X_REG_7_REVID_SHIFT		0
+
+/* Constants */
+
+#define TPS6131X_MAX_CHANNELS			3
+
+#define TPS6131X_FLASH_MAX_I_CHAN13_MA		400
+#define TPS6131X_FLASH_MAX_I_CHAN2_MA		800
+#define TPS6131X_FLASH_STEP_I_MA		25
+
+#define TPS6131X_TORCH_MAX_I_CHAN13_MA		175
+#define TPS6131X_TORCH_MAX_I_CHAN2_MA		175
+#define TPS6131X_TORCH_STEP_I_MA		25
+
+#define TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES msecs_to_jiffies(10000)
+
+#define UA_TO_MA(UA)				((UA) / 1000)
+
+enum tps6131x_mode {
+	TPS6131X_MODE_SHUTDOWN = 0x0,
+	TPS6131X_MODE_TORCH = 0x1,
+	TPS6131X_MODE_FLASH = 0x2,
+};
+
+struct tps6131x {
+	struct device *dev;
+	struct regmap *regmap;
+	struct gpio_desc *reset_gpio;
+	struct mutex lock; /* Hardware access lock*/
+	struct delayed_work torch_refresh_work;
+	bool valley_current_limit;
+	bool chan1_en;
+	bool chan2_en;
+	bool chan3_en;
+	struct fwnode_handle *led_node;
+	u32 max_flash_current_ma;
+	u32 step_flash_current_ma;
+	u32 max_torch_current_ma;
+	u32 step_torch_current_ma;
+	u32 max_timeout_us;
+	struct led_classdev_flash fled_cdev;
+	struct v4l2_flash *v4l2_flash;
+};
+
+static struct tps6131x *fled_cdev_to_tps6131x(struct led_classdev_flash *fled_cdev)
+{
+	return container_of(fled_cdev, struct tps6131x, fled_cdev);
+}
+
+/*
+ * Register contents after a power on/reset. These values ​​cannot be changed.
+ */
+static const struct reg_default tps6131x_regmap_defaults[] = {
+	{ TPS6131X_REG_0, (1 << TPS6131X_REG_0_DCLC13_SHIFT) | (2 << TPS6131X_REG_0_DCLC2_SHIFT) },
+	{ TPS6131X_REG_1, (0 << TPS6131X_REG_1_MODE_SHIFT) | (16 << TPS6131X_REG_1_FC2_SHIFT) },
+	{ TPS6131X_REG_2, (0 << TPS6131X_REG_2_MODE_SHIFT) | (8 << TPS6131X_REG_2_FC13_SHIFT) },
+	{ TPS6131X_REG_3, (6 << TPS6131X_REG_3_STIM_SHIFT) | TPS6131X_REG_3_TXMASK },
+	{ TPS6131X_REG_4, (0 << TPS6131X_REG_4_INDC_SHIFT) },
+	{ TPS6131X_REG_5, TPS6131X_REG_5_ENPSM | TPS6131X_REG_5_STSTRB1_DIR |
+				  TPS6131X_REG_5_GPIOTYPE | TPS6131X_REG_5_ENLED2 },
+	{ TPS6131X_REG_6, (9 << TPS6131X_REG_6_OV_SHIFT) },
+	{ TPS6131X_REG_7, (4 << TPS6131X_REG_7_BATDROOP_SHIFT) },
+};
+
+/*
+ * These registers contain flags that are reset when read. Ensure that these registers are not read
+ * outside of a call from the driver.
+ */
+static bool tps6131x_regmap_precious(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TPS6131X_REG_3:
+	case TPS6131X_REG_4:
+	case TPS6131X_REG_6:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config tps6131x_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = TPS6131X_REG_7,
+	.reg_defaults = tps6131x_regmap_defaults,
+	.num_reg_defaults = ARRAY_SIZE(tps6131x_regmap_defaults),
+	.cache_type = REGCACHE_FLAT,
+	.precious_reg = &tps6131x_regmap_precious,
+};
+
+struct tps6131x_timer_config {
+	u8 val;
+	u8 range;
+	u32 time_us;
+};
+
+static const struct tps6131x_timer_config tps6131x_timer_configs[] = {
+	{ .val = 0, .range = 1, .time_us = 5300 },
+	{ .val = 1, .range = 1, .time_us = 10700 },
+	{ .val = 2, .range = 1, .time_us = 16000 },
+	{ .val = 3, .range = 1, .time_us = 21300 },
+	{ .val = 4, .range = 1, .time_us = 26600 },
+	{ .val = 5, .range = 1, .time_us = 32000 },
+	{ .val = 6, .range = 1, .time_us = 37300 },
+	{ .val = 0, .range = 0, .time_us = 68200 },
+	{ .val = 7, .range = 1, .time_us = 71500 },
+	{ .val = 1, .range = 0, .time_us = 102200 },
+	{ .val = 2, .range = 0, .time_us = 136300 },
+	{ .val = 3, .range = 0, .time_us = 170400 },
+	{ .val = 4, .range = 0, .time_us = 204500 },
+	{ .val = 5, .range = 0, .time_us = 340800 },
+	{ .val = 6, .range = 0, .time_us = 579300 },
+	{ .val = 7, .range = 0, .time_us = 852000 },
+};
+
+static const struct tps6131x_timer_config *tps6131x_find_closest_timer_config(u32 timeout_us)
+{
+	const struct tps6131x_timer_config *timer_config = &tps6131x_timer_configs[0];
+	u32 diff, min_diff = U32_MAX;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tps6131x_timer_configs); i++) {
+		diff = abs(tps6131x_timer_configs[i].time_us - timeout_us);
+		if (diff < min_diff) {
+			timer_config = &tps6131x_timer_configs[i];
+			min_diff = diff;
+			if (!min_diff)
+				break;
+		}
+	}
+
+	return timer_config;
+}
+
+static int tps6131x_reset_chip(struct tps6131x *tps6131x)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(tps6131x->reset_gpio)) {
+		ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET,
+					 TPS6131X_REG_0_RESET);
+		if (ret)
+			return ret;
+
+		fsleep(100);
+
+		ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET, 0);
+		if (ret)
+			return ret;
+	} else {
+		gpiod_set_value_cansleep(tps6131x->reset_gpio, 1);
+		fsleep(10);
+		gpiod_set_value_cansleep(tps6131x->reset_gpio, 0);
+		fsleep(100);
+	}
+
+	return 0;
+}
+
+static int tps6131x_init_chip(struct tps6131x *tps6131x)
+{
+	u32 reg4, reg5, reg6;
+	int ret;
+
+	reg4 = tps6131x->valley_current_limit ? TPS6131X_REG_4_ILIM : 0;
+	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_4, reg4);
+	if (ret)
+		return ret;
+
+	reg5 = TPS6131X_REG_5_ENPSM | TPS6131X_REG_5_STSTRB1_DIR | TPS6131X_REG_5_GPIOTYPE;
+	if (tps6131x->chan1_en)
+		reg5 |= TPS6131X_REG_5_ENLED1;
+
+	if (tps6131x->chan2_en)
+		reg5 |= TPS6131X_REG_5_ENLED2;
+
+	if (tps6131x->chan3_en)
+		reg5 |= TPS6131X_REG_5_ENLED3;
+	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_5, reg5);
+	if (ret)
+		return ret;
+
+	reg6 = TPS6131X_REG_6_ENTS;
+	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, reg6);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int tps6131x_set_mode(struct tps6131x *tps6131x, enum tps6131x_mode mode, bool force)
+{
+	u8 val;
+
+	val = mode << TPS6131X_REG_1_MODE_SHIFT;
+
+	return regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_MODE, val,
+				       NULL, false, force);
+}
+
+static void tps6131x_torch_refresh_handler(struct work_struct *work)
+{
+	struct tps6131x *tps6131x = container_of(work, struct tps6131x, torch_refresh_work.work);
+
+	guard(mutex)(&tps6131x->lock);
+
+	tps6131x_set_mode(tps6131x, TPS6131X_MODE_TORCH, true);
+
+	schedule_delayed_work(&tps6131x->torch_refresh_work,
+			      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
+}
+
+static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+	u32 num_chans;
+	u32 steps_chan13, steps_chan2;
+	u32 steps_remaining;
+	u8 reg0;
+	int ret;
+
+	cancel_delayed_work_sync(&tps6131x->torch_refresh_work);
+
+	guard(mutex)(&tps6131x->lock);
+
+	/*
+	 * The brightness parameter uses the number of current steps as the unit (not the current
+	 * value itself). Since the reported step size can vary depending on the configuration,
+	 * this value must be converted into actual register steps.
+	 */
+	steps_remaining = (brightness * tps6131x->step_torch_current_ma) / TPS6131X_TORCH_STEP_I_MA;
+
+	num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en;
+
+	/*
+	 * The currents are distributed as evenly as possible across the activated channels.
+	 * Since channels 1 and 3 share the same register setting, they always use the same current
+	 * value. Channel 2 supports higher currents and thus takes over the remaining additional
+	 * portion that cannot be covered by the other channels.
+	 */
+	steps_chan13 = min_t(u32, steps_remaining / num_chans,
+			     TPS6131X_TORCH_MAX_I_CHAN13_MA / TPS6131X_TORCH_STEP_I_MA);
+	if (tps6131x->chan1_en)
+		steps_remaining -= steps_chan13;
+	if (tps6131x->chan3_en)
+		steps_remaining -= steps_chan13;
+
+	steps_chan2 = min_t(u32, steps_remaining,
+			    TPS6131X_TORCH_MAX_I_CHAN2_MA / TPS6131X_TORCH_STEP_I_MA);
+
+	reg0 = (steps_chan13 << TPS6131X_REG_0_DCLC13_SHIFT) |
+	       (steps_chan2 << TPS6131X_REG_0_DCLC2_SHIFT);
+	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0,
+				 TPS6131X_REG_0_DCLC13 | TPS6131X_REG_0_DCLC2, reg0);
+	if (ret < 0)
+		return ret;
+
+	ret = tps6131x_set_mode(tps6131x, brightness ? TPS6131X_MODE_TORCH : TPS6131X_MODE_SHUTDOWN,
+				true);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * In order to use both the flash and the video light functions purely via the I2C
+	 * interface, STRB1 must be low. If STRB1 is low, then the video light watchdog timer
+	 * is also active, which puts the device into the shutdown state after around 13 seconds.
+	 * To prevent this, the mode must be refreshed within the watchdog timeout.
+	 */
+	if (brightness)
+		schedule_delayed_work(&tps6131x->torch_refresh_work,
+				      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
+
+	return 0;
+}
+
+static int tps6131x_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
+{
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+	int ret;
+
+	guard(mutex)(&tps6131x->lock);
+
+	ret = tps6131x_set_mode(tps6131x, state ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
+				true);
+	if (ret < 0)
+		return ret;
+
+	if (state) {
+		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT,
+					      TPS6131X_REG_3_SFT, NULL, false, true);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, 0, NULL,
+				      false, true);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int tps6131x_flash_brightness_set(struct led_classdev_flash *fled_cdev, u32 brightness)
+{
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+	u32 num_chans;
+	u32 steps_chan13, steps_chan2;
+	u32 steps_remaining;
+	int ret;
+
+	guard(mutex)(&tps6131x->lock);
+
+	steps_remaining = brightness / TPS6131X_FLASH_STEP_I_MA;
+	num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en;
+	steps_chan13 = min_t(u32, steps_remaining / num_chans,
+			     TPS6131X_FLASH_MAX_I_CHAN13_MA / TPS6131X_FLASH_STEP_I_MA);
+	if (tps6131x->chan1_en)
+		steps_remaining -= steps_chan13;
+	if (tps6131x->chan3_en)
+		steps_remaining -= steps_chan13;
+	steps_chan2 = min_t(u32, steps_remaining,
+			    TPS6131X_FLASH_MAX_I_CHAN2_MA / TPS6131X_FLASH_STEP_I_MA);
+
+	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_2, TPS6131X_REG_2_FC13,
+				 steps_chan13 << TPS6131X_REG_2_FC13_SHIFT);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_FC2,
+				 steps_chan2 << TPS6131X_REG_1_FC2_SHIFT);
+	if (ret < 0)
+		return ret;
+
+	fled_cdev->brightness.val = brightness;
+
+	return 0;
+}
+
+static int tps6131x_flash_timeout_set(struct led_classdev_flash *fled_cdev, u32 timeout_us)
+{
+	const struct tps6131x_timer_config *timer_config;
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+	u8 reg3;
+	int ret;
+
+	guard(mutex)(&tps6131x->lock);
+
+	timer_config = tps6131x_find_closest_timer_config(timeout_us);
+
+	reg3 = timer_config->val << TPS6131X_REG_3_STIM_SHIFT;
+	if (timer_config->range)
+		reg3 |= TPS6131X_REG_3_SELSTIM_TO;
+
+	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_3,
+				 TPS6131X_REG_3_STIM | TPS6131X_REG_3_SELSTIM_TO, reg3);
+	if (ret < 0)
+		return ret;
+
+	fled_cdev->timeout.val = timer_config->time_us;
+
+	return 0;
+}
+
+static int tps6131x_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
+{
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+	unsigned int reg3;
+	int ret;
+
+	guard(mutex)(&tps6131x->lock);
+
+	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, &reg3);
+	if (ret)
+		return ret;
+
+	*state = !!(reg3 & TPS6131X_REG_3_SFT);
+
+	return 0;
+}
+
+static int tps6131x_flash_fault_get(struct led_classdev_flash *fled_cdev, u32 *fault)
+{
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+	unsigned int reg3, reg4, reg6;
+	int ret;
+
+	*fault = 0;
+
+	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, &reg3);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_4, &reg4);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_6, &reg6);
+	if (ret < 0)
+		return ret;
+
+	if (reg3 & TPS6131X_REG_3_HPFL)
+		*fault |= LED_FAULT_SHORT_CIRCUIT;
+
+	if (reg3 & TPS6131X_REG_3_SELSTIM_TO)
+		*fault |= LED_FAULT_TIMEOUT;
+
+	if (reg4 & TPS6131X_REG_4_HOTDIE_HI)
+		*fault |= LED_FAULT_OVER_TEMPERATURE;
+
+	if (reg6 & (TPS6131X_REG_6_LEDHOT | TPS6131X_REG_6_LEDWARN))
+		*fault |= LED_FAULT_LED_OVER_TEMPERATURE;
+
+	if (!(reg6 & TPS6131X_REG_6_LEDHDR))
+		*fault |= LED_FAULT_UNDER_VOLTAGE;
+
+	if (reg6 & TPS6131X_REG_6_LEDHOT) {
+		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_6,
+					      TPS6131X_REG_6_LEDHOT, 0, NULL, false, true);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct led_flash_ops flash_ops = {
+	.flash_brightness_set = tps6131x_flash_brightness_set,
+	.strobe_set = tps6131x_strobe_set,
+	.strobe_get = tps6131x_strobe_get,
+	.timeout_set = tps6131x_flash_timeout_set,
+	.fault_get = tps6131x_flash_fault_get,
+};
+
+static int tps6131x_parse_node(struct tps6131x *tps6131x)
+{
+	const struct tps6131x_timer_config *timer_config;
+	struct device *dev = tps6131x->dev;
+	u32 channels[TPS6131X_MAX_CHANNELS];
+	u32 current_step_multiplier;
+	u32 current_ua;
+	u32 max_current_flash_ma, max_current_torch_ma;
+	u32 num_channels;
+	u32 timeout_us;
+	int i;
+	int ret;
+
+	tps6131x->valley_current_limit = device_property_read_bool(dev, "ti,valley-current-limit");
+
+	tps6131x->led_node = fwnode_get_next_available_child_node(dev->fwnode, NULL);
+	if (!tps6131x->led_node) {
+		dev_err(dev, "Missing LED node\n");
+		return -EINVAL;
+	}
+
+	num_channels = fwnode_property_count_u32(tps6131x->led_node, "led-sources");
+	if (num_channels <= 0) {
+		dev_err(dev, "Failed to read led-sources property\n");
+		return -EINVAL;
+	}
+
+	if (num_channels > TPS6131X_MAX_CHANNELS) {
+		dev_err(dev, "led-sources count %u exceeds maximum channel count %u\n",
+			num_channels, TPS6131X_MAX_CHANNELS);
+		return -EINVAL;
+	}
+
+	ret = fwnode_property_read_u32_array(tps6131x->led_node, "led-sources", channels,
+					     num_channels);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read led-sources property\n");
+		return ret;
+	}
+
+	max_current_flash_ma = 0;
+	max_current_torch_ma = 0;
+	for (i = 0; i < num_channels; i++) {
+		switch (channels[i]) {
+		case 1:
+			tps6131x->chan1_en = true;
+			max_current_flash_ma += TPS6131X_FLASH_MAX_I_CHAN13_MA;
+			max_current_torch_ma += TPS6131X_TORCH_MAX_I_CHAN13_MA;
+			break;
+		case 2:
+			tps6131x->chan2_en = true;
+			max_current_flash_ma += TPS6131X_FLASH_MAX_I_CHAN2_MA;
+			max_current_torch_ma += TPS6131X_TORCH_MAX_I_CHAN2_MA;
+			break;
+		case 3:
+			tps6131x->chan3_en = true;
+			max_current_flash_ma += TPS6131X_FLASH_MAX_I_CHAN13_MA;
+			max_current_torch_ma += TPS6131X_TORCH_MAX_I_CHAN13_MA;
+			break;
+		default:
+			dev_err(dev, "led-source out of range [1-3]\n");
+			return -EINVAL;
+		}
+	}
+
+	/*
+	 * If only channels 1 and 3 are used, the step size is doubled because the two channels
+	 * share the same current control register.
+	 */
+	current_step_multiplier =
+		(tps6131x->chan1_en && tps6131x->chan3_en && !tps6131x->chan2_en) ? 2 : 1;
+	tps6131x->step_flash_current_ma = current_step_multiplier * TPS6131X_FLASH_STEP_I_MA;
+	tps6131x->step_torch_current_ma = current_step_multiplier * TPS6131X_TORCH_STEP_I_MA;
+
+	ret = fwnode_property_read_u32(tps6131x->led_node, "led-max-microamp", &current_ua);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read led-max-microamp property\n");
+		return ret;
+	}
+
+	tps6131x->max_torch_current_ma = UA_TO_MA(current_ua);
+
+	if (!tps6131x->max_torch_current_ma ||
+	    tps6131x->max_torch_current_ma > max_current_torch_ma ||
+	    (tps6131x->max_torch_current_ma % tps6131x->step_torch_current_ma)) {
+		dev_err(dev, "led-max-microamp out of range or not a multiple of %u\n",
+			tps6131x->step_torch_current_ma);
+		return -EINVAL;
+	}
+
+	ret = fwnode_property_read_u32(tps6131x->led_node, "flash-max-microamp", &current_ua);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read flash-max-microamp property\n");
+		return ret;
+	}
+
+	tps6131x->max_flash_current_ma = UA_TO_MA(current_ua);
+
+	if (!tps6131x->max_flash_current_ma ||
+	    tps6131x->max_flash_current_ma > max_current_flash_ma ||
+	    (tps6131x->max_flash_current_ma % tps6131x->step_flash_current_ma)) {
+		dev_err(dev, "flash-max-microamp out of range or not a multiple of %u\n",
+			tps6131x->step_flash_current_ma);
+		return -EINVAL;
+	}
+
+	ret = fwnode_property_read_u32(tps6131x->led_node, "flash-max-timeout-us", &timeout_us);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read flash-max-timeout-us property\n");
+		return ret;
+	}
+
+	timer_config = tps6131x_find_closest_timer_config(timeout_us);
+	tps6131x->max_timeout_us = timer_config->time_us;
+
+	if (tps6131x->max_timeout_us != timeout_us)
+		dev_warn(dev, "flash-max-timeout-us %u not supported (using %u)\n", timeout_us,
+			 tps6131x->max_timeout_us);
+
+	return 0;
+}
+
+static int tps6131x_led_class_setup(struct tps6131x *tps6131x)
+{
+	const struct tps6131x_timer_config *timer_config;
+	struct led_classdev *led_cdev;
+	struct led_flash_setting *setting;
+	struct led_init_data init_data = {};
+	int ret;
+
+	tps6131x->fled_cdev.ops = &flash_ops;
+
+	setting = &tps6131x->fled_cdev.timeout;
+	timer_config = tps6131x_find_closest_timer_config(0);
+	setting->min = timer_config->time_us;
+	setting->max = tps6131x->max_timeout_us;
+	setting->step = 1; /* Only some specific time periods are supported. No fixed step size. */
+	setting->val = setting->min;
+
+	setting = &tps6131x->fled_cdev.brightness;
+	setting->min = tps6131x->step_flash_current_ma;
+	setting->max = tps6131x->max_flash_current_ma;
+	setting->step = tps6131x->step_flash_current_ma;
+	setting->val = setting->min;
+
+	led_cdev = &tps6131x->fled_cdev.led_cdev;
+	led_cdev->brightness_set_blocking = tps6131x_brightness_set;
+	led_cdev->max_brightness = tps6131x->max_torch_current_ma;
+	led_cdev->flags |= LED_DEV_CAP_FLASH;
+
+	init_data.fwnode = tps6131x->led_node;
+	init_data.devicename = NULL;
+	init_data.default_label = NULL;
+	init_data.devname_mandatory = false;
+
+	ret = devm_led_classdev_flash_register_ext(tps6131x->dev, &tps6131x->fled_cdev,
+						   &init_data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int tps6131x_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
+{
+	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
+	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
+
+	guard(mutex)(&tps6131x->lock);
+
+	return tps6131x_set_mode(tps6131x, enable ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
+				 false);
+}
+
+static const struct v4l2_flash_ops tps6131x_v4l2_flash_ops = {
+	.external_strobe_set = tps6131x_flash_external_strobe_set,
+};
+
+static int tps6131x_v4l2_setup(struct tps6131x *tps6131x)
+{
+	struct v4l2_flash_config v4l2_cfg = { 0 };
+	struct led_flash_setting *intensity = &v4l2_cfg.intensity;
+
+	if (!IS_BUILTIN(CONFIG_V4L2_FLASH_LED_CLASS))
+		return 0;
+
+	intensity->min = tps6131x->step_torch_current_ma;
+	intensity->max = tps6131x->max_torch_current_ma;
+	intensity->step = tps6131x->step_torch_current_ma;
+	intensity->val = intensity->min;
+
+	strscpy(v4l2_cfg.dev_name, tps6131x->fled_cdev.led_cdev.dev->kobj.name,
+		sizeof(v4l2_cfg.dev_name));
+
+	v4l2_cfg.has_external_strobe = true;
+	v4l2_cfg.flash_faults = LED_FAULT_TIMEOUT | LED_FAULT_OVER_TEMPERATURE |
+				LED_FAULT_SHORT_CIRCUIT | LED_FAULT_UNDER_VOLTAGE |
+				LED_FAULT_LED_OVER_TEMPERATURE;
+
+	tps6131x->v4l2_flash = v4l2_flash_init(tps6131x->dev, tps6131x->led_node,
+					       &tps6131x->fled_cdev, &tps6131x_v4l2_flash_ops,
+					       &v4l2_cfg);
+	if (IS_ERR(tps6131x->v4l2_flash)) {
+		dev_err(tps6131x->dev, "Failed to initialize v4l2 flash LED\n");
+		return PTR_ERR(tps6131x->v4l2_flash);
+	}
+
+	return 0;
+}
+
+static int tps6131x_probe(struct i2c_client *client)
+{
+	struct tps6131x *tps6131x;
+	int ret;
+
+	tps6131x = devm_kzalloc(&client->dev, sizeof(*tps6131x), GFP_KERNEL);
+	if (!tps6131x)
+		return -ENOMEM;
+
+	tps6131x->dev = &client->dev;
+	i2c_set_clientdata(client, tps6131x);
+	mutex_init(&tps6131x->lock);
+	INIT_DELAYED_WORK(&tps6131x->torch_refresh_work, tps6131x_torch_refresh_handler);
+
+	ret = tps6131x_parse_node(tps6131x);
+	if (ret)
+		return -ENODEV;
+
+	tps6131x->regmap = devm_regmap_init_i2c(client, &tps6131x_regmap);
+	if (IS_ERR(tps6131x->regmap)) {
+		ret = PTR_ERR(tps6131x->regmap);
+		dev_err(&client->dev, "Failed to allocate register map\n");
+		return ret;
+	}
+
+	tps6131x->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
+	ret = tps6131x_reset_chip(tps6131x);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to reset LED controller\n");
+
+	ret = tps6131x_init_chip(tps6131x);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to initialize LED controller\n");
+
+	ret = tps6131x_led_class_setup(tps6131x);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to setup LED class\n");
+
+	ret = tps6131x_v4l2_setup(tps6131x);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to setup v4l2 flash\n");
+
+	return 0;
+}
+
+static void tps6131x_remove(struct i2c_client *client)
+{
+	struct tps6131x *tps6131x = i2c_get_clientdata(client);
+
+	v4l2_flash_release(tps6131x->v4l2_flash);
+
+	cancel_delayed_work_sync(&tps6131x->torch_refresh_work);
+}
+
+static const struct of_device_id of_tps6131x_leds_match[] = {
+	{ .compatible = "ti,tps61310" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_tps6131x_leds_match);
+
+static struct i2c_driver tps6131x_i2c_driver = {
+	.driver = {
+		.name = "tps6131x",
+		.of_match_table = of_tps6131x_leds_match,
+	},
+	.probe = tps6131x_probe,
+	.remove = tps6131x_remove,
+};
+module_i2c_driver(tps6131x_i2c_driver);
+
+MODULE_DESCRIPTION("Texas Instruments TPS6131X flash LED driver");
+MODULE_AUTHOR("Matthias Fend <matthias.fend@emfend.at>");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[kernel test robot]

Hi Matthias,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ffd294d346d185b70e28b1a28abe367bbfe53c04]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthias-Fend/dt-bindings-leds-add-Texas-Instruments-TPS6131x-flash-LED-driver/20250318-154318
base:   ffd294d346d185b70e28b1a28abe367bbfe53c04
patch link:    https://lore.kernel.org/r/20250318-leds-tps6131x-v2-2-bc09c7a50b2e%40emfend.at
patch subject: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver
reproduce: (https://download.01.org/0day-ci/archive/20250318/202503181835.693ZAgp6-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/202503181835.693ZAgp6-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: Documentation/hwmon/isl28022.rst references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/isl,isl28022.yaml
   Warning: Documentation/translations/ja_JP/SubmittingPatches references a file that doesn't exist: linux-2.6.12-vanilla/Documentation/dontdiff
   Warning: Documentation/userspace-api/netlink/index.rst references a file that doesn't exist: Documentation/networking/netlink_spec/index.rst
   Warning: Documentation/userspace-api/netlink/specs.rst references a file that doesn't exist: Documentation/networking/netlink_spec/index.rst
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/leds/ti,tps6131x.yaml
   Warning: lib/Kconfig.debug references a file that doesn't exist: Documentation/dev-tools/fault-injection/fault-injection.rst
   Using alabaster theme

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[Krzysztof Kozlowski]

On Tue, Mar 18, 2025 at 08:28:58AM +0100, Matthias Fend wrote:
> +
> +static int tps6131x_reset_chip(struct tps6131x *tps6131x)
> +{
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(tps6131x->reset_gpio)) {

No, only check for non-null, see comment in probe().

> +		ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET,
> +					 TPS6131X_REG_0_RESET);
> +		if (ret)
> +			return ret;
> +
> +		fsleep(100);
> +
> +		ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET, 0);
> +		if (ret)
> +			return ret;
> +	} else {
> +		gpiod_set_value_cansleep(tps6131x->reset_gpio, 1);
> +		fsleep(10);
> +		gpiod_set_value_cansleep(tps6131x->reset_gpio, 0);
> +		fsleep(100);
> +	}
> +
> +	return 0;
> +}
> +
> +static int tps6131x_init_chip(struct tps6131x *tps6131x)
> +{
> +	u32 reg4, reg5, reg6;
> +	int ret;
> +
> +	reg4 = tps6131x->valley_current_limit ? TPS6131X_REG_4_ILIM : 0;
> +	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_4, reg4);
> +	if (ret)
> +		return ret;
> +
> +	reg5 = TPS6131X_REG_5_ENPSM | TPS6131X_REG_5_STSTRB1_DIR | TPS6131X_REG_5_GPIOTYPE;
> +	if (tps6131x->chan1_en)
> +		reg5 |= TPS6131X_REG_5_ENLED1;
> +
> +	if (tps6131x->chan2_en)
> +		reg5 |= TPS6131X_REG_5_ENLED2;
> +
> +	if (tps6131x->chan3_en)
> +		reg5 |= TPS6131X_REG_5_ENLED3;
> +	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_5, reg5);
> +	if (ret)
> +		return ret;
> +
> +	reg6 = TPS6131X_REG_6_ENTS;
> +	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, reg6);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tps6131x_set_mode(struct tps6131x *tps6131x, enum tps6131x_mode mode, bool force)
> +{
> +	u8 val;
> +
> +	val = mode << TPS6131X_REG_1_MODE_SHIFT;
> +
> +	return regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_MODE, val,
> +				       NULL, false, force);
> +}
> +
> +static void tps6131x_torch_refresh_handler(struct work_struct *work)
> +{
> +	struct tps6131x *tps6131x = container_of(work, struct tps6131x, torch_refresh_work.work);
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	tps6131x_set_mode(tps6131x, TPS6131X_MODE_TORCH, true);
> +
> +	schedule_delayed_work(&tps6131x->torch_refresh_work,
> +			      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
> +}
> +
> +static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
> +{
> +	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +	u32 num_chans;
> +	u32 steps_chan13, steps_chan2;
> +	u32 steps_remaining;
> +	u8 reg0;
> +	int ret;
> +
> +	cancel_delayed_work_sync(&tps6131x->torch_refresh_work);
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	/*
> +	 * The brightness parameter uses the number of current steps as the unit (not the current
> +	 * value itself). Since the reported step size can vary depending on the configuration,
> +	 * this value must be converted into actual register steps.
> +	 */
> +	steps_remaining = (brightness * tps6131x->step_torch_current_ma) / TPS6131X_TORCH_STEP_I_MA;
> +
> +	num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en;
> +
> +	/*
> +	 * The currents are distributed as evenly as possible across the activated channels.
> +	 * Since channels 1 and 3 share the same register setting, they always use the same current
> +	 * value. Channel 2 supports higher currents and thus takes over the remaining additional
> +	 * portion that cannot be covered by the other channels.
> +	 */
> +	steps_chan13 = min_t(u32, steps_remaining / num_chans,
> +			     TPS6131X_TORCH_MAX_I_CHAN13_MA / TPS6131X_TORCH_STEP_I_MA);
> +	if (tps6131x->chan1_en)
> +		steps_remaining -= steps_chan13;
> +	if (tps6131x->chan3_en)
> +		steps_remaining -= steps_chan13;
> +
> +	steps_chan2 = min_t(u32, steps_remaining,
> +			    TPS6131X_TORCH_MAX_I_CHAN2_MA / TPS6131X_TORCH_STEP_I_MA);
> +
> +	reg0 = (steps_chan13 << TPS6131X_REG_0_DCLC13_SHIFT) |
> +	       (steps_chan2 << TPS6131X_REG_0_DCLC2_SHIFT);

Looks like guard should start here... or you are not synchronizing
hardware access but more.

> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0,
> +				 TPS6131X_REG_0_DCLC13 | TPS6131X_REG_0_DCLC2, reg0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tps6131x_set_mode(tps6131x, brightness ? TPS6131X_MODE_TORCH : TPS6131X_MODE_SHUTDOWN,
> +				true);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * In order to use both the flash and the video light functions purely via the I2C
> +	 * interface, STRB1 must be low. If STRB1 is low, then the video light watchdog timer
> +	 * is also active, which puts the device into the shutdown state after around 13 seconds.
> +	 * To prevent this, the mode must be refreshed within the watchdog timeout.
> +	 */
> +	if (brightness)
> +		schedule_delayed_work(&tps6131x->torch_refresh_work,
> +				      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
> +
> +	return 0;
> +}
> +
> +static int tps6131x_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
> +{
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +	int ret;
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	ret = tps6131x_set_mode(tps6131x, state ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
> +				true);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (state) {
> +		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT,
> +					      TPS6131X_REG_3_SFT, NULL, false, true);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, 0, NULL,
> +				      false, true);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tps6131x_flash_brightness_set(struct led_classdev_flash *fled_cdev, u32 brightness)
> +{
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +	u32 num_chans;
> +	u32 steps_chan13, steps_chan2;
> +	u32 steps_remaining;
> +	int ret;
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	steps_remaining = brightness / TPS6131X_FLASH_STEP_I_MA;
> +	num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en;
> +	steps_chan13 = min_t(u32, steps_remaining / num_chans,
> +			     TPS6131X_FLASH_MAX_I_CHAN13_MA / TPS6131X_FLASH_STEP_I_MA);
> +	if (tps6131x->chan1_en)
> +		steps_remaining -= steps_chan13;
> +	if (tps6131x->chan3_en)
> +		steps_remaining -= steps_chan13;
> +	steps_chan2 = min_t(u32, steps_remaining,
> +			    TPS6131X_FLASH_MAX_I_CHAN2_MA / TPS6131X_FLASH_STEP_I_MA);
> +

Same here

> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_2, TPS6131X_REG_2_FC13,
> +				 steps_chan13 << TPS6131X_REG_2_FC13_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_FC2,
> +				 steps_chan2 << TPS6131X_REG_1_FC2_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
> +	fled_cdev->brightness.val = brightness;
> +
> +	return 0;
> +}
> +
> +static int tps6131x_flash_timeout_set(struct led_classdev_flash *fled_cdev, u32 timeout_us)
> +{
> +	const struct tps6131x_timer_config *timer_config;
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +	u8 reg3;
> +	int ret;
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	timer_config = tps6131x_find_closest_timer_config(timeout_us);
> +
> +	reg3 = timer_config->val << TPS6131X_REG_3_STIM_SHIFT;
> +	if (timer_config->range)
> +		reg3 |= TPS6131X_REG_3_SELSTIM_TO;
> +
> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_3,
> +				 TPS6131X_REG_3_STIM | TPS6131X_REG_3_SELSTIM_TO, reg3);
> +	if (ret < 0)
> +		return ret;
> +
> +	fled_cdev->timeout.val = timer_config->time_us;
> +
> +	return 0;
> +}
> +
> +static int tps6131x_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
> +{
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +	unsigned int reg3;
> +	int ret;
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, &reg3);
> +	if (ret)
> +		return ret;
> +
> +	*state = !!(reg3 & TPS6131X_REG_3_SFT);
> +
> +	return 0;
> +}
> +
> +static int tps6131x_flash_fault_get(struct led_classdev_flash *fled_cdev, u32 *fault)
> +{
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +	unsigned int reg3, reg4, reg6;
> +	int ret;
> +
> +	*fault = 0;
> +

Why no lock here?

> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, &reg3);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_4, &reg4);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_6, &reg6);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (reg3 & TPS6131X_REG_3_HPFL)
> +		*fault |= LED_FAULT_SHORT_CIRCUIT;
> +
> +	if (reg3 & TPS6131X_REG_3_SELSTIM_TO)
> +		*fault |= LED_FAULT_TIMEOUT;
> +
> +	if (reg4 & TPS6131X_REG_4_HOTDIE_HI)
> +		*fault |= LED_FAULT_OVER_TEMPERATURE;
> +
> +	if (reg6 & (TPS6131X_REG_6_LEDHOT | TPS6131X_REG_6_LEDWARN))
> +		*fault |= LED_FAULT_LED_OVER_TEMPERATURE;
> +
> +	if (!(reg6 & TPS6131X_REG_6_LEDHDR))
> +		*fault |= LED_FAULT_UNDER_VOLTAGE;
> +
> +	if (reg6 & TPS6131X_REG_6_LEDHOT) {
> +		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_6,
> +					      TPS6131X_REG_6_LEDHOT, 0, NULL, false, true);

And this is not locked?

> +		if (ret < 0)
> +			return ret;
> +	}
> +

...

> +
> +static int tps6131x_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
> +{
> +	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
> +
> +	guard(mutex)(&tps6131x->lock);
> +
> +	return tps6131x_set_mode(tps6131x, enable ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
> +				 false);
> +}
> +
> +static const struct v4l2_flash_ops tps6131x_v4l2_flash_ops = {
> +	.external_strobe_set = tps6131x_flash_external_strobe_set,
> +};
> +
> +static int tps6131x_v4l2_setup(struct tps6131x *tps6131x)
> +{
> +	struct v4l2_flash_config v4l2_cfg = { 0 };
> +	struct led_flash_setting *intensity = &v4l2_cfg.intensity;
> +
> +	if (!IS_BUILTIN(CONFIG_V4L2_FLASH_LED_CLASS))

Why builtin? That's a tristate, so I don't get why driver and v4l flash
cannot be modules. You wanted REACHABLE probably... but then it is
anyway discouraged practice leading to runtime debugging. So actually
you want CONFIG_V4L2_FLASH_LED_CLASS || !CONFIG_V4L2_FLASH_LED_CLASS
dependency.

See Kconfig description.

> +		return 0;
> +
> +	intensity->min = tps6131x->step_torch_current_ma;
> +	intensity->max = tps6131x->max_torch_current_ma;
> +	intensity->step = tps6131x->step_torch_current_ma;
> +	intensity->val = intensity->min;
> +
> +	strscpy(v4l2_cfg.dev_name, tps6131x->fled_cdev.led_cdev.dev->kobj.name,
> +		sizeof(v4l2_cfg.dev_name));
> +
> +	v4l2_cfg.has_external_strobe = true;
> +	v4l2_cfg.flash_faults = LED_FAULT_TIMEOUT | LED_FAULT_OVER_TEMPERATURE |
> +				LED_FAULT_SHORT_CIRCUIT | LED_FAULT_UNDER_VOLTAGE |
> +				LED_FAULT_LED_OVER_TEMPERATURE;
> +
> +	tps6131x->v4l2_flash = v4l2_flash_init(tps6131x->dev, tps6131x->led_node,
> +					       &tps6131x->fled_cdev, &tps6131x_v4l2_flash_ops,
> +					       &v4l2_cfg);
> +	if (IS_ERR(tps6131x->v4l2_flash)) {
> +		dev_err(tps6131x->dev, "Failed to initialize v4l2 flash LED\n");
> +		return PTR_ERR(tps6131x->v4l2_flash);
> +	}
> +
> +	return 0;
> +}
> +
> +static int tps6131x_probe(struct i2c_client *client)
> +{
> +	struct tps6131x *tps6131x;
> +	int ret;
> +
> +	tps6131x = devm_kzalloc(&client->dev, sizeof(*tps6131x), GFP_KERNEL);
> +	if (!tps6131x)
> +		return -ENOMEM;
> +
> +	tps6131x->dev = &client->dev;
> +	i2c_set_clientdata(client, tps6131x);
> +	mutex_init(&tps6131x->lock);
> +	INIT_DELAYED_WORK(&tps6131x->torch_refresh_work, tps6131x_torch_refresh_handler);
> +
> +	ret = tps6131x_parse_node(tps6131x);
> +	if (ret)
> +		return -ENODEV;
> +
> +	tps6131x->regmap = devm_regmap_init_i2c(client, &tps6131x_regmap);
> +	if (IS_ERR(tps6131x->regmap)) {
> +		ret = PTR_ERR(tps6131x->regmap);
> +		dev_err(&client->dev, "Failed to allocate register map\n");
> +		return ret;

Syntax is:

return dev_err_probe

> +	}
> +
> +	tps6131x->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);

You should handle IS_ERR


Best regards,
Krzysztof

Re: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[Matthias Fend]

Hi Krzysztof,

thank you very much for your review!

Am 19.03.2025 um 10:03 schrieb Krzysztof Kozlowski:
> On Tue, Mar 18, 2025 at 08:28:58AM +0100, Matthias Fend wrote:
>> +
>> +static int tps6131x_reset_chip(struct tps6131x *tps6131x)
>> +{
>> +	int ret;
>> +
>> +	if (IS_ERR_OR_NULL(tps6131x->reset_gpio)) {
> 
> No, only check for non-null, see comment in probe().

ACK

> 
>> +		ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET,
>> +					 TPS6131X_REG_0_RESET);
>> +		if (ret)
>> +			return ret;
>> +
>> +		fsleep(100);
>> +
>> +		ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET, 0);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		gpiod_set_value_cansleep(tps6131x->reset_gpio, 1);
>> +		fsleep(10);
>> +		gpiod_set_value_cansleep(tps6131x->reset_gpio, 0);
>> +		fsleep(100);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_init_chip(struct tps6131x *tps6131x)
>> +{
>> +	u32 reg4, reg5, reg6;
>> +	int ret;
>> +
>> +	reg4 = tps6131x->valley_current_limit ? TPS6131X_REG_4_ILIM : 0;
>> +	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_4, reg4);
>> +	if (ret)
>> +		return ret;
>> +
>> +	reg5 = TPS6131X_REG_5_ENPSM | TPS6131X_REG_5_STSTRB1_DIR | TPS6131X_REG_5_GPIOTYPE;
>> +	if (tps6131x->chan1_en)
>> +		reg5 |= TPS6131X_REG_5_ENLED1;
>> +
>> +	if (tps6131x->chan2_en)
>> +		reg5 |= TPS6131X_REG_5_ENLED2;
>> +
>> +	if (tps6131x->chan3_en)
>> +		reg5 |= TPS6131X_REG_5_ENLED3;
>> +	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_5, reg5);
>> +	if (ret)
>> +		return ret;
>> +
>> +	reg6 = TPS6131X_REG_6_ENTS;
>> +	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, reg6);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_set_mode(struct tps6131x *tps6131x, enum tps6131x_mode mode, bool force)
>> +{
>> +	u8 val;
>> +
>> +	val = mode << TPS6131X_REG_1_MODE_SHIFT;
>> +
>> +	return regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_MODE, val,
>> +				       NULL, false, force);
>> +}
>> +
>> +static void tps6131x_torch_refresh_handler(struct work_struct *work)
>> +{
>> +	struct tps6131x *tps6131x = container_of(work, struct tps6131x, torch_refresh_work.work);
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	tps6131x_set_mode(tps6131x, TPS6131X_MODE_TORCH, true);
>> +
>> +	schedule_delayed_work(&tps6131x->torch_refresh_work,
>> +			      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
>> +}
>> +
>> +static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
>> +{
>> +	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +	u32 num_chans;
>> +	u32 steps_chan13, steps_chan2;
>> +	u32 steps_remaining;
>> +	u8 reg0;
>> +	int ret;
>> +
>> +	cancel_delayed_work_sync(&tps6131x->torch_refresh_work);
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	/*
>> +	 * The brightness parameter uses the number of current steps as the unit (not the current
>> +	 * value itself). Since the reported step size can vary depending on the configuration,
>> +	 * this value must be converted into actual register steps.
>> +	 */
>> +	steps_remaining = (brightness * tps6131x->step_torch_current_ma) / TPS6131X_TORCH_STEP_I_MA;
>> +
>> +	num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en;
>> +
>> +	/*
>> +	 * The currents are distributed as evenly as possible across the activated channels.
>> +	 * Since channels 1 and 3 share the same register setting, they always use the same current
>> +	 * value. Channel 2 supports higher currents and thus takes over the remaining additional
>> +	 * portion that cannot be covered by the other channels.
>> +	 */
>> +	steps_chan13 = min_t(u32, steps_remaining / num_chans,
>> +			     TPS6131X_TORCH_MAX_I_CHAN13_MA / TPS6131X_TORCH_STEP_I_MA);
>> +	if (tps6131x->chan1_en)
>> +		steps_remaining -= steps_chan13;
>> +	if (tps6131x->chan3_en)
>> +		steps_remaining -= steps_chan13;
>> +
>> +	steps_chan2 = min_t(u32, steps_remaining,
>> +			    TPS6131X_TORCH_MAX_I_CHAN2_MA / TPS6131X_TORCH_STEP_I_MA);
>> +
>> +	reg0 = (steps_chan13 << TPS6131X_REG_0_DCLC13_SHIFT) |
>> +	       (steps_chan2 << TPS6131X_REG_0_DCLC2_SHIFT);
> 
> Looks like guard should start here... or you are not synchronizing
> hardware access but more.

I just found it clearer to have the lock at the beginning.
But of course, I can easily move the lock to this position.

> 
>> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0,
>> +				 TPS6131X_REG_0_DCLC13 | TPS6131X_REG_0_DCLC2, reg0);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = tps6131x_set_mode(tps6131x, brightness ? TPS6131X_MODE_TORCH : TPS6131X_MODE_SHUTDOWN,
>> +				true);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/*
>> +	 * In order to use both the flash and the video light functions purely via the I2C
>> +	 * interface, STRB1 must be low. If STRB1 is low, then the video light watchdog timer
>> +	 * is also active, which puts the device into the shutdown state after around 13 seconds.
>> +	 * To prevent this, the mode must be refreshed within the watchdog timeout.
>> +	 */
>> +	if (brightness)
>> +		schedule_delayed_work(&tps6131x->torch_refresh_work,
>> +				      TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
>> +{
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +	int ret;
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	ret = tps6131x_set_mode(tps6131x, state ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
>> +				true);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (state) {
>> +		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT,
>> +					      TPS6131X_REG_3_SFT, NULL, false, true);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, 0, NULL,
>> +				      false, true);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_flash_brightness_set(struct led_classdev_flash *fled_cdev, u32 brightness)
>> +{
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +	u32 num_chans;
>> +	u32 steps_chan13, steps_chan2;
>> +	u32 steps_remaining;
>> +	int ret;
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	steps_remaining = brightness / TPS6131X_FLASH_STEP_I_MA;
>> +	num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en;
>> +	steps_chan13 = min_t(u32, steps_remaining / num_chans,
>> +			     TPS6131X_FLASH_MAX_I_CHAN13_MA / TPS6131X_FLASH_STEP_I_MA);
>> +	if (tps6131x->chan1_en)
>> +		steps_remaining -= steps_chan13;
>> +	if (tps6131x->chan3_en)
>> +		steps_remaining -= steps_chan13;
>> +	steps_chan2 = min_t(u32, steps_remaining,
>> +			    TPS6131X_FLASH_MAX_I_CHAN2_MA / TPS6131X_FLASH_STEP_I_MA);
>> +
> 
> Same here

ACK

> 
>> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_2, TPS6131X_REG_2_FC13,
>> +				 steps_chan13 << TPS6131X_REG_2_FC13_SHIFT);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_FC2,
>> +				 steps_chan2 << TPS6131X_REG_1_FC2_SHIFT);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	fled_cdev->brightness.val = brightness;
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_flash_timeout_set(struct led_classdev_flash *fled_cdev, u32 timeout_us)
>> +{
>> +	const struct tps6131x_timer_config *timer_config;
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +	u8 reg3;
>> +	int ret;
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	timer_config = tps6131x_find_closest_timer_config(timeout_us);
>> +
>> +	reg3 = timer_config->val << TPS6131X_REG_3_STIM_SHIFT;
>> +	if (timer_config->range)
>> +		reg3 |= TPS6131X_REG_3_SELSTIM_TO;
>> +
>> +	ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_3,
>> +				 TPS6131X_REG_3_STIM | TPS6131X_REG_3_SELSTIM_TO, reg3);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	fled_cdev->timeout.val = timer_config->time_us;
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
>> +{
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +	unsigned int reg3;
>> +	int ret;
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, &reg3);
>> +	if (ret)
>> +		return ret;
>> +
>> +	*state = !!(reg3 & TPS6131X_REG_3_SFT);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_flash_fault_get(struct led_classdev_flash *fled_cdev, u32 *fault)
>> +{
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +	unsigned int reg3, reg4, reg6;
>> +	int ret;
>> +
>> +	*fault = 0;
>> +
> 
> Why no lock here?

The individual regmap operations are already protected by the internal 
lock. If reading from a register has no other problematic effects on the 
overall function, then no additional lock is needed.

I noticed that the lock in tps6131x_strobe_get() is no longer needed. 
I'll removed that as well.

> 
>> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, &reg3);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_4, &reg4);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_6, &reg6);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (reg3 & TPS6131X_REG_3_HPFL)
>> +		*fault |= LED_FAULT_SHORT_CIRCUIT;
>> +
>> +	if (reg3 & TPS6131X_REG_3_SELSTIM_TO)
>> +		*fault |= LED_FAULT_TIMEOUT;
>> +
>> +	if (reg4 & TPS6131X_REG_4_HOTDIE_HI)
>> +		*fault |= LED_FAULT_OVER_TEMPERATURE;
>> +
>> +	if (reg6 & (TPS6131X_REG_6_LEDHOT | TPS6131X_REG_6_LEDWARN))
>> +		*fault |= LED_FAULT_LED_OVER_TEMPERATURE;
>> +
>> +	if (!(reg6 & TPS6131X_REG_6_LEDHDR))
>> +		*fault |= LED_FAULT_UNDER_VOLTAGE;
>> +
>> +	if (reg6 & TPS6131X_REG_6_LEDHOT) {
>> +		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_6,
>> +					      TPS6131X_REG_6_LEDHOT, 0, NULL, false, true);
> 
> And this is not locked?

The read modify write operation is protected by regmap. Since this 
operation does not interact with any other functions, no lock is needed 
here.

> 
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
> 
> ...
> 
>> +
>> +static int tps6131x_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
>> +{
>> +	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>> +
>> +	guard(mutex)(&tps6131x->lock);
>> +
>> +	return tps6131x_set_mode(tps6131x, enable ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
>> +				 false);
>> +}
>> +
>> +static const struct v4l2_flash_ops tps6131x_v4l2_flash_ops = {
>> +	.external_strobe_set = tps6131x_flash_external_strobe_set,
>> +};
>> +
>> +static int tps6131x_v4l2_setup(struct tps6131x *tps6131x)
>> +{
>> +	struct v4l2_flash_config v4l2_cfg = { 0 };
>> +	struct led_flash_setting *intensity = &v4l2_cfg.intensity;
>> +
>> +	if (!IS_BUILTIN(CONFIG_V4L2_FLASH_LED_CLASS))
> 
> Why builtin? That's a tristate, so I don't get why driver and v4l flash
> cannot be modules. You wanted REACHABLE probably... but then it is
> anyway discouraged practice leading to runtime debugging. So actually
> you want CONFIG_V4L2_FLASH_LED_CLASS || !CONFIG_V4L2_FLASH_LED_CLASS
> dependency.

Okay, I'll add 'depends on V4L2_FLASH_LED_CLASS || 
!V4L2_FLASH_LED_CLASS' to the Kconfig entry and do the check in the 
driver like this:
   if (!IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS))
     return 0;

Is this solution okay for you?

> 
> See Kconfig description.
> 
>> +		return 0;
>> +
>> +	intensity->min = tps6131x->step_torch_current_ma;
>> +	intensity->max = tps6131x->max_torch_current_ma;
>> +	intensity->step = tps6131x->step_torch_current_ma;
>> +	intensity->val = intensity->min;
>> +
>> +	strscpy(v4l2_cfg.dev_name, tps6131x->fled_cdev.led_cdev.dev->kobj.name,
>> +		sizeof(v4l2_cfg.dev_name));
>> +
>> +	v4l2_cfg.has_external_strobe = true;
>> +	v4l2_cfg.flash_faults = LED_FAULT_TIMEOUT | LED_FAULT_OVER_TEMPERATURE |
>> +				LED_FAULT_SHORT_CIRCUIT | LED_FAULT_UNDER_VOLTAGE |
>> +				LED_FAULT_LED_OVER_TEMPERATURE;
>> +
>> +	tps6131x->v4l2_flash = v4l2_flash_init(tps6131x->dev, tps6131x->led_node,
>> +					       &tps6131x->fled_cdev, &tps6131x_v4l2_flash_ops,
>> +					       &v4l2_cfg);
>> +	if (IS_ERR(tps6131x->v4l2_flash)) {
>> +		dev_err(tps6131x->dev, "Failed to initialize v4l2 flash LED\n");
>> +		return PTR_ERR(tps6131x->v4l2_flash);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int tps6131x_probe(struct i2c_client *client)
>> +{
>> +	struct tps6131x *tps6131x;
>> +	int ret;
>> +
>> +	tps6131x = devm_kzalloc(&client->dev, sizeof(*tps6131x), GFP_KERNEL);
>> +	if (!tps6131x)
>> +		return -ENOMEM;
>> +
>> +	tps6131x->dev = &client->dev;
>> +	i2c_set_clientdata(client, tps6131x);
>> +	mutex_init(&tps6131x->lock);
>> +	INIT_DELAYED_WORK(&tps6131x->torch_refresh_work, tps6131x_torch_refresh_handler);
>> +
>> +	ret = tps6131x_parse_node(tps6131x);
>> +	if (ret)
>> +		return -ENODEV;
>> +
>> +	tps6131x->regmap = devm_regmap_init_i2c(client, &tps6131x_regmap);
>> +	if (IS_ERR(tps6131x->regmap)) {
>> +		ret = PTR_ERR(tps6131x->regmap);
>> +		dev_err(&client->dev, "Failed to allocate register map\n");
>> +		return ret;
> 
> Syntax is:
> 
> return dev_err_probe

Of course. ACK

> 
>> +	}
>> +
>> +	tps6131x->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
> 
> You should handle IS_ERR

ACK

Thanks!
  ~Matthias

> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[Krzysztof Kozlowski]

On 19/03/2025 17:25, Matthias Fend wrote:
>>> +
>>> +	if (reg3 & TPS6131X_REG_3_HPFL)
>>> +		*fault |= LED_FAULT_SHORT_CIRCUIT;
>>> +
>>> +	if (reg3 & TPS6131X_REG_3_SELSTIM_TO)
>>> +		*fault |= LED_FAULT_TIMEOUT;
>>> +
>>> +	if (reg4 & TPS6131X_REG_4_HOTDIE_HI)
>>> +		*fault |= LED_FAULT_OVER_TEMPERATURE;
>>> +
>>> +	if (reg6 & (TPS6131X_REG_6_LEDHOT | TPS6131X_REG_6_LEDWARN))
>>> +		*fault |= LED_FAULT_LED_OVER_TEMPERATURE;
>>> +
>>> +	if (!(reg6 & TPS6131X_REG_6_LEDHDR))
>>> +		*fault |= LED_FAULT_UNDER_VOLTAGE;
>>> +
>>> +	if (reg6 & TPS6131X_REG_6_LEDHOT) {
>>> +		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_6,
>>> +					      TPS6131X_REG_6_LEDHOT, 0, NULL, false, true);
>>
>> And this is not locked?
> 
> The read modify write operation is protected by regmap. Since this 
> operation does not interact with any other functions, no lock is needed 
> here.


Following that logic no lock is needed in the first place. Define what
is the purpose of this lock, not just "hardware access". I assumed you
want to keep consistent hardware state between multiple updates. If
that's correct, how did you prevent returning value from reads happening
in the middle of concurrent update? Or how this update_bits_base is
prevented from happening while you are in the middle of earlier calls
which are protected by your lock?

That's confusing lock, considering also too short comment explaining its
purpose.

> 
>>
>>> +		if (ret < 0)
>>> +			return ret;
>>> +	}
>>> +
>>
>> ...
>>
>>> +
>>> +static int tps6131x_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
>>> +{
>>> +	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
>>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>>> +
>>> +	guard(mutex)(&tps6131x->lock);
>>> +
>>> +	return tps6131x_set_mode(tps6131x, enable ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
>>> +				 false);
>>> +}
>>> +
>>> +static const struct v4l2_flash_ops tps6131x_v4l2_flash_ops = {
>>> +	.external_strobe_set = tps6131x_flash_external_strobe_set,
>>> +};
>>> +
>>> +static int tps6131x_v4l2_setup(struct tps6131x *tps6131x)
>>> +{
>>> +	struct v4l2_flash_config v4l2_cfg = { 0 };
>>> +	struct led_flash_setting *intensity = &v4l2_cfg.intensity;
>>> +
>>> +	if (!IS_BUILTIN(CONFIG_V4L2_FLASH_LED_CLASS))
>>
>> Why builtin? That's a tristate, so I don't get why driver and v4l flash
>> cannot be modules. You wanted REACHABLE probably... but then it is
>> anyway discouraged practice leading to runtime debugging. So actually
>> you want CONFIG_V4L2_FLASH_LED_CLASS || !CONFIG_V4L2_FLASH_LED_CLASS
>> dependency.
> 
> Okay, I'll add 'depends on V4L2_FLASH_LED_CLASS || 
> !V4L2_FLASH_LED_CLASS' to the Kconfig entry and do the check in the 
> driver like this:

Only this

>    if (!IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS))
>      return 0;
> 
> Is this solution okay for you?

This should should not be needed, because there are v4l2 stubs.

Best regards,
Krzysztof

Re: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[Matthias Fend]

Hi Krzysztof,

Am 19.03.2025 um 20:28 schrieb Krzysztof Kozlowski:
> On 19/03/2025 17:25, Matthias Fend wrote:
>>>> +
>>>> +	if (reg3 & TPS6131X_REG_3_HPFL)
>>>> +		*fault |= LED_FAULT_SHORT_CIRCUIT;
>>>> +
>>>> +	if (reg3 & TPS6131X_REG_3_SELSTIM_TO)
>>>> +		*fault |= LED_FAULT_TIMEOUT;
>>>> +
>>>> +	if (reg4 & TPS6131X_REG_4_HOTDIE_HI)
>>>> +		*fault |= LED_FAULT_OVER_TEMPERATURE;
>>>> +
>>>> +	if (reg6 & (TPS6131X_REG_6_LEDHOT | TPS6131X_REG_6_LEDWARN))
>>>> +		*fault |= LED_FAULT_LED_OVER_TEMPERATURE;
>>>> +
>>>> +	if (!(reg6 & TPS6131X_REG_6_LEDHDR))
>>>> +		*fault |= LED_FAULT_UNDER_VOLTAGE;
>>>> +
>>>> +	if (reg6 & TPS6131X_REG_6_LEDHOT) {
>>>> +		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_6,
>>>> +					      TPS6131X_REG_6_LEDHOT, 0, NULL, false, true);
>>>
>>> And this is not locked?
>>
>> The read modify write operation is protected by regmap. Since this
>> operation does not interact with any other functions, no lock is needed
>> here.
> 
> 
> Following that logic no lock is needed in the first place. Define what
> is the purpose of this lock, not just "hardware access". I assumed you
> want to keep consistent hardware state between multiple updates. If
> that's correct, how did you prevent returning value from reads happening
> in the middle of concurrent update? Or how this update_bits_base is
> prevented from happening while you are in the middle of earlier calls
> which are protected by your lock?
> 
> That's confusing lock, considering also too short comment explaining its
> purpose.

Registers 0, 1, 2, and 3 control parts of the controller that are not 
completely independent of each other.
For some operations, it is therefore necessary to write to the registers 
in a specific order to avoid unwanted side effects.
Therefore, I protected write access to these registers with a lock. The 
RMW sequence in regmap_update_bits_base uses the cached value of the 
register and does not read from the hardware.

Explicit reads to the status registers can be performed at any time. If 
a flag is set, this can be reported.
Since regmap_read_bypassed actually reads from the hardware but doesn't 
update the cache, this isn't a problem either.
Therefore, I don't see any need for a lock here.

My suggestion would be to expand the comment as follows:
/* Hardware access lock for register 0, 1, 2 and 3 */

and add this additional note before it:
/*
  * Registers 0, 1, 2, and 3 control parts of the controller that are 
not completely
  * independent of each other. Since some operations require the 
registers to be written in
  * a specific order to avoid unwanted side effects, they are 
synchronized with a lock.
  */

Do you think that's acceptable?

> 
>>
>>>
>>>> +		if (ret < 0)
>>>> +			return ret;
>>>> +	}
>>>> +
>>>
>>> ...
>>>
>>>> +
>>>> +static int tps6131x_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
>>>> +{
>>>> +	struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev;
>>>> +	struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev);
>>>> +
>>>> +	guard(mutex)(&tps6131x->lock);
>>>> +
>>>> +	return tps6131x_set_mode(tps6131x, enable ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN,
>>>> +				 false);
>>>> +}
>>>> +
>>>> +static const struct v4l2_flash_ops tps6131x_v4l2_flash_ops = {
>>>> +	.external_strobe_set = tps6131x_flash_external_strobe_set,
>>>> +};
>>>> +
>>>> +static int tps6131x_v4l2_setup(struct tps6131x *tps6131x)
>>>> +{
>>>> +	struct v4l2_flash_config v4l2_cfg = { 0 };
>>>> +	struct led_flash_setting *intensity = &v4l2_cfg.intensity;
>>>> +
>>>> +	if (!IS_BUILTIN(CONFIG_V4L2_FLASH_LED_CLASS))
>>>
>>> Why builtin? That's a tristate, so I don't get why driver and v4l flash
>>> cannot be modules. You wanted REACHABLE probably... but then it is
>>> anyway discouraged practice leading to runtime debugging. So actually
>>> you want CONFIG_V4L2_FLASH_LED_CLASS || !CONFIG_V4L2_FLASH_LED_CLASS
>>> dependency.
>>
>> Okay, I'll add 'depends on V4L2_FLASH_LED_CLASS ||
>> !V4L2_FLASH_LED_CLASS' to the Kconfig entry and do the check in the
>> driver like this:
> 
> Only this
> 
>>     if (!IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS))
>>       return 0;
>>
>> Is this solution okay for you?
> 
> This should should not be needed, because there are v4l2 stubs.

True, it works that way too, you're right, of course.
I was initially tempted by the many 
'IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)' in the other drivers, and then 
by the requested switch to an early return.
I will now remove the remaining early return as well.

Thanks!
  ~Matthias

> 
> Best regards,
> Krzysztof

Re: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver

[Dan Carpenter]

Hi Matthias,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Matthias-Fend/dt-bindings-leds-add-Texas-Instruments-TPS6131x-flash-LED-driver/20250318-154318
base:   ffd294d346d185b70e28b1a28abe367bbfe53c04
patch link:    https://lore.kernel.org/r/20250318-leds-tps6131x-v2-2-bc09c7a50b2e%40emfend.at
patch subject: [PATCH v2 2/2] leds: tps6131x: add support for Texas Instruments TPS6131X flash LED driver
config: openrisc-randconfig-r073-20250321 (https://download.01.org/0day-ci/archive/20250322/202503221153.G9HPVANn-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.5.0

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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202503221153.G9HPVANn-lkp@intel.com/

smatch warnings:
drivers/leds/flash/leds-tps6131x.c:534 tps6131x_parse_node() warn: 'num_channels' unsigned <= 0

vim +/num_channels +534 drivers/leds/flash/leds-tps6131x.c

02e7ee00455be1 Matthias Fend 2025-03-18  512  static int tps6131x_parse_node(struct tps6131x *tps6131x)
02e7ee00455be1 Matthias Fend 2025-03-18  513  {
02e7ee00455be1 Matthias Fend 2025-03-18  514  	const struct tps6131x_timer_config *timer_config;
02e7ee00455be1 Matthias Fend 2025-03-18  515  	struct device *dev = tps6131x->dev;
02e7ee00455be1 Matthias Fend 2025-03-18  516  	u32 channels[TPS6131X_MAX_CHANNELS];
02e7ee00455be1 Matthias Fend 2025-03-18  517  	u32 current_step_multiplier;
02e7ee00455be1 Matthias Fend 2025-03-18  518  	u32 current_ua;
02e7ee00455be1 Matthias Fend 2025-03-18  519  	u32 max_current_flash_ma, max_current_torch_ma;
02e7ee00455be1 Matthias Fend 2025-03-18  520  	u32 num_channels;
02e7ee00455be1 Matthias Fend 2025-03-18  521  	u32 timeout_us;
02e7ee00455be1 Matthias Fend 2025-03-18  522  	int i;
02e7ee00455be1 Matthias Fend 2025-03-18  523  	int ret;
02e7ee00455be1 Matthias Fend 2025-03-18  524  
02e7ee00455be1 Matthias Fend 2025-03-18  525  	tps6131x->valley_current_limit = device_property_read_bool(dev, "ti,valley-current-limit");
02e7ee00455be1 Matthias Fend 2025-03-18  526  
02e7ee00455be1 Matthias Fend 2025-03-18  527  	tps6131x->led_node = fwnode_get_next_available_child_node(dev->fwnode, NULL);
02e7ee00455be1 Matthias Fend 2025-03-18  528  	if (!tps6131x->led_node) {
02e7ee00455be1 Matthias Fend 2025-03-18  529  		dev_err(dev, "Missing LED node\n");
02e7ee00455be1 Matthias Fend 2025-03-18  530  		return -EINVAL;
02e7ee00455be1 Matthias Fend 2025-03-18  531  	}
02e7ee00455be1 Matthias Fend 2025-03-18  532  
02e7ee00455be1 Matthias Fend 2025-03-18  533  	num_channels = fwnode_property_count_u32(tps6131x->led_node, "led-sources");
02e7ee00455be1 Matthias Fend 2025-03-18 @534  	if (num_channels <= 0) {

num_channels needs to be signed.

02e7ee00455be1 Matthias Fend 2025-03-18  535  		dev_err(dev, "Failed to read led-sources property\n");
02e7ee00455be1 Matthias Fend 2025-03-18  536  		return -EINVAL;
02e7ee00455be1 Matthias Fend 2025-03-18  537  	}
02e7ee00455be1 Matthias Fend 2025-03-18  538  
02e7ee00455be1 Matthias Fend 2025-03-18  539  	if (num_channels > TPS6131X_MAX_CHANNELS) {
02e7ee00455be1 Matthias Fend 2025-03-18  540  		dev_err(dev, "led-sources count %u exceeds maximum channel count %u\n",
02e7ee00455be1 Matthias Fend 2025-03-18  541  			num_channels, TPS6131X_MAX_CHANNELS);
02e7ee00455be1 Matthias Fend 2025-03-18  542  		return -EINVAL;
02e7ee00455be1 Matthias Fend 2025-03-18  543  	}
02e7ee00455be1 Matthias Fend 2025-03-18  544  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki