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

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

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

Uwe Kleine-König [off-list ref] 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 <redacted>
---

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Javier Martinez Canillas [off-list ref] writes:

Uwe Kleine-König [off-list ref] writes:

quoted

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

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

Hello,

On Sat, Sep 09, 2023 at 04:38:28PM +0200, Javier Martinez Canillas wrote:

Javier Martinez Canillas [off-list ref] writes:

quoted

Uwe Kleine-König [off-list ref] writes:

quoted

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

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

Uwe Kleine-König [off-list ref] writes:

Hello Uwe,

Hello,

On Sat, Sep 09, 2023 at 04:38:28PM +0200, Javier Martinez Canillas wrote:

quoted

Javier Martinez Canillas [off-list ref] writes:

quoted

Uwe Kleine-König [off-list ref] writes:

quoted

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

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

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

applied.

Thanks!
Helge

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

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