[PATCH 0/4] backlight: add new max25014 backlight driver

[PATCH 0/4] backlight: add new max25014 backlight driver

[Maud Spierings via B4 Relay]

The Maxim MAX25014 is an automotive grade backlight driver IC. Its
datasheet can be found at [1].

With its integrated boost controller, it can power 4 channels (led
strings) and has a number of different modes using pwm and or i2c.
Currently implemented is only i2c control.

link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX25014.pdf [1]

Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
Maud Spierings (4):
      dt-bindings: backlight: Add max25014 bindings
      backlight: add max25014atg backlight
      arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight
      arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight

 .../bindings/leds/backlight/maxim,max25014.yaml    |  78 ++++
 MAINTAINERS                                        |   7 +
 ...x8p-ml81-moduline-display-106-av101hdt-a10.dtso |  21 +
 ...x8p-ml81-moduline-display-106-av123z7m-n17.dtso |  19 +-
 drivers/video/backlight/Kconfig                    |   7 +
 drivers/video/backlight/Makefile                   |   1 +
 drivers/video/backlight/max25014.c                 | 449 +++++++++++++++++++++
 include/linux/platform_data/max25014.h             |  24 ++
 8 files changed, 605 insertions(+), 1 deletion(-)
---
base-commit: d7af19298454ed155f5cf67201a70f5cf836c842
change-id: 20250626-max25014-4207591e1af5

Best regards,
-- 
Maud Spierings <maudspierings@gocontroll.com>

[PATCH 1/4] dt-bindings: backlight: Add max25014 bindings

[Maud Spierings via B4 Relay]

From: Maud Spierings <maudspierings@gocontroll.com>

The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with intgrated boost controller.

Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
 .../bindings/leds/backlight/maxim,max25014.yaml    | 78 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 2 files changed, 83 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..63baf1cbcbb14ed93d7e45ba013080872060ce71
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim max25014 backlight controller
+
+maintainers:
+  - Maud Spierings <maudspierings@gocontroll.com>
+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - maxim,max25014
+
+  reg:
+    maxItems: 1
+
+  enable-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  power-supply:
+    description: Regulator which controls the boost converter input rail.
+
+  pwms:
+    maxItems: 1
+
+  maxim,iset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 11
+    description:
+      Value of the ISET register field (0-15). Default is 11.
+
+  maxim,strings:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      A 4-bit bitfield that describes which led strings to turn on.
+    minItems: 4
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - maxim,strings
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        backlight: backlight@6f {
+            reg = <0x6f>;
+            compatible = "maxim,max25014";
+            bl-name = "max25014";
+            default-brightness = <50>;
+            enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&pinctrl_backlight>;
+            maxim,iset = <7>;
+            maxim,strings = <1 1 1 1>;
+        };
+    };
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 5e18349a5556e4d793b63a6b3c600000b21b0418..9e9a45442b7bf60b00eaf3f8ebadf8f4fbaf3bef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14907,6 +14907,11 @@ F:	Documentation/userspace-api/media/drivers/max2175.rst
 F:	drivers/media/i2c/max2175*
 F:	include/uapi/linux/max2175.h
 
+MAX25014 BACKLIGHT DRIVER
+M:	Maud Spierings <maudspierings@gocontroll.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
+
 MAX31335 RTC DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
 L:	linux-rtc@vger.kernel.org

-- 
2.50.1

Re: [PATCH 1/4] dt-bindings: backlight: Add max25014 bindings

[Rob Herring (Arm)]

On Fri, 25 Jul 2025 13:09:23 +0200, Maud Spierings wrote:
> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
> with intgrated boost controller.
> 
> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> ---
>  .../bindings/leds/backlight/maxim,max25014.yaml    | 78 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 ++
>  2 files changed, 83 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.example.dtb: backlight@6f (maxim,max25014): Unevaluated properties are not allowed ('bl-name' was unexpected)
	from schema $id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250725-max25014-v1-1-0e8cce92078e@gocontroll.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/4] dt-bindings: backlight: Add max25014 bindings

[Maud Spierings]

On 7/25/25 15:27, Rob Herring (Arm) wrote:
> 
> On Fri, 25 Jul 2025 13:09:23 +0200, Maud Spierings wrote:
>> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
>> with intgrated boost controller.
>>
>> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
>> ---
>>   .../bindings/leds/backlight/maxim,max25014.yaml    | 78 ++++++++++++++++++++++
>>   MAINTAINERS                                        |  5 ++
>>   2 files changed, 83 insertions(+)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:

