[PATCH 0/2] backlight: mp3309c: Drop pwm_apply_args()

[PATCH 0/2] backlight: mp3309c: Drop pwm_apply_args()

[Uwe Kleine-König]

Hello,

the first patch of this series is what I really care about: There are
hardly any drivers left that use pwm_apply_args(). When all of them are
converted to not use it any more, I intend to drop that function.

The 2nd patch is just a change that I noticed while editing the driver
that is IMHO nice. If you don't agree and only apply the first patch, I
won't argue. It's an alternative approach to what Daniel Thompson did in
commit 7ee6478d5aa9 ("backlight: mp3309c: Fully initialize
backlight_properties during probe").

Best regards
Uwe

Uwe Kleine-König (2):
  backlight: mp3309c: Drop pwm_apply_args()
  backlight: mp3309c: Initialize backlight properties without memset

 drivers/video/backlight/mp3309c.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)


base-commit: 1343433ed38923a21425c602e92120a1f1db5f7a
-- 
2.49.0

[PATCH 1/2] backlight: mp3309c: Drop pwm_apply_args()

[Uwe Kleine-König]

pwm_apply_args() sole purpose is to initialize all parameters specified
in the device tree for consumers that rely on pwm_config() and
pwm_enable(). The mp3309c backlight driver uses pwm_apply_might_sleep()
which gets passed the full configuration and so doesn't rely on the
default being explicitly applied.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/video/backlight/mp3309c.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/backlight/mp3309c.c b/drivers/video/backlight/mp3309c.c
index 372058e26129..bb4e85531cea 100644
--- a/drivers/video/backlight/mp3309c.c
+++ b/drivers/video/backlight/mp3309c.c
@@ -222,7 +222,6 @@ static int mp3309c_parse_fwnode(struct mp3309c_chip *chip,
 		if (IS_ERR(chip->pwmd))
 			return dev_err_probe(dev, PTR_ERR(chip->pwmd), "error getting pwm data\n");
 		pdata->dimming_mode = DIMMING_PWM;
-		pwm_apply_args(chip->pwmd);
 	}
 
 	/*
-- 
2.49.0

RE: EXTERNAL: [PATCH 1/2] backlight: mp3309c: Drop pwm_apply_args()

[FLAVIO SULIGOI]

Hi Uwe,

I've tested your patch on my board, all ok.


> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> Sent: martedì 1 luglio 2025 11:23
> To: FLAVIO SULIGOI <f.suligoi@asem.it>; Lee Jones <lee@kernel.org>;
> Daniel Thompson <danielt@kernel.org>; Jingoo Han <jingoohan1@gmail.com>;
> Helge Deller <deller@gmx.de>
> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> pwm@vger.kernel.org
> Subject: EXTERNAL: [PATCH 1/2] backlight: mp3309c: Drop pwm_apply_args()
> 
> [Use caution with links & attachments]
> 
> 
> 
> pwm_apply_args() sole purpose is to initialize all parameters specified
> in the device tree for consumers that rely on pwm_config() and
> pwm_enable(). The mp3309c backlight driver uses pwm_apply_might_sleep()
> which gets passed the full configuration and so doesn't rely on the
> default being explicitly applied.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Tested-by: Flavio Suligoi <f.suligoi@asem.it>

Re: [PATCH 1/2] backlight: mp3309c: Drop pwm_apply_args()

[Daniel Thompson]

On Tue, Jul 01, 2025 at 11:22:36AM +0200, Uwe Kleine-König wrote:
> pwm_apply_args() sole purpose is to initialize all parameters specified
> in the device tree for consumers that rely on pwm_config() and
> pwm_enable(). The mp3309c backlight driver uses pwm_apply_might_sleep()
> which gets passed the full configuration and so doesn't rely on the
> default being explicitly applied.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>

Sorry for the delay. I lost track of the review on this one so Lee didn't
hoover it up!


Daniel.

[PATCH 2/2] backlight: mp3309c: Initialize backlight properties without memset

[Uwe Kleine-König]

Assigning values to a struct using a compound literal (since C99) also
guarantees that all unspecified struct members are empty-initialized, so
it properly replaces the memset to zero.

The code looks a bit nicer and more idiomatic (though that might be
subjective?). The resulting binary is a bit smaller. On ARCH=arm with
an allnoconfig + minimal changes to enable the mp3309c driver the
difference is 12 bytes.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/video/backlight/mp3309c.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/mp3309c.c b/drivers/video/backlight/mp3309c.c
index bb4e85531cea..9337110ce6e5 100644
--- a/drivers/video/backlight/mp3309c.c
+++ b/drivers/video/backlight/mp3309c.c
@@ -352,12 +352,13 @@ static int mp3309c_probe(struct i2c_client *client)
 	chip->pdata = pdata;
 
 	/* Backlight properties */
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.brightness = pdata->default_brightness;
-	props.max_brightness = pdata->max_brightness;
-	props.scale = BACKLIGHT_SCALE_LINEAR;
-	props.type = BACKLIGHT_RAW;
-	props.power = BACKLIGHT_POWER_ON;
+	props = (typeof(props)){
+		.brightness = pdata->default_brightness,
+		.max_brightness = pdata->max_brightness,
+		.scale = BACKLIGHT_SCALE_LINEAR,
+		.type = BACKLIGHT_RAW,
+		.power = BACKLIGHT_POWER_ON,
+	};
 	chip->bl = devm_backlight_device_register(dev, "mp3309c", dev, chip,
 						  &mp3309c_bl_ops, &props);
 	if (IS_ERR(chip->bl))
