RE: EXTERNAL: Re: (subset) [PATCH v1 1/1] backlight: mp3309c: fix leds flickering in pwm mode

[FLAVIO SULIGOI]

Hi Lee,

...

> Subject: Re: EXTERNAL: Re: (subset) [PATCH v1 1/1] backlight: mp3309c:
> fix leds flickering in pwm mode
> 
> On Fri, 03 May 2024, FLAVIO SULIGOI wrote:
> 
> > Hi Lee,
> >
> > ...
> >
> > > Subject: EXTERNAL: Re: (subset) [PATCH v1 1/1] backlight: mp3309c:
> > > fix leds flickering in pwm mode
> > >
> > > [Use caution with links & attachments]
> > >
> > >
> > >
> > > On Thu, 02 May 2024, Lee Jones wrote:
> > >
> > > > On Wed, 17 Apr 2024 17:31:05 +0200, Flavio Suligoi wrote:
> > > > > The mp3309 has two configuration registers, named according to
> > > > > their address (0x00 and 0x01).
> > > > > In the second register (0x01), the bit DIMS (Dimming Mode
> > > > > Select) must be always 0 (zero), in both analog (via i2c
> > > > > commands) and pwm dimming mode.
> > > > >
> > > > > In the initial driver version, the DIMS bit was set in pwm mode
> > > > > and reset in analog mode.
> > > > > But if the DIMS bit is set in pwm dimming mode and other devices
> > > > > are connected on the same i2c bus, every i2c commands on the bus
> > > > > generates a flickering on the LEDs powered by the mp3309c.
> > > > >
> > > > > [...]
> > > >
> > > > Applied, thanks!
> > > >
> > > > [1/1] backlight: mp3309c: fix leds flickering in pwm mode
> > > >   commit: ce60cddc2abf61902dfca71d630624db95315124
> > >
> > > Applied, but in future it's; I2C, PWM and LED, thanks.
> >
> > Sorry for my question, but do you mean that I also have to add the
> > I2C, PWM and LED mailing lists in my messages related to the mp33309c
> patches?
> 
> Just use proper capitalisation when you abbreviate the names these
> subsystems please.

Ah, ok, I got it!

> 
> --
> Lee Jones [李琼斯]

Thanks for your explanation,
Flavio

RE: EXTERNAL: Re: (subset) [PATCH v1 1/1] backlight: mp3309c: fix leds flickering in pwm mode

[FLAVIO SULIGOI]

Hi Lee,

...

> Subject: EXTERNAL: Re: (subset) [PATCH v1 1/1] backlight: mp3309c: fix
> leds flickering in pwm mode
> 
> [Use caution with links & attachments]
> 
> 
> 
> On Thu, 02 May 2024, Lee Jones wrote:
> 
> > On Wed, 17 Apr 2024 17:31:05 +0200, Flavio Suligoi wrote:
> > > The mp3309 has two configuration registers, named according to their
> > > address (0x00 and 0x01).
> > > In the second register (0x01), the bit DIMS (Dimming Mode Select)
> > > must be always 0 (zero), in both analog (via i2c commands) and pwm
> > > dimming mode.
> > >
> > > In the initial driver version, the DIMS bit was set in pwm mode and
> > > reset in analog mode.
> > > But if the DIMS bit is set in pwm dimming mode and other devices are
> > > connected on the same i2c bus, every i2c commands on the bus
> > > generates a flickering on the LEDs powered by the mp3309c.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/1] backlight: mp3309c: fix leds flickering in pwm mode
> >   commit: ce60cddc2abf61902dfca71d630624db95315124
> 
> Applied, but in future it's; I2C, PWM and LED, thanks.

Sorry for my question, but do you mean that I also have to add the I2C,
PWM and LED mailing lists in my messages related to the mp33309c patches?

> 
> --
> Lee Jones [李琼斯]

Thanks and best regards,
Flavio

[PATCH v1 1/1] backlight: mp3309c: fix leds flickering in pwm mode

[Flavio Suligoi]

The mp3309 has two configuration registers, named according to their
address (0x00 and 0x01).
In the second register (0x01), the bit DIMS (Dimming Mode Select) must
be always 0 (zero), in both analog (via i2c commands) and pwm dimming
mode.

In the initial driver version, the DIMS bit was set in pwm mode and
reset in analog mode.
But if the DIMS bit is set in pwm dimming mode and other devices are
connected on the same i2c bus, every i2c commands on the bus generates a
flickering on the LEDs powered by the mp3309c.

This change concerns the chip initialization and does not impact any
existing device-tree configuration.

Signed-off-by: Flavio Suligoi 
---
 drivers/video/backlight/mp3309c.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/video/backlight/mp3309c.c 
b/drivers/video/backlight/mp3309c.c
index 016c1296841c..a28036c964af 100644
--- a/drivers/video/backlight/mp3309c.c
+++ b/drivers/video/backlight/mp3309c.c
@@ -97,15 +97,10 @@ static int mp3309c_enable_device(struct mp3309c_chip *chip)
 
