Chris, On Thu, May 09, 2024 at 06:19:12PM +0000, Chris Packham wrote: > Hi Krzysztof, > > On 9/05/24 19:06, Krzysztof Kozlowski wrote: > > On 08/05/2024 23:55, Chris Packham wrote: > >> Add documentation for the pwm-initial-duty-cycle 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: > > Frequency usually has some units, so use appropriate unit suffix and > > 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. > > There's two things going on here. There's a PWM duty cycle which is > 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. > My udnerstanding is that we are supposed to use standard pwm provider 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. Hope thie helps, Guenter