diff mbox series

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

Message ID 20230403105052.426135-2-cristian.ciocaltea@collabora.com
State New
Headers
Series Add PWM fan support to Rock 5B board |

Commit Message

Cristian Ciocaltea April 3, 2023, 10:50 a.m. UTC 
  Convert the PWM fan bindings to DT schema format.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../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

Comments

Rob Herring April 3, 2023, 1:10 p.m. UTC | #1 
On Mon, 03 Apr 2023 13:50:51 +0300, Cristian Ciocaltea wrote:
> Convert the PWM fan bindings to DT schema format.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../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
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230403105052.426135-2-cristian.ciocaltea@collabora.com


pwm-fan: 'cooling-max-state', 'cooling-min-state' do not match any of the regexes: 'pinctrl-[0-9]+'
	arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dtb
	arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dtb
Cristian Ciocaltea April 3, 2023, 2:32 p.m. UTC | #2 
On 4/3/23 16:10, Rob Herring wrote:
> 
> On Mon, 03 Apr 2023 13:50:51 +0300, Cristian Ciocaltea wrote:
>> Convert the PWM fan bindings to DT schema format.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>   .../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
>>
> 
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
> 
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
> 
> Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230403105052.426135-2-cristian.ciocaltea@collabora.com
> 
> 
> pwm-fan: 'cooling-max-state', 'cooling-min-state' do not match any of the regexes: 'pinctrl-[0-9]+'
> 	arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dtb
> 	arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dtb
> 

The only references to the offending cooling-{min|max}-state are located 
in a few DTS files. Assuming they are obsolete, may I simply drop them?

$ git grep "cooling-.*-state"

arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi: cooling-min-state = <0>;
arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi: cooling-max-state = <3>;
arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts: cooling-min-state = <0>;
arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts: cooling-max-state = <3>;
arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts:        cooling-min-state = <0>;
arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts:        cooling-max-state = <3>;
arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi:        cooling-min-state = <0>;
arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi:        cooling-max-state = <9>;
AngeloGioacchino Del Regno April 3, 2023, 2:43 p.m. UTC | #3 
Il 03/04/23 16:32, Cristian Ciocaltea ha scritto:
> On 4/3/23 16:10, Rob Herring wrote:
>>
>> On Mon, 03 Apr 2023 13:50:51 +0300, Cristian Ciocaltea wrote:
>>> Convert the PWM fan bindings to DT schema format.
>>>
>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>> ---
>>>    .../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
>>>
>>
>> Running 'make dtbs_check' with the schema in this patch gives the
>> following warnings. Consider if they are expected or the schema is
>> incorrect. These may not be new warnings.
>>
>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>> This will change in the future.
>>
>> Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230403105052.426135-2-cristian.ciocaltea@collabora.com
>>
>>
>> pwm-fan: 'cooling-max-state', 'cooling-min-state' do not match any of the regexes: 'pinctrl-[0-9]+'
>> 	arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dtb
>> 	arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dtb
>>
> 
> The only references to the offending cooling-{min|max}-state are located
> in a few DTS files. Assuming they are obsolete, may I simply drop them?
> 

If they're obsolete, you can mark them as `deprecated: true` in the binding, but
dropping them entirely would be an ABI breakage, so no, you can't.

Regards,
Angelo

> $ git grep "cooling-.*-state"
> 
> arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi: cooling-min-state = <0>;
> arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi: cooling-max-state = <3>;
> arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts: cooling-min-state = <0>;
> arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts: cooling-max-state = <3>;
> arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts:        cooling-min-state = <0>;
> arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts:        cooling-max-state = <3>;
> arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi:        cooling-min-state = <0>;
> arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi:        cooling-max-state = <9>;
>
Cristian Ciocaltea April 3, 2023, 4:22 p.m. UTC | #4 
On 4/3/23 17:43, AngeloGioacchino Del Regno wrote:
> Il 03/04/23 16:32, Cristian Ciocaltea ha scritto:
>> On 4/3/23 16:10, Rob Herring wrote:
>>>
>>> On Mon, 03 Apr 2023 13:50:51 +0300, Cristian Ciocaltea wrote:
>>>> Convert the PWM fan bindings to DT schema format.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> ---
>>>>    .../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
>>>>
>>>
>>> Running 'make dtbs_check' with the schema in this patch gives the
>>> following warnings. Consider if they are expected or the schema is
>>> incorrect. These may not be new warnings.
>>>
>>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>>> This will change in the future.
>>>
>>> Full log is available here:
>>> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230403105052.426135-2-cristian.ciocaltea@collabora.com
>>>
>>>
>>> pwm-fan: 'cooling-max-state', 'cooling-min-state' do not match any of
>>> the regexes: 'pinctrl-[0-9]+'
>>>     arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dtb
>>>     arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dtb
>>>
>>
>> The only references to the offending cooling-{min|max}-state are located
>> in a few DTS files. Assuming they are obsolete, may I simply drop them?
>>
> 
> If they're obsolete, you can mark them as `deprecated: true` in the
> binding, but
> dropping them entirely would be an ABI breakage, so no, you can't.

