[PATCH 0/3] pwm: Drop useless member "pwm" from struct pwm_device

[PATCH 0/3] pwm: Drop useless member "pwm" from struct pwm_device

[Uwe Kleine-König]

Hello,

there are only two users of struct pwm_device::pwm in the tree; both use
it for some dev_dbg output. While this number allows to identify the
PWM, it's not trivial, for example the data currently available in
/sys/kernel/debug/pwm isn't enough. (You have to look in /sys/class/pwm,
pick the pwmchip with the highest number that isn't bigger than the
PWM's number.)

To be honest the label isn't always usefull either, but it's easy to use
and should be enough to identify the used PWM. The parent device + hwid
might be more useful?! On the other hand using that for a dev_dbg that
is probably only looked at by someone debugging the driver and thus
knowing the used PWM anyhow is of little value either.

Assuming this change is still considered worthwile I suggest that patches #1
and #2 go in via their respective maintainer trees and I resend patch #3 to go
via the pwm tree once these two are "in".

Best regards
Uwe

Uwe Kleine-König (3):
  drm/ssd130x: Print the PWM's label instead of its number
  video: fbdev: ssd1307fb: Print the PWM's label instead of its number
  pwm: Drop unused member "pwm" from struct pwm_device

 drivers/gpu/drm/solomon/ssd130x.c | 4 ++--
 drivers/pwm/core.c                | 1 -
 drivers/video/fbdev/ssd1307fb.c   | 4 ++--
 include/linux/pwm.h               | 1 -
 4 files changed, 4 insertions(+), 6 deletions(-)

base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
-- 
2.39.2

[PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Uwe Kleine-König]

struct pwm_device::pwm is a write-only variable in the pwm core and used
nowhere apart from this and another dev_dbg. So it isn't useful to
identify the used PWM. Emit the PWM's label instead in the debug
message.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/ssd1307fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 11c373798279..46881a691549 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -399,8 +399,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		/* Enable the PWM */
 		pwm_enable(par->pwm);
 
-		dev_dbg(&par->client->dev, "Using PWM%d with a %lluns period.\n",
-			par->pwm->pwm, pwm_get_period(par->pwm));
+		dev_dbg(&par->client->dev, "Using PWM %s with a %lluns period.\n",
+			par->pwm->label, pwm_get_period(par->pwm));
 	}
 
 	/* Set initial contrast */
-- 
2.39.2

Re: [PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Javier Martinez Canillas]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> struct pwm_device::pwm is a write-only variable in the pwm core and used
> nowhere apart from this and another dev_dbg. So it isn't useful to
> identify the used PWM. Emit the PWM's label instead in the debug
> message.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Helge Deller]

On 7/28/23 16:58, Uwe Kleine-König wrote:
> struct pwm_device::pwm is a write-only variable in the pwm core and used
> nowhere apart from this and another dev_dbg. So it isn't useful to
> identify the used PWM. Emit the PWM's label instead in the debug
> message.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

applied.

Thanks!
Helge

> ---
>   drivers/video/fbdev/ssd1307fb.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 11c373798279..46881a691549 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -399,8 +399,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>   		/* Enable the PWM */
>   		pwm_enable(par->pwm);
>
> -		dev_dbg(&par->client->dev, "Using PWM%d with a %lluns period.\n",
> -			par->pwm->pwm, pwm_get_period(par->pwm));
> +		dev_dbg(&par->client->dev, "Using PWM %s with a %lluns period.\n",
> +			par->pwm->label, pwm_get_period(par->pwm));
>   	}
>
>   	/* Set initial contrast */

Re: [PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Andy Shevchenko]

On Fri, Jul 28, 2023 at 04:58:23PM +0200, Uwe Kleine-König wrote:
> struct pwm_device::pwm is a write-only variable in the pwm core and used
> nowhere apart from this and another dev_dbg. So it isn't useful to
> identify the used PWM. Emit the PWM's label instead in the debug
> message.

Do we have firmware node of PWM available? I would print it rather than some
(possibly non-unique) string.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Javier Martinez Canillas]

Javier Martinez Canillas <javierm@redhat.com> writes:

> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>
>> struct pwm_device::pwm is a write-only variable in the pwm core and used
>> nowhere apart from this and another dev_dbg. So it isn't useful to
>> identify the used PWM. Emit the PWM's label instead in the debug
>> message.
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>

Pushed to drm-misc (drm-misc-next). Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Uwe Kleine-König]

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

Hello,

On Sat, Sep 09, 2023 at 04:38:28PM +0200, Javier Martinez Canillas wrote:
> Javier Martinez Canillas <javierm@redhat.com> writes:
> 
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> >
> >> struct pwm_device::pwm is a write-only variable in the pwm core and used
> >> nowhere apart from this and another dev_dbg. So it isn't useful to
> >> identify the used PWM. Emit the PWM's label instead in the debug
> >> message.
> >>
> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> ---
> >
> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> >
> 
> Pushed to drm-misc (drm-misc-next). Thanks!

JFTR: This patch is already in linus/master. And Javier pushed
"drm/ssd130x: Print the PWM's label instead of its number" to
drm-misc-next which is great. So the "Pushed to ..." mail is just in
reply to the wrong patch in this thread and in git everything is fine.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 2/3] video: fbdev: ssd1307fb: Print the PWM's label instead of its number

[Javier Martinez Canillas]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

Hello Uwe,

> Hello,
>
> On Sat, Sep 09, 2023 at 04:38:28PM +0200, Javier Martinez Canillas wrote:
>> Javier Martinez Canillas <javierm@redhat.com> writes:
>> 
>> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>> >
>> >> struct pwm_device::pwm is a write-only variable in the pwm core and used
>> >> nowhere apart from this and another dev_dbg. So it isn't useful to
>> >> identify the used PWM. Emit the PWM's label instead in the debug
>> >> message.
>> >>
>> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> >> ---
>> >
>> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>> >
>> 
>> Pushed to drm-misc (drm-misc-next). Thanks!
>
> JFTR: This patch is already in linus/master. And Javier pushed
> "drm/ssd130x: Print the PWM's label instead of its number" to
> drm-misc-next which is great. So the "Pushed to ..." mail is just in
> reply to the wrong patch in this thread and in git everything is fine.
>

Ups, that's correct. Thanks a lot for pointing that out!

> Thanks
> Uwe
>


-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 0/3] pwm: Drop useless member "pwm" from struct pwm_device

[Uwe Kleine-König]

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

Hello,

On Fri, Jul 28, 2023 at 04:58:21PM +0200, Uwe Kleine-König wrote:
> there are only two users of struct pwm_device::pwm in the tree; both use
> it for some dev_dbg output. While this number allows to identify the
> PWM, it's not trivial, for example the data currently available in
> /sys/kernel/debug/pwm isn't enough. (You have to look in /sys/class/pwm,
> pick the pwmchip with the highest number that isn't bigger than the
> PWM's number.)
> 
> To be honest the label isn't always usefull either, but it's easy to use
> and should be enough to identify the used PWM. The parent device + hwid
> might be more useful?! On the other hand using that for a dev_dbg that
> is probably only looked at by someone debugging the driver and thus
> knowing the used PWM anyhow is of little value either.
> 
> Assuming this change is still considered worthwile I suggest that patches #1
> and #2 go in via their respective maintainer trees and I resend patch #3 to go
> via the pwm tree once these two are "in".
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (3):
>   drm/ssd130x: Print the PWM's label instead of its number
>   video: fbdev: ssd1307fb: Print the PWM's label instead of its number
>   pwm: Drop unused member "pwm" from struct pwm_device

The two patches to stop making use of struct pwm_device::pwm are now in
Linus's tree (as of v6.7-rc1). The third patch is still "new" in
patchwork, so I don't resend.

It's great if patch #3 goes in during the next merge window.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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