RE: [PATCH v3 2/2] backlight: mp3309c: Add support for MPS MP3309C
Hi Daniel,
...
> > +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, );
> > + pwm_set_relative_duty_cycle(, brightness,
> > + chip->pdata->max_brightness);
> > + pwmstate.enabled = true;
> > + ret = pwm_apply_state(chip->pwmd, );
> > + if (ret)
> > + return ret;
> > +
> > + switch (chip->pdata->status) {
> > + case FIRST_POWER_ON:
> > + case BACKLIGHT_OFF:
> > + /*
> > +* After 20ms of low pwm signal level, the chip turns
> > + off automatically. In this case, before enabling the
> > + chip again, we must wait about 10ms for pwm signal
> to
> > + stabilize.
> > +*/
> > + if (brightness > 0) {
> > + msleep(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);
>
> Sorry to only notice after sharing a Reviewed-by:[1] but...
>
> Scaling brightness here isn't right. When running in I2C dimming mode then
> max_brightness *must* be 31 or lower, meaning the value in brightness can
> be applied directly to the hardware without scaling.
ok, right; max brightness is 31, fixed
>
> Quoting the DT binding docs about how max-brightness should be
> interpretted:
>
> Normally the maximum brightness is determined by the hardware and this
> property is not required. This property is used to put a software
> limit on the brightness apart from what the driver says, as it could
> happen that a LED can be made so bright that it gets damaged or causes
> damage due to restrictions in a specific system, such as mounting
> conditions.
>
ok
>
> Daniel.
>
>
> [1] I remember checking if this code could overflow the field but I was
> so distracted by that I ended up missing the obvious!
Thanks and best regards,
Flavio
Re: [PATCH v3 2/2] backlight: mp3309c: Add support for MPS MP3309C
On Mon, Sep 25, 2023 at 02:26:09PM +0200, Flavio Suligoi wrote:
> diff --git a/drivers/video/backlight/mp3309c.c
> b/drivers/video/backlight/mp3309c.c
> new file mode 100644
> index ..923ac7f7b291
> --- /dev/null
> +++ b/drivers/video/backlight/mp3309c.c
> @@ -0,0 +1,398 @@
> ...
> +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, );
> + pwm_set_relative_duty_cycle(, brightness,
> + chip->pdata->max_brightness);
> + pwmstate.enabled = true;
> + ret = pwm_apply_state(chip->pwmd, );
> + if (ret)
> + return ret;
> +
> + switch (chip->pdata->status) {
> + case FIRST_POWER_ON:
> + case BACKLIGHT_OFF:
> + /*
> + * After 20ms of low pwm signal level, the chip turns
> +off automatically. In this case, before enabling the
> +chip again, we must wait about 10ms for pwm signal to
> +stabilize.
> + */
> + if (brightness > 0) {
> + msleep(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);
Sorry to only notice after sharing a Reviewed-by:[1] but...
Scaling brightness here isn't right. When running in I2C dimming mode then
max_brightness *must* be 31 or lower, meaning the value in brightness can
be applied directly to the hardware without scaling.
Quoting the DT binding docs about how max-brightness should be
interpretted:
Normally the maximum brightness is determined by the hardware and this
property is not required. This property is used to put a software
limit on the brightness apart from what the driver says, as it could
happen that a LED can be made so bright that it gets damaged or causes
damage due to restrictions in a specific system, such as mounting
conditions.
Daniel.
[1] I remember checking if this code could overflow the field but I was
so distracted by that I ended up missing the obvious!
[PATCH v3 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
Reviewed-by: Daniel Thompson
---
v3:
- fix SPDX obsolete spelling
- in mp3309c_bl_update_status, change from msleep_interruptible() to msleep()
and improve the related comment
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 | 398 ++
4 files changed, 416 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
+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
L: linux-me...@vger.kernel.org
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 ..923ac7f7b291
--- /dev/null
+++ b/drivers/video/backlight/mp3309c.c
@@ -0,0 +1,398 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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_RSRV10x02
+#define REG_I2C_0_RSRV20x01
+
+#define REG_I2C_1_RSRV10x80
+#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