From the pwm-fan driver point of view, the properties are not supported
and I couldn't find any indication that they could have been used in the
past.

Hence I'm not sure adding them to the binding is the proper way to
handle this issue.

Thanks,
Cristian

> Regards,
> Angelo
> 
>> $ git grep "cooling-.*-state"
>>
>> arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi:
>> cooling-min-state = <0>;
>> arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi:
>> cooling-max-state = <3>;
>> arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts:
>> cooling-min-state = <0>;
>> arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dts:
>> cooling-max-state = <3>;
>> arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts:        cooling-min-state = <0>;
>> arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts:        cooling-max-state = <3>;
>> arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi:       
>> cooling-min-state = <0>;
>> arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi:       
>> cooling-max-state = <9>;
>>
> 
> 
>
Rob Herring April 4, 2023, 1:54 p.m. UTC | #5 
On Mon, Apr 03, 2023 at 07:22:45PM +0300, Cristian Ciocaltea wrote:
> On 4/3/23 17:43, AngeloGioacchino Del Regno wrote:
> > Il 03/04/23 16:32, Cristian Ciocaltea ha scritto:
> >> On 4/3/23 16:10, Rob Herring wrote:
> >>>
> >>> On Mon, 03 Apr 2023 13:50:51 +0300, Cristian Ciocaltea wrote:
> >>>> Convert the PWM fan bindings to DT schema format.
> >>>>
> >>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >>>> ---
> >>>>    .../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
> >>>>
> >>>
> >>> Running 'make dtbs_check' with the schema in this patch gives the
> >>> following warnings. Consider if they are expected or the schema is
> >>> incorrect. These may not be new warnings.
> >>>
> >>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> >>> This will change in the future.
> >>>
> >>> Full log is available here:
> >>> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230403105052.426135-2-cristian.ciocaltea@collabora.com
> >>>
> >>>
> >>> pwm-fan: 'cooling-max-state', 'cooling-min-state' do not match any of
> >>> the regexes: 'pinctrl-[0-9]+'
> >>>     arch/arm64/boot/dts/amlogic/meson-sm1-odroid-hc4.dtb
> >>>     arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dtb
> >>>
> >>
> >> The only references to the offending cooling-{min|max}-state are located
> >> in a few DTS files. Assuming they are obsolete, may I simply drop them?
> >>
> > 
> > If they're obsolete, you can mark them as `deprecated: true` in the
> > binding, but
> > dropping them entirely would be an ABI breakage, so no, you can't.
> 
> >From the pwm-fan driver point of view, the properties are not supported
> and I couldn't find any indication that they could have been used in the
> past.
> 
> Hence I'm not sure adding them to the binding is the proper way to
> handle this issue.

They can be omitted.

Rob
Rob Herring April 4, 2023, 2:17 p.m. UTC | #6 
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@collabora.com>
> ---
>  .../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@suse.com>
> +  - Guenter Roeck <linux@roeck-us.net>
> +
> +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
>
Guenter Roeck April 4, 2023, 2:22 p.m. UTC | #7 
On 4/4/23 07:17, Rob Herring wrote:
> 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@collabora.com>
>> ---
>>   .../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
...
>> +
>> +  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.
> 

Should be 1 to 4.

Guenter
Cristian Ciocaltea April 4, 2023, 4:32 p.m. UTC | #8 
On 4/4/23 17:17, Rob Herring wrote:
> 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@collabora.com>
>> ---
>>  .../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.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@suse.com>
>> +  - Guenter Roeck <linux@roeck-us.net>
>> +
>> +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.

The 3rd example taken from the original binding uses 2 entries. So far
it seems there are no use cases requiring more than one, so I'm going to
limit this to 5 (the driver doesn't enforce a limit).

>> +
>> +  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.

Will set it to 4, as suggested by Guenter.

>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Isn't this a scalar? 

As in the case of interrupts, the 3rd example uses 2 entries. Will set
the same limit (5).

>> +
>> +  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.

Will describe its usage according to thermal-cooling-devices binding.

Thanks for reviewing,
Cristian
diff mbox series

Patch

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@suse.com>
+  - Guenter Roeck <linux@roeck-us.net>
+
+properties:
+  compatible:
+    const: pwm-fan
+
+  cooling-levels:
+    description:
+      PWM duty cycle values in a range from 0 to 255 which correspond to
+      thermal cooling states.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  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.
+
+  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).
+      The value must be greater than zero.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  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.
+    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>;
+    };