Pretty sure I did that, but I've never gotten those tools to work quite 
right, I'll look at it for v2
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.example.dtb: backlight@6f (maxim,max25014): Unevaluated properties are not allowed ('bl-name' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#

Ah oops, leftover from old version, fixed in rv2

> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250725-max25014-v1-1-0e8cce92078e@gocontroll.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 
Kind Regards,
Maud

Re: [PATCH 1/4] dt-bindings: backlight: Add max25014 bindings

[Rob Herring]

On Fri, Jul 25, 2025 at 04:06:45PM +0200, Maud Spierings wrote:
> 
> 
> On 7/25/25 15:27, Rob Herring (Arm) wrote:
> > 
> > On Fri, 25 Jul 2025 13:09:23 +0200, Maud Spierings wrote:
> > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
> > > with intgrated boost controller.
> > > 
> > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> > > ---
> > >   .../bindings/leds/backlight/maxim,max25014.yaml    | 78 ++++++++++++++++++++++
> > >   MAINTAINERS                                        |  5 ++
> > >   2 files changed, 83 insertions(+)
> > > 
> > 
> > My bot found errors running 'make dt_binding_check' on your patch:
> 
> Pretty sure I did that, but I've never gotten those tools to work quite
> right, I'll look at it for v2

What's the issue?

Rob

Re: [PATCH 1/4] dt-bindings: backlight: Add max25014 bindings

[Maud Spierings]

On 7/25/25 21:51, Rob Herring wrote:
> On Fri, Jul 25, 2025 at 04:06:45PM +0200, Maud Spierings wrote:
>>
>>
>> On 7/25/25 15:27, Rob Herring (Arm) wrote:
>>>
>>> On Fri, 25 Jul 2025 13:09:23 +0200, Maud Spierings wrote:
>>>> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
>>>> with intgrated boost controller.
>>>>
>>>> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
>>>> ---
>>>>    .../bindings/leds/backlight/maxim,max25014.yaml    | 78 ++++++++++++++++++++++
>>>>    MAINTAINERS                                        |  5 ++
>>>>    2 files changed, 83 insertions(+)
>>>>
>>>
>>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> Pretty sure I did that, but I've never gotten those tools to work quite
>> right, I'll look at it for v2
> 
> What's the issue?

I'm on arch, and I believe there is some specific version dependencies 
in some of the libraries used, so I have a venv for doing kernel work 
but it still doesn't seem quite happy sometimes. I may have just messed 
something in that part and thought I did it correctly, but didn't.

I think I have it working properly now though, just did a quick test and 
it worked as intended.

[PATCH 2/4] backlight: add max25014atg backlight

[Maud Spierings via B4 Relay]

From: Maud Spierings <maudspierings@gocontroll.com>

The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with intgrated boost controller.

Signed-off-by: Maud Spierings maudspierings@gocontroll.com
---
 MAINTAINERS                            |   2 +
 drivers/video/backlight/Kconfig        |   7 +
 drivers/video/backlight/Makefile       |   1 +
 drivers/video/backlight/max25014.c     | 449 +++++++++++++++++++++++++++++++++
 include/linux/platform_data/max25014.h |  24 ++
 5 files changed, 483 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e9a45442b7bf60b00eaf3f8ebadf8f4fbaf3bef..3cae3402f531921a93208626ba9477018e7ef347 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14911,6 +14911,8 @@ MAX25014 BACKLIGHT DRIVER
 M:	Maud Spierings <maudspierings@gocontroll.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
+F:	drivers/video/backlight/max25014.c
+F:	include/linux/platform_data/max25014.h
 
 MAX31335 RTC DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index d9374d208ceebbf8b3c27976e9cb4d725939b942..d3bb6ccd41853d940f24c6ab8135e4b9b9ebefd7 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -262,6 +262,13 @@ config BACKLIGHT_DA9052
 	help
 	  Enable the Backlight Driver for DA9052-BC and DA9053-AA/Bx PMICs.
 
+config BACKLIGHT_MAX25014
+	tristate "Backlight driver for the Maxim MAX25014 chip"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  If you are using a MAX25014 chip as a backlight driver say Y to enable it.
+
 config BACKLIGHT_MAX8925
 	tristate "Backlight driver for MAX8925"
 	depends on MFD_MAX8925
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index dfbb169bf6ea215704859f633b6c4a887f4ebacd..1170d9ec40b8dbd52aeec1dade1cd2d2b56af466 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO)		+= locomolcd.o
 obj-$(CONFIG_BACKLIGHT_LP855X)		+= lp855x_bl.o
 obj-$(CONFIG_BACKLIGHT_LP8788)		+= lp8788_bl.o
 obj-$(CONFIG_BACKLIGHT_LV5207LP)	+= lv5207lp.o
+obj-$(CONFIG_BACKLIGHT_MAX25014)	+= max25014.o
 obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
 obj-$(CONFIG_BACKLIGHT_MP3309C)		+= mp3309c.o
 obj-$(CONFIG_BACKLIGHT_MT6370)		+= mt6370-backlight.o
diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c
new file mode 100644
index 0000000000000000000000000000000000000000..371b6017953ae5955f4dfef921980dfdedd65d85
--- /dev/null
+++ b/drivers/video/backlight/max25014.c
@@ -0,0 +1,449 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Backlight driver for Maxim MAX25014
+ *
+ * Copyright (C) 2025 GOcontroll B.V.
+ * Author: Maud Spierings <maudspierings@gocontroll.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/platform_data/max25014.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define MAX25014_ISET_DEFAULT_100 11U
+#define MAX_BRIGHTNESS (100U)
+#define MIN_BRIGHTNESS (0U)
+#define TON_MAX (130720U) /* @153Hz */
+#define TON_STEP (1307U) /* @153Hz */
+#define TON_MIN (0U)
+
+#define MAX25014_DEV_ID         (0x00U)
+#define MAX25014_REV_ID         (0x01U)
+#define MAX25014_ISET           (0x02U)
+#define MAX25014_IMODE          (0x03U)
+#define MAX25014_TON1H          (0x04U)
+#define MAX25014_TON1L          (0x05U)
+#define MAX25014_TON2H          (0x06U)
+#define MAX25014_TON2L          (0x07U)
+#define MAX25014_TON3H          (0x08U)
+#define MAX25014_TON3L          (0x09U)
+#define MAX25014_TON4H          (0x0AU)
+#define MAX25014_TON4L          (0x0BU)
+#define MAX25014_TON_1_4_LSB    (0x0CU)
+#define MAX25014_SETTING        (0x12U)
+#define MAX25014_DISABLE        (0x13U)
+#define MAX25014_BSTMON         (0x14U)
+#define MAX25014_IOUT1          (0x15U)
+#define MAX25014_IOUT2          (0x16U)
+#define MAX25014_IOUT3          (0x17U)
+#define MAX25014_IOUT4          (0x18U)
+#define MAX25014_OPEN           (0x1BU)
+#define MAX25014_SHORT_GND      (0x1CU)
+#define MAX25014_SHORT_LED      (0x1DU)
+#define MAX25014_MASK           (0x1EU)
+#define MAX25014_DIAG           (0x1FU)
+
+#define MAX25014_IMODE_HDIM     BIT(2)
+#define MAX25014_ISET_ENABLE    BIT(5)
+#define MAX25014_ISET_PSEN      BIT(4)
+#define MAX25014_DIAG_HW_RST    BIT(2)
+#define MAX25014_SETTING_FPWM   GENMASK(6, 4)
+
+struct max25014;
+
+struct max25014 {
+	const char *chipname;
+	struct i2c_client *client;
+	struct backlight_device *bl;
+	struct device *dev;
+	struct regmap *regmap;
+	struct max25014_platform_data *pdata;
+	struct gpio_desc *enable;
+	struct regulator *vin; /* regulator for boost converter Vin rail */
+};
+
+static const struct regmap_config max25014_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MAX25014_DIAG,
+};
+
+/**
+ * @brief get the bit mask for the DISABLE register.
+ *
+ * @param strings the led string configuration array.
+ * @return uint8_t bits to set in the register.
+ */
+static uint8_t strings_mask(struct max25014 *maxim)
+{
+	uint8_t res, i;
+
+	for (i = 0; i < 4; i++) {
+		if (maxim->pdata->strings[i] == 0)
+			res |= 1 << i;
+	}
+	return res;
+}
+
+/**
+ * @brief control the brightness with i2c registers
+ *
+ * @param regmap trivial
+ * @param brt brightness
+ * @return int
+ */
+static int max25014_register_control(struct regmap *regmap, uint32_t brt)
+{
+	uint32_t reg = TON_STEP * brt;
+	int ret;
+	/*
+	 * 18 bit number lowest, 2 bits in first register,
+	 * next lowest 8 in the L register, next 8 in the H register
+	 * Seemingly setting the strength of only one string controls all of
+	 * them, individual settings don't affect the outcome.
+	 */
+
+	ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011);
+	if (ret != 0)
+		return ret;
+	ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111);
+	if (ret != 0)
+		return ret;
+	return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111);
+}
+
+static int max25014_check_errors(struct max25014 *maxim)
+{
+	uint8_t i;
+	int ret;
+	uint32_t val;
+
+	ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val);
+	if (ret != 0)
+		return ret;
+	if (val > 0) {
+		dev_err(maxim->dev, "Open led strings detected on:\n");
+		for (i = 0; i < 4; i++) {
+			if (val & 1 << i)
+				dev_err(maxim->dev, "string %d\n", i + 1);
+		}
+		return -EIO;
+	}
+
+	ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
+	if (ret != 0)
+		return ret;
+	if (val > 0) {
+		dev_err(maxim->dev, "Short to ground detected on:\n");
+		for (i = 0; i < 4; i++) {
+			if (val & 1 << i)
+				dev_err(maxim->dev, "string %d\n", i + 1);
+		}
+		return -EIO;
+	}
+
+	ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
+	if (ret != 0)
+		return ret;
+	if (val > 0) {
+		dev_err(maxim->dev, "Shorted led detected on:\n");
+		for (i = 0; i < 4; i++) {
+			if (val & 1 << i)
+				dev_err(maxim->dev, "string %d\n", i + 1);
+		}
+		return -EIO;
+	}
+
+	ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val);
+	if (ret != 0)
+		return ret;
+	/*
+	 * The HW_RST bit always starts at 1 after power up.
+	 * It is reset on first read, does not indicate an error.
+	 */
+	if (val > 0 && val != MAX25014_DIAG_HW_RST) {
+		if (val & 0b1)
+			dev_err(maxim->dev, "Overtemperature shutdown\n");
+		if (val & 0b10)
+			dev_warn(maxim->dev,
+				 "Chip is getting too hot (>125C)\n");
+		if (val & 0b1000)
+			dev_err(maxim->dev, "Boost converter overvoltage\n");
+		if (val & 0b10000)
+			dev_err(maxim->dev, "Boost converter undervoltage\n");
+		if (val & 0b100000)
+			dev_err(maxim->dev, "IREF out of range\n");
+		return -EIO;
+	}
+	return 0;
+}
+
+/*
+ * 1. disable unused strings
+ * 2. set dim mode
+ * 3. set initial brightness
+ * 4. set setting register
+ * 5. enable the backlight
+ */
+static int max25014_configure(struct max25014 *maxim)
+{
+	int ret;
+	uint32_t val;
+
+	ret = regmap_write(maxim->regmap, MAX25014_DISABLE,
+			   strings_mask(maxim));
+	if (ret != 0)
+		return ret;
+
+	ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM);
+	if (ret != 0)
+		return ret;
+
+	max25014_register_control(maxim->regmap,
+				  maxim->pdata->initial_brightness);
+
+	ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val);
+	if (ret != 0)
+		return ret;
+
+	ret = regmap_write(
+		maxim->regmap, MAX25014_SETTING,
+		val & ~MAX25014_SETTING_FPWM);
+	if (ret != 0)
+		return ret;
+
+	ret = regmap_write(maxim->regmap, MAX25014_ISET,
+			   maxim->pdata->iset | MAX25014_ISET_ENABLE | MAX25014_ISET_PSEN);
+	return ret;
+}
+
+static int max25014_update_status(struct backlight_device *bl_dev)
+{
+	struct max25014 *maxim = bl_get_data(bl_dev);
+
+	if (bl_dev->props.state & BL_CORE_SUSPENDED)
+		bl_dev->props.brightness = 0;
+
+	return max25014_register_control(maxim->regmap, bl_dev->props.brightness);
+}
+
+static const struct backlight_ops max25014_bl_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = max25014_update_status,
+};
+
+static int max25014_backlight_register(struct max25014 *maxim)
+{
+	struct backlight_device *bl;
+	struct backlight_properties props;
+	struct max25014_platform_data *pdata = maxim->pdata;
+
+	memset(&props, 0, sizeof(props));
+	props.type = BACKLIGHT_PLATFORM;
+	props.max_brightness = MAX_BRIGHTNESS;
+
+	if (pdata->initial_brightness > props.max_brightness)
+		pdata->initial_brightness = props.max_brightness;
+
+	props.brightness = pdata->initial_brightness;
+
+	bl = devm_backlight_device_register(maxim->dev, maxim->chipname, maxim->dev,
+					    maxim, &max25014_bl_ops, &props);
+	if (IS_ERR(bl))
+		return PTR_ERR(bl);
+
+	maxim->bl = bl;
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static int max25014_parse_dt(struct max25014 *maxim)
+{
+	struct device *dev = maxim->dev;
+	struct device_node *node = dev->of_node;
+	struct max25014_platform_data *pdata;
+
+	int res;
+
+	if (!node) {
+		dev_err(dev, "no platform data\n");
+		return -EINVAL;
+	}
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	res = of_property_count_u32_elems(node, "maxim,strings");
+	if (res == 4) {
+		of_property_read_u32_array(node, "maxim,strings", pdata->strings, 4);
+	} else {
+		dev_err(dev, "strings property not correctly defined\n");
+		return -EINVAL;
+	}
+
+	pdata->initial_brightness = 50U;
+	of_property_read_u32(node, "default-brightness", &pdata->initial_brightness);
+	pdata->iset = MAX25014_ISET_DEFAULT_100;
+	of_property_read_u32(node, "maxim,iset", &pdata->iset);
+
+	if (pdata->iset < 0 || pdata->iset > 15) {
+		dev_err(dev,
+			"Invalid iset, should be a value from 0-15, entered was %d\n",
+			pdata->iset);
+		return -EINVAL;
+	}
+
+	if (pdata->initial_brightness < 0 || pdata->initial_brightness > 100) {
+		dev_err(dev,
+			"Invalid initial brightness, should be a value from 0-100, entered was %d\n",
+			pdata->initial_brightness);
+		return -EINVAL;
+	}
+
+	maxim->pdata = pdata;
+
+	return 0;
+}
+#else
+static int max25014_parse_dt(struct max25014 *maxim)
+{
+	dev_err(maxim->dev,
+		"CONFIG_OF not configured, unable to parse devicetree");
+	return -EINVAL;
+}
+#endif
+
+static int max25014_probe(struct i2c_client *cl)
+{
+	const struct i2c_device_id *id = i2c_client_get_device_id(cl);
+	struct max25014 *maxim;
+	int ret;
+
+	maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL);
+	if (!maxim)
+		return -ENOMEM;
+
+	maxim->client = cl;
+	maxim->dev = &cl->dev;
+	maxim->chipname = id->name;
+	maxim->pdata = dev_get_platdata(&cl->dev);
+
+	if (!maxim->pdata) {
+		ret = max25014_parse_dt(maxim);
+		if (ret < 0)
+			return ret;
+	}
+
+	maxim->vin = devm_regulator_get(maxim->dev, "power");
+	if (IS_ERR(maxim->vin)) {
+		if (PTR_ERR(maxim->vin) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		maxim->vin = NULL;
+	}
+
+	if (maxim->vin) {
+		ret = regulator_enable(maxim->vin);
+		if (ret < 0) {
+			dev_err(maxim->dev, "failed to enable Vin: %d\n", ret);
+			return ret;
+		}
+	}
+
+	maxim->enable =
+		devm_gpiod_get_optional(maxim->dev, "enable", GPIOD_ASIS);
+	if (IS_ERR(maxim->enable)) {
+		ret = PTR_ERR(maxim->enable);
+		dev_err(maxim->dev, "failed to get enable gpio: %d\n", ret);
+		goto disable_vin;
+	}
+
+	if (maxim->enable) {
+		gpiod_set_value_cansleep(maxim->enable, 1);
+
+		/* Datasheet Electrical Characteristics tSTARTUP 2ms */
+		usleep_range(2000, 2500);
+	}
+
+	maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config);
+	if (IS_ERR(maxim->regmap)) {
+		ret = PTR_ERR(maxim->regmap);
+		dev_err(maxim->dev, "failed to initialize the i2c regmap: %d\n", ret);
+		goto disable_full;
+	}
+
+	i2c_set_clientdata(cl, maxim);
+
+	ret = max25014_check_errors(maxim);
+	if (ret) { /* error is already reported in the above function */
+		goto disable_full;
+	}
+
+	ret = max25014_configure(maxim);
+	if (ret) {
+		dev_err(maxim->dev, "device config err: %d", ret);
+		goto disable_full;
+	}
+
+	ret = max25014_backlight_register(maxim);
+	if (ret) {
+		dev_err(maxim->dev, "failed to register backlight. err: %d\n",
+			ret);
+		goto disable_full;
+	}
+
+	dev_dbg(maxim->dev, "max25014 probed.\n");
+
+	return 0;
+
+disable_full:
+	if (maxim->enable)
+		gpiod_set_value_cansleep(maxim->enable, 0);
+disable_vin:
+	if (maxim->vin)
+		regulator_disable(maxim->vin);
+	return ret;
+}
+
+static void max25014_remove(struct i2c_client *cl)
+{
+	struct max25014 *maxim = i2c_get_clientdata(cl);
+
+	maxim->bl->props.brightness = 0;
+	max25014_update_status(maxim->bl);
+	if (maxim->enable)
+		gpiod_set_value_cansleep(maxim->enable, 0);
+	if (maxim->vin)
+		regulator_disable(maxim->vin);
+}
+
+static const struct of_device_id max25014_dt_ids[] = {
+	{ .compatible = "maxim,max25014", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max25014_dt_ids);
+
+static const struct i2c_device_id max25014_ids[] = {
+	{ "max25014" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max25014_ids);
+
+static struct i2c_driver max25014_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(max25014_dt_ids),
+	},
+	.probe = max25014_probe,
+	.remove = max25014_remove,
+	.id_table = max25014_ids,
+};
+module_i2c_driver(max25014_driver);
+
+MODULE_DESCRIPTION("Maxim MAX25014 backlight driver");
+MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/platform_data/max25014.h b/include/linux/platform_data/max25014.h
new file mode 100644
index 0000000000000000000000000000000000000000..048f11015e9551e619ddec9167e3f10394f44bb9
--- /dev/null
+++ b/include/linux/platform_data/max25014.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Backlight driver for Maxim MAX25014
+ *
+ * Copyright (C) 2025 GOcontroll B.V.
+ * Author: Maud Spierings <maudspierings@gocontroll.com>
+ */
+
+#ifndef _MAX25014_H
+#define _MAX25014_H
+
+/**
+ * struct max25014_platform_data
+ * @initial_brightness : Initial value of the backlight brightness.
+ * @iset : Value of the iset field which scales the amperage/limits it.
+ * @strings : Which, out of four, led strings are in use.
+ */
+struct max25014_platform_data {
+	uint32_t initial_brightness;
+	uint32_t iset;
+	uint32_t strings[4];
+};
+
+#endif

