Re: [PATCH RFC 02/10] backlight: add Kinetic KTD3136 driver
From: Daniel Thompson <hidden>
Date: 2026-09-22 09:52:49
Also in:
dri-devel, linux-arm-msm, linux-devicetree, linux-fbdev, linux-leds, lkml
On Fri, Sep 11, 2026 at 11:50:59AM +0300, YİĞİTCAN KAVAKLI via B4 Relay wrote:
quoted hunk ↗ jump to hunk
obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.odiff --git a/drivers/video/backlight/ktd3136-backlight.c b/drivers/video/backlight/ktd3136-backlight.c new file mode 100644 index 0000000000000000000000000000000000000000..a8ebe3b23c99f3ada9f45c60688f8b8c78d72def --- /dev/null +++ b/drivers/video/backlight/ktd3136-backlight.c@@ -0,0 +1,262 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Kinetic Technologies KTD3136 3-Channel LED Backlight Driver + * + * Copyright (C) 2026 + */ + +#include <linux/backlight.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/property.h> +#include <linux/regulator/consumer.h> + +#define KTD3136_REG_DEV_ID 0x00 +#define KTD3136_REG_SW_RESET 0x01 +#define KTD3136_REG_MODE 0x02 +#define KTD3136_REG_CONTROL 0x03 +#define KTD3136_REG_RATIO_LSB 0x04 +#define KTD3136_REG_RATIO_MSB 0x05 +#define KTD3136_REG_PWM 0x06 +#define KTD3136_REG_STATUS 0x0A + +#define KTD3136_DEV_ID_VAL 0x18 +#define KTD3136_DEV_ID_VAL2 0x19 + +#define KTD3136_MODE_ON 0xC9 /* Boost ON, 3 channels enabled */ +#define KTD3136_MODE_STANDBY 0x98 /* Standby mode */ +#define KTD3136_CONTROL_LINEAR 0x02 /* Linear dimming mapping */ +#define KTD3136_PWM_DEFAULT 0x1B /* Default PWM/boost frequency */ + +#define KTD3136_DEFAULT_MAX_BRIGHTNESS 2047 /* 11-bit resolution */ +#define KTD3136_DEFAULT_BRIGHTNESS 2047 + +struct ktd3136_data { + struct i2c_client *client; + struct backlight_device *bd; + struct gpio_desc *enable_gpio; + struct regulator *vin; + /* Protects chip registers and state */ + struct mutex lock; + bool is_enabled; +}; + +static int ktd3136_write(struct ktd3136_data *chip, u8 reg, u8 val) +{ + int ret; + + ret = i2c_smbus_write_byte_data(chip->client, reg, val); + if (ret < 0) + dev_err(&chip->client->dev, "failed to write reg 0x%02x: %d\n", reg, ret); + + return ret; +} + +static int ktd3136_read(struct ktd3136_data *chip, u8 reg) +{ + int ret; + + ret = i2c_smbus_read_byte_data(chip->client, reg); + if (ret < 0) + dev_err(&chip->client->dev, "failed to read reg 0x%02x: %d\n", reg, ret); + + return ret; +} + +static int ktd3136_power_on(struct ktd3136_data *chip) +{ + int ret; + + if (chip->is_enabled) + return 0; + + if (chip->enable_gpio) { + gpiod_set_value_cansleep(chip->enable_gpio, 1); + /* Allow oscillator and internal logic to stabilize */ + usleep_range(2000, 3000); + } + + ret = ktd3136_write(chip, KTD3136_REG_CONTROL, KTD3136_CONTROL_LINEAR); + if (ret < 0) + goto err_off; + + ret = ktd3136_write(chip, KTD3136_REG_PWM, KTD3136_PWM_DEFAULT); + if (ret < 0) + goto err_off; + + chip->is_enabled = true; + return 0; + +err_off: + if (chip->enable_gpio) + gpiod_set_value_cansleep(chip->enable_gpio, 0); + return ret; +} + +static void ktd3136_power_off(struct ktd3136_data *chip) +{ + if (!chip->is_enabled) + return; + + ktd3136_write(chip, KTD3136_REG_MODE, KTD3136_MODE_STANDBY); + + if (chip->enable_gpio) + gpiod_set_value_cansleep(chip->enable_gpio, 0);
Why does this powering off leave the regulator active?
+
+ chip->is_enabled = false;
+}
+
+static int ktd3136_update_status(struct backlight_device *bd)
+{
+ struct ktd3136_data *chip = bl_get_data(bd);
+ int brightness = backlight_get_brightness(bd);
+ int ret = 0;
+ u8 lsb, msb;
+
+ mutex_lock(&chip->lock);
+
+ if (backlight_is_blank(bd) || brightness == 0) {
+ ktd3136_power_off(chip);
+ goto out;
+ }
+
+ ret = ktd3136_power_on(chip);
+ if (ret < 0)
+ goto out;
+
+ lsb = brightness & 0x07;
+ msb = (brightness >> 3) & 0xFF;
+
+ ret = ktd3136_write(chip, KTD3136_REG_RATIO_LSB, lsb);
+ if (ret < 0)
+ goto out;
+
+ ret = ktd3136_write(chip, KTD3136_REG_RATIO_MSB, msb);
+ if (ret < 0)
+ goto out;
+
+ ret = ktd3136_write(chip, KTD3136_REG_MODE, KTD3136_MODE_ON);
+
+out:
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
+static const struct backlight_ops ktd3136_backlight_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = ktd3136_update_status,
+};
+
+static int ktd3136_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct backlight_properties props;
+ struct ktd3136_data *chip;
+ int ret, val;
+ u32 def_brightness = KTD3136_DEFAULT_BRIGHTNESS;
+ u32 max_brightness = KTD3136_DEFAULT_MAX_BRIGHTNESS;
+
+ chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->client = client;
+ mutex_init(&chip->lock);
+
+ chip->vin = devm_regulator_get_optional(dev, "vin");
+ if (IS_ERR(chip->vin)) {
+ ret = PTR_ERR(chip->vin);
+ if (ret != -ENODEV)
+ return dev_err_probe(dev, ret, "Failed to get vin regulator\n");
+ chip->vin = NULL;
+ }
+
+ if (chip->vin) {
+ ret = regulator_enable(chip->vin);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable vin regulator\n");
+ }Why is the regulator managed here rather than in the power_on/off functions? Also, you need to keep track of the regulator state. You must "put" the regulator before allowing devm to clean it up (handling the regulator state in power_on/off would solve this).
+
+ chip->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+ if (IS_ERR(chip->enable_gpio))
+ return dev_err_probe(dev, PTR_ERR(chip->enable_gpio),
+ "Failed to get enable GPIO\n");
+
+ if (chip->enable_gpio)
+ usleep_range(2000, 3000);
+
+ val = ktd3136_read(chip, KTD3136_REG_DEV_ID);
+ if (val < 0)
+ return dev_err_probe(dev, val, "Failed to read device ID\n");
+
+ if (val != KTD3136_DEV_ID_VAL && val != KTD3136_DEV_ID_VAL2) {
+ dev_err(dev, "Unknown device ID: 0x%02x\n", val);
+ return -ENODEV;
+ }
+
+ dev_info(dev, "Kinetic KTD3136 detected (ID: 0x%02x)\n", val);Remove this. Happy noises are still noise!
+ + device_property_read_u32(dev, "max-brightness", &max_brightness); + if (max_brightness > KTD3136_DEFAULT_MAX_BRIGHTNESS) + max_brightness = KTD3136_DEFAULT_MAX_BRIGHTNESS; + + device_property_read_u32(dev, "default-brightness", &def_brightness); + if (def_brightness > max_brightness) + def_brightness = max_brightness; + + memset(&props, 0, sizeof(props)); + props.type = BACKLIGHT_RAW; + props.max_brightness = max_brightness; + props.brightness = def_brightness;
The scale property needs to be set here. Daniel.