Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7522b13d-ad50-f2bc-1242-97a23d15c3b1@linaro.org>
Date:   Fri, 28 Oct 2022 12:58:11 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.1
Subject: Re: [PATCH 3/4] dt-bindings: thermal: Add qcom,qmi-tmd-device and
 qcom,tmd-device yaml bindings
To:     Bhupesh Sharma <bhupesh.sharma@linaro.org>,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org
Cc:     agross@kernel.org, linux-arm-msm@vger.kernel.org,
        daniel.lezcano@linaro.org, robh@kernel.org, andersson@kernel.org,
        rafael@kernel.org, bhupesh.linux@gmail.com
References: <20220912085049.3517140-1-bhupesh.sharma@linaro.org>
 <20220912085049.3517140-4-bhupesh.sharma@linaro.org>
Content-Language: en-US
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20220912085049.3517140-4-bhupesh.sharma@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/09/2022 04:50, Bhupesh Sharma wrote:
> Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings.
> 
> Qualcomm QMI based TMD cooling device(s) are used for various
> mitigations for remote subsystem(s) including remote processor
> mitigation, rail voltage restriction etc.
> 
> Each child node represents one remote subsystem and each child
> of this subsystem in-turn represents separate TMD cooling device.
> 
> Cc: daniel.lezcano@linaro.org
> Cc: rafael@kernel.org
> Cc: andersson@kernel.org
> Cc: robh@kernel.org
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  .../bindings/thermal/qcom,qmi-tmd-device.yaml |  78 +++++++++++
>  .../bindings/thermal/qcom,tmd-device.yaml     | 122 ++++++++++++++++++
>  include/dt-bindings/thermal/qcom,tmd.h        |  14 ++

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

>  3 files changed, 214 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
>  create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
>  create mode 100644 include/dt-bindings/thermal/qcom,tmd.h
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> new file mode 100644
> index 000000000000..dfda5b611a93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,qmi-tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes.

> +
> +title: Qualcomm QMI based thermal mitigation (TMD) cooling devices.
> +
> +maintainers:
> +  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
> +
> +description:
> +  Qualcomm QMI based TMD cooling device(s) are used for various
> +  mitigations for remote subsystem(s) including remote processor
> +  mitigation, rail voltage restriction etc.
> +
> +properties:
> +  $nodename:
> +    const: qmi-tmd-devices

Why enforcing node name?
> +
> +  compatible:
> +    items:
> +      - const: qcom,qmi-tmd-devices
> +
> +  modem0:

Why 0? Do you think it will grow for more devices?

> +    $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +  adsp:
> +    $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +  cdsp:
> +    $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +  slpi:
> +    $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/thermal/qcom,tmd.h>
> +    qmi-tmd-devices {
> +      compatible = "qcom,qmi-tmd-devices";
> +
> +      modem0 {
> +        qcom,instance-id = <MODEM0_INSTANCE_ID>;
> +
> +        modem0_pa: tmd-device0 {
> +          label = "pa";
> +          #cooling-cells = <2>;
> +        };
> +
> +        modem0_proc: tmd-device1 {
> +          label = "modem";
> +          #cooling-cells = <2>;
> +        };
> +
> +        modem0_current: tmd-device2 {
> +          label = "modem_current";
> +          #cooling-cells = <2>;
> +        };
> +
> +        modem0_skin: tmd-device3 {
> +          label = "modem_skin";
> +          #cooling-cells = <2>;
> +        };
> +
> +        modem0_vdd: tmd-device4 {
> +          label = "cpuv_restriction_cold";
> +          #cooling-cells = <2>;
> +        };
> +      };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> new file mode 100644
> index 000000000000..38ac62f03376
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm thermal mitigation (TMD) cooling devices
> +
> +maintainers:
> +  - Bhupesh Sharma <bhupesh.sharma@linaro.org>
> +
> +description:
> +  Qualcomm thermal mitigation (TMD) cooling devices. Each child node
> +  represents one remote subsystem and each child of this subsystem in-turn
> +  represents separate cooling devices.
> +
> +properties:
> +  $nodename:
> +    pattern: "^(modem|adsp|cdsp|slpi[0-9])?$"

No need to enforce node name.

> +
> +  qcom,instance-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Remote subsystem QMI server instance id to be used for communicating with QMI.
> +
> +patternProperties:
> +  "^tmd-device[0-9]?$":
> +    type: object
> +    description:
> +      Subnodes indicating tmd cooling device of a specific category.
> +    properties:
> +      label:
> +        maxItems: 1
> +        description: |
> +          Remote subsystem device identifier. Acceptable device names -
> +          "pa" -> for pa cooling device,
> +          "cpuv_restriction_cold" -> for vdd restriction,
> +          "cx_vdd_limit" -> for vdd limit,
> +          "modem" -> for processor passive cooling device,
> +          "modem_current" -> for current limiting device,
> +          "modem_bw" ->  for bus bandwidth limiting device,
> +          "cpr_cold" -> for cpr restriction.

Why these are acceptable and others are not? What does it mean here the
"acceptable"?

Best regards,
Krzysztof