-- 
2.50.1

Re: [PATCH 2/4] backlight: add max25014atg backlight

[kernel test robot]

Hi Maud,

kernel test robot noticed the following build warnings:

[auto build test WARNING on d7af19298454ed155f5cf67201a70f5cf836c842]

url:    https://github.com/intel-lab-lkp/linux/commits/Maud-Spierings-via-B4-Relay/dt-bindings-backlight-Add-max25014-bindings/20250725-191221
base:   d7af19298454ed155f5cf67201a70f5cf836c842
patch link:    https://lore.kernel.org/r/20250725-max25014-v1-2-0e8cce92078e%40gocontroll.com
patch subject: [PATCH 2/4] backlight: add max25014atg backlight
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250727/202507270543.G0TT6f25-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250727/202507270543.G0TT6f25-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/202507270543.G0TT6f25-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/video/backlight/max25014.c:86:4: warning: variable 'res' is uninitialized when used here [-Wuninitialized]
      86 |                         res |= 1 << i;
         |                         ^~~
   drivers/video/backlight/max25014.c:82:13: note: initialize the variable 'res' to silence this warning
      82 |         uint8_t res, i;
         |                    ^
         |                     = '\0'
   1 warning generated.


vim +/res +86 drivers/video/backlight/max25014.c

    73	
    74	/**
    75	 * @brief get the bit mask for the DISABLE register.
    76	 *
    77	 * @param strings the led string configuration array.
    78	 * @return uint8_t bits to set in the register.
    79	 */
    80	static uint8_t strings_mask(struct max25014 *maxim)
    81	{
    82		uint8_t res, i;
    83	
    84		for (i = 0; i < 4; i++) {
    85			if (maxim->pdata->strings[i] == 0)
  > 86				res |= 1 << i;
    87		}
    88		return res;
    89	}
    90	

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

Re: [PATCH 2/4] backlight: add max25014atg backlight

[Daniel Thompson]

On Fri, Jul 25, 2025 at 01:09:24PM +0200, Maud Spierings via B4 Relay wrote:
> From: Maud Spierings <maudspierings@gocontroll.com>
>
> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
> with intgrated boost controller.
>
> Signed-off-by: Maud Spierings maudspierings@gocontroll.com
> ---
>  MAINTAINERS                            |   2 +
>  drivers/video/backlight/Kconfig        |   7 +
>  drivers/video/backlight/Makefile       |   1 +
>  drivers/video/backlight/max25014.c     | 449 +++++++++++++++++++++++++++++++++
>  include/linux/platform_data/max25014.h |  24 ++

Who else included this header file? Can the code here simply be included
in the C file?


> diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..371b6017953ae5955f4dfef921980dfdedd65d85
> --- /dev/null
> +++ b/drivers/video/backlight/max25014.c
> @@ -0,0 +1,449 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Backlight driver for Maxim MAX25014
> + *
> + * Copyright (C) 2025 GOcontroll B.V.
> + * Author: Maud Spierings <maudspierings@gocontroll.com>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/platform_data/max25014.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define MAX25014_ISET_DEFAULT_100 11U
> +#define MAX_BRIGHTNESS (100U)
> +#define MIN_BRIGHTNESS (0U)
> +#define TON_MAX (130720U) /* @153Hz */
> +#define TON_STEP (1307U) /* @153Hz */
> +#define TON_MIN (0U)
> +
> +#define MAX25014_DEV_ID         (0x00U)
> +#define MAX25014_REV_ID         (0x01U)
> +#define MAX25014_ISET           (0x02U)
> +#define MAX25014_IMODE          (0x03U)
> +#define MAX25014_TON1H          (0x04U)
> +#define MAX25014_TON1L          (0x05U)
> +#define MAX25014_TON2H          (0x06U)
> +#define MAX25014_TON2L          (0x07U)
> +#define MAX25014_TON3H          (0x08U)
> +#define MAX25014_TON3L          (0x09U)
> +#define MAX25014_TON4H          (0x0AU)
> +#define MAX25014_TON4L          (0x0BU)
> +#define MAX25014_TON_1_4_LSB    (0x0CU)
> +#define MAX25014_SETTING        (0x12U)
> +#define MAX25014_DISABLE        (0x13U)
> +#define MAX25014_BSTMON         (0x14U)
> +#define MAX25014_IOUT1          (0x15U)
> +#define MAX25014_IOUT2          (0x16U)
> +#define MAX25014_IOUT3          (0x17U)
> +#define MAX25014_IOUT4          (0x18U)
> +#define MAX25014_OPEN           (0x1BU)
> +#define MAX25014_SHORT_GND      (0x1CU)
> +#define MAX25014_SHORT_LED      (0x1DU)
> +#define MAX25014_MASK           (0x1EU)
> +#define MAX25014_DIAG           (0x1FU)

Forcing all these constants to be unsigned is unusual. Is it really
needed?


> +#define MAX25014_IMODE_HDIM     BIT(2)
> +#define MAX25014_ISET_ENABLE    BIT(5)
> +#define MAX25014_ISET_PSEN      BIT(4)
> +#define MAX25014_DIAG_HW_RST    BIT(2)
> +#define MAX25014_SETTING_FPWM   GENMASK(6, 4)
> +
> +struct max25014;

This is redundant. Remove.
> +
> +struct max25014 {
> +	const char *chipname;

Why keep this value around? It is only used during the probe.

> +	struct i2c_client *client;
> +	struct backlight_device *bl;
> +	struct device *dev;

It is necessary to cache this, is it just a copy of client->dev)?


> +	struct regmap *regmap;
> +	struct max25014_platform_data *pdata;
> +	struct gpio_desc *enable;
> +	struct regulator *vin; /* regulator for boost converter Vin rail */
> +};
> +
> +static const struct regmap_config max25014_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX25014_DIAG,
> +};
> +
> +/**
> + * @brief get the bit mask for the DISABLE register.
> + *
> + * @param strings the led string configuration array.
> + * @return uint8_t bits to set in the register.
> + */
> +static uint8_t strings_mask(struct max25014 *maxim)
> +{
> +	uint8_t res, i;
> +
> +	for (i = 0; i < 4; i++) {
> +		if (maxim->pdata->strings[i] == 0)
> +			res |= 1 << i;
> +	}
> +	return res;

Could this converison have happened during DT parsing?
> +}
> +
> +/**
> + * @brief control the brightness with i2c registers
> + *
> + * @param regmap trivial
> + * @param brt brightness
> + * @return int
> + */
> +static int max25014_register_control(struct regmap *regmap, uint32_t brt)
> +{
> +	uint32_t reg = TON_STEP * brt;
> +	int ret;
> +	/*
> +	 * 18 bit number lowest, 2 bits in first register,
> +	 * next lowest 8 in the L register, next 8 in the H register
> +	 * Seemingly setting the strength of only one string controls all of
> +	 * them, individual settings don't affect the outcome.
> +	 */
> +
> +	ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011);
> +	if (ret != 0)
> +		return ret;
> +	ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111);
> +	if (ret != 0)
> +		return ret;
> +	return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111);
> +}
> +
> +static int max25014_check_errors(struct max25014 *maxim)
> +{
> +	uint8_t i;
> +	int ret;
> +	uint32_t val;
> +
> +	ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val);
> +	if (ret != 0)
> +		return ret;
> +	if (val > 0) {
> +		dev_err(maxim->dev, "Open led strings detected on:\n");
> +		for (i = 0; i < 4; i++) {
> +			if (val & 1 << i)
> +				dev_err(maxim->dev, "string %d\n", i + 1);
> +		}
> +		return -EIO;
> +	}
> +
> +	ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
> +	if (ret != 0)
> +		return ret;
> +	if (val > 0) {
> +		dev_err(maxim->dev, "Short to ground detected on:\n");
> +		for (i = 0; i < 4; i++) {
> +			if (val & 1 << i)
> +				dev_err(maxim->dev, "string %d\n", i + 1);
> +		}
> +		return -EIO;
> +	}
> +
> +	ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
> +	if (ret != 0)
> +		return ret;
> +	if (val > 0) {
> +		dev_err(maxim->dev, "Shorted led detected on:\n");
> +		for (i = 0; i < 4; i++) {
> +			if (val & 1 << i)
> +				dev_err(maxim->dev, "string %d\n", i + 1);
> +		}
> +		return -EIO;
> +	}
> +
> +	ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val);
> +	if (ret != 0)
> +		return ret;
> +	/*
> +	 * The HW_RST bit always starts at 1 after power up.
> +	 * It is reset on first read, does not indicate an error.
> +	 */
> +	if (val > 0 && val != MAX25014_DIAG_HW_RST) {
> +		if (val & 0b1)
> +			dev_err(maxim->dev, "Overtemperature shutdown\n");
> +		if (val & 0b10)
> +			dev_warn(maxim->dev,
> +				 "Chip is getting too hot (>125C)\n");
> +		if (val & 0b1000)
> +			dev_err(maxim->dev, "Boost converter overvoltage\n");
> +		if (val & 0b10000)
> +			dev_err(maxim->dev, "Boost converter undervoltage\n");
> +		if (val & 0b100000)
> +			dev_err(maxim->dev, "IREF out of range\n");
> +		return -EIO;
> +	}
> +	return 0;
> +}
> +
> +/*
> + * 1. disable unused strings
> + * 2. set dim mode
> + * 3. set initial brightness
> + * 4. set setting register
> + * 5. enable the backlight
> + */
> +static int max25014_configure(struct max25014 *maxim)
> +{
> +	int ret;
> +	uint32_t val;
> +
> +	ret = regmap_write(maxim->regmap, MAX25014_DISABLE,
> +			   strings_mask(maxim));
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM);
> +	if (ret != 0)
> +		return ret;
> +
> +	max25014_register_control(maxim->regmap,
> +				  maxim->pdata->initial_brightness);
> +
> +	ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = regmap_write(
> +		maxim->regmap, MAX25014_SETTING,
> +		val & ~MAX25014_SETTING_FPWM);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = regmap_write(maxim->regmap, MAX25014_ISET,
> +			   maxim->pdata->iset | MAX25014_ISET_ENABLE | MAX25014_ISET_PSEN);
> +	return ret;
> +}
> +
> +static int max25014_update_status(struct backlight_device *bl_dev)
> +{
> +	struct max25014 *maxim = bl_get_data(bl_dev);
> +
> +	if (bl_dev->props.state & BL_CORE_SUSPENDED)
> +		bl_dev->props.brightness = 0;
> +
> +	return max25014_register_control(maxim->regmap, bl_dev->props.brightness);
> +}
> +
> +static const struct backlight_ops max25014_bl_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
> +	.update_status = max25014_update_status,
> +};
> +
> +static int max25014_backlight_register(struct max25014 *maxim)
> +{
> +	struct backlight_device *bl;
> +	struct backlight_properties props;
> +	struct max25014_platform_data *pdata = maxim->pdata;
> +
> +	memset(&props, 0, sizeof(props));
> +	props.type = BACKLIGHT_PLATFORM;
> +	props.max_brightness = MAX_BRIGHTNESS;
> +
> +	if (pdata->initial_brightness > props.max_brightness)
> +		pdata->initial_brightness = props.max_brightness;

Handle this during DT parsing.

> +
> +	props.brightness = pdata->initial_brightness;
> +
> +	bl = devm_backlight_device_register(maxim->dev, maxim->chipname, maxim->dev,
> +					    maxim, &max25014_bl_ops, &props);
> +	if (IS_ERR(bl))
> +		return PTR_ERR(bl);
> +
> +	maxim->bl = bl;
> +
> +	return 0;
> +}

