On 22/07/24 16:27, Guenter Roeck wrote:
On 7/21/24 21:09, Chris Packham wrote:
On 22/07/24 15:53, Guenter Roeck wrote:
On 7/21/24 17:58, Chris Packham wrote:
By default the PWM duty cycle in hardware is 100%. On some systems
this
can cause unwanted fan noise. Add the ability to specify the fan
connections and initial state of the PWMs via device properties.
Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
---
Notes:
Changes in v6:
- Use do_div() instead of plain /
- Use a helper function to avoid repetition between the of and
non-of
code paths.
Changes in v5:
- Deal with PWM frequency and duty cycle being specified in
nanoseconds
Changes in v4:
- Support DT and ACPI fwnodes
- Put PWM into manual mode
Changes in v3:
- Use the pwm provider/consumer bindings
Changes in v2:
- Use correct device property string for frequency
- Allow -EINVAL and only warn on error
- Use a frequency of 0 to indicate that the hardware should be
left as-is
drivers/hwmon/adt7475.c | 130
++++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 4224ffb30483..fc5605d34f36 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -21,6 +21,8 @@
#include <linux/of.h>
#include <linux/util_macros.h>
+#include <dt-bindings/pwm/pwm.h>
+
/* Indexes for the sysfs hooks */
#define INPUT 0
@@ -1662,6 +1664,130 @@ static int adt7475_set_pwm_polarity(struct
i2c_client *client)
return 0;
}
+struct adt7475_pwm_config {
+ int index;
+ int freq;
+ int flags;
+ int duty;
+};
+
+static int _adt7475_pwm_properties_parse_args(u32 args[4], struct
adt7475_pwm_config *cfg)
+{
+ unsigned long freq_hz;
+ unsigned long duty;
+
+ if (args[1] == 0)
+ return -EINVAL;
+
+ freq_hz = 1000000000UL;
+ do_div(freq_hz, args[1]);
+ duty = 255 * args[3];
+ do_div(duty, args[1]);
+
Gues I am a bit at loss here, just as 0-day. Why use do_div ? It is
only needed
for 64-bit divide operations.
Mainly because of Uwe's comment on v5. I think I've avoided the
original u64 issue now that I'm converting
fwnode_reference_args::args to a u32 array. I can probably get away
with plain division, although 255 * args[3] / args[1] might overflow
in theory but shouldn't in practice.
I'll let the earth turn and send out a v7 that uses plain division
unless someone has a strong opinion that I should sprinkle some more
u64s around.
You lost me, sorry. Neither duty nor freq_hz are u64. What u64 variables
are you talking about ? Using so_div doesn't make those variables u64.
One way of fixing the 0-day complaint (I think) is to declare freq_hz
and duty as u64 which would avoid all the theoretical overflow issues.
But plain division is probably easier to understand for everyone so I'll
send out something like this in v7
(unsigned?) int freq_hz;
(unsigned?) int duty;
...
freq_hz = 1000000000UL / args[1];
duty = 255 * args[3] / args[1];
...
Guenter
Thanks,
Guenter
