Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Convert to DT schema

From: Rob Herring
Date: Tue Apr 04 2023 - 10:17:56 EST


On Mon, Apr 03, 2023 at 01:50:51PM +0300, Cristian Ciocaltea wrote:
> Convert the PWM fan bindings to DT schema format.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/hwmon/pwm-fan.txt | 68 +----------
> .../devicetree/bindings/hwmon/pwm-fan.yaml | 109 ++++++++++++++++++
> 2 files changed, 110 insertions(+), 67 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
> index 4509e688623a..48886f0ce415 100644
> --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt
> @@ -1,67 +1 @@
> -Bindings for a fan connected to the PWM lines
> -
> -Required properties:
> -- compatible : "pwm-fan"
> -- pwms : the PWM that is used to control the PWM fan
> -- cooling-levels : PWM duty cycle values in a range from 0 to 255
> - which correspond to thermal cooling states
> -
> -Optional properties:
> -- fan-supply : phandle to the regulator that provides power to the fan
> -- interrupts : This contains an interrupt specifier for each fan
> - tachometer output connected to an interrupt source.
> - The output signal must generate a defined number of
> - interrupts per fan revolution, which require that
> - it must be self resetting edge interrupts. See
> - interrupt-controller/interrupts.txt for the format.
> -- pulses-per-revolution : define the number of pulses per fan revolution for
> - each tachometer input as an integer (default is 2
> - interrupts per revolution). The value must be
> - greater than zero.
> -
> -Example:
> - fan0: pwm-fan {
> - compatible = "pwm-fan";
> - #cooling-cells = <2>;
> - pwms = <&pwm 0 10000 0>;
> - cooling-levels = <0 102 170 230>;
> - };
> -
> - thermal-zones {
> - cpu_thermal: cpu-thermal {
> - thermal-sensors = <&tmu 0>;
> - polling-delay-passive = <0>;
> - polling-delay = <0>;
> - trips {
> - cpu_alert1: cpu-alert1 {
> - temperature = <100000>; /* millicelsius */
> - hysteresis = <2000>; /* millicelsius */
> - type = "passive";
> - };
> - };
> - cooling-maps {
> - map0 {
> - trip = <&cpu_alert1>;
> - cooling-device = <&fan0 0 1>;
> - };
> - };
> - };
> -
> -Example 2:
> - fan0: pwm-fan {
> - compatible = "pwm-fan";
> - pwms = <&pwm 0 40000 0>;
> - fan-supply = <&reg_fan>;
> - interrupt-parent = <&gpio5>;
> - interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
> - pulses-per-revolution = <2>;
> - };
> -
> -Example 3:
> - fan0: pwm-fan {
> - compatible = "pwm-fan";
> - pwms = <&pwm1 0 25000 0>;
> - interrupts-extended = <&gpio1 1 IRQ_TYPE_EDGE_FALLING>,
> - <&gpio2 5 IRQ_TYPE_EDGE_FALLING>;
> - pulses-per-revolution = <2>, <1>;
> - };
> +This file has moved to pwm-fan.yaml.
> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> new file mode 100644
> index 000000000000..448b48ec5d73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/pwm-fan.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Fan connected to PWM lines
> +
> +maintainers:
> + - Jean Delvare <jdelvare@xxxxxxxx>
> + - Guenter Roeck <linux@xxxxxxxxxxxx>
> +
> +properties:
> + compatible:
> + const: pwm-fan
> +
> + cooling-levels:
> + description:
> + PWM duty cycle values in a range from 0 to 255 which correspond to

Don't put constraints in plain text:

items:
maximum: 255

> + thermal cooling states.
> + $ref: /schemas/types.yaml#/definitions/uint32-array

Unfortunately, looks like we've wound up with same property with 2
differing types. A problem for another day...

> +
> + fan-supply:
> + description: Phandle to the regulator that provides power to the fan.
> +
> + interrupts:
> + description:
> + This contains an interrupt specifier for each fan tachometer output
> + connected to an interrupt source. The output signal must generate a
> + defined number of interrupts per fan revolution, which require that
> + it must be self resetting edge interrupts.

How many entries? I'm not sure how more than 1 makes sense.

> +
> + pulses-per-revolution:
> + description:
> + Define the number of pulses per fan revolution for each tachometer
> + input as an integer (default is 2 interrupts per revolution).

default: 2

> + The value must be greater than zero.

minimum: 1

maximum: ??? I assume there's some practical limit here much less than
2^32.

> + $ref: /schemas/types.yaml#/definitions/uint32-array

Isn't this a scalar?
> +
> + pwms:
> + description: The PWM that is used to control the fan.
> + maxItems: 1
> +
> + pwm-names: true
> +
> + "#cooling-cells":
> + description: The PWM fan can be referenced as a cooling-device.

Not that useful. What would be is what's in the 2 cells.

> + const: 2
> +
> +required:
> + - compatible
> + - pwms
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pwm-fan {
> + compatible = "pwm-fan";
> + cooling-levels = <0 102 170 230>;
> + pwms = <&pwm 0 10000 0>;
> + #cooling-cells = <2>;
> + };
> +
> + thermal-zones {
> + cpu_thermal: cpu-thermal {
> + thermal-sensors = <&tmu 0>;
> + polling-delay-passive = <0>;
> + polling-delay = <0>;
> +
> + trips {
> + cpu_alert1: cpu-alert1 {
> + temperature = <100000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&cpu_alert1>;
> + cooling-device = <&fan0 0 1>;
> + };
> + };
> + };
> + };
> +
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pwm-fan {
> + compatible = "pwm-fan";
> + pwms = <&pwm 0 40000 0>;
> + fan-supply = <&reg_fan>;
> + interrupt-parent = <&gpio5>;
> + interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
> + pulses-per-revolution = <2>;
> + };
> +
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pwm-fan {
> + compatible = "pwm-fan";
> + pwms = <&pwm1 0 25000 0>;
> + interrupts-extended = <&gpio1 1 IRQ_TYPE_EDGE_FALLING>,
> + <&gpio2 5 IRQ_TYPE_EDGE_FALLING>;
> + pulses-per-revolution = <2>, <1>;
> + };
> --
> 2.40.0
>