Can max25014_backlight_register() be moved into the probe function?
There is no special control flow here so this function doesn't make the
probe function any simpler.

> +
> +#ifdef CONFIG_OF
> +static int max25014_parse_dt(struct max25014 *maxim)
> +{
> +	struct device *dev = maxim->dev;
> +	struct device_node *node = dev->of_node;
> +	struct max25014_platform_data *pdata;
> +
> +	int res;
> +
> +	if (!node) {
> +		dev_err(dev, "no platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	res = of_property_count_u32_elems(node, "maxim,strings");
> +	if (res == 4) {
> +		of_property_read_u32_array(node, "maxim,strings", pdata->strings, 4);
> +	} else {
> +		dev_err(dev, "strings property not correctly defined\n");
> +		return -EINVAL;
> +	}
> +
> +	pdata->initial_brightness = 50U;
> +	of_property_read_u32(node, "default-brightness", &pdata->initial_brightness);
> +	pdata->iset = MAX25014_ISET_DEFAULT_100;
> +	of_property_read_u32(node, "maxim,iset", &pdata->iset);
> +
> +	if (pdata->iset < 0 || pdata->iset > 15) {
> +		dev_err(dev,
> +			"Invalid iset, should be a value from 0-15, entered was %d\n",
> +			pdata->iset);
> +		return -EINVAL;
> +	}
> +
> +	if (pdata->initial_brightness < 0 || pdata->initial_brightness > 100) {
> +		dev_err(dev,
> +			"Invalid initial brightness, should be a value from 0-100, entered was %d\n",
> +			pdata->initial_brightness);
> +		return -EINVAL;
> +	}
> +
> +	maxim->pdata = pdata;
> +
> +	return 0;
> +}
> +#else
> +static int max25014_parse_dt(struct max25014 *maxim)
> +{
> +	dev_err(maxim->dev,
> +		"CONFIG_OF not configured, unable to parse devicetree");
> +	return -EINVAL;
> +}

What is the point of this method? New drivers shouldn't support platform
data so CONFIG_OF is required for this driver to work at all.


> +#endif
> +
> +static int max25014_probe(struct i2c_client *cl)
> ...


Daniel.

Re: [PATCH 2/4] backlight: add max25014atg backlight

[Maud Spierings]

Hi Daniel,

Thank you very much for your review, for some reason it ended in my spam 
box, so I only saw it just now.

On 8/11/25 16:15, Daniel Thompson wrote:
> On Fri, Jul 25, 2025 at 01:09:24PM +0200, Maud Spierings via B4 Relay wrote:
>> From: Maud Spierings <maudspierings@gocontroll.com>
>>
>> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
>> with intgrated boost controller.
>>
>> Signed-off-by: Maud Spierings maudspierings@gocontroll.com
>> ---
>>   MAINTAINERS                            |   2 +
>>   drivers/video/backlight/Kconfig        |   7 +
>>   drivers/video/backlight/Makefile       |   1 +
>>   drivers/video/backlight/max25014.c     | 449 +++++++++++++++++++++++++++++++++
>>   include/linux/platform_data/max25014.h |  24 ++
> 
> Who else included this header file? Can the code here simply be included
> in the C file?

That was my instinct too, I was following a clearly incorrect pattern 
from another driver, merged the fields from that struct into the main 
max25014 struct.

> 
>> diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..371b6017953ae5955f4dfef921980dfdedd65d85
>> --- /dev/null
>> +++ b/drivers/video/backlight/max25014.c
>> @@ -0,0 +1,449 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Backlight driver for Maxim MAX25014
>> + *
>> + * Copyright (C) 2025 GOcontroll B.V.
>> + * Author: Maud Spierings <maudspierings@gocontroll.com>
>> + */
>> +
>> +#include <linux/backlight.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/platform_data/max25014.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#define MAX25014_ISET_DEFAULT_100 11U
>> +#define MAX_BRIGHTNESS (100U)
>> +#define MIN_BRIGHTNESS (0U)
>> +#define TON_MAX (130720U) /* @153Hz */
>> +#define TON_STEP (1307U) /* @153Hz */
>> +#define TON_MIN (0U)
>> +
>> +#define MAX25014_DEV_ID         (0x00U)
>> +#define MAX25014_REV_ID         (0x01U)
>> +#define MAX25014_ISET           (0x02U)
>> +#define MAX25014_IMODE          (0x03U)
>> +#define MAX25014_TON1H          (0x04U)
>> +#define MAX25014_TON1L          (0x05U)
>> +#define MAX25014_TON2H          (0x06U)
>> +#define MAX25014_TON2L          (0x07U)
>> +#define MAX25014_TON3H          (0x08U)
>> +#define MAX25014_TON3L          (0x09U)
>> +#define MAX25014_TON4H          (0x0AU)
>> +#define MAX25014_TON4L          (0x0BU)
>> +#define MAX25014_TON_1_4_LSB    (0x0CU)
>> +#define MAX25014_SETTING        (0x12U)
>> +#define MAX25014_DISABLE        (0x13U)
>> +#define MAX25014_BSTMON         (0x14U)
>> +#define MAX25014_IOUT1          (0x15U)
>> +#define MAX25014_IOUT2          (0x16U)
>> +#define MAX25014_IOUT3          (0x17U)
>> +#define MAX25014_IOUT4          (0x18U)
>> +#define MAX25014_OPEN           (0x1BU)
>> +#define MAX25014_SHORT_GND      (0x1CU)
>> +#define MAX25014_SHORT_LED      (0x1DU)
>> +#define MAX25014_MASK           (0x1EU)
>> +#define MAX25014_DIAG           (0x1FU)
> 
> Forcing all these constants to be unsigned is unusual. Is it really
> needed?

Removed all the U's

> 
>> +#define MAX25014_IMODE_HDIM     BIT(2)
>> +#define MAX25014_ISET_ENABLE    BIT(5)
>> +#define MAX25014_ISET_PSEN      BIT(4)
>> +#define MAX25014_DIAG_HW_RST    BIT(2)
>> +#define MAX25014_SETTING_FPWM   GENMASK(6, 4)
>> +
>> +struct max25014;
> 
> This is redundant. Remove.

Thats an interesting leftover, removed.

>> +
>> +struct max25014 {
>> +	const char *chipname;
> 
> Why keep this value around? It is only used during the probe.
> 
>> +	struct i2c_client *client;
>> +	struct backlight_device *bl;
>> +	struct device *dev;
> 
> It is necessary to cache this, is it just a copy of client->dev)?

yep completely unnecessary, removed.

> 
>> +	struct regmap *regmap;
>> +	struct max25014_platform_data *pdata;
>> +	struct gpio_desc *enable;
>> +	struct regulator *vin; /* regulator for boost converter Vin rail */
>> +};
>> +
>> +static const struct regmap_config max25014_regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.max_register = MAX25014_DIAG,
>> +};
>> +
>> +/**
>> + * @brief get the bit mask for the DISABLE register.
>> + *
>> + * @param strings the led string configuration array.
>> + * @return uint8_t bits to set in the register.
>> + */
>> +static uint8_t strings_mask(struct max25014 *maxim)
>> +{
>> +	uint8_t res, i;
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		if (maxim->pdata->strings[i] == 0)
>> +			res |= 1 << i;
>> +	}
>> +	return res;
> 
> Could this converison have happened during DT parsing?