/*
 * 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;
-- 
2.34.1

[PATCH v1 0/1] backlight: mp3309c: fix leds flickering in pwm mode

[Flavio Suligoi]

The mp3309 has two configuration registers, named according to their
address (0x00 and 0x01).
In the second register (0x01), the bit DIMS (Dimming Mode Select) must
be always 0 (zero), in both analog (via i2c commands) and pwm dimming
mode.

In the initial driver version, the DIMS bit was set in pwm mode and
reset in analog mode.
But if the DIMS bit is set in pwm dimming mode and other devices are
connected on the same i2c bus, every i2c commands on the bus generates a
flickering on the LEDs powered by the mp3309c.

This change concerns the chip initialization and does not impact any
existing device-tree configuration.

I created this device driver for one of our boards, where both dimming
modes (pwm and analog by i2c commands) can be used.
This board uses the same i2c bus for the mp3309c and for an at24cs32
eeprom.
During further tests, I realized that, when the mp3309c is used in pwm
mode, every read operation on the eeprom caused a backlight flickering.
This is why I made this device driver modification.

Flavio Suligoi (1):
  backlight: mp3309c: fix leds flickering in pwm mode

 drivers/video/backlight/mp3309c.c | 5 -
 1 file changed, 5 deletions(-)

-- 
2.34.1

RE: EXTERNAL: Re: [PATCH v2 6/6] backlight: Remove fb_blank from struct backlight_properties

[FLAVIO SULIGOI]

HI Thomas,

...
> >> Remove the field fb_blank from struct backlight_properties and remove
> >> all code that still sets or reads it. Backlight blank status is now
> >> tracked exclusively in struct backlight_properties.state.
> >>
> >> The core backlight code keeps the fb_blank and state fields in sync,
> >> but doesn't do anything else with fb_blank. Several drivers
> >> initialize fb_blank to FB_BLANK_UNBLANK to enable the backlight. This
> >> is already the default for the state field. So we can delete the
> >> fb_blank code from core and drivers and rely on the state field.
> >>
> >> Signed-off-by: Thomas Zimmermann 
> >> Cc: Flavio Suligoi 
> >> Cc: Nicolas Ferre 
> >> Cc: Alexandre Belloni 
> >> Cc: Claudiu Beznea 
> >> Tested-by: Flavio Suligoi 
> >> Reviewed-by: Daniel Thompson 
> >> Reviewed-by: Dan Carpenter 
> > ...
> >
> > Can you explain what are the differences between the version 1 and version
> 2 of the patch?
> 
> There are none. It's simply labeled v2 because it is part of the version
> 2 of this series.
...

Ah, ok. Sorry for my question, but having received only this email from the 
series,
I didn't understand why version two.
It's all clear now!

Best regards,
Flavio

RE: [PATCH v2 6/6] backlight: Remove fb_blank from struct backlight_properties

[FLAVIO SULIGOI]

Hi Thomas,

...
> Remove the field fb_blank from struct backlight_properties and remove all
> code that still sets or reads it. Backlight blank status is now tracked 
> exclusively
> in struct backlight_properties.state.
> 
> The core backlight code keeps the fb_blank and state fields in sync, but 
> doesn't
> do anything else with fb_blank. Several drivers initialize fb_blank to
> FB_BLANK_UNBLANK to enable the backlight. This is already the default for
> the state field. So we can delete the fb_blank code from core and drivers and
> rely on the state field.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Flavio Suligoi 
> Cc: Nicolas Ferre 
> Cc: Alexandre Belloni 
> Cc: Claudiu Beznea 
> Tested-by: Flavio Suligoi 
> Reviewed-by: Daniel Thompson 
> Reviewed-by: Dan Carpenter 
...

Can you explain what are the differences between the version 1 and version 2 of 
the patch?
Thanks,
Flavio

RE: [PATCH] backlight: mp3309c: fix signedness bug in mp3309c_parse_fwnode()

[FLAVIO SULIGOI]

Hi Dan,

> The "num_levels" variable is used to store error codes from
> device_property_count_u32() so it needs to be signed.  This doesn't cause an
> issue at runtime because devm_kcalloc() won't allocate negative sizes.
> However, it's still worth fixing.
> 
> Fixes: b54c828bdba9 ("backlight: mp3309c: Make use of device properties")
> Signed-off-by: Dan Carpenter 

I've just tested on my board with the mp3309c chip, all is ok.
Thanks!

Tested-by: Flavio Suligoi 

RE: [PATCH 6/6] backlight: Remove fb_blank from struct backlight_properties

[FLAVIO SULIGOI]

Hi Thomas,

...

>  drivers/video/backlight/mp3309c.c |  1 -

...

> diff --git a/drivers/video/backlight/mp3309c.c
> b/drivers/video/backlight/mp3309c.c
> index 34d71259fac1d..059f4ddbbc842 100644
> --- a/drivers/video/backlight/mp3309c.c
> +++ b/drivers/video/backlight/mp3309c.c
> @@ -378,7 +378,6 @@ static int mp3309c_probe(struct i2c_client *client)
> 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,
>   _bl_ops, ); 

...

I've just tested your change with my board with the mp3309c backlight and all 
is ok.
Thanks and best regards
Flavio

Tested-by: Flavio Suligoi 

RE: [PATCH v2 0/3] backlight: mp3309c: Allow to use on non-OF platforms

[Flavio Suligoi]

Hi Andy,

...

> Subject: [PATCH v2 0/3] backlight: mp3309c: Allow to use on non-OF
> platforms
> 
> Allow to use driver on non-OF platforms and other cleanups.
> 
> Changelog v2:
> - rename pm3309c_parse_dt_node() --> mp3309c_parse_fwnode() (Daniel)
> - add tags (Daniel, Flavio)
> - new patch 2
> 
> Andy Shevchenko (3):
>   backlight: mp3309c: Make use of device properties
>   backlight: mp3309c: use dev_err_probe() instead of dev_err()
>   backlight: mp3309c: Utilise temporary variable for struct device
...
I've just tested again all your three patches (using the last 8.8.0-rc1 
for-backlight-next )  on my board and all is ok.

Thanks,
Flavio

RE: [PATCH v2 2/3] backlight: mp3309c: use dev_err_probe() instead of dev_err()

[Flavio Suligoi]

Hi Andy,

> Replace dev_err() with dev_err_probe().
> 
> This helps in simplifing code and standardizing the error output.
> 
> Signed-off-by: Andy Shevchenko 

Tested-by: Flavio Suligoi 

Flavio

RE: [PATCH] backlight: mp3309c: Use pwm_apply_might_sleep()

[Flavio Suligoi]

Hi Sean,

I've just tested your change on my board that uses the mp3309c.
All ok, thanks!

...

> Subject: [PATCH] backlight: mp3309c: Use pwm_apply_might_sleep()
> 
> pwm_apply_state() is deprecated since commit c748a6d77c06a ("pwm:
> Rename
> pwm_apply_state() to pwm_apply_might_sleep()"). This is the final user in the
> tree.
> 
> Signed-off-by: Sean Young 
> ---

Tested-by: Flavio Suligoi 

RE: [PATCH v1 1/2] backlight: mp3309c: Make use of device properties

[Flavio Suligoi]

Hi Daniel,

...

> Subject: Re: [PATCH v1 1/2] backlight: mp3309c: Make use of device
> properties
> > +++ b/drivers/video/backlight/mp3309c.c
> > @@ -15,6 +15,8 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -202,15 +204,12 @@ static const struct backlight_ops mp3309c_bl_ops
> = {
> >  static int pm3309c_parse_dt_node(struct mp3309c_chip *chip,
> 
> Pretty minor... but I wonder if it should be renamed:
> mp3309c_parse_fwnode().

Right! It was my oversight!
Thanks!

> 
> 
> Daniel.

Flavio

RE: [PATCH v1 2/2] backlight: mp3309c: Utilise temporary variable for struct device

[Flavio Suligoi]

> Subject: [PATCH v1 2/2] backlight: mp3309c: Utilise temporary variable
> for struct device
> 
> We have a temporary variable to keep pointer to struct device.
> Utilise it where it makes sense.
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/video/backlight/mp3309c.c | 38 +--
>  1 file changed, 16 insertions(+), 22 deletions(-)
> 

Hi Andy,

I tested the patch in both pwm and analog-ic dimming-mode and everything is ok.
Thanks for the optimizations!

Tested-by: Flavio Suligoi 

RE: [PATCH v1 1/2] backlight: mp3309c: Make use of device properties

[Flavio Suligoi]

> Subject: [PATCH v1 1/2] backlight: mp3309c: Make use of device
> properties
> 
> Convert the module to be property provider agnostic and allow
> it to be used on non-OF platforms.
> 
> Add mod_devicetable.h include.
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/video/backlight/mp3309c.c | 38 ---
>  1 file changed, 15 insertions(+), 23 deletions(-)

Hi Andy,

I tested the patch in both pwm and analog-ic dimming-mode and everything is ok.
Thanks for the optimizations!

Tested-by: Flavio Suligoi 

[PATCH v2] backlight: mp3309c: fix uninitialized local variable

[Flavio Suligoi]

In the function "pm3309c_parse_dt_node", when the dimming analog control
mode (by I2C messages) is enabled, the local variable "prop_levels" is
tested without any initialization, as indicated by the following smatch
warning:

drivers/video/backlight/mp3309c.c:279 pm3309c_parse_dt_node() error: 
uninitialized symbol 'prop_levels'.

To avoid any problem in case of undefined behavior, we need to initialize
it to "NULL".

Reported-by: Dan Carpenter 
Closes: 
https://lore.kernel.org/dri-devel/af0a1870-693b-442f-9b11-0503cfcd944a@moroto.mountain/
Fixes: 2e914516a58c ("backlight: mp3309c: Add support for MPS MP3309C")
Signed-off-by: Flavio Suligoi 
---

v2:
 - remove redundant initialization of "prop_pwms" variable
 - remove "thanks to Dan Carpenter for the report"
 - add "Reported-by: Dan Carpenter " tag
 - add "Closes:" tag

 drivers/video/backlight/mp3309c.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/mp3309c.c 
b/drivers/video/backlight/mp3309c.c
index 3fe4469ef43f..34d71259fac1 100644
--- a/drivers/video/backlight/mp3309c.c
+++ b/drivers/video/backlight/mp3309c.c
@@ -203,7 +203,8 @@ 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_pwms, *prop_levels;
+   struct property *prop_pwms;
+   struct property *prop_levels = NULL;
int length = 0;
int ret, i;
unsigned int num_levels, tmp_value;
-- 
2.34.1

RE: [bug report] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

Hi Dan,

Can I add the "Reported-by" tag, with your name, in my 2nd vers of
the commit to fix this bug?

Thanks and regards,

Flavio


> -Original Message-
> From: Flavio Suligoi 
> Sent: Tuesday, November 28, 2023 9:24 AM
> To: Dan Carpenter 
> Cc: dri-devel@lists.freedesktop.org
> Subject: RE: [bug report] backlight: mp3309c: Add support for MPS
> MP3309C
> 
> Hi Dan,
> 
> Thanks for the report, I'll fix the bug as soon as possible.
> 
> Regards,
> Flavio
> 
> > -Original Message-
> > From: Dan Carpenter 
> > Sent: Tuesday, November 28, 2023 8:20 AM
> > To: Flavio Suligoi 
> > Cc: dri-devel@lists.freedesktop.org
> > Subject: [bug report] backlight: mp3309c: Add support for MPS MP3309C
> >
> > Hello Flavio Suligoi,
> >
> > The patch 2e914516a58c: "backlight: mp3309c: Add support for MPS
> > MP3309C" from Nov 16, 2023 (linux-next), leads to the following
> > Smatch static checker warning:
> >
> > drivers/video/backlight/mp3309c.c:277 pm3309c_parse_dt_node()
> > error: uninitialized symbol 'prop_levels'.
> >
> > drivers/video/backlight/mp3309c.c
> > 202 static int pm3309c_parse_dt_node(struct mp3309c_chip *chip,
> > 203  struct mp3309c_platform_data
> > *pdata)
> > 204 {
> > 205 struct device_node *node = chip->dev->of_node;
> > 206 struct property *prop_pwms, *prop_levels;
> > 207 int length = 0;
> > 208 int ret, i;
> > 209 unsigned int num_levels, tmp_value;
> > 210
> > 211 if (!node) {
> > 212 dev_err(chip->dev, "failed to get DT node\n");
> > 213 return -ENODEV;
> > 214 }
> > 215
> > 216 /*
> > 217  * Dimming mode: the MP3309C provides two dimming
> > control mode:
> > 218  *
> > 219  * - PWM mode
> > 220  * - Analog by I2C control mode (default)
> > 221  *
> > 222  * I2C control mode is assumed as default but, if the
> > pwms property is
> > 223  * found in the backlight node, the mode switches to
> PWM
> > mode.
> > 224  */
> > 225 pdata->dimming_mode = DIMMING_ANALOG_I2C;
> > 226 prop_pwms = of_find_property(node, "pwms", );
> > 227 if (prop_pwms) {
> > 228 chip->pwmd = devm_pwm_get(chip->dev, NULL);
> > 229 if (IS_ERR(chip->pwmd))
> > 230 return dev_err_probe(chip->dev,
> > PTR_ERR(chip->pwmd),
> > 231  "error getting
> pwm
> > data\n");
> > 232 pdata->dimming_mode = DIMMING_PWM;
> > 233 pwm_apply_args(chip->pwmd);
> > 234 }
> > 235
> > 236 /*
> > 237  * In I2C control mode the dimming levels (0..31) are
> > fixed by the
> > 238  * hardware, while in PWM control mode they can be
> > chosen by the user,
> > 239  * to allow nonlinear mappings.
> > 240  */
> > 241 if  (pdata->dimming_mode == DIMMING_ANALOG_I2C) {
> > 242 /*
> > 243  * Analog (by I2C commands) control mode:
> fixed
> > 0..31 brightness
> > 244  * levels
> > 245  */
> > 246 num_levels = ANALOG_I2C_NUM_LEVELS;
> > 247
> > 248 /* Enable GPIO used in I2C dimming mode only
> */
> > 249 chip->enable_gpio = devm_gpiod_get(chip->dev,
> > "enable",
> > 250
> > GPIOD_OUT_HIGH);
> > 251 if (IS_ERR(chip->enable_gpio))
> > 252 return dev_err_probe(chip->dev,
> > 253  PTR_ERR(chip-
> > >enable_gpio),
> > 254  "error getting
> > enable gpio\n");
> >
> > prop_levels not initialized on this path.
> >
> > 255 } else {
> > 256 /*
> > 257  * PWM control mode: check for brightness
> level
> > in DT
> > 258  */
> > 259 prop_levels = of_find_property(node,
> > "brightness-

[PATCH] backlight: mp3309c: fix uninitialized local variable

[Flavio Suligoi]

In the function "pm3309c_parse_dt_node", when the dimming analog control
mode (by I2C messages) is enabled, the local variable "prop_levels" is
tested without any initialization, as indicated by the following smatch
warning (thanks to Dan Carpenter for the report):

drivers/video/backlight/mp3309c.c:279 pm3309c_parse_dt_node() error: 
uninitialized symbol 'prop_levels'.

To avoid any problem in case of undefined behavior, we need to initialize
it to "NULL".
For consistency, I also initialize the other similar variable
"prop_pwms" in the same way.

Signed-off-by: Flavio Suligoi 
---
 drivers/video/backlight/mp3309c.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/mp3309c.c 
b/drivers/video/backlight/mp3309c.c
index 3fe4469ef43f..733cad1dd15c 100644
--- a/drivers/video/backlight/mp3309c.c
+++ b/drivers/video/backlight/mp3309c.c
@@ -203,7 +203,8 @@ 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_pwms, *prop_levels;
+   struct property *prop_pwms = NULL;
+   struct property *prop_levels = NULL;
int length = 0;
int ret, i;
unsigned int num_levels, tmp_value;
-- 
2.34.1

RE: [bug report] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

Hi Dan,

Thanks for the report, I'll fix the bug as soon as possible.

Regards,
Flavio

> -Original Message-
> From: Dan Carpenter 
> Sent: Tuesday, November 28, 2023 8:20 AM
> To: Flavio Suligoi 
> Cc: dri-devel@lists.freedesktop.org
> Subject: [bug report] backlight: mp3309c: Add support for MPS MP3309C
> 
> Hello Flavio Suligoi,
> 
> The patch 2e914516a58c: "backlight: mp3309c: Add support for MPS
> MP3309C" from Nov 16, 2023 (linux-next), leads to the following
> Smatch static checker warning:
> 
>   drivers/video/backlight/mp3309c.c:277 pm3309c_parse_dt_node()
>   error: uninitialized symbol 'prop_levels'.
> 
> drivers/video/backlight/mp3309c.c
> 202 static int pm3309c_parse_dt_node(struct mp3309c_chip *chip,
> 203  struct mp3309c_platform_data
> *pdata)
> 204 {
> 205 struct device_node *node = chip->dev->of_node;
> 206 struct property *prop_pwms, *prop_levels;
> 207 int length = 0;
> 208 int ret, i;
> 209 unsigned int num_levels, tmp_value;
> 210
> 211 if (!node) {
> 212 dev_err(chip->dev, "failed to get DT node\n");
> 213 return -ENODEV;
> 214 }
> 215
> 216 /*
> 217  * Dimming mode: the MP3309C provides two dimming
> control mode:
> 218  *
> 219  * - PWM mode
> 220  * - Analog by I2C control mode (default)
> 221  *
> 222  * I2C control mode is assumed as default but, if the
> pwms property is
> 223  * found in the backlight node, the mode switches to PWM
> mode.
> 224  */
> 225 pdata->dimming_mode = DIMMING_ANALOG_I2C;
> 226 prop_pwms = of_find_property(node, "pwms", );
> 227 if (prop_pwms) {
> 228 chip->pwmd = devm_pwm_get(chip->dev, NULL);
> 229 if (IS_ERR(chip->pwmd))
> 230 return dev_err_probe(chip->dev,
> PTR_ERR(chip->pwmd),
> 231  "error getting pwm
> data\n");
> 232 pdata->dimming_mode = DIMMING_PWM;
> 233 pwm_apply_args(chip->pwmd);
> 234 }
> 235
> 236 /*
> 237  * In I2C control mode the dimming levels (0..31) are
> fixed by the
> 238  * hardware, while in PWM control mode they can be
> chosen by the user,
> 239  * to allow nonlinear mappings.
> 240  */
> 241 if  (pdata->dimming_mode == DIMMING_ANALOG_I2C) {
> 242 /*
> 243  * Analog (by I2C commands) control mode: fixed
> 0..31 brightness
> 244  * levels
> 245  */
> 246 num_levels = ANALOG_I2C_NUM_LEVELS;
> 247
> 248 /* Enable GPIO used in I2C dimming mode only */
> 249 chip->enable_gpio = devm_gpiod_get(chip->dev,
> "enable",
> 250
> GPIOD_OUT_HIGH);
> 251 if (IS_ERR(chip->enable_gpio))
> 252 return dev_err_probe(chip->dev,
> 253  PTR_ERR(chip-
> >enable_gpio),
> 254  "error getting
> enable gpio\n");
> 
> prop_levels not initialized on this path.
> 
> 255 } else {
> 256 /*
> 257  * PWM control mode: check for brightness level
> in DT
> 258  */
> 259 prop_levels = of_find_property(node,
> "brightness-levels",
> 260);
> 261 if (prop_levels) {
> 262 /* Read brightness levels from DT */
> 263 num_levels = length / sizeof(u32);
> 264 if (num_levels < 2)
> 265 return -EINVAL;
> 266 } else {
> 267 /* Use default brightness levels */
> 268 num_levels =
> MP3309C_PWM_DEFAULT_NUM_LEVELS;
> 269 }
> 270 }
> 271
> 272 /* Fill brightness levels array */
> 273 pdata->levels = devm_kcalloc(chip->dev, num_levels,
> 274  sizeof(*pdata->levels),
> GFP_KERNEL);
> 275 if (!pdata->

[PATCH v7 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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 
---

v7:
 - add missed patch history
 - add missed "Reviewed-by" added in one of the previous version
v6:
 - check and resend for updated kernel 6.7.0-rc1
v5:
 - check and resend for updated kernel 6.6.0-rc1
v4:
 - add brightness-levels property
 - force fixed 32 brightness levels (0..31) in analog-i2c dimming mode
 - remove useless irq and reset_gpio from mp3309c_chip structure
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   |   7 +
 drivers/video/backlight/Kconfig   |  11 +
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/mp3309c.c | 443 ++
 4 files changed, 462 insertions(+)
 create mode 100644 drivers/video/backlight/mp3309c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c9f868e13b6..d033c2a2d120 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14657,6 +14657,13 @@ S: Maintained
 F: Documentation/driver-api/tty/moxa-smartio.rst
 F: drivers/tty/mxser.*
 
+MP3309C BACKLIGHT DRIVER
+M: Flavio Suligoi 
+L: dri-devel@lists.freedesktop.org
+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 ..3fe4469ef43f
--- /dev/null
+++ b/drivers/video/backlight/mp3309c.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * This driver support both analog (by I2C commands) and PWM dimming control
+ * modes.
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi 
+ *
+ * Based on pwm_bl.c
+ */
+
+#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_

[PATCH v7 1/2] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

The two properties:

- max-brightness
- default brightness

are not really required, so they can be removed from the "required"
section.
The "max-brightness" is no longer used in the current version
of the driver (it was used only in the first version).
The "default-brightness", if omitted in the DT, is managed by the
device driver, using a default value. This value depends on the dimming
mode used:

- for the "analog mode", via I2C commands, this value is fixed by
  hardware (=31)
- while in case of pwm mode the default used is the last value of the
  brightness-levels array.

Also the brightness-levels array is not required:

- in "analog mode", via I2C commands, the brightness-level array is
  fixed by hardware (0..31).;
- in pwm dimming mode, the driver uses a default array of 0..255 and
  the "default-brightness" is the last one, which is "255"

NOTE: there are no compatibility problems with the previous version,
  since the device driver has not yet been included in any kernel.
  Only this dt-binding yaml file is already included in the current
  v6.7.0-rc1 kernel version.
  No developer may have used it.

Other changes:

- improve the backlight working mode description, in the "description"
  section
- update the example, removing the "max-brightness" and introducing the
  "brightess-levels" property

Signed-off-by: Flavio Suligoi 
Acked-by: Conor Dooley 
---

v7:
 - add missed patch history
 - add missed "Acked-by" added in one of the previous version
v6:
 - check and resend for updated kernel 6.7.0-rc1 (nothing changed compared
   to the previous versions)
 - add this patch to the same patchset of the MP3309C device driver
   Note: the patch related to this file was previously a separate patch and
  sent in two versions (v1 and v2).
  It has now been included in this patchset, starting with the
  version v6.
v5...v3:
 - non-existing versions (the last was v2, the next is v6)
v2:
 - add more explanations in commit message, about the the non-existence of
   compatibility issues with the previous versions of the yaml file
v1:
 - first version

 .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
index 4191e33626f5..527a37368ed7 100644
--- a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -14,8 +14,8 @@ description: |
   programmable switching frequency to optimize efficiency.
   It supports two different dimming modes:
 
-  - analog mode, via I2C commands (default)
-  - PWM controlled mode.
+  - analog mode, via I2C commands, as default mode (32 dimming levels)
+  - PWM controlled mode (optional)
 
   The datasheet is available at:
   https://www.monolithicpower.com/en/mp3309c.html
@@ -50,8 +50,6 @@ properties:
 required:
   - compatible
   - reg
-  - max-brightness
-  - default-brightness
 
 unevaluatedProperties: false
 
@@ -66,8 +64,8 @@ examples:
 compatible = "mps,mp3309c";
 reg = <0x17>;
 pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
-max-brightness = <100>;
-default-brightness = <80>;
+brightness-levels = <0 4 8 16 32 64 128 255>;
+default-brightness = <6>;
 mps,overvoltage-protection-microvolt = <2400>;
 };
 };
-- 
2.34.1

[PATCH v7 0/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

This patchset (rebased on v6.7.0-rc1 kernel version):

- includes and updates the mps,mp3309c.yaml dt bindings file:
- Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
Note: the patch related to this file was previously a separate patch
  and sent in two versions (v1 and v2).
  It has now been included in this patchset, starting with the
  version v6.
- adds the related device driver to support the MPS MP3309C backlight chip
- adds missed history of previous versions
- adds missed "Acked-by" and "Reviewed-by" added in previous versions

Note: about the first point, the mps,mp3309c.yaml file updating, there are
  no compatibility problems with the previous version, since the
  related device driver has not yet been included in any kernel.
  Only this dt-binding yaml file is already included in the current
  v6.7.0-rc1 kernel version.
  No developer may have used it.

Flavio Suligoi (2):
  dt-bindings: backlight: mp3309c: remove two required properties
  backlight: mp3309c: Add support for MPS MP3309C

 .../bindings/leds/backlight/mps,mp3309c.yaml  |  10 +-
 MAINTAINERS   |   7 +
 drivers/video/backlight/Kconfig   |  11 +
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/mp3309c.c | 443 ++
 5 files changed, 466 insertions(+), 6 deletions(-)
 create mode 100644 drivers/video/backlight/mp3309c.c

-- 
2.34.1

[PATCH v6 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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 
---
 MAINTAINERS   |   7 +
 drivers/video/backlight/Kconfig   |  11 +
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/mp3309c.c | 443 ++
 4 files changed, 462 insertions(+)
 create mode 100644 drivers/video/backlight/mp3309c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c9f868e13b6..d033c2a2d120 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14657,6 +14657,13 @@ S: Maintained
 F: Documentation/driver-api/tty/moxa-smartio.rst
 F: drivers/tty/mxser.*
 
+MP3309C BACKLIGHT DRIVER
+M:     Flavio Suligoi 
+L: dri-devel@lists.freedesktop.org
+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 ..3fe4469ef43f
--- /dev/null
+++ b/drivers/video/backlight/mp3309c.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * This driver support both analog (by I2C commands) and PWM dimming control
+ * modes.
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi 
+ *
+ * Based on pwm_bl.c
+ */
+
+#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_I2C_NUM_LEVELS  32  /* 0..31 */
+#define ANALOG_I2C_REG_MASK0x7c
+
+#define MP3309C_PWM_DEFAULT_NUM_LEVELS 256 /* 0..255 */
+
+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 {
+   unsigned int max_brightness;
+   unsigned int default_brightness;
+   unsigned int *levels;
+   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;
+};
+
+static const struct regmap_config mp3309c_regmap = {
+   .name = "mp3309c_regmap",
+   .reg_bits = 8

[PATCH v6 0/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

This patchset (rebased on v6.7-rc1 kernel version):

- updates the mps,mp3309c.yaml dt bindings file:
- Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml

- adds the related device driver to support the MPS MP3309C backlight chip.

Note: about the first point, the mps,mp3309c.yaml file updating, there are
  no compatibility problems with the previous version, since the
  related device driver has not yet been included in any kernel.
  Only this dt-binding yaml file is already included in the current
  v6.7-rc1 kernel version.
  No developer may have used it.

Flavio Suligoi (2):
  dt-bindings: backlight: mp3309c: remove two required properties
  backlight: mp3309c: Add support for MPS MP3309C

 .../bindings/leds/backlight/mps,mp3309c.yaml  |  10 +-
 MAINTAINERS   |   7 +
 drivers/video/backlight/Kconfig   |  11 +
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/mp3309c.c | 443 ++
 5 files changed, 466 insertions(+), 6 deletions(-)
 create mode 100644 drivers/video/backlight/mp3309c.c

-- 
2.34.1

[PATCH v6 1/2] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

The two properties:

- max-brightness
- default brightness

are not really required, so they can be removed from the "required"
section.
The "max-brightness" is no longer used in the current version
of the driver (it was used only in the first version).
The "default-brightness", if omitted in the DT, is managed by the
device driver, using a default value. This value depends on the dimming
mode used:

- for the "analog mode", via I2C commands, this value is fixed by
  hardware (=31)
- while in case of pwm mode the default used is the last value of the
  brightness-levels array.

Also the brightness-levels array is not required:

- in "analog mode", via I2C commands, the brightness-level array is
  fixed by hardware (0..31).;
- in pwm dimming mode, the driver uses a default array of 0..255 and
  the "default-brightness" is the last one, which is "255"

NOTE: there are no compatibility problems with the previous version,
  since the device driver has not yet been included in any kernel.
  Only this dt-binding yaml file is already included in the kernel
  repository.
  No developer may have used it.

Other changes:

- improve the backlight working mode description, in the "description"
  section
- update the example, removing the "max-brightness" and introducing the
  "brightess-levels" property

Signed-off-by: Flavio Suligoi 
---
 .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
index 4191e33626f5..527a37368ed7 100644
--- a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -14,8 +14,8 @@ description: |
   programmable switching frequency to optimize efficiency.
   It supports two different dimming modes:
 
-  - analog mode, via I2C commands (default)
-  - PWM controlled mode.
+  - analog mode, via I2C commands, as default mode (32 dimming levels)
+  - PWM controlled mode (optional)
 
   The datasheet is available at:
   https://www.monolithicpower.com/en/mp3309c.html
@@ -50,8 +50,6 @@ properties:
 required:
   - compatible
   - reg
-  - max-brightness
-  - default-brightness
 
 unevaluatedProperties: false
 
@@ -66,8 +64,8 @@ examples:
 compatible = "mps,mp3309c";
 reg = <0x17>;
 pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
-max-brightness = <100>;
-default-brightness = <80>;
+brightness-levels = <0 4 8 16 32 64 128 255>;
+default-brightness = <6>;
 mps,overvoltage-protection-microvolt = <2400>;
 };
 };
-- 
2.34.1

RE: [PATCH v2 1/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

Hi Daniel,

> On Wed, Oct 25, 2023 at 05:50:57PM +0200, Flavio Suligoi wrote:
> > NOTE: there are no compatibility problems with the previous version,
> >   since the device driver has not yet been included in any kernel.
> >   Only this dt-binding yaml file is already included in the
> >   "for-backlight-next" branch of the "backlight" kernel repository.
> >   No developer may have used it.
> 
> I'm afraid I got confused by the fragmented MP3309C patches from all the
> different patchsets.
> 
> Please can you rebase whatever is left on v6.7-rc1 and send a single patchset
> with all pending changes as a single patch set.
> 

No problem, I'll do it!

> 
> Thanks
> 
> Daniel.

Regards,
Flavio

[PATCH v2 1/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

The two properties:

- max-brightness
- default brightness

are not really required, so they can be removed from the "required"
section.
The "max-brightness" is no longer used in the current version
of the driver (it was used only in the first version).
The "default-brightness", if omitted in the DT, is managed by the
device driver, using a default value. This value depends on the dimming
mode used:

- for the "analog mode", via I2C commands, this value is fixed by
  hardware (=31)
- while in case of pwm mode the default used is the last value of the
  brightness-levels array.

Also the brightness-levels array is not required:

- in "analog mode", via I2C commands, the brightness-level array is
  fixed by hardware (0..31).;
- in pwm dimming mode, the driver uses a default array of 0..255 and
  the "default-brightness" is the last one, which is "255"

NOTE: there are no compatibility problems with the previous version,
  since the device driver has not yet been included in any kernel.
  Only this dt-binding yaml file is already included in the
  "for-backlight-next" branch of the "backlight" kernel repository.
  No developer may have used it.

Other changes:

- improve the backlight working mode description, in the "description"
  section
- update the example, removing the "max-brightness" and introducing the
  "brightess-levels" property

Signed-off-by: Flavio Suligoi 
---
 .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
index 4191e33626f5..527a37368ed7 100644
--- a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -14,8 +14,8 @@ description: |
   programmable switching frequency to optimize efficiency.
   It supports two different dimming modes:
 
-  - analog mode, via I2C commands (default)
-  - PWM controlled mode.
+  - analog mode, via I2C commands, as default mode (32 dimming levels)
+  - PWM controlled mode (optional)
 
   The datasheet is available at:
   https://www.monolithicpower.com/en/mp3309c.html
@@ -50,8 +50,6 @@ properties:
 required:
   - compatible
   - reg
-  - max-brightness
-  - default-brightness
 
 unevaluatedProperties: false
 
@@ -66,8 +64,8 @@ examples:
 compatible = "mps,mp3309c";
 reg = <0x17>;
 pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
-max-brightness = <100>;
-default-brightness = <80>;
+brightness-levels = <0 4 8 16 32 64 128 255>;
+default-brightness = <6>;
 mps,overvoltage-protection-microvolt = <2400>;
 };
 };
-- 
2.34.1

[PATCH v2 0/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

This patch remove the following two not-required properties from the
"required" section:

- max-brightness
- default brightness

These properties are not really required, so they can be removed from the
"required" section.
The "max-brightness" is no longer used in the current version
of the driver (it was used only in the first version).
The "default-brightness", if omitted in the DT, is managed by the
device driver, using a default value. This value depends on the dimming
mode used:

- for the "analog mode", via I2C commands, this value is fixed by
  hardware (=31);
- while in case of pwm mode the default used is the last value of the
  brightness-levels array.

Also the brightness-levels array is not required:

- in "analog mode", via I2C commands, the brightness-level array is
  fixed by hardware (0..31);
- in pwm dimming mode, the driver uses a default array of 0..255 and
  the "default-brightness" is the last one, which is "255".

NOTE: there are no compatibility problems with the previous version,
  since the device driver has not yet been included in any kernel.
  Only this dt-binding yaml file is already included in the
  "for-backlight-next" branch of the "backlight" kernel repository.
  No developer may have used it.

Other changes:

- improve the backlight working mode descripion in the "description"
  section
- update the example, removing the "max-brightness" and introducing the
  "brightess-levels" property

NOTE: the "brightess-levels" property is present in the last version of the
  common.yaml file, so it is not decalared here.
  For this last version of common.yaml file, see my patch:
      
[1/1] dt-bindings: backlight: add brightness-levels related common
 properties
commit: d5272d39995f4150062a67e6f2cef556edece740

Flavio Suligoi (1):
  dt-bindings: backlight: mp3309c: remove two required properties

 .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.34.1

RE: [PATCH 1/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

Hi Conor,

...

> > > > > > The two properties:
> > > > > >
> > > > > > - max-brightness
> > > > > > - default brightness
> > > > > >
> > > > > > are not really required, so they can be removed from the "required"
> > > > > > section.
> > > > >
> > > > > Why are they not required? You need to provide an explanation.
> > > >
> > > > The "max-brightness" is not more used now in the driver (I used it
> > > > in the first version of the driver).
> > >
> > > If it is not used any more, what happens when someone passes an old
> > > devicetree to the kernel, that contains max-brightness, but not any
> > > of your new properties?
> >
> > This is not a problem, because the device driver has not yet been included 
> > in
> any kernel.
> > My patch for the device driver is still being analyzed by the maintainers.
> > Only this dt-binding yaml file is already included in the
> > "for-backlight-next" branch of the "backlight" kernel repository.
> > At the moment, this driver is used only in a i.MX8MM board produced in
> > my company, under my full control. No other developer is using it now.
> 
> Right. This is exactly the sort of commentary that you need to provide up
> front, to have us spent a bunch of time going back and forth to figure out :(

I'm sorry for wasting your time, I'll add this information in the next commit.

> 
> > > > The "default-brightness", if omitted in the DT, is managed by the
> > > > device driver, using a default value. This depends on the dimming
> > > > mode
> > > used:
> > >
> > > For default-brightness, has here always been support in the driver
> > > for the property being omitted, or is this newly added?
> >
> > In the first version of the driver this property was a "required
> > property", but nobody has used this driver before, so this should be not a
> problem.
> 
> > > What I would like is an explanation in the commit message as to why
> > > the revised example is more helpful than the existing (and
> > > must-remain-valid) one.
> >
> > As said before, no one may have ever used this device driver, so I
> > would leave only this new version of the example.
> 
> Okay. Please improve the commit message explaining why it is okay to make
> these changes & send a v2.
> The alternative is that Lee drops the dt-binding patch & you submit a revised
> version of the binding alongside the next iteration of the driver.

Ok, I'll send a new commit v2, explaining the reasons for the changes.

> 
> Cheers,
> Conor.

Thank you Conor,
Flavio.

RE: [PATCH 1/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

Hi Conor,

...

> On Mon, Oct 23, 2023 at 09:28:03AM +0000, Flavio Suligoi wrote:
> > > On Fri, Oct 20, 2023 at 03:54:33PM +0200, Flavio Suligoi wrote:
> > > > The two properties:
> > > >
> > > > - max-brightness
> > > > - default brightness
> > > >
> > > > are not really required, so they can be removed from the "required"
> > > > section.
> > >
> > > Why are they not required? You need to provide an explanation.
> >
> > The "max-brightness" is not more used now in the driver (I used it in
> > the first version of the driver).
> 
> If it is not used any more, what happens when someone passes an old
> devicetree to the kernel, that contains max-brightness, but not any of your
> new properties?

This is not a problem, because the device driver has not yet been included in 
any kernel.
My patch for the device driver is still being analyzed by the maintainers.
Only this dt-binding yaml file is already included in the "for-backlight-next" 
branch
of the "backlight" kernel repository.
At the moment, this driver is used only in a i.MX8MM board produced in my 
company,
under my full control. No other developer is using it now.

> > The "default-brightness", if omitted in the DT, is managed by the
> > device driver, using a default value. This depends on the dimming mode
> used:
> 
> For default-brightness, has here always been support in the driver for the
> property being omitted, or is this newly added?

In the first version of the driver this property was a "required property",
but nobody has used this driver before, so this should be not a problem.

> 
> > - for the "analog mode", via I2C commands, this value is fixed by
> > hardware (=31)
> > - while in case of pwm mode the default used is the last value of the
> >   brightness-levels array.
> >
> > Also the brightness-levels array is not required; if it is omitted,
> > the driver uses a default array of 0..255 and the "default-brightness" is 
> > the
> last one, which is "255".
> 
> Firstly, this is the sort of rationale that needs to be put into your commit
> messages, rather than bullet pointed lists of what you have done.

You are absolutely right, I'll include these details in the next commit message.

> 
> Secondly, what about other operating systems etc, do any of those support
> this platform and depend on presence of these properties?

I used this backlight driver in our i.MX8MM board only, with Linux only.

> 
> >
> > > > Other changes:
> > > >
> > > > - improve the backlight working mode description, in the "description"
> > > >   section
> > >
> > > > - update the example, removing the "max-brightness" and introducing
> the
> > > >   "brightess-levels" property
> > >
> > > Why is this more useful?
> >
> > I introduced the "brightness-levels" instead of "max-brightness" for
> > homogeneity, since the "analog mode" dimming has a brightness-levels array
> fixed by hardware (0..31).
> > In this way also the "pwm" mode can use the same concepts of array of
> levels.
> 
> What I would like is an explanation in the commit message as to why the
> revised example is more helpful than the existing (and
> must-remain-valid) one.

As said before, no one may have ever used this device driver,
so I would leave only this new version of the example.

> 
> Cheers,
> Conor.

Thanks for your help and best regards,
Flavio.

> 
> > >
> > > >
> > > > Signed-off-by: Flavio Suligoi 
> > > > ---
> > > >  .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
> > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git
> > > a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > > b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > > > index 4191e33626f5..527a37368ed7 100644
> > > > ---
> > > a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > > > +++
> > > b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > > > @@ -14,8 +14,8 @@ description: |
> > > >programmable switching frequency to optimize efficiency.
> > > >It supports two different dimming modes:
> > > >
> > > > -  - analog mode, via I2C commands (default)
> > > > -  - PWM controlled mode.
> &

RE: [PATCH 1/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

Hi Conor,

...

> On Fri, Oct 20, 2023 at 03:54:33PM +0200, Flavio Suligoi wrote:
> > The two properties:
> >
> > - max-brightness
> > - default brightness
> >
> > are not really required, so they can be removed from the "required"
> > section.
> 
> Why are they not required? You need to provide an explanation.

The "max-brightness" is not more used now in the driver (I used it in the first 
version
of the driver).
The "default-brightness", if omitted in the DT, is managed by the device driver,
using a default value. This depends on the dimming mode used:

- for the "analog mode", via I2C commands, this value is fixed by hardware (=31)
- while in case of pwm mode the default used is the last value of the 
  brightness-levels array.

Also the brightness-levels array is not required; if it is omitted, the driver 
uses 
a default array of 0..255 and the "default-brightness" is the last one, which 
is "255".

> > Other changes:
> >
> > - improve the backlight working mode description, in the "description"
> >   section
> 
> > - update the example, removing the "max-brightness" and introducing the
> >   "brightess-levels" property
> 
> Why is this more useful?

I introduced the "brightness-levels" instead of "max-brightness" for 
homogeneity,
since the "analog mode" dimming has a brightness-levels array fixed by hardware 
(0..31).
In this way also the "pwm" mode can use the same concepts of array of levels.

> 
> Cheers,
> Conor.

Thanks for your help.
Flavio.

> 
> >
> > Signed-off-by: Flavio Suligoi 
> > ---
> >  .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git
> a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > index 4191e33626f5..527a37368ed7 100644
> > ---
> a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > +++
> b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > @@ -14,8 +14,8 @@ description: |
> >programmable switching frequency to optimize efficiency.
> >It supports two different dimming modes:
> >
> > -  - analog mode, via I2C commands (default)
> > -  - PWM controlled mode.
> > +  - analog mode, via I2C commands, as default mode (32 dimming levels)
> > +  - PWM controlled mode (optional)
> >
> >The datasheet is available at:
> >https://www.monolithicpower.com/en/mp3309c.html
> > @@ -50,8 +50,6 @@ properties:
> >  required:
> >- compatible
> >- reg
> > -  - max-brightness
> > -  - default-brightness
> >
> >  unevaluatedProperties: false
> >
> > @@ -66,8 +64,8 @@ examples:
> >  compatible = "mps,mp3309c";
> >  reg = <0x17>;
> >  pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
> > -max-brightness = <100>;
> > -default-brightness = <80>;
> > +brightness-levels = <0 4 8 16 32 64 128 255>;
> > +default-brightness = <6>;
> >  mps,overvoltage-protection-microvolt = <2400>;
> >  };
> >  };
> > --
> > 2.34.1
> >

RE: [PATCH v5] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

Hi Conor,

...

> Subject: Re: [PATCH v5] backlight: mp3309c: Add support for MPS MP3309C
> 
> On Fri, Oct 20, 2023 at 03:54:34PM +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 
> > ---
> >
> > v5:
> >  - checked and resend for updated kernel 6.6.0-rc1
> 
> Why is this v5 patch attached to a 1 patch "series" that only purports to
> contain a binding patch?

Sorry Conor, what do you mean for "binding patch"? The cover letter of this 
patch or the
other patch about the dt-bindings for the mp3309c backlight ?

> Thanks,
> Conor.

Thanks,
Flavio.


> 
> > v4:
> >  - add brightness-levels property
> >  - force fixed 32 brightness levels (0..31) in analog-i2c dimming mode
> >  - remove useless irq and reset_gpio from mp3309c_chip structure
> > 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   |   7 +
> >  drivers/video/backlight/Kconfig   |  11 +
> >  drivers/video/backlight/Makefile  |   1 +
> >  drivers/video/backlight/mp3309c.c | 443
> > ++
> >  4 files changed, 462 insertions(+)
> >  create mode 100644 drivers/video/backlight/mp3309c.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > 90f13281d297..3d23b869e2aa 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14474,6 +14474,13 @@ S: Maintained
> >  F: Documentation/driver-api/tty/moxa-smartio.rst
> >  F: drivers/tty/mxser.*
> >
> > +MP3309C BACKLIGHT DRIVER
> > +M: Flavio Suligoi 
> > +L: dri-devel@lists.freedesktop.org
> > +S: Maintained
> > +F:
>   Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.ya
> ml
> > +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

[PATCH 1/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

The two properties:

- max-brightness
- default brightness

are not really required, so they can be removed from the "required"
section.

Other changes:

- improve the backlight working mode description, in the "description"
  section
- update the example, removing the "max-brightness" and introducing the
  "brightess-levels" property

Signed-off-by: Flavio Suligoi 
---
 .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
index 4191e33626f5..527a37368ed7 100644
--- a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -14,8 +14,8 @@ description: |
   programmable switching frequency to optimize efficiency.
   It supports two different dimming modes:
 
-  - analog mode, via I2C commands (default)
-  - PWM controlled mode.
+  - analog mode, via I2C commands, as default mode (32 dimming levels)
+  - PWM controlled mode (optional)
 
   The datasheet is available at:
   https://www.monolithicpower.com/en/mp3309c.html
@@ -50,8 +50,6 @@ properties:
 required:
   - compatible
   - reg
-  - max-brightness
-  - default-brightness
 
 unevaluatedProperties: false
 
@@ -66,8 +64,8 @@ examples:
 compatible = "mps,mp3309c";
 reg = <0x17>;
 pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
-max-brightness = <100>;
-default-brightness = <80>;
+brightness-levels = <0 4 8 16 32 64 128 255>;
+default-brightness = <6>;
 mps,overvoltage-protection-microvolt = <2400>;
 };
 };
-- 
2.34.1

[PATCH v5] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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 
---

v5:
 - checked and resend for updated kernel 6.6.0-rc1
v4:
 - add brightness-levels property
 - force fixed 32 brightness levels (0..31) in analog-i2c dimming mode
 - remove useless irq and reset_gpio from mp3309c_chip structure
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   |   7 +
 drivers/video/backlight/Kconfig   |  11 +
 drivers/video/backlight/Makefile  |   1 +
 drivers/video/backlight/mp3309c.c | 443 ++
 4 files changed, 462 insertions(+)
 create mode 100644 drivers/video/backlight/mp3309c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 90f13281d297..3d23b869e2aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14474,6 +14474,13 @@ S: Maintained
 F: Documentation/driver-api/tty/moxa-smartio.rst
 F: drivers/tty/mxser.*
 
+MP3309C BACKLIGHT DRIVER
+M:     Flavio Suligoi 
+L: dri-devel@lists.freedesktop.org
+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 ..3fe4469ef43f
--- /dev/null
+++ b/drivers/video/backlight/mp3309c.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * This driver support both analog (by I2C commands) and PWM dimming control
+ * modes.
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi 
+ *
+ * Based on pwm_bl.c
+ */
+
+#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_

[PATCH 0/1] dt-bindings: backlight: mp3309c: remove two required properties

[Flavio Suligoi]

This patch remove the following two not-required properties from the
"required" section:

- max-brightness
- default brightness

Other changes:

- improve the backlight working mode descripion in the "description"
  section
- update the example, removing the "max-brightness" and introducing the
  "brightess-levels" property

Note: the "brightess-levels" property is present in the last version of the
  common.yaml file, so it is not decalared here.
  For this last version see my patch:
  
[1/1] dt-bindings: backlight: add brightness-levels related common
 properties
commit: d5272d39995f4150062a67e6f2cef556edece740

Flavio Suligoi (1):
  dt-bindings: backlight: mp3309c: remove two required properties

 .../bindings/leds/backlight/mps,mp3309c.yaml   | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH v1] dt-bindings: backlight: add brightness-levels related common properties

[Flavio Suligoi]

Both files pwm-backlight.yaml and led-backlight.yaml contain properties
in common with each other, regarding the brightness levels:

- brightness-levels
- default-brightness-level

These properties can then be moved to backlight/common.yaml.

Signed-off-by: Flavio Suligoi 
---
 .../bindings/leds/backlight/common.yaml   | 17 
 .../leds/backlight/led-backlight.yaml | 19 --
 .../leds/backlight/pwm-backlight.yaml | 20 ---
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/backlight/common.yaml 
b/Documentation/devicetree/bindings/leds/backlight/common.yaml
index 3b60afbab68b..e0983e44934c 100644
--- a/Documentation/devicetree/bindings/leds/backlight/common.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/common.yaml
@@ -33,4 +33,21 @@ properties:
   due to restrictions in a specific system, such as mounting conditions.
 $ref: /schemas/types.yaml#/definitions/uint32
 
+  brightness-levels:
+description:
+  Array of distinct brightness levels. The levels must be in the range
+  accepted by the underlying LED device. Typically these are in the range
+  from 0 to 255, but any range starting at 0 will do, as long as they are
+  accepted by the LED.
+  The 0 value means a 0% of brightness (darkest/off), while the last value
+  in the array represents a full 100% brightness (brightest).
+  If this array is not provided, the driver default mapping is used.
+$ref: /schemas/types.yaml#/definitions/uint32-array
+
+  default-brightness-level:
+description:
+  The default brightness level (index into the array defined by the
+  "brightness-levels" property).
+$ref: /schemas/types.yaml#/definitions/uint32
+
 additionalProperties: true
diff --git 
a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml 
b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
index d7b78198abc2..f5554da6bc6c 100644
--- a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
@@ -16,6 +16,9 @@ description:
   can also be used to describe a backlight device controlled by the output of
   a LED driver.
 
+allOf:
+  - $ref: common.yaml#
+
 properties:
   compatible:
 const: led-backlight
@@ -26,25 +29,11 @@ properties:
 items:
   maxItems: 1
 
-  brightness-levels:
-description:
-  Array of distinct brightness levels. The levels must be in the range
-  accepted by the underlying LED devices. This is used to translate a
-  backlight brightness level into a LED brightness level. If it is not
-  provided, the identity mapping is used.
-$ref: /schemas/types.yaml#/definitions/uint32-array
-
-  default-brightness-level:
-description:
-  The default brightness level (index into the array defined by the
-  "brightness-levels" property).
-$ref: /schemas/types.yaml#/definitions/uint32
-
 required:
   - compatible
   - leds
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
diff --git 
a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml 
b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
index 535690288990..b71f6454a4ac 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
@@ -11,6 +11,9 @@ maintainers:
   - Daniel Thompson 
   - Jingoo Han 
 
+allOf:
+  - $ref: common.yaml#
+
 properties:
   compatible:
 const: pwm-backlight
@@ -39,21 +42,6 @@ properties:
   Delay in ms between disabling the backlight using GPIO and setting PWM
   value to 0.
 
-  brightness-levels:
-description:
-  Array of distinct brightness levels. Typically these are in the range
-  from 0 to 255, but any range starting at 0 will do. The actual brightness
-  level (PWM duty cycle) will be interpolated from these values. 0 means a
-  0% duty cycle (darkest/off), while the last value in the array represents
-  a 100% duty cycle (brightest).
-$ref: /schemas/types.yaml#/definitions/uint32-array
-
-  default-brightness-level:
-description:
-  The default brightness level (index into the array defined by the
-  "brightness-levels" property).
-$ref: /schemas/types.yaml#/definitions/uint32
-
   num-interpolated-steps:
 description:
   Number of interpolated steps between each value of brightness-levels
@@ -69,7 +57,7 @@ required:
   - compatible
   - pwms
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.34.1

RE: [PATCH v4 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Rob,

just a question about led-backlight.yaml and pwm-backlight.yaml
common properties.

...

> > +
> > +  brightness-levels:
> > +description:
> > +  Array of distinct brightness levels, in PWM dimming mode.
> > +  Typically these are in the range from 0 to 255, but any range 
> > starting
> > +  at 0 will do.
> > +  The 0 value means a 0% duty cycle (darkest/off), while the last 
> > value in
> > +  the array represents a 100% duty cycle (brightest).
> > +$ref: /schemas/types.yaml#/definitions/uint32-array
> 
> This already has a type defined. Please add it to backlight/common.yaml and
> remove from led-backlight.yaml and pwm-backlight.yaml.

As well as the brightness-levels property, also the default-brightness-level is 
in common
between led-backlight.yaml and pwm-backlight.yaml.
What do you think about removing it from both led-backlight.yaml and 
pwm-backlight.yaml, and
moving it into backlight/common.yaml?

> 
> You say 0-255 here, but your example is 0-10. One of those seems wrong.
> Anyways, don't define constraints in prose, use a schema:
> 
> items:
>   maximum: 10 (or 255?)
> 
> > +
> > +  default-brightness:
> > +description:
> > +  The default brightness (index into the levels array).
> > +$ref: /schemas/types.yaml#/definitions/uint32
> 
> Already has a type. You need to reference backlight/common.yaml.
> 
> > +
> > +  mps,overvoltage-protection-microvolt:
> > +description: Overvoltage protection (13.5V, 24V or 35.5V).
> > +enum: [ 1350, 2400, 3550 ]
> > +default: 3550
> > +
> > +  mps,no-sync-mode:
> > +description: disable synchronous rectification mode
> > +type: boolean
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +i2c {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +/* Backlight with PWM control */
> > +backlight_pwm: backlight@17 {
> > +compatible = "mps,mp3309c";
> > +reg = <0x17>;
> > +pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
> > +brightness-levels = <0 1 2 3 4 5 6 7 8 9 10>;
> > +default-brightness = <8>;
> > +mps,overvoltage-protection-microvolt = <2400>;
> > +};
> > +};
> > --
> > 2.34.1
> >

Regards,
Flavio

RE: [PATCH v4 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Rob,

... 
> >  .../bindings/leds/backlight/mps,mp3309c.yaml  | 82
> > +++
> >  1 file changed, 82 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > new file mode 100644
> > index ..e2f9ae2b3fb4
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yam
> > +++ l
> > @@ -0,0 +1,82 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MPS MP3309C backlight
> > +
> > +maintainers:
> > +  - Flavio Suligoi 
> > +
> > +description: |
> > +  The Monolithic Power (MPS) MP3309C is a WLED step-up converter,
> > +featuring a
> > +  programmable switching frequency to optimize efficiency.
> > +  It supports two different dimming modes:
> > +
> > +  - analog mode, via I2C commands, as default mode (32 dimming
> > + levels)
> > +  - PWM controlled mode (optional)
> > +
> > +  The datasheet is available at:
> > +  https://www.monolithicpower.com/en/mp3309c.html
> > +
> > +properties:
> > +  compatible:
> > +const: mps,mp3309c
> > +
> > +  reg:
> > +maxItems: 1
> > +
> > +  pwms:
> > +description: if present, the backlight is controlled in PWM mode.
> > +maxItems: 1
> > +
> > +  enable-gpios:
> > +description: GPIO used to enable the backlight in "analog-i2c" dimming
> mode.
> > +maxItems: 1
> > +
> > +  brightness-levels:
> > +description:
> > +  Array of distinct brightness levels, in PWM dimming mode.
> > +  Typically these are in the range from 0 to 255, but any range 
> > starting
> > +  at 0 will do.
> > +  The 0 value means a 0% duty cycle (darkest/off), while the last 
> > value in
> > +  the array represents a 100% duty cycle (brightest).
> > +$ref: /schemas/types.yaml#/definitions/uint32-array
> 
> This already has a type defined. Please add it to backlight/common.yaml and
> remove from led-backlight.yaml and pwm-backlight.yaml.

Ok, I'll add common.yaml and I'll prepare two other separate patches for these.

> 
> You say 0-255 here, but your example is 0-10. One of those seems wrong.
> Anyways, don't define constraints in prose, use a schema:
> 
> items:
>   maximum: 10 (or 255?)

Ok, I'll add maximum, and I'll change the example, using 0..255

> 
> > +
> > +  default-brightness:
> > +description:
> > +  The default brightness (index into the levels array).
> > +$ref: /schemas/types.yaml#/definitions/uint32
> 
> Already has a type. You need to reference backlight/common.yaml.

Ok

> 
> > +
> > +  mps,overvoltage-protection-microvolt:
> > +description: Overvoltage protection (13.5V, 24V or 35.5V).
> > +enum: [ 1350, 2400, 3550 ]
> > +default: 3550
> > +
> > +  mps,no-sync-mode:
> > +description: disable synchronous rectification mode
> > +type: boolean
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +i2c {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +/* Backlight with PWM control */
> > +backlight_pwm: backlight@17 {
> > +compatible = "mps,mp3309c";
> > +reg = <0x17>;
> > +pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
> > +brightness-levels = <0 1 2 3 4 5 6 7 8 9 10>;
> > +default-brightness = <8>;
> > +mps,overvoltage-protection-microvolt = <2400>;
> > +};
> > +};
> > --
> > 2.34.1
> >

Thanks,
Flavio

[PATCH v4 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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 
---

v4:
 - add brightness-levels property
 - force fixed 32 brightness levels (0..31) in analog-i2c dimming mode
 - remove useless irq and reset_gpio from mp3309c_chip structure
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 | 443 ++
 4 files changed, 461 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 ..3fe4469ef43f
--- /dev/null
+++ b/drivers/video/backlight/mp3309c.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * This driver support both analog (by I2C commands) and PWM dimming control
+ * modes.
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi 
+ *
+ * Based on pwm_bl.c
+ */
+
+#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_I2C_NUM_LEVELS  32  /* 0..31 *

[PATCH v4 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

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 device driver details, please refer to:
- drivers/video/backlight/mp3309c_bl.c

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

Signed-off-by: Flavio Suligoi 
---

v4:
 - remove not more used allOf keyword
 - add brightness-levels and default-brightness properties
 - remove max-brightness and default-brightness from required properties
 - update example, adding brightness-levels and default-brightness properties
v3:
 - add default value for mps,overvoltage-protection-microvolt property
 - fix the example, changing from "mps,mp3309c-backlight" to "mps,mp3309c" in
   compatible property
v2:
 - remove useless properties (dimming-mode, pinctrl-names, pinctrl-0,
   switch-on-delay-ms, switch-off-delay-ms, reset-gpios, reset-on-delay-ms,
   reset-on-length-ms)
 - add common.yaml#
 - remove already included properties (default-brightness, max-brightness)
 - substitute three boolean properties, used for the overvoltage-protection
   values, with a single enum property
 - remove some conditional definitions
 - remove the 2nd example
v1:
 - first version

 .../bindings/leds/backlight/mps,mp3309c.yaml  | 82 +++
 1 file changed, 82 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
new file mode 100644
index ..e2f9ae2b3fb4
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MPS MP3309C backlight
+
+maintainers:
+  - Flavio Suligoi 
+
+description: |
+  The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a
+  programmable switching frequency to optimize efficiency.
+  It supports two different dimming modes:
+
+  - analog mode, via I2C commands, as default mode (32 dimming levels)
+  - PWM controlled mode (optional)
+
+  The datasheet is available at:
+  https://www.monolithicpower.com/en/mp3309c.html
+
+properties:
+  compatible:
+const: mps,mp3309c
+
+  reg:
+maxItems: 1
+
+  pwms:
+description: if present, the backlight is controlled in PWM mode.
+maxItems: 1
+
+  enable-gpios:
+description: GPIO used to enable the backlight in "analog-i2c" dimming 
mode.
+maxItems: 1
+
+  brightness-levels:
+description:
+  Array of distinct brightness levels, in PWM dimming mode.
+  Typically these are in the range from 0 to 255, but any range starting
+  at 0 will do.
+  The 0 value means a 0% duty cycle (darkest/off), while the last value in
+  the array represents a 100% duty cycle (brightest).
+$ref: /schemas/types.yaml#/definitions/uint32-array
+
+  default-brightness:
+description:
+  The default brightness (index into the levels array).
+$ref: /schemas/types.yaml#/definitions/uint32
+
+  mps,overvoltage-protection-microvolt:
+description: Overvoltage protection (13.5V, 24V or 35.5V).
+enum: [ 1350, 2400, 3550 ]
+default: 3550
+
+  mps,no-sync-mode:
+description: disable synchronous rectification mode
+type: boolean
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+/* Backlight with PWM control */
+backlight_pwm: backlight@17 {
+compatible = "mps,mp3309c";
+reg = <0x17>;
+pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
+brightness-levels = <0 1 2 3 4 5 6 7 8 9 10>;
+default-brightness = <8>;
+mps,overvoltage-protection-microvolt = <2400>;
+};
+};
-- 
2.34.1

RE: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

HI Daniel,

...
> > ...
> > > ...
> > > > > > > +required:
> > > > > > > +  - compatible
> > > > > > > +  - reg
> > > > > > > +  - max-brightness
> > > > > >
> > > > > > Why is this mandatory?
> > > > > >
> > > > > > There's no point in setting max-brightness when running in I2C
> > > > > > mode
> > > > > > (max- brightness should default to 31 in that case).
> > > > > >
> > > > > >
> > > > > > > +  - default-brightness
> > > > > >
> > > > > > Again. I'm not clear why this needs to be mandatory.
> > > > > >
> > > > > >
> > > > >
> > > > > Ok, you are right, I'll remove max-brightness and
> > > > > default-brightness from required properties list. I think to
> > > > > change these properties, for the pwm dimming, into a clearer:
> > > > >
> > > > > - brightness-levels (uint32)
> > > > > - default-brightness-levels (uint32).
> > > > >
> > > > > For example:
> > > > >
> > > > >   brightness-levels:
> > > > > description:
> > > > >   Number of brightness levels. The actual brightness
> > > > >   level (PWM duty cycle) will be interpolated from 0 to this 
> > > > > value.
> > > > >   0 means a  0% duty cycle (darkest/off), while the
> > > > > brightness-levels
> > > > represents
> > > > >   a 100% duty cycle (brightest).
> > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > >
> > > > >   default-brightness-level:
> > > > > description:
> > > > >   The default brightness level (from 0 to brightness-levels)
> > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > >
> > > > > Example:
> > > > > brightness-levels = <10>;
> > > > > default-brightness-level = <6>;
> > > > >
> > > > > What do you think about this solution?
> > > >
> > > > If you want to introduce a brightness-levels property then I would
> > > > expect it to be defined with the same meaning as pwm-backlight
> > > > (it's not relevant to the bindings but ideally it would be
> > > > implemented by refactoring and reusing the code from pwm_bl.c).
> > >
> > > ok, I'll use the brightness-levels property as used in pwm-backlight
> > >
> > > >
> > > > Same with default-brightness-level although I'm not sure why one
> > > > wouldn't just use default-brightness for new bindings (doesn't
> > > > default-brightness-level simply do exactly the same thing as
> > > > default-
> > > brightness).
> > >
> > > ok for default-brightness instead of default-brightness-level
> >
> > Just a question: default-brightness-level is the index into the brightness-
> levels array.
> > But, if I use default-brightness instead of default-brightness-level,
> > should I consider default-brightness also as an index into brightness-levels
> array?
> 
> Yes.
> 
> 
> > Or, in this case, have the default-brightness to be equal to one of
> > the values inside the brightness-levels array?
> 
> When there is a brightness array (and there is no interpolation) then it is
> indexed by brightness. The values in the array are not brightness (e.g. the
> controlable value describing the output of the hardware). The values in the
> table are merely the PWM duty cycle...

ok

> 
> Main difference is, with a correct table the brightness can use an appropriate
> logarithmic power scale (which matches how humans perceive
> brightness) instead of the linear scale provided by the PWM duty cycle.
> 
> 
> Daniel.
> 
> 
> Brightness and "index into the brightness-levels array" should be one and the
> same thing

ok, I'll use default-brightness, thanks for the explanations!

> >
> > >
> > > >
> > > >
> > > > Daniel.
> > >
> > > Thanks an best regards,
> > > Flavio
> >
> > Thanks,
> >
> > Flavio

Flavio

RE: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Daniel,
...
> ...
> > > > > +required:
> > > > > +  - compatible
> > > > > +  - reg
> > > > > +  - max-brightness
> > > >
> > > > Why is this mandatory?
> > > >
> > > > There's no point in setting max-brightness when running in I2C
> > > > mode
> > > > (max- brightness should default to 31 in that case).
> > > >
> > > >
> > > > > +  - default-brightness
> > > >
> > > > Again. I'm not clear why this needs to be mandatory.
> > > >
> > > >
> > >
> > > Ok, you are right, I'll remove max-brightness and default-brightness
> > > from required properties list. I think to change these properties,
> > > for the pwm dimming, into a clearer:
> > >
> > > - brightness-levels (uint32)
> > > - default-brightness-levels (uint32).
> > >
> > > For example:
> > >
> > >   brightness-levels:
> > > description:
> > >   Number of brightness levels. The actual brightness
> > >   level (PWM duty cycle) will be interpolated from 0 to this value.
> > >   0 means a  0% duty cycle (darkest/off), while the
> > > brightness-levels
> > represents
> > >   a 100% duty cycle (brightest).
> > > $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > >   default-brightness-level:
> > > description:
> > >   The default brightness level (from 0 to brightness-levels)
> > > $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > > Example:
> > > brightness-levels = <10>;
> > > default-brightness-level = <6>;
> > >
> > > What do you think about this solution?
> >
> > If you want to introduce a brightness-levels property then I would
> > expect it to be defined with the same meaning as pwm-backlight (it's
> > not relevant to the bindings but ideally it would be implemented by
> > refactoring and reusing the code from pwm_bl.c).
> 
> ok, I'll use the brightness-levels property as used in pwm-backlight
> 
> >
> > Same with default-brightness-level although I'm not sure why one
> > wouldn't just use default-brightness for new bindings (doesn't
> > default-brightness-level simply do exactly the same thing as default-
> brightness).
> 
> ok for default-brightness instead of default-brightness-level

Just a question: default-brightness-level is the index into the 
brightness-levels array.
But, if I use default-brightness instead of default-brightness-level,  
should I consider default-brightness also as an index into brightness-levels 
array?
Or, in this case, have the default-brightness to be equal to one of the values 
inside the
brightness-levels array?

> 
> >
> >
> > Daniel.
> 
> Thanks an best regards,
> Flavio

Thanks,

Flavio

RE: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Daniel,

...
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - max-brightness
> > >
> > > Why is this mandatory?
> > >
> > > There's no point in setting max-brightness when running in I2C mode
> > > (max- brightness should default to 31 in that case).
> > >
> > >
> > > > +  - default-brightness
> > >
> > > Again. I'm not clear why this needs to be mandatory.
> > >
> > >
> >
> > Ok, you are right, I'll remove max-brightness and default-brightness
> > from required properties list. I think to change these properties, for
> > the pwm dimming, into a clearer:
> >
> > - brightness-levels (uint32)
> > - default-brightness-levels (uint32).
> >
> > For example:
> >
> >   brightness-levels:
> > description:
> >   Number of brightness levels. The actual brightness
> >   level (PWM duty cycle) will be interpolated from 0 to this value.
> >   0 means a  0% duty cycle (darkest/off), while the brightness-levels
> represents
> >   a 100% duty cycle (brightest).
> > $ref: /schemas/types.yaml#/definitions/uint32
> >
> >   default-brightness-level:
> > description:
> >   The default brightness level (from 0 to brightness-levels)
> > $ref: /schemas/types.yaml#/definitions/uint32
> >
> > Example:
> > brightness-levels = <10>;
> > default-brightness-level = <6>;
> >
> > What do you think about this solution?
> 
> If you want to introduce a brightness-levels property then I would expect it 
> to
> be defined with the same meaning as pwm-backlight (it's not relevant to the
> bindings but ideally it would be implemented by refactoring and reusing the
> code from pwm_bl.c).

ok, I'll use the brightness-levels property as used in pwm-backlight

> 
> Same with default-brightness-level although I'm not sure why one wouldn't
> just use default-brightness for new bindings (doesn't default-brightness-level
> simply do exactly the same thing as default-brightness).

ok for default-brightness instead of default-brightness-level

> 
> 
> Daniel.

Thanks an best regards,
Flavio

RE: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Daniel,

...

> > +required:
> > +  - compatible
> > +  - reg
> > +  - max-brightness
> 
> Why is this mandatory?
> 
> There's no point in setting max-brightness when running in I2C mode (max-
> brightness should default to 31 in that case).
> 
> 
> > +  - default-brightness
> 
> Again. I'm not clear why this needs to be mandatory.
> 
> 

Ok, you are right, I'll remove max-brightness and default-brightness from 
required properties list.
I think to change these properties, for the pwm dimming, into a clearer:

- brightness-levels (uint32) 
- default-brightness-levels (uint32).

For example:

  brightness-levels:
description:
  Number of brightness levels. The actual brightness
  level (PWM duty cycle) will be interpolated from 0 to this value.
  0 means a  0% duty cycle (darkest/off), while the brightness-levels 
represents
  a 100% duty cycle (brightest).
$ref: /schemas/types.yaml#/definitions/uint32

  default-brightness-level:
description:
  The default brightness level (from 0 to brightness-levels)
$ref: /schemas/types.yaml#/definitions/uint32

Example:
brightness-levels = <10>;
default-brightness-level = <6>;

What do you think about this solution?

> Daniel.

Thanks for your help,
Flavio

RE: [PATCH v3 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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

[PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

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 device driver details, please refer to:
- drivers/video/backlight/mp3309c_bl.c

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

Signed-off-by: Flavio Suligoi 
Reviewed-by: Rob Herring 
---

v3:
 - add default value for mps,overvoltage-protection-microvolt property
 - fix the example, changing from "mps,mp3309c-backlight" to "mps,mp3309c" in
   compatible property
v2:
 - remove useless properties (dimming-mode, pinctrl-names, pinctrl-0,
   switch-on-delay-ms, switch-off-delay-ms, reset-gpios, reset-on-delay-ms,
   reset-on-length-ms)
 - add common.yaml#
 - remove already included properties (default-brightness, max-brightness)
 - substitute three boolean properties, used for the overvoltage-protection
   values, with a single enum property
 - remove some conditional definitions
 - remove the 2nd example
v1:
 - first version

 .../bindings/leds/backlight/mps,mp3309c.yaml  | 73 +++
 1 file changed, 73 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
new file mode 100644
index ..4191e33626f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MPS MP3309C backlight
+
+maintainers:
+  - Flavio Suligoi 
+
+description: |
+  The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a
+  programmable switching frequency to optimize efficiency.
+  It supports two different dimming modes:
+
+  - analog mode, via I2C commands (default)
+  - PWM controlled mode.
+
+  The datasheet is available at:
+  https://www.monolithicpower.com/en/mp3309c.html
+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+const: mps,mp3309c
+
+  reg:
+maxItems: 1
+
+  pwms:
+description: if present, the backlight is controlled in PWM mode.
+maxItems: 1
+
+  enable-gpios:
+description: GPIO used to enable the backlight in "analog-i2c" dimming 
mode.
+maxItems: 1
+
+  mps,overvoltage-protection-microvolt:
+description: Overvoltage protection (13.5V, 24V or 35.5V).
+enum: [ 1350, 2400, 3550 ]
+default: 3550
+
+  mps,no-sync-mode:
+description: disable synchronous rectification mode
+type: boolean
+
+required:
+  - compatible
+  - reg
+  - max-brightness
+  - default-brightness
+
+unevaluatedProperties: false
+
+examples:
+  - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+/* Backlight with PWM control */
+backlight_pwm: backlight@17 {
+compatible = "mps,mp3309c";
+reg = <0x17>;
+pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
+max-brightness = <100>;
+default-brightness = <80>;
+mps,overvoltage-protection-microvolt = <2400>;
+};
+};
-- 
2.34.1

[PATCH v3 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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;
+   

RE: [PATCH v2 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

HI Rob,

...

> Subject: Re: [PATCH v2 1/2] dt-bindings: backlight: Add MPS MP3309C
> 

...

> > +
> > +  mps,overvoltage-protection-microvolt:
> > +description: Overvoltage protection (13.5V, 24V or 35.5V). If missing, 
> > the
> > +  hardware default of 35.5V is used.
> 
> default: 3550
> 
> instead of prose saying the same thing.
> 
> With that,
> 
> Reviewed-by: Rob Herring 

Ok, right, I'll fix it in my next version.

I prefer to wait a few days before sending the next version,
to wait if there are other comments on the device driver file as well.

Just a question about the procedure: in my next version, I have to add the 
sentence:

"Reviewed-by: Rob Herring "

in my commit or it will be added again by you?

Thanks for your help,
Flavio 

RE: [PATCH v2 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Conor,

...


> On Fri, Sep 15, 2023 at 04:05:15PM +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 device driver details, please refer to:
> > - drivers/video/backlight/mp3309c_bl.c
> >
> > The datasheet is available at:
> > - https://www.monolithicpower.com/en/mp3309c.html
> >
> > Signed-off-by: Flavio Suligoi 
> > ---
> >
> > v2:
> >  - remove useless properties (dimming-mode, pinctrl-names, pinctrl-0,
> >switch-on-delay-ms, switch-off-delay-ms, reset-gpios, reset-on-delay-ms,
> >reset-on-length-ms)
> >  - add common.yaml#
> >  - remove already included properties (default-brightness,
> > max-brightness)
> >  - substitute three boolean properties, used for the overvoltage-protection
> >values, with a single enum property
> >  - remove some conditional definitions
> >  - remove the 2nd example
> > v1:
> >  - first version
> >
> >  .../bindings/leds/backlight/mps,mp3309c.yaml  | 73
> > +++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > new file mode 100644
> > index ..99ccdba2c08f
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yam
> > +++ l
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MPS MP3309C backlight
> > +
> > +maintainers:
> > +  - Flavio Suligoi 

...

> > +  mps,overvoltage-protection-microvolt:
> > +description: Overvoltage protection (13.5V, 24V or 35.5V). If missing, 
> > the
> > +  hardware default of 35.5V is used.
> > +enum: [ 1350, 2400, 3550 ]
> You can add "default: 3550" and drop the free form default as text in the
> description.

Ok, thanks.

> 
> Cheers,
> Conor.
> 
> > +
> > +  mps,no-sync-mode:
> > +description: disable synchronous rectification mode
> > +type: boolean
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - max-brightness
> > +  - default-brightness
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +i2c {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +/* Backlight with PWM control */
> > +backlight_pwm: backlight@17 {
> > +compatible = "mps,mp3309c-backlight";
> 
> As the bot pointed out, the compatible doesn't contain "backlight".

Right, fixed!

Thanks and regards,
Flavio

[PATCH v2 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

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 device driver details, please refer to:
- drivers/video/backlight/mp3309c_bl.c

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

Signed-off-by: Flavio Suligoi 
---

v2:
 - remove useless properties (dimming-mode, pinctrl-names, pinctrl-0,
   switch-on-delay-ms, switch-off-delay-ms, reset-gpios, reset-on-delay-ms,
   reset-on-length-ms)
 - add common.yaml#
 - remove already included properties (default-brightness, max-brightness)
 - substitute three boolean properties, used for the overvoltage-protection
   values, with a single enum property
 - remove some conditional definitions
 - remove the 2nd example
v1:
 - first version

 .../bindings/leds/backlight/mps,mp3309c.yaml  | 73 +++
 1 file changed, 73 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
new file mode 100644
index ..99ccdba2c08f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MPS MP3309C backlight
+
+maintainers:
+  - Flavio Suligoi 
+
+description: |
+  The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a
+  programmable switching frequency to optimize efficiency.
+  It supports two different dimming modes:
+
+  - analog mode, via I2C commands (default)
+  - PWM controlled mode.
+
+  The datasheet is available at:
+  https://www.monolithicpower.com/en/mp3309c.html
+
+allOf:
+  - $ref: common.yaml#
+
+properties:
+  compatible:
+const: mps,mp3309c
+
+  reg:
+maxItems: 1
+
+  pwms:
+description: if present, the backlight is controlled in PWM mode.
+maxItems: 1
+
+  enable-gpios:
+description: GPIO used to enable the backlight in "analog-i2c" dimming 
mode.
+maxItems: 1
+
+  mps,overvoltage-protection-microvolt:
+description: Overvoltage protection (13.5V, 24V or 35.5V). If missing, the
+  hardware default of 35.5V is used.
+enum: [ 1350, 2400, 3550 ]
+
+  mps,no-sync-mode:
+description: disable synchronous rectification mode
+type: boolean
+
+required:
+  - compatible
+  - reg
+  - max-brightness
+  - default-brightness
+
+unevaluatedProperties: false
+
+examples:
+  - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+/* Backlight with PWM control */
+backlight_pwm: backlight@17 {
+compatible = "mps,mp3309c-backlight";
+reg = <0x17>;
+pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
+max-brightness = <100>;
+default-brightness = <80>;
+overvoltage-protection-microvolt = <2400>;
+};
+};
-- 
2.34.1

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

[Flavio Suligoi]

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 
---

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 
+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 ..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 
+ */
+
+#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  dimming_mode;
+   u8  over_voltage_protection;
+   bool sync_mode;
+   u8 status;
+};
+
+struct mp3309c_chip {
+   struct device *dev

RE: [PATCH v1 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Krzysztof,

...

> >>> +
> >>> +  reg:
> >>> +maxItems: 1
> >>> +
> >>> +  mps,dimming-mode:
> >>> +description: The dimming mode (PWM or analog by I2C commands).
> >>> +$ref: '/schemas/types.yaml#/definitions/string'
> >>
> >> Drop quotes, you should see warnings for this.
> >>
> >> It does not look like you tested the bindings, at least after quick
> >> look. Please run `make dt_binding_check` (see
> >> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> >> Maybe you need to update your dtschema and yamllint.
> >>
> >>> +enum:
> >>> +  - pwm
> >>> +  - analog-i2c
> >>
> >> Why do you think this is a property of a board? Is PWM signal optional?
> >> If so, its presence would define it. Otherwise it seems you want to
> >> control the driver.
> >>
> >
> > The MP3309C device always need a I2C bus to rd/wr its internal registers.
> > But the brightness can be controlled in one of the following ways
> > (mutually exclusive, but mandatory):
> > - a PWM input signal
> > or
> > - a I2C command
> > So, the driver needs a property to select the dimming mode used; this
> property is mandatory.
> 
> No, it's not a proof. Don't mix properties and hardware signals.
> 
> > This is the reason of the existence of the ' mps,dimming-mode' property.
> > PWM signal is not optional, it is required if and only if the 'pwm' dimming
> mode is used.
> 
> So the pwms determine the mode. That's it, no need for this property.
> 
> 
> > If the 'analog-i2c' dimming mode is used, instead, the PWM signal must not
> be used.
> > So the property 'mps,dimming-mode' controls how the MP3309C is used.
> > I can add more details about this in the description section.
> 
> No, drop the property or explain more, e.g. is I2C mode of control used while
> having PWMs signals connected?

Ok, I think I understand what you mean:
since the I2C bus is always present to configure the chip, the 'analog-i2c'
dimming mode is the default mode.
The other type of dimming mode, the 'pwm' dimming mode, is an option.
When the pwm is present in the DT, the pwm signal is used as dimming control 
mode
instead of the i2c dimming control mode.
In this way the ' mps,dimming-mode' property is not more necessary and can be 
eliminated.

> 
> > ...
> >
> >>> +
> >>> +  mps,overvoltage-protection-13v:
> >>> +description: overvoltage protection set to 13.5V.
> >>> +type: boolean
> >>> +  mps,overvoltage-protection-24v:
> >>> +description: overvoltage protection set to 24V.
> >>> +type: boolean
> >>> +  mps,overvoltage-protection-35v:
> >>> +description: overvoltage protection set to 35.5V.
> >>> +type: boolean
> >>
> >> Nope for these three. Use -microvolt suffix for one property.
> >
> > Ok
> >
> >>
> >>> +
> >>> +  mps,reset-gpios:
> >>> +description: optional GPIO to reset an external device (LCD panel,
> FPGA,
> >>> +  etc.) when the backlight is switched on.
> >>> +maxItems: 1
> >>
> >> No, you should not add here GPIOs for other devices.
> >
> > Do you mean that I have to remove this property or that I have to move it
> somewhere else?
> > I added this feature because sometimes, in embedded boards, you need a
> > pulse signal to
> 
> How you described it, this is not the property of this device.
> 
> > use after the backlight probing, for example to reset another device
> > in sync with the backlight probe.
> 
> There is no term as "probe" in hardware, so you describe drivers.
> 
> > Do you think I have to remove this feature from the driver?
> 
> You cannot request GPIO after removing it from the bindings, obviously, but
> whether your backlight should reset something else? Don't care, don't know. I
> talk about bindings.

This GPIO was used in one of our boards to solve a sync problem, but it is no 
more
necessary. I'll eliminate it, thanks.

> 
> >
> > ...
> >
> >>> +allOf:
> >>> +  - $ref: common.yaml#
> >>> +  - if:
> >>> +  properties:
> >>> +mps,dimming-mode:
> >>> +  contains:
> >>> +enum:
> >>> +  - pwm
> >>> +then:
> >>> +  required:
> >>> +- pwms
> >>
> >> So this proves the point - mps,dimming-mode looks redundant and not
> >> hardware related.
> >
> > See my previous comment.
> 
> No, it still proves the point till you explain why pwms cannot be used to
> determine this. Read my messages.
> 
> Best regards,
> Krzysztof

Thanks for your help!
Flavio

RE: [PATCH v1 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

HI Daniel,

> -Original Message-
> From: Daniel Thompson 
> Sent: Tuesday, August 29, 2023 6:28 PM
> To: Flavio Suligoi 
> Cc: Lee Jones ; Jingoo Han ; Helge
> Deller ; Pavel Machek ; Rob Herring
> ; Krzysztof Kozlowski
> ; Conor Dooley ;
> dri-devel@lists.freedesktop.org; linux-l...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-fb...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v1 2/2] backlight: mp3309c: Add support for MPS
> MP3309C
> 
> On Tue, Aug 29, 2023 at 12:15:46PM +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 
> 
> > diff --git a/drivers/video/backlight/Kconfig
> > b/drivers/video/backlight/Kconfig index 51387b1ef012..65d0ac9f611d
> > 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -389,6 +389,19 @@ config BACKLIGHT_LM3639
> > help
> >   This supports TI LM3639 Backlight + 1.5A Flash LED Driver
> >
> > +config BACKLIGHT_MP3309C
> > +   tristate "Backlight Driver for MPS MP3309C"
> > +   depends on I2C
> > +   select REGMAP_I2C
> > +   select NEW_LEDS
> > +   select LEDS_CLASS
> 
> This doesn't seem right.
> 
> Shouldn't PWM and GPIOLIB be listed here? Why are NEW_LEDS and
> LEDS_CLASS needed?

ok, I'll fix it

> 
> > +   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_bl.
> > +
> >  config BACKLIGHT_LP855X
> > tristate "Backlight driver for TI LP855X"
> > depends on I2C && PWM
> 
> > +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_init_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;
> > +   } else {
> > +   /*
> > +* Analog dimming mode by I2C commands
> > +*
> > +* 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) {
> > +   switch (chip->pdata->status) {
> > +   case FIRST_POWER_ON:
> > +   /*
> > +* Only for the first time, wait for the optional
> > +* switch-on delay and then enable the device.
> > +* Otherwise enable the backlight immediately.
> > +*/
> > +   schedule_delayed_work(>enable_work,
> > + 

RE: [PATCH v1 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

Hi Krzysztof,

Thanks for your quick replay and corrections!
Just some questions about some of your remarks:

> > @@ -0,0 +1,202 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> 
> > +
> > +  reg:
> > +maxItems: 1
> > +
> > +  mps,dimming-mode:
> > +description: The dimming mode (PWM or analog by I2C commands).
> > +$ref: '/schemas/types.yaml#/definitions/string'
> 
> Drop quotes, you should see warnings for this.
> 
> It does not look like you tested the bindings, at least after quick look. 
> Please
> run `make dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.
> 
> > +enum:
> > +  - pwm
> > +  - analog-i2c
> 
> Why do you think this is a property of a board? Is PWM signal optional?
> If so, its presence would define it. Otherwise it seems you want to control 
> the
> driver.
> 

The MP3309C device always need a I2C bus to rd/wr its internal registers.
But the brightness can be controlled in one of the following ways (mutually 
exclusive,
but mandatory):
- a PWM input signal
or
- a I2C command
So, the driver needs a property to select the dimming mode used; this property 
is mandatory.
This is the reason of the existence of the ' mps,dimming-mode' property.
PWM signal is not optional, it is required if and only if the 'pwm' dimming 
mode is used.
If the 'analog-i2c' dimming mode is used, instead, the PWM signal must not be 
used.
So the property 'mps,dimming-mode' controls how the MP3309C is used.
I can add more details about this in the description section.
...
 
> > +
> > +  mps,overvoltage-protection-13v:
> > +description: overvoltage protection set to 13.5V.
> > +type: boolean
> > +  mps,overvoltage-protection-24v:
> > +description: overvoltage protection set to 24V.
> > +type: boolean
> > +  mps,overvoltage-protection-35v:
> > +description: overvoltage protection set to 35.5V.
> > +type: boolean
> 
> Nope for these three. Use -microvolt suffix for one property.

Ok

> 
> > +
> > +  mps,reset-gpios:
> > +description: optional GPIO to reset an external device (LCD panel, 
> > FPGA,
> > +  etc.) when the backlight is switched on.
> > +maxItems: 1
> 
> No, you should not add here GPIOs for other devices.

Do you mean that I have to remove this property or that I have to move it 
somewhere else?
I added this feature because sometimes, in embedded boards, you need a pulse 
signal to
use after the backlight probing, for example to reset another device in sync 
with the backlight
probe.
Do you think I have to remove this feature from the driver?

...

> > +allOf:
> > +  - $ref: common.yaml#
> > +  - if:
> > +  properties:
> > +mps,dimming-mode:
> > +  contains:
> > +enum:
> > +  - pwm
> > +then:
> > +  required:
> > +- pwms
> 
> So this proves the point - mps,dimming-mode looks redundant and not
> hardware related.

See my previous comment.

> 
> > +  not:
> > +required:
> > +  - enable-gpios
> > +
> > +  - if:
> > +  properties:
> > +mps,dimming-mode:
> > +  contains:
> > +enum:
> > +  - analog-i2c
> > +then:
> > +  required:
> > +- enable-gpios
> > +  not:
> > +required:
> > +  - pwms
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - mps,dimming-mode
> > +  - max-brightness
> > +  - default-brightness
> > +
> > +additionalProperties: false
> 
> Instead:
> unevaluatedProperties: false
> 

Ok

> > +
> > +examples:
> > +  - |
> > +#include 
> > +i2c3 {
> 
> i2c
> 
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +clock-frequency = <10>;
> 
> Drop
> 
> > +pinctrl-names = "default";
> > +pinctrl-0 = <_i2c3>;
> > +status = "okay";
> 
> Drop all except of cells.

Ok

> 
> > +
> > +/* Backlight with PWM control */
> > +backlight_pwm: backlight@17 {
> > +compatible = "mps,mp3309c-backlight";
> > +reg = <0x17>;
> > +mps,dimming-mode = "pwm";
> > +pinctrl-names = "default";
> > +pinctrl-0 = <_fpga_reset>;
> > +pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
> > +max-brightness = <100>;
> > +default-brightness = <80>;
> > +mps,switch-on-delay-ms = <800>;
> > +mps,switch-off-delay-ms = <10>;
> > +mps,overvoltage-protection-24v;
> > +
> > +/*
> > + * Enable an FPGA reset pulse when MIPI data are stable,
> > + * before switch on the backlight
> > + */
> > +mps,reset-gpios = < 20 GPIO_ACTIVE_HIGH>;
> 
> Nope, nope. FPGA reset pin is not related to this device.

See my previous comment/question about this feature.

> 
> > +mps,reset-on-delay-ms = <100>;
> > +  

[PATCH v1 2/2] backlight: mp3309c: Add support for MPS MP3309C

[Flavio Suligoi]

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 
---
 MAINTAINERS  |   6 +
 drivers/video/backlight/Kconfig  |  13 +
 drivers/video/backlight/Makefile |   1 +
 drivers/video/backlight/mp3309c_bl.c | 491 +++
 4 files changed, 511 insertions(+)
 create mode 100644 drivers/video/backlight/mp3309c_bl.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3be1bdfe8ecc..895c56ff4f1e 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_bl.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..65d0ac9f611d 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -389,6 +389,19 @@ config BACKLIGHT_LM3639
help
  This supports TI LM3639 Backlight + 1.5A Flash LED Driver
 
+config BACKLIGHT_MP3309C
+   tristate "Backlight Driver for MPS MP3309C"
+   depends on I2C
+   select REGMAP_I2C
+   select NEW_LEDS
+   select LEDS_CLASS
+   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_bl.
+
 config BACKLIGHT_LP855X
tristate "Backlight driver for TI LP855X"
depends on I2C && PWM
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index f72e1c3c59e9..c42c5bccc5ac 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_bl.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_bl.c 
b/drivers/video/backlight/mp3309c_bl.c
new file mode 100644
index ..7cb7a542ceca
--- /dev/null
+++ b/drivers/video/backlight/mp3309c_bl.c
@@ -0,0 +1,491 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for MPS MP3309C White LED driver with I2C interface
+ *
+ * Copyright (C) 2023 ASEM Srl
+ * Author: Flavio Suligoi 
+ */
+
+#include 
+#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 backlight_status {
+   FIRST_POWER_ON,
+   BACKLIGHT_OFF,
+   BACKLIGHT_ON,
+};
+
+enum dimming_mode_value {
+   DIMMING_PWM,
+   DIMMING_ANALOG_I2C,
+};
+
+struct mp3309c_platform_data {
+   u32 max_brightness;
+   u32 brightness;
+   u32 switch_on_delay_ms;
+   u32 switch_off_delay_ms;
+   u32 reset_on_delay_ms;
+   u32 reset_on_length_ms;
+   u8  dimming_mode;
+   u8  reset_pulse_enable;
+   u8  over_voltage_protection;
+
+   unsigned int 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;
+
+   struct delayed_work enable_work;
+   struct delayed_work reset_gpio_work;
+   int irq;
+
+   struct gpio_desc *reset_gpio;
+};
+
+static const struct regmap_config mp3309c_regmap = {
+   .name = &

[PATCH v1 1/2] dt-bindings: backlight: Add MPS MP3309C

[Flavio Suligoi]

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 device driver details, please refer to:
- drivers/video/backlight/mp3309c_bl.c

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

Signed-off-by: Flavio Suligoi 
---
 .../bindings/leds/backlight/mps,mp3309c.yaml  | 202 ++
 1 file changed, 202 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml 
b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
new file mode 100644
index ..a58904f2a271
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
@@ -0,0 +1,202 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MPS MP3309C backlight
+
+maintainers:
+  - Flavio Suligoi 
+
+description: |
+  The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a
+  programmable switching frequency to optimize efficiency.
+  It supports both analog (via I2C commands) and PWM dimming mode.
+
+  The datasheet is available at:
+  https://www.monolithicpower.com/en/mp3309c.html
+
+properties:
+  compatible:
+const: mps,mp3309c-backlight
+
+  reg:
+maxItems: 1
+
+  mps,dimming-mode:
+description: The dimming mode (PWM or analog by I2C commands).
+$ref: '/schemas/types.yaml#/definitions/string'
+enum:
+  - pwm
+  - analog-i2c
+
+  pinctrl-names:
+items:
+  - const: default
+
+  pinctrl-0: true
+
+  pwms:
+description: PWM channel used for controlling the backlight in "pwm" 
dimming
+  mode.
+maxItems: 1
+
+  default-brightness:
+minimum: 0
+
+  max-brightness:
+minimum: 1
+
+  enable-gpios:
+description: GPIO used to enable the backlight in "analog-i2c" dimming 
mode.
+maxItems: 1
+
+  mps,switch-on-delay-ms:
+description: delay (in ms) before switch on the backlight, to wait for 
image
+  stabilization.
+default: 10
+
+  mps,switch-off-delay-ms:
+description: delay (in ms) after the switch off command to the backlight.
+default: 0
+
+  mps,overvoltage-protection-13v:
+description: overvoltage protection set to 13.5V.
+type: boolean
+  mps,overvoltage-protection-24v:
+description: overvoltage protection set to 24V.
+type: boolean
+  mps,overvoltage-protection-35v:
+description: overvoltage protection set to 35.5V.
+type: boolean
+
+  mps,reset-gpios:
+description: optional GPIO to reset an external device (LCD panel, FPGA,
+  etc.) when the backlight is switched on.
+maxItems: 1
+
+  mps,reset-on-delay-ms:
+description: delay (in s) before generating the reset-gpios.
+default: 10
+
+  mps,reset-on-length-ms:
+description: pulse length (in ms) for reset-gpios.
+default: 10
+
+oneOf:
+  - required:
+  - mps,overvoltage-protection-13v
+  - required:
+  - mps,overvoltage-protection-24v
+  - required:
+  - mps,overvoltage-protection-35.5v
+
+allOf:
+  - $ref: common.yaml#
+  - if:
+  properties:
+mps,dimming-mode:
+  contains:
+enum:
+  - pwm
+then:
+  required:
+- pwms
+  not:
+required:
+  - enable-gpios
+
+  - if:
+  properties:
+mps,dimming-mode:
+  contains:
+enum:
+  - analog-i2c
+then:
+  required:
+- enable-gpios
+  not:
+required:
+  - pwms
+
+required:
+  - compatible
+  - reg
+  - mps,dimming-mode
+  - max-brightness
+  - default-brightness
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c3 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+clock-frequency = <10>;
+pinctrl-names = "default";
+pinctrl-0 = <_i2c3>;
+status = "okay";
+
+/* Backlight with PWM control */
+backlight_pwm: backlight@17 {
+compatible = "mps,mp3309c-backlight";
+reg = <0x17>;
+mps,dimming-mode = "pwm";
+pinctrl-names = "default";
+pinctrl-0 = <_fpga_reset>;
+pwms = < 0 333 0>; /* 300 Hz --> (1/f) * 1*10^9 */
+max-brightness = <100>;
+default-brightness = <80>;
+mps,switch-on-delay-ms = <800>;
+mps,switch-off-delay-ms = <10>;
+mps,overvoltage-protection-24v;
+
+/*
+ * Enab

[PATCH v2] i2c: imx: fix typo in comment

[Flavio Suligoi]

to provid --> to provide

Signed-off-by: Flavio Suligoi 
---
v2:
 - fix typo in subject

 drivers/i2c/busses/i2c-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index e9e2db68b9fb..78fb1a4274a6 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -66,7 +66,7 @@
 
 /* IMX I2C registers:
  * the I2C register offset is different between SoCs,
- * to provid support for all these chips, split the
+ * to provide support for all these chips, split the
  * register offset into a fixed base address and a
  * variable shift value, then the full register offset
  * will be calculated by
-- 
2.25.1

[PATCH v1] drm/i915: Fix spelling mistake in i915_reg.h

[Flavio Suligoi]

Fix typo: "TRIGER" --> "TRIGGER"

The two misplelled macros:

1) OAREPORTTRIG1_EDGE_LEVEL_TRIGER_SELECT_MASK
2) OAREPORTTRIG5_EDGE_LEVEL_TRIGER_SELECT_MASK

are not used in any other sources of the kernel,
so this change can be consider only a local change
for the i915_reg.h file.

Signed-off-by: Flavio Suligoi 
Reviewed-by: Chris Wilson 
---
v1: add "Reviewed-by: Chris Wilson "

 drivers/gpu/drm/i915/i915_reg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9d6536afc94b..c2153364724a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -868,7 +868,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 
 #define OAREPORTTRIG1 _MMIO(0x2740)
 #define OAREPORTTRIG1_THRESHOLD_MASK 0x
-#define OAREPORTTRIG1_EDGE_LEVEL_TRIGER_SELECT_MASK 0x /* 0=level */
+#define OAREPORTTRIG1_EDGE_LEVEL_TRIGGER_SELECT_MASK 0x /* 0=level */
 
 #define OAREPORTTRIG2 _MMIO(0x2744)
 #define OAREPORTTRIG2_INVERT_A_0  (1 << 0)
@@ -921,7 +921,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 
 #define OAREPORTTRIG5 _MMIO(0x2750)
 #define OAREPORTTRIG5_THRESHOLD_MASK 0x
-#define OAREPORTTRIG5_EDGE_LEVEL_TRIGER_SELECT_MASK 0x /* 0=level */
+#define OAREPORTTRIG5_EDGE_LEVEL_TRIGGER_SELECT_MASK 0x /* 0=level */
 
 #define OAREPORTTRIG6 _MMIO(0x2754)
 #define OAREPORTTRIG6_INVERT_A_0  (1 << 0)
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: Fix spelling mistake in i915_reg.h

[Flavio Suligoi]

Fix typo: "TRIGER" --> "TRIGGER"

The two misplelled macros:

1) OAREPORTTRIG1_EDGE_LEVEL_TRIGER_SELECT_MASK
2) OAREPORTTRIG5_EDGE_LEVEL_TRIGER_SELECT_MASK

are not used in any other sources of the kernel,
so this change can be consider only a local change
for the i915_reg.h file.

Signed-off-by: Flavio Suligoi 
---
 drivers/gpu/drm/i915/i915_reg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9d6536afc94b..c2153364724a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -868,7 +868,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 
 #define OAREPORTTRIG1 _MMIO(0x2740)
 #define OAREPORTTRIG1_THRESHOLD_MASK 0x
-#define OAREPORTTRIG1_EDGE_LEVEL_TRIGER_SELECT_MASK 0x /* 0=level */
+#define OAREPORTTRIG1_EDGE_LEVEL_TRIGGER_SELECT_MASK 0x /* 0=level */
 
 #define OAREPORTTRIG2 _MMIO(0x2744)
 #define OAREPORTTRIG2_INVERT_A_0  (1 << 0)
@@ -921,7 +921,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 
 #define OAREPORTTRIG5 _MMIO(0x2750)
 #define OAREPORTTRIG5_THRESHOLD_MASK 0x
-#define OAREPORTTRIG5_EDGE_LEVEL_TRIGER_SELECT_MASK 0x /* 0=level */
+#define OAREPORTTRIG5_EDGE_LEVEL_TRIGGER_SELECT_MASK 0x /* 0=level */
 
 #define OAREPORTTRIG6 _MMIO(0x2754)
 #define OAREPORTTRIG6_INVERT_A_0  (1 << 0)
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel