lkml.org 
[lkml]   [2021]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] pwm: Rename pwm_get_state() to better reflect its semantic
On Tue, Apr 06, 2021 at 03:43:56PM +0200, Uwe Kleine-König wrote:
> Hello Thierry,
>
> On Tue, Apr 06, 2021 at 01:16:31PM +0200, Thierry Reding wrote:
> > On Tue, Apr 06, 2021 at 09:30:36AM +0200, Uwe Kleine-König wrote:
> > > Given that lowlevel drivers usually cannot implement exactly what a
> > > consumer requests with pwm_apply_state() there is some rounding involved.
> > >
> > > pwm_get_state() traditionally returned the setting that was requested most
> > > recently by the consumer (opposed to what was actually implemented in
> > > hardware in reply to the last request). To make this semantic obvious
> > > rename the function.
> > >
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Documentation/driver-api/pwm.rst | 6 +++-
> > > drivers/clk/clk-pwm.c | 2 +-
> > > drivers/gpu/drm/i915/display/intel_panel.c | 4 +--
> > > drivers/input/misc/da7280.c | 2 +-
> > > drivers/input/misc/pwm-beeper.c | 2 +-
> > > drivers/input/misc/pwm-vibra.c | 4 +--
> > > drivers/pwm/core.c | 4 +--
> > > drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
> > > drivers/pwm/pwm-atmel.c | 2 +-
> > > drivers/pwm/pwm-imx27.c | 2 +-
> > > drivers/pwm/pwm-rockchip.c | 2 +-
> > > drivers/pwm/pwm-stm32-lp.c | 4 +--
> > > drivers/pwm/pwm-sun4i.c | 2 +-
> > > drivers/pwm/sysfs.c | 18 ++++++------
> > > drivers/regulator/pwm-regulator.c | 4 +--
> > > drivers/video/backlight/pwm_bl.c | 10 +++----
> > > include/linux/pwm.h | 34 ++++++++++++++--------
> > > 17 files changed, 59 insertions(+), 45 deletions(-)
> >
> > Honestly, I don't think this is worth the churn. If you think people
> > will easily get confused by this then a better solution might be to more
> > explicitly document the pwm_get_state() function to say exactly what it
> > returns.
>
> I'm not so optimistic that people become aware of the semantic just
> because there is documentation describing it and I strongly believe that
> a good name for functions is more important than accurate documentation.
>
> If you don't agree, what do you think about the updated wording in
> Documentation/driver-api/pwm.rst?

Yeah, that clarifies this a bit. I can apply that hunk of the patch
separately.

> > But there's no need to make life difficult for everyone by
> > renaming this to something as cumbersome as this.
>
> I don't expect any merge conflicts (and if still a problem occurs
> resolving should be trivial enough). So I obviously don't agree to your
> weighing.

I wasn't talking about merge conflicts but instead about the extra churn
of changing all consumers and having to type all these extra characters
for no benefit.

Thierry
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-04-09 14:09    [W:0.036 / U:0.284 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site