-- 
2.49.0

RE: EXTERNAL: [PATCH 2/2] backlight: mp3309c: Initialize backlight properties without memset

[FLAVIO SULIGOI]

Hi Uwe,

I've tested your patch on my board, all ok.

> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> Sent: martedì 1 luglio 2025 11:23
> To: FLAVIO SULIGOI <f.suligoi@asem.it>; Lee Jones <lee@kernel.org>;
> Daniel Thompson <danielt@kernel.org>; Jingoo Han <jingoohan1@gmail.com>;
> Helge Deller <deller@gmx.de>
> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> pwm@vger.kernel.org
> Subject: EXTERNAL: [PATCH 2/2] backlight: mp3309c: Initialize backlight
> properties without memset
> 
> [Use caution with links & attachments]
> 
> 
> 
> Assigning values to a struct using a compound literal (since C99) also
> guarantees that all unspecified struct members are empty-initialized, so
> it properly replaces the memset to zero.
> 
> The code looks a bit nicer and more idiomatic (though that might be
> subjective?). The resulting binary is a bit smaller. On ARCH=arm with an
> allnoconfig + minimal changes to enable the mp3309c driver the
> difference is 12 bytes.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Tested-by: Flavio Suligoi <f.suligoi@asem.it>

Re: [PATCH 2/2] backlight: mp3309c: Initialize backlight properties without memset

[Daniel Thompson]

On Tue, Jul 01, 2025 at 11:22:37AM +0200, Uwe Kleine-König wrote:
> Assigning values to a struct using a compound literal (since C99) also
> guarantees that all unspecified struct members are empty-initialized, so
> it properly replaces the memset to zero.
>
> The code looks a bit nicer and more idiomatic (though that might be
> subjective?). The resulting binary is a bit smaller. On ARCH=arm with
> an allnoconfig + minimal changes to enable the mp3309c driver the
> difference is 12 bytes.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>


Daniel.

Re: [PATCH 2/2] backlight: mp3309c: Initialize backlight properties without memset

[Lee Jones]

On Mon, 18 Aug 2025, Daniel Thompson wrote:

> On Tue, Jul 01, 2025 at 11:22:37AM +0200, Uwe Kleine-König wrote:
> > Assigning values to a struct using a compound literal (since C99) also
> > guarantees that all unspecified struct members are empty-initialized, so
> > it properly replaces the memset to zero.
> >
> > The code looks a bit nicer and more idiomatic (though that might be
> > subjective?). The resulting binary is a bit smaller. On ARCH=arm with
> > an allnoconfig + minimal changes to enable the mp3309c driver the
> > difference is 12 bytes.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> 
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>

Looks like you cannot send tags from non-related email accounts:

NOTE: some trailers ignored due to from/email mismatches:
    ! Trailer: Reviewed-by: "Daniel Thompson (RISCstar)" <danielt@kernel.org>
     Msg From: Daniel Thompson <daniel@riscstar.com>

I'll add the tags manually this time.

-- 
Lee Jones [李琼斯]

RE: EXTERNAL: [PATCH 0/2] backlight: mp3309c: Drop pwm_apply_args()

[FLAVIO SULIGOI]

Hi Uwe,

> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> Sent: martedì 1 luglio 2025 11:23
> To: FLAVIO SULIGOI <f.suligoi@asem.it>; Lee Jones <lee@kernel.org>;
> Daniel Thompson <danielt@kernel.org>; Jingoo Han <jingoohan1@gmail.com>;
> Helge Deller <deller@gmx.de>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>; dri-
> devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> pwm@vger.kernel.org
> Subject: EXTERNAL: [PATCH 0/2] backlight: mp3309c: Drop pwm_apply_args()
> 
> [Use caution with links & attachments]
> 
> 
> 
> Hello,
> 
> the first patch of this series is what I really care about: There are
> hardly any drivers left that use pwm_apply_args(). When all of them are
> converted to not use it any more, I intend to drop that function.
> 
> The 2nd patch is just a change that I noticed while editing the driver
> that is IMHO nice. If you don't agree and only apply the first patch, I
> won't argue. It's an alternative approach to what Daniel Thompson did in
> commit 7ee6478d5aa9 ("backlight: mp3309c: Fully initialize
> backlight_properties during probe").

I've tested your patches on my board and all is ok.
Thanks!

> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (2):
>   backlight: mp3309c: Drop pwm_apply_args()
>   backlight: mp3309c: Initialize backlight properties without memset
> 
>  drivers/video/backlight/mp3309c.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> 
> base-commit: 1343433ed38923a21425c602e92120a1f1db5f7a
> --
> 2.49.0


Flavio

Re: EXTERNAL: [PATCH 0/2] backlight: mp3309c: Drop pwm_apply_args()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

[Updating Daniel's email address as the linaro one stopped working]

Hello,

On Mon, Jul 07, 2025 at 03:44:25PM +0000, FLAVIO SULIGOI wrote:
> > the first patch of this series is what I really care about: There are
> > hardly any drivers left that use pwm_apply_args(). When all of them are
> > converted to not use it any more, I intend to drop that function.
> > 
> > The 2nd patch is just a change that I noticed while editing the driver
> > that is IMHO nice. If you don't agree and only apply the first patch, I
> > won't argue. It's an alternative approach to what Daniel Thompson did in
> > commit 7ee6478d5aa9 ("backlight: mp3309c: Fully initialize
> > backlight_properties during probe").
> 
> I've tested your patches on my board and all is ok.

@Flavio:
A Tested-by in this reply to the cover letter is understood by b4 (which
is the tool most maintainers use to apply patches from the mailing
list). So there wouldn't have been a need to reply to each mail
individually.

@backlight maintainers:
This patch didn't make it into next yet, I guess it's too late for
6.17-rc1 now?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: EXTERNAL: [PATCH 0/2] backlight: mp3309c: Drop pwm_apply_args()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

Hello,

On Tue, Jul 29, 2025 at 10:17:20PM +0200, Uwe Kleine-König wrote:
> [Updating Daniel's email address as the linaro one stopped working]
> 
> Hello,
> 
> On Mon, Jul 07, 2025 at 03:44:25PM +0000, FLAVIO SULIGOI wrote:
> > > the first patch of this series is what I really care about: There are
> > > hardly any drivers left that use pwm_apply_args(). When all of them are
> > > converted to not use it any more, I intend to drop that function.
> > > 
> > > The 2nd patch is just a change that I noticed while editing the driver
> > > that is IMHO nice. If you don't agree and only apply the first patch, I
> > > won't argue. It's an alternative approach to what Daniel Thompson did in
> > > commit 7ee6478d5aa9 ("backlight: mp3309c: Fully initialize
> > > backlight_properties during probe").
> > 
> > I've tested your patches on my board and all is ok.
> 
> @Flavio:
> A Tested-by in this reply to the cover letter is understood by b4 (which
> is the tool most maintainers use to apply patches from the mailing
> list). So there wouldn't have been a need to reply to each mail
> individually.
> 
> @backlight maintainers:
> This patch didn't make it into next yet, I guess it's too late for
> 6.17-rc1 now?

Confirmed, it didn't make it into 6.17-rc1. In next (and also v6.17-rc1)
we have:

$ git grep pwm_apply_args next/master
next/master:drivers/video/backlight/mp3309c.c:          pwm_apply_args(chip->pwmd);
next/master:include/linux/pwm.h:static inline void pwm_apply_args(struct pwm_device *pwm)
next/master:include/linux/pwm.h:         * PWM users calling pwm_apply_args() expect to have a fresh config
next/master:include/linux/pwm.h:         * pwm_apply_args().

so this patch series is the single stopper before I can remove
pwm_apply_args().

Can we please get this into next for 6.18-rc1?

Looking at
https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git I see
that the branches merged regularily into next (for-backlight-next +
for-backlight-fixes) still points to 6.16-rc1 and 6.10-rc1 respectively.
If that means I should better take this series (or at least the first
patch) through my tree, please tell me.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]