inlined it, changed the strings field in to strings_mask and only store 
the mask it calculates.

>> +}
>> +
>> +/**
>> + * @brief control the brightness with i2c registers
>> + *
>> + * @param regmap trivial
>> + * @param brt brightness
>> + * @return int
>> + */
>> +static int max25014_register_control(struct regmap *regmap, uint32_t brt)
>> +{
>> +	uint32_t reg = TON_STEP * brt;
>> +	int ret;
>> +	/*
>> +	 * 18 bit number lowest, 2 bits in first register,
>> +	 * next lowest 8 in the L register, next 8 in the H register
>> +	 * Seemingly setting the strength of only one string controls all of
>> +	 * them, individual settings don't affect the outcome.
>> +	 */
>> +
>> +	ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011);
>> +	if (ret != 0)
>> +		return ret;
>> +	ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111);
>> +	if (ret != 0)
>> +		return ret;
>> +	return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111);
>> +}
>> +
>> +static int max25014_check_errors(struct max25014 *maxim)
>> +{
>> +	uint8_t i;
>> +	int ret;
>> +	uint32_t val;
>> +
>> +	ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val);
>> +	if (ret != 0)
>> +		return ret;
>> +	if (val > 0) {
>> +		dev_err(maxim->dev, "Open led strings detected on:\n");
>> +		for (i = 0; i < 4; i++) {
>> +			if (val & 1 << i)
>> +				dev_err(maxim->dev, "string %d\n", i + 1);
>> +		}
>> +		return -EIO;
>> +	}
>> +
>> +	ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
>> +	if (ret != 0)
>> +		return ret;
>> +	if (val > 0) {
>> +		dev_err(maxim->dev, "Short to ground detected on:\n");
>> +		for (i = 0; i < 4; i++) {
>> +			if (val & 1 << i)
>> +				dev_err(maxim->dev, "string %d\n", i + 1);
>> +		}
>> +		return -EIO;
>> +	}
>> +
>> +	ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val);
>> +	if (ret != 0)
>> +		return ret;
>> +	if (val > 0) {
>> +		dev_err(maxim->dev, "Shorted led detected on:\n");
>> +		for (i = 0; i < 4; i++) {
>> +			if (val & 1 << i)
>> +				dev_err(maxim->dev, "string %d\n", i + 1);
>> +		}
>> +		return -EIO;
>> +	}
>> +
>> +	ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val);
>> +	if (ret != 0)
>> +		return ret;
>> +	/*
>> +	 * The HW_RST bit always starts at 1 after power up.
>> +	 * It is reset on first read, does not indicate an error.
>> +	 */
>> +	if (val > 0 && val != MAX25014_DIAG_HW_RST) {
>> +		if (val & 0b1)
>> +			dev_err(maxim->dev, "Overtemperature shutdown\n");
>> +		if (val & 0b10)
>> +			dev_warn(maxim->dev,
>> +				 "Chip is getting too hot (>125C)\n");
>> +		if (val & 0b1000)
>> +			dev_err(maxim->dev, "Boost converter overvoltage\n");
>> +		if (val & 0b10000)
>> +			dev_err(maxim->dev, "Boost converter undervoltage\n");
>> +		if (val & 0b100000)
>> +			dev_err(maxim->dev, "IREF out of range\n");
>> +		return -EIO;
>> +	}
>> +	return 0;
>> +}
>> +
>> +/*
>> + * 1. disable unused strings
>> + * 2. set dim mode
>> + * 3. set initial brightness
>> + * 4. set setting register
>> + * 5. enable the backlight
>> + */
>> +static int max25014_configure(struct max25014 *maxim)
>> +{
>> +	int ret;
>> +	uint32_t val;
>> +
>> +	ret = regmap_write(maxim->regmap, MAX25014_DISABLE,
>> +			   strings_mask(maxim));
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	max25014_register_control(maxim->regmap,
>> +				  maxim->pdata->initial_brightness);
>> +
>> +	ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	ret = regmap_write(
>> +		maxim->regmap, MAX25014_SETTING,
>> +		val & ~MAX25014_SETTING_FPWM);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	ret = regmap_write(maxim->regmap, MAX25014_ISET,
>> +			   maxim->pdata->iset | MAX25014_ISET_ENABLE | MAX25014_ISET_PSEN);
>> +	return ret;
>> +}
>> +
>> +static int max25014_update_status(struct backlight_device *bl_dev)
>> +{
>> +	struct max25014 *maxim = bl_get_data(bl_dev);
>> +
>> +	if (bl_dev->props.state & BL_CORE_SUSPENDED)
>> +		bl_dev->props.brightness = 0;
>> +
>> +	return max25014_register_control(maxim->regmap, bl_dev->props.brightness);
>> +}
>> +
>> +static const struct backlight_ops max25014_bl_ops = {
>> +	.options = BL_CORE_SUSPENDRESUME,
>> +	.update_status = max25014_update_status,
>> +};
>> +
>> +static int max25014_backlight_register(struct max25014 *maxim)
>> +{
>> +	struct backlight_device *bl;
>> +	struct backlight_properties props;
>> +	struct max25014_platform_data *pdata = maxim->pdata;
>> +
>> +	memset(&props, 0, sizeof(props));
>> +	props.type = BACKLIGHT_PLATFORM;
>> +	props.max_brightness = MAX_BRIGHTNESS;
>> +
>> +	if (pdata->initial_brightness > props.max_brightness)
>> +		pdata->initial_brightness = props.max_brightness;
> 
> Handle this during DT parsing.

It is already handled there, this is double, so dropped.

>> +
>> +	props.brightness = pdata->initial_brightness;
>> +
>> +	bl = devm_backlight_device_register(maxim->dev, maxim->chipname, maxim->dev,
>> +					    maxim, &max25014_bl_ops, &props);
>> +	if (IS_ERR(bl))
>> +		return PTR_ERR(bl);
>> +
>> +	maxim->bl = bl;
>> +
>> +	return 0;
>> +}
> 
> Can max25014_backlight_register() be moved into the probe function?
> There is no special control flow here so this function doesn't make the
> probe function any simpler.

