PWM implementation in HWMON and backlight

PWM implementation in HWMON and backlight

[Richard Weinberger]

Hello,

The backlight of a board I am working with is controlled via PWM.
Naturally, I thought this would be a straightforward task using the
pwm-backlight driver.

However, the PWM in question is implemented using an NCT6106D chip.
The associated HWMON driver, nct6775-core.c, does not implement a
standard PWM device interface but rather its own custom one.

I am a bit puzzled, is there a specific reason why HWMON does not
utilize the standard PWM framework in this case?
Also, in case I have overlooked something: do we have a backlight
driver available that can interface with an HWMON-based PWM?

-- 
Thanks,
//richard

Re: PWM implementation in HWMON and backlight

[Uwe Kleine-König]

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

Hello Guenter,

On Wed, Feb 11, 2026 at 10:28:55AM +0100, Richard Weinberger wrote:
> The backlight of a board I am working with is controlled via PWM.
> Naturally, I thought this would be a straightforward task using the
> pwm-backlight driver.
> 
> However, the PWM in question is implemented using an NCT6106D chip.
> The associated HWMON driver, nct6775-core.c, does not implement a
> standard PWM device interface but rather its own custom one.

Looking around in drivers/hwmon made me a sad. There are four drivers
that handle parsing #pwm-cells:

	$ git grep pwm-cell drivers/hwmon/
	drivers/hwmon/adt7475.c:        ret = fwnode_property_get_reference_args(fwnode, "pwms", "#pwm-cells", 0, 0, &rargs);
	drivers/hwmon/amc6821.c:        if (of_parse_phandle_with_args(fan_np, "pwms", "#pwm-cells", 0, &args))
	drivers/hwmon/emc2305.c:        ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", 0, &args);
	drivers/hwmon/nct7363.c:        ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells",

instead of using the pwm subsystem. Also the driver mentioned by Richard
above has some self-made PWM handling including a set of driver specific
sysfs files to control the PWMs. I stopped looking at the output of

	git grep pwm drivers/hwmon/

after finding some more sad things. (My "favourite" so far was:

	dev_dbg(dev, "chmod -w pwm%d failed\n", nr + 1);
 
.)

> I am a bit puzzled, is there a specific reason why HWMON does not
> utilize the standard PWM framework in this case?

Yes please!

I think that the PWM waveform API that exists since v6.13-rc1 is
flexible enough that hwmon chips should be able to both implement and
use it properly.

Can you please make sure that the next hardware driver for a pwm capable
chip uses a proper PWM chip? Feel free to send patch authors in my
direction for that.

And if I'm wrong and using the pwm subsystem in these cases is a burden,
I want to hear about that and discuss how this can be made better.

Thanks
Uwe

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

Re: PWM implementation in HWMON and backlight

[Guenter Roeck]

On 2/11/26 02:46, Uwe Kleine-König wrote:
> Hello Guenter,
> 
> On Wed, Feb 11, 2026 at 10:28:55AM +0100, Richard Weinberger wrote:
>> The backlight of a board I am working with is controlled via PWM.
>> Naturally, I thought this would be a straightforward task using the
>> pwm-backlight driver.
>>
>> However, the PWM in question is implemented using an NCT6106D chip.
>> The associated HWMON driver, nct6775-core.c, does not implement a
>> standard PWM device interface but rather its own custom one.
> 
> Looking around in drivers/hwmon made me a sad. There are four drivers
> that handle parsing #pwm-cells:
> 
> 	$ git grep pwm-cell drivers/hwmon/
> 	drivers/hwmon/adt7475.c:        ret = fwnode_property_get_reference_args(fwnode, "pwms", "#pwm-cells", 0, 0, &rargs);
> 	drivers/hwmon/amc6821.c:        if (of_parse_phandle_with_args(fan_np, "pwms", "#pwm-cells", 0, &args))
> 	drivers/hwmon/emc2305.c:        ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", 0, &args);
> 	drivers/hwmon/nct7363.c:        ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells",
> 
> instead of using the pwm subsystem. Also the driver mentioned by Richard
> above has some self-made PWM handling including a set of driver specific
> sysfs files to control the PWMs. I stopped looking at the output of
> 
> 	git grep pwm drivers/hwmon/
> 
> after finding some more sad things. (My "favourite" so far was:
> 
> 	dev_dbg(dev, "chmod -w pwm%d failed\n", nr + 1);
>   

That code is from 2006. Are you serious ? Did you bother to read
the context ? Did you bother considering that this was possibly the
best means available at the time to control visibility of an
attribute file ?

If calls to sysfs_chmod_file() bother you, I would suggest to send
patches updating affected drivers and to drop the API function from
the kernel.

> .)
> 
>> I am a bit puzzled, is there a specific reason why HWMON does not
>> utilize the standard PWM framework in this case?
> 
> Yes please!
> 
> I think that the PWM waveform API that exists since v6.13-rc1 is
> flexible enough that hwmon chips should be able to both implement and
> use it properly.
> 
> Can you please make sure that the next hardware driver for a pwm capable
> chip uses a proper PWM chip? Feel free to send patch authors in my
> direction for that.
> 
> And if I'm wrong and using the pwm subsystem in these cases is a burden,
> I want to hear about that and discuss how this can be made better.
> 

