Messages in this thread Patch in this message | | | Date | Thu, 28 Oct 2021 15:52:35 +0200 | From | David Heidelberg <> | Subject | Re: [PATCH] WIP: dt-bindings: arm: hwmon: gpio-fan: Convert txt bindings to yaml |
| |
On Tue, Oct 19 2021 at 16:35:17 -0500, Rob Herring <robh@kernel.org> 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@ixit.cz> >> --- >> .../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@kernel.org> > Just robh@kernel.org > >> + >> +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 >> >>
| |