Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191127052935.1719897-1-anarsoul@gmail.com> <20191127052935.1719897-3-anarsoul@gmail.com>
 <20191127174434.wousbqosmm5vxcsu@gilmour.lan> <20191127180743.ww5npenlg2urxtjn@core.my.home>
In-Reply-To: <20191127180743.ww5npenlg2urxtjn@core.my.home>
From:   Vasily Khoruzhick <anarsoul@gmail.com>
Date:   Wed, 27 Nov 2019 10:52:55 -0800
Message-ID: <CA+E=qVeXhutfeJ6m8fuokzy8aRmNqWWyRGn0Lbtv_9hNCXzSeg@mail.gmail.com>
Subject: Re: [PATCH v6 2/7] dt-bindings: thermal: add YAML schema for
 sun8i-thermal driver bindings
To:     Maxime Ripard <mripard@kernel.org>,
        Vasily Khoruzhick <anarsoul@gmail.com>,
        Yangtao Li <tiny.windzz@gmail.com>,
        Zhang Rui <rui.zhang@intel.com>,
        Eduardo Valentin <edubezval@gmail.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Amit Kucheria <amit.kucheria@verdurent.com>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        arm-linux <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Nov 27, 2019 at 10:07 AM Ond=C5=99ej Jirman <megous@megous.com> wro=
te:
>
> Hi,
>
> On Wed, Nov 27, 2019 at 06:44:34PM +0100, Maxime Ripard wrote:
> > Hi,
> >
> > On Tue, Nov 26, 2019 at 09:29:30PM -0800, Vasily Khoruzhick wrote:
> > > From: Yangtao Li <tiny.windzz@gmail.com>
> > >
> > > sun8i-thermal driver supports thermal sensor in wide range of Allwinn=
er
> > > SoCs. Add YAML schema for its bindings.
> > >
> > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > > ---
> > >  .../thermal/allwinner,sun8i-a83t-ths.yaml     | 103 ++++++++++++++++=
++
> > >  1 file changed, 103 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/thermal/allwinn=
er,sun8i-a83t-ths.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8=
i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i=
-a83t-ths.yaml
> > > new file mode 100644
> > > index 000000000000..e622f0a4be90
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-=
ths.yaml
> > > @@ -0,0 +1,103 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/thermal/allwinner,sun8i-a83t-ths.=
yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Allwinner SUN8I Thermal Controller Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Yangtao Li <tiny.windzz@gmail.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - const: allwinner,sun8i-a83t-ths
> > > +      - const: allwinner,sun8i-h3-ths
> > > +      - const: allwinner,sun8i-r40-ths
> > > +      - const: allwinner,sun50i-a64-ths
> > > +      - const: allwinner,sun50i-h5-ths
> > > +      - const: allwinner,sun50i-h6-ths
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  clock-names:
> > > +    anyOf:
> > > +      - items:
> > > +        - const: bus
> > > +        - const: mod
> > > +      - items:
> > > +        - const: bus
> >
> > This can be:
> >
> > clock-names:
> >   minItems: 1
>
> Additionally, minItems should be 0, since A83T doesn't have bus clock/res=
et. And
> then there should be a special case for A83T too with min/maxItems =3D 0 =
for both
> resets and clocks.

That's why I removed clocks, clock-names and resets from required
properties. If they're present min/maxItems should be 1 and 2
accordingly.

> regards,
>         o.
>
> >   maxItems: 2
> >   items:
> >     - const: bus
> >     - const: mod
> >
> > And the length should be checked based on the compatible value, with
> > something like
> >
> > if:
> >   properties:
> >     compatible:
> >       contains:
> >         const: allwinner,sun50i-h6-ths
> >
> > then:
> >   properties:
> >     clocks:
> >       maxItems: 1
> >
> >     clock-names:
> >       maxItems: 1
> >
> > else:
> >   properties:
> >     clocks:
> >       maxItems: 2
> >
> >     clock-names:
> >       maxItems: 2
> >
> > > +
> > > +  '#thermal-sensor-cells':
> > > +    enum: [ 0, 1 ]
> > > +    description: |
> > > +      Definition depends on soc version:
> > > +
> > > +      For "allwinner,sun8i-h3-ths",
> > > +      value must be 0.
> > > +      For all other compatibles
> > > +      value must be 1.
> >
> > This should be checked using an if as well.
> >
> > > +
> > > +  nvmem-cells:
> > > +    maxItems: 1
> > > +    items:
> > > +      - description: Calibration data for thermal sensors
> >
> > You can drop the items and just move the description up one level,
> > under nvmem-cells
> >
> > > +
> > > +  nvmem-cell-names:
> > > +    items:
> > > +      - const: calibration
> >
> > Ditto for the const
> >
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - '#thermal-sensor-cells'
> >
> > Whether clocks, clock-names and resets are thereshould be check using
> > an if statement as well.
> >
> > > +
> > > +examples:
> > > +  - |
> > > +    ths_a83t: ths@1f04000 {
> >
> > You don't need the label at all, and the node name should be
> > temperature-sensor according to the DT spec, not ths. This applies to
> > all you examples.
> >
> > > +         compatible =3D "allwinner,sun8i-a83t-ths";
> > > +         reg =3D <0x01f04000 0x100>;
> > > +         interrupts =3D <0 31 0>;
> > > +         nvmem-cells =3D <&ths_calibration>;
> > > +         nvmem-cell-names =3D "calibration";
> > > +         #thermal-sensor-cells =3D <1>;
> > > +    };
> >
> > New line.
> >
> > Thanks!
> > Maxime
>
>