linux-kernel - Re: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Move auxdac binding to yaml

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230602-pacify-dares-0989f2c759b2@spud>
Date:   Fri, 2 Jun 2023 18:53:28 +0100
From:   Conor Dooley <conor@...nel.org>
To:     matthias.bgg@...nel.org
Cc:     rafael@...nel.org, Daniel Lezcano <daniel.lezcano@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        devicetree@...r.kernel.org, Amit Kucheria <amitk@...nel.org>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mediatek@...ts.infradead.org,
        Zhang Rui <rui.zhang@...el.com>,
        linux-arm-kernel@...ts.infradead.org,
        Matthias Brugger <matthias.bgg@...il.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Move auxdac
 binding to yaml

On Thu, Jun 01, 2023 at 03:44:24PM +0200, matthias.bgg@...nel.org wrote:
> From: Matthias Brugger <matthias.bgg@...il.com>
> 
> Convert the older binding to yaml syntax.
> The thermal IP has several sensors, to reflect that
> thermal-sensors-cells is set to '1'. Apart optional regulator
> for bank supply wasn't part of the old binding description,
> this patch adds them.
> 
> Signed-off-by: Matthias Brugger <matthias.bgg@...il.com>
> 
> ---
> 
> Changes in v2:
> - drop auxadc and apmixedsys nodes
> - maxItems for clocks
> - drop obvious description of reset
> - mention optional regulators in commit message
> - mention thermal-sensors-cells change in commit message
> 
>  .../bindings/thermal/mediatek,thermal.yaml    | 152 ++++++++++++++++++
>  .../bindings/thermal/mediatek-thermal.txt     |  52 ------
>  2 files changed, 152 insertions(+), 52 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml
>  delete mode 100644 Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
> 
> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml
> new file mode 100644
> index 000000000000..bbc9c2935da0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml
> @@ -0,0 +1,152 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/mediatek,thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Thermal Sensor
> +
> +maintainers:
> +  - Matthias Brugger <matthias.bgg@...il.com>
> +
> +description: |
                ^
You don't need this btw, unless you have formatting to preserve.

> +  The MediaTek thermal controller measures the on-SoC temperatures.
> +  This device does not have its own ADC, instead it directly controls
> +  the AUXADC via AHB bus accesses. For this reason this device needs
> +  phandles to the AUXADC. Also it controls a mux in the apmixedsys
> +  register space via AHB bus accesses, so a phandle to the APMIXEDSYS
> +  is also needed.

> +  reset-names:
> +    items:
> +      - const: therm

> +  nvmem-cell-names:
> +    items:
> +      - const: calibration-data

You don't need the "items: - const:", "const:" alone is sufficient.

> +
> +  mediatek,auxadc:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle to the AUXADC which the thermal controller uses...
...to read the on-SoC temperatures as it does not have its own ADC.

Moving the information out of the description down here seems like it
would be better than what the original text binding does. Sorry for not
point that out last time around.

> +
> +  mediatek,apmixedsys:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle to the APMIXEDSYS controller...

Ideally the information about the purpose of the mux would go here, but
I don't know the purpose of it :)

> +  "#thermal-sensor-cells":
> +    const: 1
> +
> +  bank0-supply:
> +    description: Regulator supplying voltage to the first bank

Since I am an eejit, first bank of what? Sensors?

Otherwise, LGTM.

Cheers,
Conor.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)