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

Frank Li
2025-11-07T15:51:24Z

On Fri, Nov 07, 2025 at 01:49:59PM +0100, 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 integrated boost controller.
>
> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> ---
> MAINTAINERS | 1 +
> drivers/video/backlight/Kconfig | 7 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 418 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 606ce086f758..d082d3f8cfae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15265,6 +15265,7 @@ 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
>
> MAX31335 RTC DRIVER
> M: Antoniu Miclaus <antoniu.miclaus@analog.com>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index d9374d208cee..d3bb6ccd4185 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 dfbb169bf6ea..1170d9ec40b8 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 000000000000..36bae508697e
> --- /dev/null
...
> +
> +struct max25014 {
> + struct i2c_client *client;
> + struct backlight_device *bl;
> + struct regmap *regmap;
> + struct gpio_desc *enable;
> + struct regulator *vin; /* regulator for boost converter Vin rail */
> + uint32_t iset;
> + uint8_t strings_mask;
> +};
> +
> +static const struct regmap_config max25014_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = MAX25014_DIAG,
> +};
> +
> +/**
> + * @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)

if (ret), check others regmap_*()

> + 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) {

uint32 always >= 0

So
if (val)

> + dev_err(&maxim->client->dev, "Open led strings detected on:\n");
> + for (i = 0; i < 4; i++) {
> + if (val & 1 << i)
> + dev_err(&maxim->client->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->client->dev, "Short to ground detected on:\n");
> + for (i = 0; i < 4; i++) {
> + if (val & 1 << i)
> + dev_err(&maxim->client->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->client->dev, "Shorted led detected on:\n");
> + for (i = 0; i < 4; i++) {
> + if (val & 1 << i)
> + dev_err(&maxim->client->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)

BIT(0)

> + dev_err(&maxim->client->dev,
> + "Overtemperature shutdown\n");
> + if (val & 0b10)
> + dev_err(&maxim->client->dev,
> + "Chip is getting too hot (>125C)\n");
> + if (val & 0b1000)
> + dev_err(&maxim->client->dev,
> + "Boost converter overvoltage\n");
> + if (val & 0b10000)
> + dev_err(&maxim->client->dev,
> + "Boost converter undervoltage\n");
> + if (val & 0b100000)
> + dev_err(&maxim->client->dev, "IREF out of range\n");
> + return -EIO;
> + }
> + return 0;
> +}
> +
...
> +static int max25014_parse_dt(struct max25014 *maxim,
> + uint32_t *initial_brightness)
> +{
> + struct device *dev = &maxim->client->dev;
> + struct device_node *node = dev->of_node;
> + struct fwnode_handle *child;
> + uint32_t strings[4];
> + int res, i;
> +
> + if (!node) {
> + dev_err(dev, "no platform data\n");
> + return -EINVAL;
> + }

call from probe, check other place

return dev_err_probe()

> +
> + child = device_get_next_child_node(dev, NULL);
> + if (child) {
> + res = fwnode_property_count_u32(child, "led-sources");
> + if (res > 0) {
> + fwnode_property_read_u32_array(child, "led-sources",
> + strings, res);
> +
> + /* set all strings as disabled, then enable those in led-sources*/
> + maxim->strings_mask = 0xf;
> + for (i = 0; i < res; i++) {
> + if (strings[i] <= 4)
> + maxim->strings_mask &= ~BIT(strings[i]);
> + }
> + }
> +
> + fwnode_property_read_u32(child, "default-brightness",
> + initial_brightness);
> +
> + fwnode_handle_put(child);
> + }
> +
> + maxim->iset = MAX25014_ISET_DEFAULT_100;
> + of_property_read_u32(node, "maxim,iset", &maxim->iset);
> +
> + if (maxim->iset < 0 || maxim->iset > 15) {
> + dev_err(dev,
> + "Invalid iset, should be a value from 0-15, entered was %d\n",
> + maxim->iset);
> + return -EINVAL;
> + }
> +
> + if (*initial_brightness < 0 || *initial_brightness > 100) {
> + dev_err(dev,
> + "Invalid initial brightness, should be a value from 0-100, entered was %d\n",
> + *initial_brightness);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int max25014_probe(struct i2c_client *cl)
> +{
> + struct backlight_device *bl;
> + const struct i2c_device_id *id = i2c_client_get_device_id(cl);
> + struct max25014 *maxim;
> + struct backlight_properties props;
> + int ret;
> + uint32_t initial_brightness = 50;

try keep reverise christmas order

> +
> + maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL);
> + if (!maxim)
> + return -ENOMEM;
> +
> + maxim->client = cl;
> +
> + ret = max25014_parse_dt(maxim, &initial_brightness);
> + if (ret < 0)
> + return ret;
> +
> + maxim->vin = devm_regulator_get_optional(&maxim->client->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->client->dev,
> + "failed to enable Vin: %d\n", ret);
> + return ret;
> + }
> + }

use devm_regulator_get_enable_optional() to combine devm_regulator_get_optional()
and regulator_enable() to one call.

> +
> + maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable",
> + GPIOD_ASIS);
> + if (IS_ERR(maxim->enable)) {
> + ret = PTR_ERR(maxim->enable);
> + dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n",
> + ret);
> + goto disable_vin;
> + }
> +
> + if (maxim->enable)
> + gpiod_set_value_cansleep(maxim->enable, 1);

gpiod_set_value_cansleep() tolerate NULL, so needn't if check here

and if you pass GPIOD_OUT_HIGH at devm_gpiod_get_optional(), needn't call
this function.

> +
> + /* Enable can be tied to vin rail wait if either is available */
> + if (maxim->enable || maxim->vin) {
> + /* Datasheet Electrical Characteristics tSTARTUP 2ms */
> + usleep_range(2000, 2500);

now perfer use fsleep()

Frank
> + }
> +
> + maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config);
> + if (IS_ERR(maxim->regmap)) {
> + ret = PTR_ERR(maxim->regmap);
> + dev_err(&maxim->client->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->client->dev, "device config err: %d", ret);
> + goto disable_full;
> + }
> +
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_PLATFORM;
> + props.max_brightness = MAX_BRIGHTNESS;
> + props.brightness = initial_brightness;
> + props.scale = BACKLIGHT_SCALE_LINEAR;
> +
> + bl = devm_backlight_device_register(&maxim->client->dev, id->name,
> + &maxim->client->dev, maxim,
> + &max25014_bl_ops, &props);
> + if (IS_ERR(bl))
> + return PTR_ERR(bl);
> +
> + maxim->bl = bl;
> +
> + 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");
>
> --
> 2.51.2
>
>