On 12/07/2024 14:43, Satya Priya Kakitapalli wrote: > Add bindings support for the MBG Temp alarm peripheral found on > pm8775 pmics. > > Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@xxxxxxxxxxx> > --- > .../bindings/thermal/qcom-spmi-mbg-tm.yaml | 63 ++++++++++++++++++++++ A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > 1 file changed, 63 insertions(+) > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml > new file mode 100644 > index 000000000000..9b6d1bc34a11 > --- /dev/null > +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/thermal/qcom-spmi-mbg-tm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies, Inc. SPMI PMIC MBG Thermal Monitoring > + > +maintainers: > + - Satya Priya Kakitapalli <quic_skakitap@xxxxxxxxxxx> > + > +description: | Do not need '|' unless you need to preserve formatting. > + Qualcomm's thermal driver for the MBG thermal monitoring device. Driver as Linux driver? Instead please describe the hardware. Missing $ref to thermal-sensor. > + > +properties: > + compatible: > + const: qcom,spmi-mbg-tm Instead use SoC specific compatible. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + io-channels: Missing constraints. Use items with description. > + description: > + IIO channel specifier for the ADC channel, which reports And drop redundant part - "IIO channel specifier for". This cannot be anything else. > + chip die temperature. > + > + io-channel-names: > + const: thermal > + > + "#thermal-sensor-cells": > + const: 0 > + > +required: > + - compatible > + - reg > + - interrupts > + - io-channels > + - io-channel-names > + - "#thermal-sensor-cells" And this won't be needed. > + > +additionalProperties: false unevaluatedProperties instead > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h> > + spmi_bus { Eh... No. Is this really directly on SPMI bus? Anyway, use correct node names. > + #address-cells = <1>; > + #size-cells = <0>; > + > + pm8775_sail_1_tz: qcom,mbg-tm@d700 { Oh no, please don't bring downstream crap. Do you see any node called like this? Also, drop unused label. > + compatible = "qcom,spmi-mbg-tm"; > + reg = <0xd700>; > + interrupts = <0x1 0xd7 0x0 IRQ_TYPE_EDGE_RISING>; This suggests it is not on SPMI bus but part of PMIC. Why doing something entirely different then entire Linux kernel? Do not use downstream as template, that's a no go. Best regards, Krzysztof