For the most part the pwm implementation in hwmon chips is tied to supporting
pwm output for fans and isn't usable for anything else. This gets worse for
chips where pwm vs. voltage control on the output signal can be selected.

Unless there is an actual use case for utilizing the pwm subsystem for a
given chip, doing so would just create overhead. _If_ there is a proven
use case, I don't mind if people submit patches to add generic support
for the the pwm subsystem to such drivers. Keep in mind though that support
for the ability to switch between pwm and voltage control (as is very common
for fans) is mandatory for chips with that capability.

Talking about this specific driver, it has been in the upstream kernel since v3.10
(2013). Almost all hwmon drivers supporting pwm fan control are much older than
v6.13. While many of those would benefit from a modernization update, supporting
the pwm subsystem just because it exists would, from my perspective, be a waste
of time. I most certainly won't do it.

In my opinion calling it "sad" that drivers are not re-implemented to use a
newly available out-of-subsystem API is close to being an insult to those
who would have to do that work.

Sorry, you folks really got me on the wrong foot. If there is anything sad,
it is people complaining about 20 year old code without doing anything about
it themselves. I by now spent months if not years of my time modernizing hwmon
drivers. Did you ?

Guenter

Re: PWM implementation in HWMON and backlight

[Uwe Kleine-König]

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

Hello Guenter,

On Wed, Feb 11, 2026 at 07:47:36AM -0800, Guenter Roeck wrote:
> On 2/11/26 02:46, Uwe Kleine-König wrote:
> > On Wed, Feb 11, 2026 at 10:28:55AM +0100, Richard Weinberger wrote:
> > > The backlight of a board I am working with is controlled via PWM.
> > > Naturally, I thought this would be a straightforward task using the
> > > pwm-backlight driver.
> > > 
> > > However, the PWM in question is implemented using an NCT6106D chip.
> > > The associated HWMON driver, nct6775-core.c, does not implement a
> > > standard PWM device interface but rather its own custom one.
> > 
> > Looking around in drivers/hwmon made me a sad. There are four drivers
> > that handle parsing #pwm-cells:
> > 
> > 	$ git grep pwm-cell drivers/hwmon/
> > 	drivers/hwmon/adt7475.c:        ret = fwnode_property_get_reference_args(fwnode, "pwms", "#pwm-cells", 0, 0, &rargs);
> > 	drivers/hwmon/amc6821.c:        if (of_parse_phandle_with_args(fan_np, "pwms", "#pwm-cells", 0, &args))
> > 	drivers/hwmon/emc2305.c:        ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", 0, &args);
> > 	drivers/hwmon/nct7363.c:        ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells",
> > 
> > instead of using the pwm subsystem. Also the driver mentioned by Richard
> > above has some self-made PWM handling including a set of driver specific
> > sysfs files to control the PWMs. I stopped looking at the output of
> > 
> > 	git grep pwm drivers/hwmon/
> > 
> > after finding some more sad things. (My "favourite" so far was:
> > 
> > 	dev_dbg(dev, "chmod -w pwm%d failed\n", nr + 1);
> 
> That code is from 2006. Are you serious ? Did you bother to read
> the context ? Did you bother considering that this was possibly the
> best means available at the time to control visibility of an
> attribute file ?

This was just something that I spotted while looking at the git-grep
output. The thing that actually triggered my mail was commit
46b94c485ed197bc681da242440c6e2315697c57 and less that it doesn't use
the pwm stuff (which was only in next at that time probably), but more
that I wasn't involved.

> For the most part the pwm implementation in hwmon chips is tied to supporting
> pwm output for fans and isn't usable for anything else. This gets worse for
> chips where pwm vs. voltage control on the output signal can be selected.
> 
> Unless there is an actual use case for utilizing the pwm subsystem for a
> given chip, doing so would just create overhead. _If_ there is a proven
> use case, I don't mind if people submit patches to add generic support
> for the the pwm subsystem to such drivers. Keep in mind though that support
> for the ability to switch between pwm and voltage control (as is very common
> for fans) is mandatory for chips with that capability.
> 
> Talking about this specific driver, it has been in the upstream kernel since v3.10
> (2013). Almost all hwmon drivers supporting pwm fan control are much older than
> v6.13. While many of those would benefit from a modernization update, supporting
> the pwm subsystem just because it exists would, from my perspective, be a waste
> of time. I most certainly won't do it.

The bit I don't like about these drivers is that their binding (using
#pwm-cells) suggests that these chips are usable as generic PWM. That's
what Richard seems to have expected, too.

> In my opinion calling it "sad" that drivers are not re-implemented to use a
> newly available out-of-subsystem API is close to being an insult to those
> who would have to do that work.
> 
> Sorry, you folks really got me on the wrong foot. If there is anything sad,
> it is people complaining about 20 year old code without doing anything about
> it themselves. I by now spent months if not years of my time modernizing hwmon
> drivers. Did you ?

That wasn't the message that I intended to transport and I'm taking the
blame for that. The actual intended take away was: Please for the next
driver implementing PWM stuff, poke me, such that the nice things in
drivers/pwm are reused instead of partly reimplemented with less
functionality and that the maybe not so nice things are improved.

I didn't want to blame you (or anyone else) with my mail, and I'm sorry
that this was how you received my mail.

Thanks for being honest,
Uwe

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