Done.

>> +
>> +#ifdef CONFIG_OF
>> +static int max25014_parse_dt(struct max25014 *maxim)
>> +{
>> +	struct device *dev = maxim->dev;
>> +	struct device_node *node = dev->of_node;
>> +	struct max25014_platform_data *pdata;
>> +
>> +	int res;
>> +
>> +	if (!node) {
>> +		dev_err(dev, "no platform data\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +	if (!pdata)
>> +		return -ENOMEM;
>> +
>> +	res = of_property_count_u32_elems(node, "maxim,strings");
>> +	if (res == 4) {
>> +		of_property_read_u32_array(node, "maxim,strings", pdata->strings, 4);
>> +	} else {
>> +		dev_err(dev, "strings property not correctly defined\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	pdata->initial_brightness = 50U;
>> +	of_property_read_u32(node, "default-brightness", &pdata->initial_brightness);
>> +	pdata->iset = MAX25014_ISET_DEFAULT_100;
>> +	of_property_read_u32(node, "maxim,iset", &pdata->iset);
>> +
>> +	if (pdata->iset < 0 || pdata->iset > 15) {
>> +		dev_err(dev,
>> +			"Invalid iset, should be a value from 0-15, entered was %d\n",
>> +			pdata->iset);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (pdata->initial_brightness < 0 || pdata->initial_brightness > 100) {
>> +		dev_err(dev,
>> +			"Invalid initial brightness, should be a value from 0-100, entered was %d\n",
>> +			pdata->initial_brightness);
>> +		return -EINVAL;
>> +	}
>> +
>> +	maxim->pdata = pdata;
>> +
>> +	return 0;
>> +}
>> +#else
>> +static int max25014_parse_dt(struct max25014 *maxim)
>> +{
>> +	dev_err(maxim->dev,
>> +		"CONFIG_OF not configured, unable to parse devicetree");
>> +	return -EINVAL;
>> +}
> 
> What is the point of this method? New drivers shouldn't support platform
> data so CONFIG_OF is required for this driver to work at all.

I think it is me following a bad pattern again, dropped the ifdef.

>> +#endif
>> +
>> +static int max25014_probe(struct i2c_client *cl)
>> ...

Kind regards,
Maud

[PATCH 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight

[Maud Spierings via B4 Relay]

From: Maud Spierings <maudspierings@gocontroll.com>

Add the missing backlight driver.

Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
 ...tx8p-ml81-moduline-display-106-av101hdt-a10.dtso | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
index e3965caca6be42a17aa89b77bd5b919382c84151..143243ba95cd7a69c7b043fa0fb32c37b67e1064 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
@@ -17,6 +17,7 @@
 
 	panel {
 		compatible = "boe,av101hdt-a10";
+		backlight = <&backlight>;
 		enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
 		pinctrl-0 = <&pinctrl_panel>;
 		pinctrl-names = "default";
@@ -40,7 +41,27 @@ reg_vbus: regulator-vbus {
 	};
 };
 
+&i2c4 {
+	backlight: backlight@6f {
+		reg = <0x6f>;
+		compatible = "maxim,max25014";
+		default-brightness = <50>;
+		enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_backlight>;
+		maxim,iset = <7>;
+		maxim,strings = <1 1 1 0>;
+	};
+};
+
 &iomuxc {
+	pinctrl_backlight: backlightgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04
+				(MX8MP_PULL_UP | MX8MP_PULL_ENABLE)
+		>;
+	};
+
 	pinctrl_panel: panelgrp {
 		fsl,pins = <
 			MX8MP_IOMUXC_GPIO1_IO07__GPIO1_IO07

-- 
2.50.1

[PATCH 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight

[Maud Spierings via B4 Relay]

From: Maud Spierings <maudspierings@gocontroll.com>

Add the missing backlight.

Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
 ...p-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
index 3eb665ce9d5d2a1c742ffb4feca046e406e29956..9124cd87cce54a5aa7b7ad674f70f814d1dc3515 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
@@ -16,6 +16,7 @@
 
 	panel {
 		compatible = "boe,av123z7m-n17";
+		backlight = <&backlight>;
 		enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
 		pinctrl-0 = <&pinctrl_panel>;
 		pinctrl-names = "default";
@@ -91,10 +92,26 @@ lvds1_out: endpoint {
 		};
 	};
 
-	/* max25014 @ 0x6f */
+	backlight: backlight@6f {
+		reg = <0x6f>;
+		compatible = "maxim,max25014";
+		default-brightness = <50>;
+		enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_backlight>;
+		maxim,iset = <7>;
+		maxim,strings = <1 1 1 1>;
+	};
 };
 
 &iomuxc {
+	pinctrl_backlight: backlightgrp {
+		fsl,pins = <
+			MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04
+				(MX8MP_PULL_UP | MX8MP_PULL_ENABLE)
+		>;
+	};
+
 	pinctrl_lvds_bridge: lvdsbridgegrp {
 		fsl,pins = <
 			MX8MP_IOMUXC_SAI1_TXD2__GPIO4_IO14

-- 
2.50.1