Hi Patrick, a few comments in the following. Sam On Sat, Mar 09, 2024 at 02:24:56PM +0100, Patrick Gansterer wrote: > This is a general driver for LM3509 backlight chip of TI. > LM3509 is High Efficiency Boost for White LEDs and/or OLED Displays with > Dual Current Sinks. This driver supports OLED/White LED select, brightness > control and sub/main control. > The datasheet can be found at http://www.ti.com/product/lm3509. > > Signed-off-by: Patrick Gansterer <paroga@xxxxxxxxxx> > --- > drivers/video/backlight/Kconfig | 7 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/lm3509_bl.c | 340 ++++++++++++++++++++++++++++ > 3 files changed, 348 insertions(+) > create mode 100644 drivers/video/backlight/lm3509_bl.c > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index ea2d0d69bd8c..96ad5dc584b6 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -366,6 +366,13 @@ config BACKLIGHT_AAT2870 > If you have a AnalogicTech AAT2870 say Y to enable the > backlight driver. > > +config BACKLIGHT_LM3509 > + tristate "Backlight Driver for LM3509" > + depends on I2C > + select REGMAP_I2C > + help > + This supports TI LM3509 Backlight Driver > + > config BACKLIGHT_LM3630A > tristate "Backlight Driver for LM3630A" > depends on I2C && PWM > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > index 06966cb20459..51a4ac5d0530 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -35,6 +35,7 @@ obj-$(CONFIG_BACKLIGHT_HP700) += jornada720_bl.o > obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO) += ipaq_micro_bl.o > obj-$(CONFIG_BACKLIGHT_KTD253) += ktd253-backlight.o > obj-$(CONFIG_BACKLIGHT_KTZ8866) += ktz8866.o > +obj-$(CONFIG_BACKLIGHT_LM3509) += lm3509_bl.o > obj-$(CONFIG_BACKLIGHT_LM3533) += lm3533_bl.o > obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.o > obj-$(CONFIG_BACKLIGHT_LM3639) += lm3639_bl.o > diff --git a/drivers/video/backlight/lm3509_bl.c b/drivers/video/backlight/lm3509_bl.c > new file mode 100644 > index 000000000000..bfad0aaffa0d > --- /dev/null > +++ b/drivers/video/backlight/lm3509_bl.c > @@ -0,0 +1,340 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#define LM3509_NAME "lm3509_bl" > + > +#define LM3509_SINK_MAIN 0 > +#define LM3509_SINK_SUB 1 > +#define LM3509_NUM_SINKS 2 > + > +#define LM3509_DEF_BRIGHTNESS 0x12 > +#define LM3509_MAX_BRIGHTNESS 0x1F > + > +#define REG_GP 0x10 > +#define REG_BMAIN 0xA0 > +#define REG_BSUB 0xB0 > +#define REG_MAX 0xFF > + > +enum { > + REG_GP_ENM_BIT = 0, > + REG_GP_ENS_BIT, > + REG_GP_UNI_BIT, > + REG_GP_RMP0_BIT, > + REG_GP_RMP1_BIT, > + REG_GP_OLED_BIT, > +}; > + > +struct lm3509_bl { > + struct regmap *regmap; > + struct backlight_device *bl_main; > + struct backlight_device *bl_sub; > + struct gpio_desc *reset_gpio; > +}; > + > +struct lm3509_bl_led_pdata { > + const char *label; > + int led_sources; > + u32 brightness; > + u32 max_brightness; > +}; > + > +static void lm3509_reset(struct lm3509_bl *data) > +{ > + if (data->reset_gpio) { > + gpiod_set_value(data->reset_gpio, 1); > + udelay(1); > + gpiod_set_value(data->reset_gpio, 0); > + udelay(10); > + } > +} > + > +static int lm3509_update_status(struct backlight_device *bl, > + unsigned int en_mask, unsigned int br_reg) > +{ > + struct lm3509_bl *data = bl_get_data(bl); > + int ret; > + bool en; > + > + ret = regmap_write(data->regmap, br_reg, bl->props.brightness); Here you can use backlight_get_brightness() thus avoiding direct access to backlight internal properties. > + if (ret < 0) > + return ret; > + > + en = bl->props.power <= FB_BLANK_NORMAL; Use backlight_is_blank() here. Sam > + return regmap_update_bits(data->regmap, REG_GP, en_mask, > + en ? en_mask : 0); > +} > + > +static int lm3509_main_update_status(struct backlight_device *bl) > +{ > + return lm3509_update_status(bl, BIT(REG_GP_ENM_BIT), REG_BMAIN); > +} > + > +static const struct backlight_ops lm3509_main_ops = { > + .options = BL_CORE_SUSPENDRESUME, > + .update_status = lm3509_main_update_status, > +}; > + > +static int lm3509_sub_update_status(struct backlight_device *bl) > +{ > + return lm3509_update_status(bl, BIT(REG_GP_ENS_BIT), REG_BSUB); > +} > + > +static const struct backlight_ops lm3509_sub_ops = { > + .options = BL_CORE_SUSPENDRESUME, > + .update_status = lm3509_sub_update_status, > +}; > + > +static struct backlight_device * > +lm3509_backlight_register(struct device *dev, const char *name_suffix, > + struct lm3509_bl *data, > + const struct backlight_ops *ops, > + const struct lm3509_bl_led_pdata *pdata) > + > +{ > + struct backlight_device *bd; > + struct backlight_properties props; > + const char *label = pdata->label; > + char name[64]; > + > + memset(&props, 0, sizeof(props)); > + props.type = BACKLIGHT_RAW; > + props.brightness = pdata->brightness; > + props.max_brightness = pdata->max_brightness; > + props.power = pdata->brightness > 0 ? FB_BLANK_UNBLANK : > + FB_BLANK_POWERDOWN; props.power is not supposed to be set by the user - is is maintained by the backlight core. Sam
