On 10/05/24 15:36, Guenter Roeck wrote:
Chris,
On Thu, May 09, 2024 at 06:19:12PM +0000, Chris Packham wrote:
Hi Krzysztof,My udnerstanding is that we are supposed to use standard pwm provider
On 9/05/24 19:06, Krzysztof Kozlowski wrote:
On 08/05/2024 23:55, Chris Packham wrote:There's two things going on here. There's a PWM duty cycle which is
Add documentation for the pwm-initial-duty-cycle andFrequency usually has some units, so use appropriate unit suffix and
pwm-initial-frequency properties. These allow the starting state of the
PWM outputs to be set to cater for hardware designs where undesirable
amounts of noise is created by the default hardware state.
Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
---
Notes:
Changes in v2:
- Document 0 as a valid value (leaves hardware as-is)
.../devicetree/bindings/hwmon/adt7475.yaml | 27 ++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/hwmon/adt7475.yaml b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
index 051c976ab711..97deda082b4a 100644
--- a/Documentation/devicetree/bindings/hwmon/adt7475.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adt7475.yaml
@@ -51,6 +51,30 @@ properties:
enum: [0, 1]
default: 1
+ adi,pwm-initial-duty-cycle:
+ description: |
+ Configures the initial duty cycle for the PWM outputs. The hardware
+ default is 100% but this may cause unwanted fan noise at startup. Set
+ this to a value from 0 (0% duty cycle) to 255 (100% duty cycle).
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 3
+ maxItems: 3
+ items:
+ minimum: 0
+ maximum: 255
+ default: 255
+
+ adi,pwm-initial-frequency:
drop $ref. Maybe that's just target-rpm property?
But isn't this duplicating previous property? This is fan controller,
not PWM provider (in any case you miss proper $refs to pwm.yaml or
fan-common.yaml), so the only thing you initially want to configure is
the fan rotation, not specific PWM waveform. If you you want to
configure specific PWM waveform, then it's a PWM provider... but it is
not... Confused.
configurable from 0% to 100%. It might be nice if this was expressed as
a percentage instead of 0-255 but I went with the latter because that's
how the sysfs ABI for the duty cycle works.
The frequency (which I'll call adi,pwm-initial-frequency-hz in v3)
affects how that duty cycle is presented to the fans. So you could still
have a duty cycle of 50% at any frequency. What frequency is best
depends on the kind of fans being used. In my particular case the lower
frequencies end up with the fans oscillating annoyingly so I use the
highest setting.
properties. The property description is provider specicic, so I think
we can pretty much just make it up.
Essentially you'd first define a pwm provider which defines all the
pwm parameters needed, such as pwm freqency, default duty cycle,
and flags such as PWM_POLARITY_INVERTED. You'd then add something like
pwms = <&pwm index frequency duty_cycle ... flags>;
to the node for each fan, and be done.
That doesn't mean that we would actually have to register the chip
as pwm provider with the pwm subsystem; all we would have to do is to
interpret the property values.
We've already got the pwm-active-state as a separate property so that
might be tricky to deal with, I guess it could be deprecated in favour
of something else. Looking at pwm.yaml and fan-common.yaml I can't quite
see how that'd help here. Were you thinking maybe something like
pwm: hwmon@2e {
compatible = "adi,adt7476";
reg = <0x2e>;
#pwm-cells = <4>;
fan-0 {
pwms = <&pwm 0 255 22500 PWM_POLARITY_INVERTED>;
pwm-names = "PWM1";
tach-ch = <0>;
};
fan-1 {
// controlled by pwm 0
tach-ch = <1>
};
fan-0 {
pwms = <&pwm 2 255 22500 PWM_POLARITY_INVERTED>;
pwm-names = "PWM3";
tach-ch <2>;
};
fan-1 {
// controlled by pwm 2
tach-ch = <3>