2023-09-15 18:40:37

by FLAVIO SULIGOI

[permalink] [raw]
Subject: [PATCH v2 2/2] backlight: mp3309c: Add support for MPS MP3309C

The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a
programmable switching frequency to optimize efficiency.
The brightness can be controlled either by I2C commands (called "analog"
mode) or by a PWM input signal (PWM mode).
This driver supports both modes.

For DT configuration details, please refer to:
- Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml

The datasheet is available at:
- https://www.monolithicpower.com/en/mp3309c.html

Signed-off-by: Flavio Suligoi <[email protected]>
---

v2:
- fix dependecies in Kconfig
- fix Kconfig MP3309C entry order
- remove switch-on-delay-ms property
- remove optional gpio property to reset external devices
- remove dimming-mode property (the analog-i2c dimming mode is the default; the
presence of the pwms property, in DT, selects automatically the pwm dimming
mode)
- substitute three boolean properties, used for the overvoltage-protection
values, with a single enum property
- drop simple tracing messages
- use dev_err_probe() in probe function
- change device name from mp3309c_bl to the simple mp3309c
- remove shutdown function
v1:
- first version

MAINTAINERS | 6 +
drivers/video/backlight/Kconfig | 11 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/mp3309c.c | 395 ++++++++++++++++++++++++++++++
4 files changed, 413 insertions(+)
create mode 100644 drivers/video/backlight/mp3309c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3be1bdfe8ecc..f779df433af1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14333,6 +14333,12 @@ S: Maintained
F: Documentation/driver-api/tty/moxa-smartio.rst
F: drivers/tty/mxser.*

+MP3309C BACKLIGHT DRIVER
+M: Flavio Suligoi <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
+F: drivers/video/backlight/mp3309c.c
+
MR800 AVERMEDIA USB FM RADIO DRIVER
M: Alexey Klimov <[email protected]>
L: [email protected]
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 51387b1ef012..1144a54a35c0 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -402,6 +402,17 @@ config BACKLIGHT_LP8788
help
This supports TI LP8788 backlight driver.

