Hi Daniel,
Thanks for the review!
On 8/22/25 09:20, Daniel Thompson wrote:
On Tue, Aug 19, 2025 at 12:59:00PM +0200, Maud Spierings via B4 Relay wrote:
From: Maud Spierings <maudspieri...@gocontroll.com>
The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with intgrated boost controller.
Signed-off-by: Maud Spierings maudspieri...@gocontroll.com
Looking good but still a few small comments (below).
diff --git a/drivers/video/backlight/max25014.c
b/drivers/video/backlight/max25014.c
new file mode 100644
index
0000000000000000000000000000000000000000..fe5e0615cf6d151868b56ebb9544b175b09dfcee
--- /dev/null
+++ b/drivers/video/backlight/max25014.c
@@ -0,0 +1,395 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Backlight driver for Maxim MAX25014
+ *
+ * Copyright (C) 2025 GOcontroll B.V.
+ * Author: Maud Spierings <maudspieri...@gocontroll.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define MAX25014_ISET_DEFAULT_100 11
+#define MAX_BRIGHTNESS (100)
+#define MIN_BRIGHTNESS (0)
+#define TON_MAX (130720) /* @153Hz */
+#define TON_STEP (1307) /* @153Hz */
+#define TON_MIN (0)
+
+#define MAX25014_DEV_ID (0x00)
+#define MAX25014_REV_ID (0x01)
+#define MAX25014_ISET (0x02)
+#define MAX25014_IMODE (0x03)
+#define MAX25014_TON1H (0x04)
+#define MAX25014_TON1L (0x05)
+#define MAX25014_TON2H (0x06)
+#define MAX25014_TON2L (0x07)
+#define MAX25014_TON3H (0x08)
+#define MAX25014_TON3L (0x09)
+#define MAX25014_TON4H (0x0A)
+#define MAX25014_TON4L (0x0B)
+#define MAX25014_TON_1_4_LSB (0x0C)
+#define MAX25014_SETTING (0x12)
+#define MAX25014_DISABLE (0x13)
+#define MAX25014_BSTMON (0x14)
+#define MAX25014_IOUT1 (0x15)
+#define MAX25014_IOUT2 (0x16)
+#define MAX25014_IOUT3 (0x17)
+#define MAX25014_IOUT4 (0x18)
+#define MAX25014_OPEN (0x1B)
+#define MAX25014_SHORT_GND (0x1C)
+#define MAX25014_SHORT_LED (0x1D)
+#define MAX25014_MASK (0x1E)
+#define MAX25014_DIAG (0x1F)
There is no need to put raw numbers in brackets.
gone in the next version, I'll also align the defines at the top better.
+
+#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 i2c_client *client;
+ struct backlight_device *bl;
+ struct regmap *regmap;
+ struct max25014_platform_data *pdata;
This appears to be unused.
oops forgot to remove that, how did it even build with that?
+ struct gpio_desc *enable;
+ struct regulator *vin; /* regulator for boost converter Vin rail */
+ uint32_t initial_brightness;
It is important to keep the initial_brightness for the lifetime of the
driver?
I guess not, though I am not sure how to comfortably transport this
value to the max25014_configure() otherwise. Some temporary value that I
create in the probe function then pass as reference to
max25014_parse_dt() and then to max25014_configure()?
+ uint32_t iset;
+ uint8_t strings_mask;
+};
+
Kind regards,
Maud
