Re: [PATCH] WIP: dt-bindings: arm: hwmon: gpio-fan: Convert txt bindings to yaml

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On Tue, Oct 19 2021 at 16:35:17 -0500, Rob Herring <robh@xxxxxxxxxx> wrote:
On Sat, Oct 09, 2021 at 12:43:09PM +0200, David Heidelberg wrote:
 Convert fan devices connected to GPIOs to the YAML syntax.

 Signed-off-by: David Heidelberg <david@xxxxxxx>
 ---
  .../devicetree/bindings/hwmon/gpio-fan.txt    | 41 -----------
.../devicetree/bindings/hwmon/gpio-fan.yaml | 69 +++++++++++++++++++
  2 files changed, 69 insertions(+), 41 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/hwmon/gpio-fan.txt create mode 100644 Documentation/devicetree/bindings/hwmon/gpio-fan.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/gpio-fan.txt b/Documentation/devicetree/bindings/hwmon/gpio-fan.txt
 deleted file mode 100644
 index f4cfa350f6a1..000000000000
 --- a/Documentation/devicetree/bindings/hwmon/gpio-fan.txt
 +++ /dev/null
 @@ -1,41 +0,0 @@
 -Bindings for fan connected to GPIO lines
 -
 -Required properties:
 -- compatible : "gpio-fan"
 -
 -Optional properties:
 -- gpios: Specifies the pins that map to bits in the control value,
 -  ordered MSB-->LSB.
 -- gpio-fan,speed-map: A mapping of possible fan RPM speeds and the
 -  control value that should be set to achieve them. This array
 -  must have the RPM values in ascending order.
-- alarm-gpios: This pin going active indicates something is wrong with
 -  the fan, and a udev event will be fired.
 -- #cooling-cells: If used as a cooling device, must be <2>
 -  Also see:
- Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
 -  min and max states are derived from the speed-map of the fan.
 -
-Note: At least one the "gpios" or "alarm-gpios" properties must be set.
 -
 -Examples:
 -
 -	gpio_fan {
 -		compatible = "gpio-fan";
 -		gpios = <&gpio1 14 1
 -			 &gpio1 13 1>;
 -		gpio-fan,speed-map = <0    0
 -				      3000 1
 -				      6000 2>;
 -		alarm-gpios = <&gpio1 15 1>;
 -	};
 -	gpio_fan_cool: gpio_fan {
 -		compatible = "gpio-fan";
 -		gpios = <&gpio2 14 1
 -			 &gpio2 13 1>;
 -		gpio-fan,speed-map =	<0    0>,
 -					<3000 1>,
 -					<6000 2>;
 -		alarm-gpios = <&gpio2 15 1>;
 -		#cooling-cells = <2>; /* min followed by max */
 -	};
diff --git a/Documentation/devicetree/bindings/hwmon/gpio-fan.yaml b/Documentation/devicetree/bindings/hwmon/gpio-fan.yaml
 new file mode 100644
 index 000000000000..e2db65d58a92
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/hwmon/gpio-fan.yaml
 @@ -0,0 +1,69 @@
 +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 +%YAML 1.2
 +---
 +$id: "http://devicetree.org/schemas/hwmon/gpio-fan.yaml#";
 +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
 +
 +title: Bindings for fan connected to GPIO lines
 +
 +maintainers:
 +  - Rob Herring <robh+dt@xxxxxxxxxx>

Just robh@xxxxxxxxxx

 +
 +properties:
 +  compatible:
 +    const: gpio-fan
 +
 +  gpios:
 +    description: |
 +      Specifies the pins that map to bits in the control value,
 +      ordered MSB-->LSB.

minItems: 1
maxItems: 7 ?

 +
 +  gpio-fan,speed-map:
 +    $ref: /schemas/types.yaml#/definitions/uint32-array
 +    minItems: 4
 +    maxItems: 254
 +    description: |
 +      A mapping of possible fan RPM speeds and the
 +      control value that should be set to achieve them. This array
 +      must have the RPM values in ascending order.

Really this should be a uint32-matrix with this schema:

items:
  minItems: 2
  maxItems: 127
  items:
    - description: fan speed in RPMs
    - description: control value

 +
 +  alarm-gpios:
 +    description: |
 +      This pin going active indicates something is wrong with
 +      the fan, and a udev event will be fired.

maxItems: 1

udev is a linuxism and shouldn't be in the binding.

 +
 +  '#cooling-cells':
 +    const: 2
 +
 +required:
 +  - compatible
 +  - gpio-fan,speed-map
 +
 +anyOf:
 +  - required: [gpios]

How is 'gpios' not always required?

 +  - required: [alarm-gpios]
 +
 +additionalProperties: false
 +
 +examples:
 +  - |
 +    gpio_fan {
 +      compatible = "gpio-fan";
 +      gpios = <&gpio1 14 1
 +               &gpio1 13 1>;
 +      gpio-fan,speed-map = <0    0
 +                            3000 1
 +                            6000 2>;

Brackets needed around each pair.

Well, that's the issue. I would love to use u32-matrix, but all the drivers use < x1 x2 y1 x2 ... z1 z2 > syntax and driver suggests it's the right solution.
        * Speed map is in the form <RPM ctrl_val RPM ctrl_val ...>

Someone had to rewrite the driver and the DTS files to fix it. We could mark old format as deprecated and use u32-matrix, but for now with current drivers it's not a solution.

What you think? Should I document it as it is (so u32-array)?

David


 +      alarm-gpios = <&gpio1 15 1>;
 +    };
 +  - |
 +    gpio_fan_cool: gpio_fan {
 +      compatible = "gpio-fan";
 +      gpios = <&gpio2 14 1
 +               &gpio2 13 1>;
 +      gpio-fan,speed-map = <0    0
 +                            3000 1
 +                            6000 2>;
 +      alarm-gpios = <&gpio2 15 1>;
 +      #cooling-cells = <2>; /* min followed by max */
 +    };
 --
 2.33.0







[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux