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 <par...@paroga.com>
> ---
> 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
