Re: [PATCH 6/6] mfd: lochnagar: Move binding over to dtschema

From: Rob Herring
Date: Fri May 01 2020 - 16:56:07 EST

On Mon, Apr 27, 2020 at 11:28:12AM +0100, Charles Keepax wrote:
> Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/mfd/cirrus,lochnagar.txt | 85 ----------
> .../devicetree/bindings/mfd/cirrus,lochnagar.yaml | 183 +++++++++++++++++++++
> MAINTAINERS | 12 +-
> 3 files changed, 189 insertions(+), 91 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/mfd/cirrus,lochnagar.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/cirrus,lochnagar.yaml

> diff --git a/Documentation/devicetree/bindings/mfd/cirrus,lochnagar.yaml b/Documentation/devicetree/bindings/mfd/cirrus,lochnagar.yaml
> new file mode 100644
> index 0000000000000..b2967f141b2af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/cirrus,lochnagar.yaml
> @@ -0,0 +1,183 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/cirrus,lochnagar.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic Lochnagar Audio Development Board
> +
> +maintainers:
> + - patches@xxxxxxxxxxxxxxxxxxxxx
> +
> +description: |
> + Lochnagar is an evaluation and development board for Cirrus Logic
> + Smart CODEC and Amp devices. It allows the connection of most Cirrus
> + Logic devices on mini-cards, as well as allowing connection of
> + various application processor systems to provide a full evaluation
> + platform. Audio system topology, clocking and power can all be
> + controlled through the Lochnagar, allowing the device under test
> + to be used in a variety of possible use cases.
> +
> + Also see these documents for generic binding information:
> + [1] GPIO : ../gpio/gpio.txt
> +
> + And these for relevant defines:
> + [2] include/dt-bindings/pinctrl/lochnagar.h
> + [3] include/dt-bindings/clock/lochnagar.h
> +
> + And these documents for the required sub-node binding details:
> + [4] Clock: ../clock/cirrus,lochnagar.yaml
> + [5] Pinctrl: ../pinctrl/cirrus,lochnagar.yaml
> + [6] Regulator: ../regulator/cirrus,lochnagar.yaml
> + [7] Sound: ../sound/cirrus,lochnagar.yaml
> + [8] Hardware Monitor: ../hwmon/cirrus,lochnagar.yaml
> +
> +allOf:
> + - $ref: /schemas/clock/cirrus,lochnagar.yaml#
> + - $ref: /schemas/pinctrl/cirrus,lochnagar.yaml#
> + - $ref: /schemas/regulator/cirrus,lochnagar.yaml#

The preferred form is to define the child nodes in this schema and then
reference the child schema:

lochnagar-clk:
type: object
$ref: /schemas/clock/cirrus,lochnagar.yaml#

> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - cirrus,lochnagar2
> + then:
> + allOf:
> + - $ref: /schemas/sound/cirrus,lochnagar.yaml#
> + - $ref: /schemas/hwmon/cirrus,lochnagar.yaml#

That makes this part a bit more challenging:

then:
required:
- lochnagar-sc
else:
not:
required:
- lochnagar-sc

> +
> +properties:
> + compatible:
> + contains:

So this is valid?

"foo", "cirrus,lochnagar1"

I think you just want to drop 'contains'.

> + enum:
> + - cirrus,lochnagar1
> + - cirrus,lochnagar2
> +
> + reg:
> + description:
> + I2C slave address.

No need for generic descriptions of common properties.