+config BACKLIGHT_MP3309C
+ tristate "Backlight Driver for MPS MP3309C"
+ depends on I2C && PWM
+ select REGMAP_I2C
+ help
+ This supports MPS MP3309C backlight WLED driver in both PWM and
+ analog/I2C dimming modes.
+
+ To compile this driver as a module, choose M here: the module will
+ be called mp3309c.
+
config BACKLIGHT_PANDORA
tristate "Backlight driver for Pandora console"
depends on TWL4030_CORE
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index f72e1c3c59e9..1af583de665b 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o
obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o
obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o
+obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o
obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o
obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
diff --git a/drivers/video/backlight/mp3309c.c b/drivers/video/backlight/mp3309c.c
new file mode 100644
index 000000000000..470c960d7438
--- /dev/null
+++ b/drivers/video/backlight/mp3309c.c
@@ -0,0 +1,395 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi <[email protected]>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+
+#define REG_I2C_0 0x00
+#define REG_I2C_1 0x01
+
+#define REG_I2C_0_EN 0x80
+#define REG_I2C_0_D0 0x40
+#define REG_I2C_0_D1 0x20
+#define REG_I2C_0_D2 0x10
+#define REG_I2C_0_D3 0x08
+#define REG_I2C_0_D4 0x04
+#define REG_I2C_0_RSRV1 0x02
+#define REG_I2C_0_RSRV2 0x01
+
+#define REG_I2C_1_RSRV1 0x80
+#define REG_I2C_1_DIMS 0x40
+#define REG_I2C_1_SYNC 0x20
+#define REG_I2C_1_OVP0 0x10
+#define REG_I2C_1_OVP1 0x08
+#define REG_I2C_1_VOS 0x04
+#define REG_I2C_1_LEDO 0x02
+#define REG_I2C_1_OTP 0x01
+
+#define ANALOG_MAX_VAL 31
+#define ANALOG_REG_MASK 0x7c
+
+enum mp3309c_status_value {
+ FIRST_POWER_ON,
+ BACKLIGHT_OFF,
+ BACKLIGHT_ON,
+};
+
+enum mp3309c_dimming_mode_value {
+ DIMMING_PWM,
+ DIMMING_ANALOG_I2C,
+};
+
+struct mp3309c_platform_data {
+ u32 max_brightness;
+ u32 default_brightness;
+ u8 dimming_mode;
+ u8 over_voltage_protection;
+ bool sync_mode;
+ u8 status;
+};
+
+struct mp3309c_chip {
+ struct device *dev;
+ struct mp3309c_platform_data *pdata;
+ struct backlight_device *bl;
+ struct gpio_desc *enable_gpio;
+ struct regmap *regmap;
+ struct pwm_device *pwmd;
+ int irq;
+ struct gpio_desc *reset_gpio;
+};
+
+static const struct regmap_config mp3309c_regmap = {
+ .name = "mp3309c_regmap",
+ .reg_bits = 8,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .max_register = REG_I2C_1,
+};
+
+static int mp3309c_enable_device(struct mp3309c_chip *chip)
+{
+ u8 reg_val;
+ int ret;
+
+ /* I2C register #0 - Device enable */
+ ret = regmap_update_bits(chip->regmap, REG_I2C_0, REG_I2C_0_EN,
+ REG_I2C_0_EN);
+ if (ret)
+ return ret;
+
+ /*
+ * I2C register #1 - Set working mode:
+ * - set one of the two dimming mode:
+ * - PWM dimming using an external PWM dimming signal
+ * - analog dimming using I2C commands
+ * - enable/disable synchronous mode
+ * - set overvoltage protection (OVP)
+ */
+ reg_val = 0x00;
+ if (chip->pdata->dimming_mode == DIMMING_PWM)
+ reg_val |= REG_I2C_1_DIMS;
+ if (chip->pdata->sync_mode)
+ reg_val |= REG_I2C_1_SYNC;
+ reg_val |= chip->pdata->over_voltage_protection;
+ ret = regmap_write(chip->regmap, REG_I2C_1, reg_val);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mp3309c_bl_update_status(struct backlight_device *bl)
+{
+ struct mp3309c_chip *chip = bl_get_data(bl);
+ int brightness = backlight_get_brightness(bl);
+ struct pwm_state pwmstate;
+ unsigned int analog_val, bits_val;
+ int i, ret;
+
+ if (chip->pdata->dimming_mode == DIMMING_PWM) {
+ /*
+ * PWM dimming mode
+ */
+ pwm_get_state(chip->pwmd, &pwmstate);
+ pwm_set_relative_duty_cycle(&pwmstate, brightness,
+ chip->pdata->max_brightness);
+ pwmstate.enabled = true;
+ ret = pwm_apply_state(chip->pwmd, &pwmstate);
+ if (ret)
+ return ret;
+
+ switch (chip->pdata->status) {
+ case FIRST_POWER_ON:
+ case BACKLIGHT_OFF:
+ /*
+ * After 20ms of pwm off, we must enable the chip again
+ */
+ if (brightness > 0) {
+ msleep_interruptible(10);
+ mp3309c_enable_device(chip);
+ chip->pdata->status = BACKLIGHT_ON;
+ } else {
+ chip->pdata->status = BACKLIGHT_OFF;
+ }
+ break;
+ case BACKLIGHT_ON:
+ if (brightness == 0)
+ chip->pdata->status = BACKLIGHT_OFF;
+ break;
+ }
+ } else {
+ /*
+ * Analog dimming (by I2C command) dimming mode
+ *
+ * The first time, before setting brightness, we must enable the
+ * device
+ */
+ if (chip->pdata->status == FIRST_POWER_ON)
+ mp3309c_enable_device(chip);
+
+ /*
+ * Dimming mode I2C command
+ *
+ * The 5 bits of the dimming analog value D4..D0 is allocated
+ * in the I2C register #0, in the following way:
+ *
+ * +--+--+--+--+--+--+--+--+
+ * |EN|D0|D1|D2|D3|D4|XX|XX|
+ * +--+--+--+--+--+--+--+--+
+ */
+ analog_val = DIV_ROUND_UP(ANALOG_MAX_VAL * brightness,
+ chip->pdata->max_brightness);
+ bits_val = 0;
+ for (i = 0; i <= 5; i++)
+ bits_val += ((analog_val >> i) & 0x01) << (6 - i);
+ ret = regmap_update_bits(chip->regmap, REG_I2C_0,
+ ANALOG_REG_MASK, bits_val);
+ if (ret)
+ return ret;
+
+ if (brightness > 0)
+ chip->pdata->status = BACKLIGHT_ON;
+ else
+ chip->pdata->status = BACKLIGHT_OFF;
+ }
+
+ return 0;
+}
+
+static const struct backlight_ops mp3309c_bl_ops = {
+ .update_status = mp3309c_bl_update_status,
+};
+
+static int pm3309c_parse_dt_node(struct mp3309c_chip *chip,
+ struct mp3309c_platform_data *pdata)
+{
+ struct device_node *node = chip->dev->of_node;
+ struct property *prop;
+ int length, ret;
+ unsigned int tmp_value;
+
+ if (!node) {
+ dev_err(chip->dev, "failed to get DT node\n");
+ return -ENODEV;
+ }
+
+ /*
+ * Dimming mode: the MP3309C provides two dimming control mode:
+ *
+ * - PWM mode
+ * - Analog by I2C control mode (default)
+ *
+ * I2C control mode is assumed as default but, if the pwms property is
+ * found in the backlight node, the mode switches to PWM mode.
+ */
+ pdata->dimming_mode = DIMMING_ANALOG_I2C;
+ prop = of_find_property(node, "pwms", &length);
+ if (prop) {
+ chip->pwmd = devm_pwm_get(chip->dev, NULL);
+ if (IS_ERR(chip->pwmd))
+ return dev_err_probe(chip->dev, PTR_ERR(chip->pwmd),
+ "error getting pwm data\n");
+ pdata->dimming_mode = DIMMING_PWM;
+ pwm_apply_args(chip->pwmd);
+ }
+
+ /* Dimming steps (levels): 0 .. max_brightness */
+ ret = of_property_read_u32(node, "max-brightness",
+ &pdata->max_brightness);
+ if (ret < 0) {
+ dev_err(chip->dev, "failed to get max-brightness from DT\n");
+ return ret;
+ }
+
+ /* Default brightness at startup */
+ ret = of_property_read_u32(node, "default-brightness",
+ &pdata->default_brightness);
+ if (ret < 0) {
+ dev_err(chip->dev,
+ "failed to get default-brightness from DT\n");
+ return ret;
+ }
+
+ /* Enable GPIO used in I2C dimming mode */
+ if (pdata->dimming_mode == DIMMING_ANALOG_I2C) {
+ chip->enable_gpio = devm_gpiod_get(chip->dev, "enable",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(chip->enable_gpio))
+ return dev_err_probe(chip->dev, PTR_ERR(chip->enable_gpio),
+ "error getting enable gpio\n");
+ }
+
+ /*
+ * Over-voltage protection (OVP)
+ *
+ * This (optional) property values are:
+ *
+ * - 13.5V
+ * - 24V
+ * - 35.5V (hardware default setting)
+ *
+ * If missing, the default value for OVP is 35.5V
+ */
+ pdata->over_voltage_protection = REG_I2C_1_OVP1;
+ if (!of_property_read_u32(node, "mps,overvoltage-protection-microvolt",
+ &tmp_value)) {
+ switch (tmp_value) {
+ case 13500000:
+ pdata->over_voltage_protection = 0x00;
+ break;
+ case 24000000:
+ pdata->over_voltage_protection = REG_I2C_1_OVP0;
+ break;
+ case 35500000:
+ pdata->over_voltage_protection = REG_I2C_1_OVP1;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ /* Synchronous (default) and non-synchronous mode */
+ pdata->sync_mode = true;
+ if (of_property_read_bool(node, "mps,no-sync-mode"))
+ pdata->sync_mode = false;
+
+ return 0;
+}
+
+static int mp3309c_probe(struct i2c_client *client)
+{
+ struct mp3309c_platform_data *pdata = dev_get_platdata(&client->dev);
+ struct mp3309c_chip *chip;
+ struct backlight_properties props;
+ struct pwm_state pwmstate;
+ int ret;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(&client->dev, "failed to check i2c functionality\n");
+ return -EOPNOTSUPP;
+ }
+
+ chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->dev = &client->dev;
+
+ chip->regmap = devm_regmap_init_i2c(client, &mp3309c_regmap);
+ if (IS_ERR(chip->regmap))
+ return dev_err_probe(&client->dev, PTR_ERR(chip->regmap),
+ "failed to allocate register map\n");
+
+ i2c_set_clientdata(client, chip);
+
+ if (!pdata) {
+ pdata = devm_kzalloc(chip->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ ret = pm3309c_parse_dt_node(chip, pdata);
+ if (ret)
+ return ret;
+ }
+ chip->pdata = pdata;
+
+ /* Backlight properties */
+ props.brightness = pdata->default_brightness;
+ props.max_brightness = pdata->max_brightness;
+ props.scale = BACKLIGHT_SCALE_LINEAR;
+ props.type = BACKLIGHT_RAW;
+ props.power = FB_BLANK_UNBLANK;
+ props.fb_blank = FB_BLANK_UNBLANK;
+ chip->bl = devm_backlight_device_register(chip->dev, "mp3309c",
+ chip->dev, chip, &mp3309c_bl_ops,
+ &props);
+ if (IS_ERR(chip->bl))
+ return dev_err_probe(chip->dev, PTR_ERR(chip->bl),
+ "error registering backlight device\n");
+
+ /* In PWM dimming mode, enable pwm device */
+ if (chip->pdata->dimming_mode == DIMMING_PWM) {
+ pwm_init_state(chip->pwmd, &pwmstate);
+ pwm_set_relative_duty_cycle(&pwmstate,
+ chip->pdata->default_brightness,
+ chip->pdata->max_brightness);
+ pwmstate.enabled = true;
+ ret = pwm_apply_state(chip->pwmd, &pwmstate);
+ if (ret)
+ return dev_err_probe(chip->dev, ret,
+ "error setting pwm device\n");
+ }
+
+ chip->pdata->status = FIRST_POWER_ON;
+ backlight_update_status(chip->bl);
+
+ return 0;
+}
+
+static void mp3309c_remove(struct i2c_client *client)
+{
+ struct mp3309c_chip *chip = i2c_get_clientdata(client);
+ struct backlight_device *bl = chip->bl;
+
+ bl->props.power = FB_BLANK_POWERDOWN;
+ bl->props.brightness = 0;
+ backlight_update_status(chip->bl);
+}
+
+static const struct of_device_id mp3309c_match_table[] = {
+ { .compatible = "mps,mp3309c", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mp3309c_match_table);
+
+static const struct i2c_device_id mp3309c_id[] = {
+ { "mp3309c", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mp3309c_id);
+
+static struct i2c_driver mp3309c_i2c_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = mp3309c_match_table,
+ },
+ .probe = mp3309c_probe,
+ .remove = mp3309c_remove,
+ .id_table = mp3309c_id,
+};
+
+module_i2c_driver(mp3309c_i2c_driver);
+
+MODULE_DESCRIPTION("Backlight Driver for MPS MP3309C");
+MODULE_AUTHOR("Flavio Suligoi <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.34.1


2023-09-20 21:49:19

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] backlight: mp3309c: Add support for MPS MP3309C

On Fri, Sep 15, 2023 at 04:05:16PM +0200, Flavio Suligoi wrote:
> The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a
> programmable switching frequency to optimize efficiency.
> The brightness can be controlled either by I2C commands (called "analog"
> mode) or by a PWM input signal (PWM mode).
> This driver supports both modes.
>
> For DT configuration details, please refer to:
> - Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
>
> The datasheet is available at:
> - https://www.monolithicpower.com/en/mp3309c.html
>
> Signed-off-by: Flavio Suligoi <[email protected]>
> ---
>
> v2:
> - fix dependecies in Kconfig
> - fix Kconfig MP3309C entry order
> - remove switch-on-delay-ms property
> - remove optional gpio property to reset external devices
> - remove dimming-mode property (the analog-i2c dimming mode is the default; the
> presence of the pwms property, in DT, selects automatically the pwm dimming
> mode)
> - substitute three boolean properties, used for the overvoltage-protection
> values, with a single enum property
> - drop simple tracing messages
> - use dev_err_probe() in probe function
> - change device name from mp3309c_bl to the simple mp3309c
> - remove shutdown function
> v1:
> - first version
>
> MAINTAINERS | 6 +
> drivers/video/backlight/Kconfig | 11 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/mp3309c.c | 395 ++++++++++++++++++++++++++++++
> 4 files changed, 413 insertions(+)
> create mode 100644 drivers/video/backlight/mp3309c.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3be1bdfe8ecc..f779df433af1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14333,6 +14333,12 @@ S: Maintained
> F: Documentation/driver-api/tty/moxa-smartio.rst
> F: drivers/tty/mxser.*
>
> +MP3309C BACKLIGHT DRIVER
> +M: Flavio Suligoi <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> +F: drivers/video/backlight/mp3309c.c
> +
> MR800 AVERMEDIA USB FM RADIO DRIVER
> M: Alexey Klimov <[email protected]>
> L: [email protected]
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 51387b1ef012..1144a54a35c0 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -402,6 +402,17 @@ config BACKLIGHT_LP8788
> help
> This supports TI LP8788 backlight driver.
>
> +config BACKLIGHT_MP3309C
> + tristate "Backlight Driver for MPS MP3309C"
> + depends on I2C && PWM
> + select REGMAP_I2C
> + help
> + This supports MPS MP3309C backlight WLED driver in both PWM and
> + analog/I2C dimming modes.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called mp3309c.
> +
> config BACKLIGHT_PANDORA
> tristate "Backlight driver for Pandora console"
> depends on TWL4030_CORE
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index f72e1c3c59e9..1af583de665b 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
> obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o
> obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o
> obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o
> +obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o
> obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o
> obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
> obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
> diff --git a/drivers/video/backlight/mp3309c.c b/drivers/video/backlight/mp3309c.c
> new file mode 100644
> index 000000000000..470c960d7438
> --- /dev/null
> +++ b/drivers/video/backlight/mp3309c.c
> @@ -0,0 +1,395 @@
> +// SPDX-License-Identifier: GPL-2.0+

This is an obsolete spelling. Should be:
https://spdx.org/licenses/GPL-2.0-or-later.html


> +static int mp3309c_bl_update_status(struct backlight_device *bl)
> +{
> + struct mp3309c_chip *chip = bl_get_data(bl);
> + int brightness = backlight_get_brightness(bl);
> + struct pwm_state pwmstate;
> + unsigned int analog_val, bits_val;
> + int i, ret;
> +
> + if (chip->pdata->dimming_mode == DIMMING_PWM) {
> + /*
> + * PWM dimming mode
> + */
> + pwm_get_state(chip->pwmd, &pwmstate);
> + pwm_set_relative_duty_cycle(&pwmstate, brightness,
> + chip->pdata->max_brightness);
> + pwmstate.enabled = true;
> + ret = pwm_apply_state(chip->pwmd, &pwmstate);
> + if (ret)
> + return ret;
> +
> + switch (chip->pdata->status) {
> + case FIRST_POWER_ON:
> + case BACKLIGHT_OFF:
> + /*
> + * After 20ms of pwm off, we must enable the chip again
> + */
> + if (brightness > 0) {
> + msleep_interruptible(10);

This is either missing a return code check (and working bail-out logic)
or, more likely, shouldn't be interruptible.

Also it looks like the delay time and the comment do not match.

However, when these and the license comment is addressed please add my:
Reviewed-by: Daniel Thompson <[email protected]>


Daniel.