Re: [PATCH 1/2] dt-bindings: thermal: qcom: add pm8901-temp-alarm

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The binding introduces a child node without updating the parent MFD schema to allow it, and the provided device tree example violates parent requirements.
- [Medium] Meaningful mismatch between the commit message and the actual binding implementation regarding interrupts and parent references.
- [Low] The binding defines a thermal sensor provider but fails to reference the core `thermal-sensor.yaml` schema.
--

commit 547082ca486fab0b441c0b967105187e700c4d9c
Author: Herman van Hazendonk <github.com@herrie.org>

dt-bindings: thermal: qcom: add pm8901-temp-alarm

This commit adds a device tree binding for the temperature-alarm block
inside the Qualcomm PM8901 PMIC. The driver exposes the PM8901
die-temperature trip stages to the thermal framework via a thermal-zone.

> The binding describes the SSBI sub-node address, the GIC interrupt
> the alarm raises on a stage transition, and the parent PMIC
> reference.

[Severity: Medium]
Does the commit message match the binding implementation? The message
states it describes the GIC interrupt and parent PMIC reference, but the
schema actually defines PMIC-internal interrupts (indices 52 and 53), and
relies purely on the standard device tree parent-child hierarchy rather
than a parent PMIC reference property.

> diff --git a/Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml b/Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml
> new file mode 100644
> index 0000000000000..569943b4aedc5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,pm8901-temp-alarm.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: qcom,pm8901-temp-alarm

[Severity: Low]
Should this binding include an allOf block containing a $ref to
/schemas/thermal/thermal-sensor.yaml#?

Standard device tree bindings for thermal sensors usually include this
reference to inherit core schema definitions. By omitting it and defining
additionalProperties: false, the schema will reject future common thermal
sensor properties.

[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    ssbi {
> +      pmic {
> +        compatible = "qcom,pm8901";
> +        #interrupt-cells = <2>;
> +        interrupt-controller;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pm8901_temp: temp-alarm@23 {

[Severity: Medium]
Will this pass dtbs_check and dt_binding_check?

The qcom,pm8901 parent schema in qcom-pm8xxx.yaml strictly prohibits
undocumented child nodes via additionalProperties: false and requires an
interrupts property on the pmic node.

It seems the qcom,pm8901-temp-alarm node was added without adding a
corresponding temp-alarm@[0-9a-f]+$ pattern to the parent schema.
Additionally, the pmic node in this example lacks the mandatory
interrupts property.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780148149.git.github.com@herrie.org?part=1