> + const: 0x22
> +
> + reset-gpios:
> + description: |
> + Reset line to the Lochnagar, see [1].
> + maxItems: 1
> +
> + present-gpios:
> + description: |
> + Host present line, indicating the presence of a
> + host system, see [1]. This can be omitted if the present line is
> + tied in hardware.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - reset-gpios
> + - lochnagar-clk
> + - lochnagar-pinctrl
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clk/lochnagar.h>
> + #include <dt-bindings/pinctrl/lochnagar.h>
> + i2c@e0004000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0xe0004000 0x1000>;
> +
> + lochnagar: lochnagar@22 {
> + compatible = "cirrus,lochnagar2";
> + reg = <0x22>;
> +
> + reset-gpios = <&gpio0 55 0>;
> + present-gpios = <&gpio0 60 0>;
> +
> + lochnagarclk: lochnagar-clk {
> + compatible = "cirrus,lochnagar2-clk";
> +
> + #clock-cells = <1>;
> + clocks = <&clkaudio>, <&clkpmic>;
> + clock-names = "ln-gf-mclk2", "ln-pmic-32k";
> +
> + assigned-clocks = <&lochnagarclk LOCHNAGAR_CDC_MCLK1>,
> + <&lochnagarclk LOCHNAGAR_CDC_MCLK2>;
> + assigned-clock-parents = <&clkaudio>, <&clkpmic>;
> + };
> +
> + clkpmic: lochnagar-pmic32k {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + };
> +
> + lochnagar-pinctrl {
> + compatible = "cirrus,lochnagar-pinctrl";
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&lochnagar 0 0 LOCHNAGAR2_PIN_NUM_GPIOS>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinsettings>;
> +
> + pinsettings: pin-settings {
> + ap2aif {
> + input-enable;
> + groups = "gf-aif1";
> + function = "codec-aif3";
> + };
> + codec2aif {
> + output-enable;
> + groups = "codec-aif3";
> + function = "gf-aif1";
> + };
> + };
> + };
> +
> + lochnagar-sc {
> + compatible = "cirrus,lochnagar2-soundcard";
> +
> + #sound-dai-cells = <1>;
> +
> + clocks = <&lochnagarclk LOCHNAGAR_SOUNDCARD_MCLK>;
> + clock-names = "mclk";
> + };
> +
> + lochnagar-hwmon {
> + compatible = "cirrus,lochnagar2-hwmon";
> + };
> +
> + MIC1VDD {
> + compatible = "cirrus,lochnagar2-mic1vdd";
> +
> + MICBIAS1-supply = <&eric>;
> +
> + cirrus,micbias-input = <3>;
> + };
> +
> + MICVDD {
> + compatible = "cirrus,lochnagar2-micvdd";
> +
> + SYSVDD-supply = <&wallvdd>;
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + VDD1V8 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "VDD1V8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-boot-on;
> + regulator-always-on;
> +
> + vin-supply = <&wallvdd>;
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e64e5db314976..a2b94e8cf585c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4062,12 +4062,12 @@ M: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> M: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> L: patches@xxxxxxxxxxxxxxxxxxxxx
> S: Supported
> -F: Documentation/devicetree/bindings/clock/cirrus,lochnagar.txt
> -F: Documentation/devicetree/bindings/hwmon/cirrus,lochnagar.txt
> -F: Documentation/devicetree/bindings/mfd/cirrus,lochnagar.txt
> -F: Documentation/devicetree/bindings/pinctrl/cirrus,lochnagar.txt
> -F: Documentation/devicetree/bindings/regulator/cirrus,lochnagar.txt
> -F: Documentation/devicetree/bindings/sound/cirrus,lochnagar.txt
> +F: Documentation/devicetree/bindings/clock/cirrus,lochnagar.yaml
> +F: Documentation/devicetree/bindings/hwmon/cirrus,lochnagar.yaml
> +F: Documentation/devicetree/bindings/mfd/cirrus,lochnagar.yaml
> +F: Documentation/devicetree/bindings/pinctrl/cirrus,lochnagar.yaml
> +F: Documentation/devicetree/bindings/regulator/cirrus,lochnagar.yaml
> +F: Documentation/devicetree/bindings/sound/cirrus,lochnagar.yaml
> F: Documentation/hwmon/lochnagar.rst
> F: drivers/clk/clk-lochnagar.c
> F: drivers/hwmon/lochnagar-hwmon.c
> --
> 2.11.0
>