[PATCH 0/5] Add support for MBG Thermal monitoring device

[PATCH 0/5] Add support for MBG Thermal monitoring device

[Satya Priya Kakitapalli]

Add bindings, driver and DT for the Qualcomm's MBG thermal
monitoring device.

Please note that this series is dependent on [1] which adds
ADC5-GEN3 support.

[1] https://lore.kernel.org/linux-iio/20231231171237.3322376-1-quic_jprakash@quicinc.com/

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
Satya Priya Kakitapalli (5):
      dt-bindings: iio: adc: Add ADC5 GEN3 Channel info for pm8775 PMIC
      dt-bindings: thermal: qcom: Add MBG thermal monitor bindings
      thermal: qcom: Add support for MBG Temp monitor
      ARM: dts: qcom: Add vadc support for pm8775 pmic on SA8775P
      ARM: dts: qcom: Add support for MBG TM for pm8775 on SA8775P

 .../bindings/thermal/qcom-spmi-mbg-tm.yaml         |  63 +++++
 arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi        | 210 ++++++++++++++++
 drivers/thermal/qcom/Kconfig                       |  11 +
 drivers/thermal/qcom/Makefile                      |   1 +
 drivers/thermal/qcom/qcom-spmi-mbg-tm.c            | 269 +++++++++++++++++++++
 .../iio/adc/qcom,spmi-adc5-gen3-pm8775.h           |  42 ++++
 6 files changed, 596 insertions(+)
---
base-commit: c27723304c1f6af79f7bece5edacace6a8d46167
change-id: 20240627-mbg-tm-support-7bbf25c246e1

Best regards,
-- 
Satya Priya Kakitapalli <quic_skakitap@quicinc.com>

[PATCH 1/5] dt-bindings: iio: adc: Add ADC5 GEN3 Channel info for pm8775 PMIC

[Satya Priya Kakitapalli]

Add definitions for ADC5 GEN3 virtual channels(combination of ADC channel
number and PMIC SID number) used by PM8775.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 .../iio/adc/qcom,spmi-adc5-gen3-pm8775.h           | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h b/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h
new file mode 100644
index 000000000000..84ab07ed73cc
--- /dev/null
+++ b/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PM8775_H
+#define _DT_BINDINGS_QCOM_SPMI_VADC_PM8775_H
+
+#include <dt-bindings/iio/adc/qcom,spmi-vadc.h>
+
+/* ADC channels for PM8775_ADC for PMIC5 Gen3 */
+#define PM8775_ADC5_GEN3_REF_GND(sid)			((sid) << 8 | ADC5_GEN3_REF_GND)
+#define PM8775_ADC5_GEN3_1P25VREF(sid)			((sid) << 8 | ADC5_GEN3_1P25VREF)
+#define PM8775_ADC5_GEN3_VREF_VADC(sid)			((sid) << 8 | ADC5_GEN3_VREF_VADC)
+#define PM8775_ADC5_GEN3_DIE_TEMP(sid)			((sid) << 8 | ADC5_GEN3_DIE_TEMP)
+
+#define PM8775_ADC5_GEN3_AMUX1_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX1_THM)
+#define PM8775_ADC5_GEN3_AMUX2_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX2_THM)
+#define PM8775_ADC5_GEN3_AMUX3_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX3_THM)
+#define PM8775_ADC5_GEN3_AMUX4_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX4_THM)
+#define PM8775_ADC5_GEN3_AMUX5_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX5_THM)
+#define PM8775_ADC5_GEN3_AMUX6_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX6_THM)
+#define PM8775_ADC5_GEN3_AMUX1_GPIO9(sid)		((sid) << 8 | ADC5_GEN3_AMUX1_GPIO)
+#define PM8775_ADC5_GEN3_AMUX2_GPIO10(sid)		((sid) << 8 | ADC5_GEN3_AMUX2_GPIO)
+#define PM8775_ADC5_GEN3_AMUX3_GPIO11(sid)		((sid) << 8 | ADC5_GEN3_AMUX3_GPIO)
+#define PM8775_ADC5_GEN3_AMUX4_GPIO12(sid)		((sid) << 8 | ADC5_GEN3_AMUX4_GPIO)
+
+/* 100k pull-up2 */
+#define PM8775_ADC5_GEN3_AMUX1_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX1_THM_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX2_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX2_THM_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX3_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX3_THM_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX4_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX4_THM_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX5_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX5_THM_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX6_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX6_THM_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX1_GPIO9_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX1_GPIO_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX2_GPIO10_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX2_GPIO_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX3_GPIO11_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX3_GPIO_100K_PU)
+#define PM8775_ADC5_GEN3_AMUX4_GPIO12_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX4_GPIO_100K_PU)
+
+#define PM8775_ADC5_GEN3_VPH_PWR(sid)			((sid) << 8 | ADC5_GEN3_VPH_PWR)
+
+#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PM8775_H */

-- 
2.25.1

Re: [PATCH 1/5] dt-bindings: iio: adc: Add ADC5 GEN3 Channel info for pm8775 PMIC

[Krzysztof Kozlowski]

On 12/07/2024 14:43, Satya Priya Kakitapalli wrote:
> Add definitions for ADC5 GEN3 virtual channels(combination of ADC channel
> number and PMIC SID number) used by PM8775.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
>  .../iio/adc/qcom,spmi-adc5-gen3-pm8775.h           | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 

This is part of binding patch, so squash it.


Best regards,
Krzysztof

Re: [PATCH 1/5] dt-bindings: iio: adc: Add ADC5 GEN3 Channel info for pm8775 PMIC

[Jonathan Cameron]

On Fri, 12 Jul 2024 18:13:28 +0530
Satya Priya Kakitapalli <quic_skakitap@quicinc.com> wrote:

> Add definitions for ADC5 GEN3 virtual channels(combination of ADC channel
> number and PMIC SID number) used by PM8775.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
as I presume this will go with the rest via the thermal tree.

> ---
>  .../iio/adc/qcom,spmi-adc5-gen3-pm8775.h           | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h b/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h
> new file mode 100644
> index 000000000000..84ab07ed73cc
> --- /dev/null
> +++ b/include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PM8775_H
> +#define _DT_BINDINGS_QCOM_SPMI_VADC_PM8775_H
> +
> +#include <dt-bindings/iio/adc/qcom,spmi-vadc.h>
> +
> +/* ADC channels for PM8775_ADC for PMIC5 Gen3 */
> +#define PM8775_ADC5_GEN3_REF_GND(sid)			((sid) << 8 | ADC5_GEN3_REF_GND)
> +#define PM8775_ADC5_GEN3_1P25VREF(sid)			((sid) << 8 | ADC5_GEN3_1P25VREF)
> +#define PM8775_ADC5_GEN3_VREF_VADC(sid)			((sid) << 8 | ADC5_GEN3_VREF_VADC)
> +#define PM8775_ADC5_GEN3_DIE_TEMP(sid)			((sid) << 8 | ADC5_GEN3_DIE_TEMP)
> +
> +#define PM8775_ADC5_GEN3_AMUX1_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX1_THM)
> +#define PM8775_ADC5_GEN3_AMUX2_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX2_THM)
> +#define PM8775_ADC5_GEN3_AMUX3_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX3_THM)
> +#define PM8775_ADC5_GEN3_AMUX4_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX4_THM)
> +#define PM8775_ADC5_GEN3_AMUX5_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX5_THM)
> +#define PM8775_ADC5_GEN3_AMUX6_THM(sid)			((sid) << 8 | ADC5_GEN3_AMUX6_THM)
> +#define PM8775_ADC5_GEN3_AMUX1_GPIO9(sid)		((sid) << 8 | ADC5_GEN3_AMUX1_GPIO)
> +#define PM8775_ADC5_GEN3_AMUX2_GPIO10(sid)		((sid) << 8 | ADC5_GEN3_AMUX2_GPIO)
> +#define PM8775_ADC5_GEN3_AMUX3_GPIO11(sid)		((sid) << 8 | ADC5_GEN3_AMUX3_GPIO)
> +#define PM8775_ADC5_GEN3_AMUX4_GPIO12(sid)		((sid) << 8 | ADC5_GEN3_AMUX4_GPIO)
> +
> +/* 100k pull-up2 */
> +#define PM8775_ADC5_GEN3_AMUX1_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX1_THM_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX2_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX2_THM_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX3_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX3_THM_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX4_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX4_THM_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX5_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX5_THM_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX6_THM_100K_PU(sid)		((sid) << 8 | ADC5_GEN3_AMUX6_THM_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX1_GPIO9_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX1_GPIO_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX2_GPIO10_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX2_GPIO_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX3_GPIO11_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX3_GPIO_100K_PU)
> +#define PM8775_ADC5_GEN3_AMUX4_GPIO12_100K_PU(sid)	((sid) << 8 | ADC5_GEN3_AMUX4_GPIO_100K_PU)
> +
> +#define PM8775_ADC5_GEN3_VPH_PWR(sid)			((sid) << 8 | ADC5_GEN3_VPH_PWR)
> +
> +#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PM8775_H */
> 

[PATCH 2/5] dt-bindings: thermal: qcom: Add MBG thermal monitor bindings

[Satya Priya Kakitapalli]

Add bindings support for the MBG Temp alarm peripheral found on
pm8775 pmics.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 63 ++++++++++++++++++++++
 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@quicinc.com>
+
+description: |
+  Qualcomm's thermal driver for the MBG thermal monitoring device.
+
+properties:
+  compatible:
+    const: qcom,spmi-mbg-tm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  io-channels:
+    description:
+      IIO channel specifier for the ADC channel, which reports
+      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"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h>
+    spmi_bus {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pm8775_sail_1_tz: qcom,mbg-tm@d700 {
+        compatible = "qcom,spmi-mbg-tm";
+        reg = <0xd700>;
+        interrupts = <0x1 0xd7 0x0 IRQ_TYPE_EDGE_RISING>;
+        io-channels = <&pm8775_1_adc PM8775_ADC5_GEN3_DIE_TEMP(1)>;
+        io-channel-names = "thermal";
+        #thermal-sensor-cells = <0>;
+      };
+    };
+...

-- 
2.25.1

Re: [PATCH 2/5] dt-bindings: thermal: qcom: Add MBG thermal monitor bindings

[Rob Herring (Arm)]

On Fri, 12 Jul 2024 18:13:29 +0530, 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@quicinc.com>
> ---
>  .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 63 ++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
In file included from Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.example.dts:25:
./scripts/dtc/include-prefixes/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h:9:10: fatal error: dt-bindings/iio/adc/qcom,spmi-vadc.h: No such file or directory
    9 | #include <dt-bindings/iio/adc/qcom,spmi-vadc.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.lib:427: Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240712-mbg-tm-support-v1-2-7d78bec920ca@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 2/5] dt-bindings: thermal: qcom: Add MBG thermal monitor bindings

[Krzysztof Kozlowski]

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@quicinc.com>
> ---
>  .../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@quicinc.com>
> +
> +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

Re: [PATCH 2/5] dt-bindings: thermal: qcom: Add MBG thermal monitor bindings

[Konrad Dybcio]

On 12.07.2024 7:25 PM, Krzysztof Kozlowski wrote:
> 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@quicinc.com>
>> ---

Please also describe what MBG stands for and how it differs from the
currently supported temp alarm that's been in use for the past 10 years
on various PMICs

Konrad

Re: [PATCH 2/5] dt-bindings: thermal: qcom: Add MBG thermal monitor bindings

[Dmitry Baryshkov]

On Fri, Jul 12, 2024 at 06:13:29PM GMT, 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@quicinc.com>
> ---
>  .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 63 ++++++++++++++++++++++
>  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@quicinc.com>
> +
> +description: |
> +  Qualcomm's thermal driver for the MBG thermal monitoring device.

I was hoping for the binding to tell me, what is MBG. But they don't.
Could you please fix that?

> +
> +properties:
> +  compatible:
> +    const: qcom,spmi-mbg-tm

-- 
With best wishes
Dmitry

Re: [PATCH 2/5] dt-bindings: thermal: qcom: Add MBG thermal monitor bindings

[Satya Priya Kakitapalli]

On 7/13/2024 9:44 PM, Dmitry Baryshkov wrote:
> On Fri, Jul 12, 2024 at 06:13:29PM GMT, 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@quicinc.com>
>> ---
>>   .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 63 ++++++++++++++++++++++
>>   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@quicinc.com>
>> +
>> +description: |
>> +  Qualcomm's thermal driver for the MBG thermal monitoring device.
> I was hoping for the binding to tell me, what is MBG. But they don't.
> Could you please fix that?


Sure, I'll add the description and post v2 patches.


>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,spmi-mbg-tm

[PATCH 3/5] thermal: qcom: Add support for MBG Temp monitor

[Satya Priya Kakitapalli]

Add driver for the MBG thermal monitoring device. It monitors
the die temperature, and when there is a level 1 upper threshold
violation, it receives an interrupt over spmi. The driver reads
the fault status register and notifies thermal accordingly.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 drivers/thermal/qcom/Kconfig            |  11 ++
 drivers/thermal/qcom/Makefile           |   1 +
 drivers/thermal/qcom/qcom-spmi-mbg-tm.c | 269 ++++++++++++++++++++++++++++++++
 3 files changed, 281 insertions(+)

diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
index 2c7f3f9a26eb..46045094020c 100644
--- a/drivers/thermal/qcom/Kconfig
+++ b/drivers/thermal/qcom/Kconfig
@@ -21,6 +21,17 @@ config QCOM_SPMI_ADC_TM5
 	  Thermal client sets threshold temperature for both warm and cool and
 	  gets updated when a threshold is reached.
 
+config QCOM_SPMI_MBG_TM
+	tristate "Qualcomm Technologies, Inc. SPMI PMIC MBG Temperature monitor"
+	depends on OF && SPMI && IIO
+	select REGMAP_SPMI
+	help
+	 This enables a thermal driver for the MBG thermal monitoring device.
+	 It shows up in sysfs as a thermal sensor with two trip points.
+	 It notifies the thermal framework when level 1 high threshold is
+	 violated. The temperature reported by the thermal sensor reflects
+	 the real time die temperature through ADC channel.
+
 config QCOM_SPMI_TEMP_ALARM
 	tristate "Qualcomm SPMI PMIC Temperature Alarm"
 	depends on OF && SPMI && IIO
diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
index 0fa2512042e7..bc18e08ee3e2 100644
--- a/drivers/thermal/qcom/Makefile
+++ b/drivers/thermal/qcom/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_QCOM_TSENS)	+= qcom_tsens.o
 qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
 				   tsens-8960.o
 obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
+obj-$(CONFIG_QCOM_SPMI_MBG_TM)	+= qcom-spmi-mbg-tm.o
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_QCOM_LMH)		+= lmh.o
diff --git a/drivers/thermal/qcom/qcom-spmi-mbg-tm.c b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
new file mode 100644
index 000000000000..70964ea5a48d
--- /dev/null
+++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+#include <linux/iio/consumer.h>
+
+#include "../thermal_core.h"
+
+#define MBG_TEMP_MON_MM_MON2_FAULT_STATUS	0x50
+
+#define MON_FAULT_STATUS_MASK			GENMASK(7, 6)
+#define MON_FAULT_STATUS_SHIFT			6
+#define MON2_LVL1_ERR				0x1
+
+#define MON2_LVL1_UP_THRESH			0x59
+
+#define MBG_TEMP_MON_MM_MON2_MISC_CFG		0x5f
+#define UP_THRESH_EN				BIT(1)
+
+#define STEP_MV					8
+#define MBG_DEFAULT_TEMP_MV			600
+#define MBG_TEMP_CONSTANT			1000
+#define MIN_TRIP_TEMP				25000
+#define MAX_SUPPORTED_TEMP			160000
+
+struct mbg_tm_chip {
+	struct regmap			*map;
+	struct device			*dev;
+	struct thermal_zone_device	*tz_dev;
+	struct mutex                    lock;
+	unsigned int			base;
+	int				irq;
+	int				last_temp;
+	bool				last_temp_set;
+	struct iio_channel		*adc;
+};
+
+struct mbg_map_table {
+	int min_temp;
+	int vtemp0;
+	int tc;
+};
+
+static const struct mbg_map_table map_table[] = {
+	/* minT	vtemp0	tc */
+	{ -60000, 4337, 1967 },
+	{ -40000, 4731, 1964 },
+	{ -20000, 5124, 1957  },
+	{ 0,      5515, 1949 },
+	{ 20000,  5905, 1940 },
+	{ 40000,  6293, 1930 },
+	{ 60000,  6679, 1921 },
+	{ 80000,  7064, 1910 },
+	{ 100000, 7446, 1896 },
+	{ 120000, 7825, 1878 },
+	{ 140000, 8201, 1859 },
+};
+
+static int mbg_tm_read(struct mbg_tm_chip *chip, u16 addr, int *data)
+{
+	return regmap_read(chip->map, chip->base + addr, data);
+}
+
+static int mbg_tm_write(struct mbg_tm_chip *chip, u16 addr, int data)
+{
+	return regmap_write(chip->map, chip->base + addr, data);
+}
+
+static int mbg_tm_reg_update(struct mbg_tm_chip *chip, u16 addr, u8 mask, u8 val)
+{
+	return regmap_write_bits(chip->map, chip->base + addr, mask, val);
+}
+
+static int mbg_tm_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
+	int ret, milli_celsius;
+
+	if (!temp)
+		return -EINVAL;
+
+	if (chip->last_temp_set) {
+		pr_debug("last_temp: %d\n", chip->last_temp);
+		chip->last_temp_set = false;
+		*temp = chip->last_temp;
+		return 0;
+	}
+
+	ret = iio_read_channel_processed(chip->adc, &milli_celsius);
+	if (ret < 0) {
+		dev_err(chip->dev, "failed to read iio channel %d\n", ret);
+		return ret;
+	}
+
+	*temp = milli_celsius;
+
+	return 0;
+}
+
+static int temp_to_vtemp(int temp)
+{
+
+	int idx, vtemp, tc = 0, t0 = 0, vtemp0 = 0;
+
+	if (temp > MAX_SUPPORTED_TEMP)
+		temp = MAX_SUPPORTED_TEMP - MBG_TEMP_CONSTANT;
+
+	for (idx = 0; idx < ARRAY_SIZE(map_table); idx++)
+		if (temp >= map_table[idx].min_temp &&
+				temp < (map_table[idx].min_temp + 20000)) {
+			tc = map_table[idx].tc;
+			t0 = map_table[idx].min_temp;
+			vtemp0 = map_table[idx].vtemp0;
+			break;
+		}
+
+	/*
+	 * Formula to calculate vtemp(mV) from a given temp
+	 * vtemp = (temp - minT) * tc + vtemp0
+	 * tc, t0 and vtemp0 values are mentioned in the map_table array.
+	 */
+	vtemp = ((temp - t0) * tc + vtemp0 * 100000) / 1000000;
+
+	return abs(vtemp - MBG_DEFAULT_TEMP_MV) / STEP_MV;
+}
+
+static int mbg_tm_set_trip_temp(struct thermal_zone_device *tz, int low_temp,
+						int temp)
+{
+	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
+	int ret = 0, vtemp = 0;
+
+	mutex_lock(&chip->lock);
+
+	/* The HW has a limitation that the trip set must be above 25C */
+	if (temp > MIN_TRIP_TEMP && temp < INT_MAX) {
+		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
+					 UP_THRESH_EN, UP_THRESH_EN);
+		vtemp = temp_to_vtemp(temp);
+		ret = mbg_tm_write(chip, MON2_LVL1_UP_THRESH, vtemp);
+		if (ret < 0) {
+			mutex_unlock(&chip->lock);
+			return ret;
+		}
+	} else {
+		dev_dbg(chip->dev, "Setting %d failed, set trip between 25C and INT_MAX\n", temp);
+		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
+					UP_THRESH_EN, 0);
+	}
+
+	mutex_unlock(&chip->lock);
+
+	/*
+	 * Configure the last_temp one degree higher, to ensure the
+	 * violated temp is returned to thermal framework when it reads
+	 * temperature for the first time after the violation happens.
+	 * This is needed to account for the inaccuracy in the conversion
+	 * formula used which leads to the thermal framework setting back
+	 * the same thresholds in case the temperature it reads does not
+	 * show violation.
+	 */
+	chip->last_temp = temp + MBG_TEMP_CONSTANT;
+
+	return ret;
+}
+
+static const struct thermal_zone_device_ops mbg_tm_ops = {
+	.get_temp = mbg_tm_get_temp,
+	.set_trips = mbg_tm_set_trip_temp,
+};
+
+static irqreturn_t mbg_tm_isr(int irq, void *data)
+{
+	struct mbg_tm_chip *chip = data;
+	int ret;
+	int val = 0;
+
+	mutex_lock(&chip->lock);
+
+	ret = mbg_tm_read(chip, MBG_TEMP_MON_MM_MON2_FAULT_STATUS, &val);
+
+	mutex_unlock(&chip->lock);
+
+	if (ret < 0)
+		return IRQ_HANDLED;
+
+	val &= MON_FAULT_STATUS_MASK;
+	if ((val >> MON_FAULT_STATUS_SHIFT) & MON2_LVL1_ERR) {
+		chip->last_temp_set = true;
+		thermal_zone_device_update(chip->tz_dev,
+					THERMAL_TRIP_VIOLATED);
+		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
+	} else {
+		dev_dbg(chip->dev, "Lvl 1 upper threshold not violated, ignoring interrupt\n");
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int mbg_tm_probe(struct platform_device *pdev)
+{
+	struct mbg_tm_chip *chip;
+	struct device_node *node = pdev->dev.of_node;
+	u32 res;
+	int ret = 0;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = &pdev->dev;
+
+	mutex_init(&chip->lock);
+
+	chip->map = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!chip->map)
+		return -ENXIO;
+
+	ret = of_property_read_u32(node, "reg", &res);
+	if (ret < 0)
+		return ret;
+
+	chip->base = res;
+
+	chip->irq = platform_get_irq(pdev, 0);
+	if (chip->irq < 0)
+		return chip->irq;
+
+	chip->adc = devm_iio_channel_get(&pdev->dev, "thermal");
+	if (IS_ERR(chip->adc))
+		return PTR_ERR(chip->adc);
+
+	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev,
+				 0, chip, &mbg_tm_ops);
+	if (IS_ERR(chip->tz_dev)) {
+		dev_err(&pdev->dev, "failed to register sensor\n");
+		return PTR_ERR(chip->tz_dev);
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, chip->irq, NULL,
+			mbg_tm_isr, IRQF_ONESHOT, node->name, chip);
+
+	return ret;
+}
+
+static const struct of_device_id mbg_tm_match_table[] = {
+	{ .compatible = "qcom,spmi-mbg-tm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mbg_tm_match_table);
+
+static struct platform_driver mbg_tm_driver = {
+	.driver = {
+		.name = "qcom-spmi-mbg-tm",
+		.of_match_table = mbg_tm_match_table,
+	},
+	.probe  = mbg_tm_probe,
+};
+module_platform_driver(mbg_tm_driver);
+
+MODULE_DESCRIPTION("PMIC MBG Temperature monitor driver");
+MODULE_LICENSE("GPL");

-- 
2.25.1

Re: [PATCH 3/5] thermal: qcom: Add support for MBG Temp monitor

[Jonathan Cameron]

On Fri, 12 Jul 2024 18:13:30 +0530
Satya Priya Kakitapalli <quic_skakitap@quicinc.com> wrote:

> Add driver for the MBG thermal monitoring device. It monitors
> the die temperature, and when there is a level 1 upper threshold
> violation, it receives an interrupt over spmi. The driver reads
> the fault status register and notifies thermal accordingly.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>

Hi Satya,

Some comments inline.

Jonathan

> diff --git a/drivers/thermal/qcom/qcom-spmi-mbg-tm.c b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> new file mode 100644
> index 000000000000..70964ea5a48d
> --- /dev/null
> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/thermal.h>
> +#include <linux/iio/consumer.h>
> +
> +#include "../thermal_core.h"
> +
> +#define MBG_TEMP_MON_MM_MON2_FAULT_STATUS	0x50
> +
> +#define MON_FAULT_STATUS_MASK			GENMASK(7, 6)
> +#define MON_FAULT_STATUS_SHIFT			6
Use FIELD_GET() and FIELD_PREP() then no need to have shift defined.

> +#define MON2_LVL1_ERR				0x1
> +
> +#define MON2_LVL1_UP_THRESH			0x59
> +
> +#define MBG_TEMP_MON_MM_MON2_MISC_CFG		0x5f
> +#define UP_THRESH_EN				BIT(1)
> +
> +#define STEP_MV					8
> +#define MBG_DEFAULT_TEMP_MV			600
> +#define MBG_TEMP_CONSTANT			1000
> +#define MIN_TRIP_TEMP				25000
> +#define MAX_SUPPORTED_TEMP			160000
these are all device specific but don't sound it. I'd prefix them with MBG_TEMP


> +
> +static int mbg_tm_set_trip_temp(struct thermal_zone_device *tz, int low_temp,
> +						int temp)
> +{
> +	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
> +	int ret = 0, vtemp = 0;
> +
> +	mutex_lock(&chip->lock);

	guard(mutex)(&chip->lock);


As then you don't need to carefully unlock as it will be done at exit of scope.

> +
> +	/* The HW has a limitation that the trip set must be above 25C */
> +	if (temp > MIN_TRIP_TEMP && temp < INT_MAX) {
		int vtemp;

reduce the scope and it becomes clear no need to init.

> +		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
> +					 UP_THRESH_EN, UP_THRESH_EN);
> +		vtemp = temp_to_vtemp(temp);
> +		ret = mbg_tm_write(chip, MON2_LVL1_UP_THRESH, vtemp);
> +		if (ret < 0) {
> +			mutex_unlock(&chip->lock);
> +			return ret;
> +		}
> +	} else {
> +		dev_dbg(chip->dev, "Setting %d failed, set trip between 25C and INT_MAX\n", temp);
> +		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
> +					UP_THRESH_EN, 0);
> +	}
> +
> +	mutex_unlock(&chip->lock);
> +
> +	/*
> +	 * Configure the last_temp one degree higher, to ensure the
> +	 * violated temp is returned to thermal framework when it reads
> +	 * temperature for the first time after the violation happens.
> +	 * This is needed to account for the inaccuracy in the conversion
> +	 * formula used which leads to the thermal framework setting back
> +	 * the same thresholds in case the temperature it reads does not
> +	 * show violation.
> +	 */
> +	chip->last_temp = temp + MBG_TEMP_CONSTANT;
> +
> +	return ret;
> +}
> +
> +static const struct thermal_zone_device_ops mbg_tm_ops = {
> +	.get_temp = mbg_tm_get_temp,
> +	.set_trips = mbg_tm_set_trip_temp,
> +};
> +
> +static irqreturn_t mbg_tm_isr(int irq, void *data)
> +{
> +	struct mbg_tm_chip *chip = data;
> +	int ret;
> +	int val = 0;
> +
> +	mutex_lock(&chip->lock);
> +
> +	ret = mbg_tm_read(chip, MBG_TEMP_MON_MM_MON2_FAULT_STATUS, &val);
> +
> +	mutex_unlock(&chip->lock);
	scoped_guard(mutex, &chip->lock) {
		ret = mbg...
		if (ret < 0)
			return IRQ_HANDLED;
	}
avoids need to unlock before return and to me at least gives slightly more
readable code.

> +
> +	if (ret < 0)
> +		return IRQ_HANDLED;
> +
> +	val &= MON_FAULT_STATUS_MASK;
> +	if ((val >> MON_FAULT_STATUS_SHIFT) & MON2_LVL1_ERR) {

as above use FIELD_GET() with the mask - it is more readable and
does the mask and shift in one call.



> +		chip->last_temp_set = true;
> +		thermal_zone_device_update(chip->tz_dev,
> +					THERMAL_TRIP_VIOLATED);

I think the above is 80 chars on one line so no need to wrap.

> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
> +	} else {
> +		dev_dbg(chip->dev, "Lvl 1 upper threshold not violated, ignoring interrupt\n");
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mbg_tm_probe(struct platform_device *pdev)
> +{
> +	struct mbg_tm_chip *chip;
> +	struct device_node *node = pdev->dev.of_node;
> +	u32 res;
> +	int ret = 0;
It's always set, so don't intiialize here.

> +
> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	chip->dev = &pdev->dev;
> +
> +	mutex_init(&chip->lock);
> +
> +	chip->map = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!chip->map)
> +		return -ENXIO;
> +
> +	ret = of_property_read_u32(node, "reg", &res);

I'm not sure on thermal subsystem opinion on this but I'd use
the property.h generic firmware property reading stuff rather than of
specific.  
device_property_read_u32()

> +	if (ret < 0)
> +		return ret;
> +
> +	chip->base = res;
> +
> +	chip->irq = platform_get_irq(pdev, 0);
> +	if (chip->irq < 0)
> +		return chip->irq;
> +
> +	chip->adc = devm_iio_channel_get(&pdev->dev, "thermal");
> +	if (IS_ERR(chip->adc))
> +		return PTR_ERR(chip->adc);
> +
> +	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev,
> +				 0, chip, &mbg_tm_ops);
> +	if (IS_ERR(chip->tz_dev)) {
> +		dev_err(&pdev->dev, "failed to register sensor\n");
> +		return PTR_ERR(chip->tz_dev);

		return dev_err_probe(...)

> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, chip->irq, NULL,
> +			mbg_tm_isr, IRQF_ONESHOT, node->name, chip);
> +
> +	return ret;

	return devm_request...

> +}

Re: [PATCH 3/5] thermal: qcom: Add support for MBG Temp monitor

[Satya Priya Kakitapalli]

On 7/13/2024 5:29 PM, Jonathan Cameron wrote:
> On Fri, 12 Jul 2024 18:13:30 +0530
> Satya Priya Kakitapalli <quic_skakitap@quicinc.com> wrote:
>
>> Add driver for the MBG thermal monitoring device. It monitors
>> the die temperature, and when there is a level 1 upper threshold
>> violation, it receives an interrupt over spmi. The driver reads
>> the fault status register and notifies thermal accordingly.
>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> Hi Satya,
>
> Some comments inline.


Thanks for review, I'll address the comments in my V2 patches.


>
> Jonathan
>
>> diff --git a/drivers/thermal/qcom/qcom-spmi-mbg-tm.c b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
>> new file mode 100644
>> index 000000000000..70964ea5a48d
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
>> @@ -0,0 +1,269 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +//Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/thermal.h>
>> +#include <linux/iio/consumer.h>
>> +
>> +#include "../thermal_core.h"
>> +
>> +#define MBG_TEMP_MON_MM_MON2_FAULT_STATUS	0x50
>> +
>> +#define MON_FAULT_STATUS_MASK			GENMASK(7, 6)
>> +#define MON_FAULT_STATUS_SHIFT			6
> Use FIELD_GET() and FIELD_PREP() then no need to have shift defined.
>
>> +#define MON2_LVL1_ERR				0x1
>> +
>> +#define MON2_LVL1_UP_THRESH			0x59
>> +
>> +#define MBG_TEMP_MON_MM_MON2_MISC_CFG		0x5f
>> +#define UP_THRESH_EN				BIT(1)
>> +
>> +#define STEP_MV					8
>> +#define MBG_DEFAULT_TEMP_MV			600
>> +#define MBG_TEMP_CONSTANT			1000
>> +#define MIN_TRIP_TEMP				25000
>> +#define MAX_SUPPORTED_TEMP			160000
> these are all device specific but don't sound it. I'd prefix them with MBG_TEMP
>
>
>> +
>> +static int mbg_tm_set_trip_temp(struct thermal_zone_device *tz, int low_temp,
>> +						int temp)
>> +{
>> +	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
>> +	int ret = 0, vtemp = 0;
>> +
>> +	mutex_lock(&chip->lock);
> 	guard(mutex)(&chip->lock);
>
>
> As then you don't need to carefully unlock as it will be done at exit of scope.
>
>> +
>> +	/* The HW has a limitation that the trip set must be above 25C */
>> +	if (temp > MIN_TRIP_TEMP && temp < INT_MAX) {
> 		int vtemp;
>
> reduce the scope and it becomes clear no need to init.
>
>> +		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
>> +					 UP_THRESH_EN, UP_THRESH_EN);
>> +		vtemp = temp_to_vtemp(temp);
>> +		ret = mbg_tm_write(chip, MON2_LVL1_UP_THRESH, vtemp);
>> +		if (ret < 0) {
>> +			mutex_unlock(&chip->lock);
>> +			return ret;
>> +		}
>> +	} else {
>> +		dev_dbg(chip->dev, "Setting %d failed, set trip between 25C and INT_MAX\n", temp);
>> +		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
>> +					UP_THRESH_EN, 0);
>> +	}
>> +
>> +	mutex_unlock(&chip->lock);
>> +
>> +	/*
>> +	 * Configure the last_temp one degree higher, to ensure the
>> +	 * violated temp is returned to thermal framework when it reads
>> +	 * temperature for the first time after the violation happens.
>> +	 * This is needed to account for the inaccuracy in the conversion
>> +	 * formula used which leads to the thermal framework setting back
>> +	 * the same thresholds in case the temperature it reads does not
>> +	 * show violation.
>> +	 */
>> +	chip->last_temp = temp + MBG_TEMP_CONSTANT;
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct thermal_zone_device_ops mbg_tm_ops = {
>> +	.get_temp = mbg_tm_get_temp,
>> +	.set_trips = mbg_tm_set_trip_temp,
>> +};
>> +
>> +static irqreturn_t mbg_tm_isr(int irq, void *data)
>> +{
>> +	struct mbg_tm_chip *chip = data;
>> +	int ret;
>> +	int val = 0;
>> +
>> +	mutex_lock(&chip->lock);
>> +
>> +	ret = mbg_tm_read(chip, MBG_TEMP_MON_MM_MON2_FAULT_STATUS, &val);
>> +
>> +	mutex_unlock(&chip->lock);
> 	scoped_guard(mutex, &chip->lock) {
> 		ret = mbg...
> 		if (ret < 0)
> 			return IRQ_HANDLED;
> 	}
> avoids need to unlock before return and to me at least gives slightly more
> readable code.
>
>> +
>> +	if (ret < 0)
>> +		return IRQ_HANDLED;
>> +
>> +	val &= MON_FAULT_STATUS_MASK;
>> +	if ((val >> MON_FAULT_STATUS_SHIFT) & MON2_LVL1_ERR) {
> as above use FIELD_GET() with the mask - it is more readable and
> does the mask and shift in one call.
>
>
>
>> +		chip->last_temp_set = true;
>> +		thermal_zone_device_update(chip->tz_dev,
>> +					THERMAL_TRIP_VIOLATED);
> I think the above is 80 chars on one line so no need to wrap.
>
>> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
>> +	} else {
>> +		dev_dbg(chip->dev, "Lvl 1 upper threshold not violated, ignoring interrupt\n");
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int mbg_tm_probe(struct platform_device *pdev)
>> +{
>> +	struct mbg_tm_chip *chip;
>> +	struct device_node *node = pdev->dev.of_node;
>> +	u32 res;
>> +	int ret = 0;
> It's always set, so don't intiialize here.
>
>> +
>> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>> +	if (!chip)
>> +		return -ENOMEM;
>> +
>> +	chip->dev = &pdev->dev;
>> +
>> +	mutex_init(&chip->lock);
>> +
>> +	chip->map = dev_get_regmap(pdev->dev.parent, NULL);
>> +	if (!chip->map)
>> +		return -ENXIO;
>> +
>> +	ret = of_property_read_u32(node, "reg", &res);
> I'm not sure on thermal subsystem opinion on this but I'd use
> the property.h generic firmware property reading stuff rather than of
> specific.
> device_property_read_u32()
>
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	chip->base = res;
>> +
>> +	chip->irq = platform_get_irq(pdev, 0);
>> +	if (chip->irq < 0)
>> +		return chip->irq;
>> +
>> +	chip->adc = devm_iio_channel_get(&pdev->dev, "thermal");
>> +	if (IS_ERR(chip->adc))
>> +		return PTR_ERR(chip->adc);
>> +
>> +	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev,
>> +				 0, chip, &mbg_tm_ops);
>> +	if (IS_ERR(chip->tz_dev)) {
>> +		dev_err(&pdev->dev, "failed to register sensor\n");
>> +		return PTR_ERR(chip->tz_dev);
> 		return dev_err_probe(...)
>
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, chip->irq, NULL,
>> +			mbg_tm_isr, IRQF_ONESHOT, node->name, chip);
>> +
>> +	return ret;
> 	return devm_request...
>
>> +}

Re: [PATCH 3/5] thermal: qcom: Add support for MBG Temp monitor

[Konrad Dybcio]

On 12.07.2024 2:43 PM, Satya Priya Kakitapalli wrote:
> Add driver for the MBG thermal monitoring device. It monitors
> the die temperature, and when there is a level 1 upper threshold
> violation, it receives an interrupt over spmi. The driver reads
> the fault status register and notifies thermal accordingly.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---

[...]

>  drivers/thermal/qcom/Kconfig            |  11 ++
>  drivers/thermal/qcom/Makefile           |   1 +
>  drivers/thermal/qcom/qcom-spmi-mbg-tm.c | 269 ++++++++++++++++++++++++++++++++
>  3 files changed, 281 insertions(+)
> 
> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index 2c7f3f9a26eb..46045094020c 100644
> --- a/drivers/thermal/qcom/Kconfig
> +++ b/drivers/thermal/qcom/Kconfig
> @@ -21,6 +21,17 @@ config QCOM_SPMI_ADC_TM5
>  	  Thermal client sets threshold temperature for both warm and cool and
>  	  gets updated when a threshold is reached.
>  
> +config QCOM_SPMI_MBG_TM
> +	tristate "Qualcomm Technologies, Inc. SPMI PMIC MBG Temperature monitor"
> +	depends on OF && SPMI && IIO
> +	select REGMAP_SPMI
> +	help
> +	 This enables a thermal driver for the MBG thermal monitoring device.
> +	 It shows up in sysfs as a thermal sensor with two trip points.
> +	 It notifies the thermal framework when level 1 high threshold is
> +	 violated. The temperature reported by the thermal sensor reflects
> +	 the real time die temperature through ADC channel.
> +
>  config QCOM_SPMI_TEMP_ALARM
>  	tristate "Qualcomm SPMI PMIC Temperature Alarm"
>  	depends on OF && SPMI && IIO
> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> index 0fa2512042e7..bc18e08ee3e2 100644
> --- a/drivers/thermal/qcom/Makefile
> +++ b/drivers/thermal/qcom/Makefile
> @@ -4,5 +4,6 @@ obj-$(CONFIG_QCOM_TSENS)	+= qcom_tsens.o
>  qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
>  				   tsens-8960.o
>  obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
> +obj-$(CONFIG_QCOM_SPMI_MBG_TM)	+= qcom-spmi-mbg-tm.o
>  obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>  obj-$(CONFIG_QCOM_LMH)		+= lmh.o
> diff --git a/drivers/thermal/qcom/qcom-spmi-mbg-tm.c b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> new file mode 100644
> index 000000000000..70964ea5a48d
> --- /dev/null
> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/thermal.h>
> +#include <linux/iio/consumer.h>
> +
> +#include "../thermal_core.h"
> +
> +#define MBG_TEMP_MON_MM_MON2_FAULT_STATUS	0x50
> +
> +#define MON_FAULT_STATUS_MASK			GENMASK(7, 6)
> +#define MON_FAULT_STATUS_SHIFT			6
> +#define MON2_LVL1_ERR				0x1
> +
> +#define MON2_LVL1_UP_THRESH			0x59
> +
> +#define MBG_TEMP_MON_MM_MON2_MISC_CFG		0x5f
> +#define UP_THRESH_EN				BIT(1)
> +
> +#define STEP_MV					8
> +#define MBG_DEFAULT_TEMP_MV			600
> +#define MBG_TEMP_CONSTANT			1000
> +#define MIN_TRIP_TEMP				25000
> +#define MAX_SUPPORTED_TEMP			160000
> +
> +struct mbg_tm_chip {
> +	struct regmap			*map;
> +	struct device			*dev;
> +	struct thermal_zone_device	*tz_dev;
> +	struct mutex                    lock;
> +	unsigned int			base;
> +	int				irq;
> +	int				last_temp;
> +	bool				last_temp_set;
> +	struct iio_channel		*adc;
> +};
> +
> +struct mbg_map_table {
> +	int min_temp;
> +	int vtemp0;
> +	int tc;
> +};
> +
> +static const struct mbg_map_table map_table[] = {
> +	/* minT	vtemp0	tc */
> +	{ -60000, 4337, 1967 },
> +	{ -40000, 4731, 1964 },
> +	{ -20000, 5124, 1957  },
> +	{ 0,      5515, 1949 },
> +	{ 20000,  5905, 1940 },
> +	{ 40000,  6293, 1930 },
> +	{ 60000,  6679, 1921 },
> +	{ 80000,  7064, 1910 },
> +	{ 100000, 7446, 1896 },
> +	{ 120000, 7825, 1878 },
> +	{ 140000, 8201, 1859 },
> +};
> +
> +static int mbg_tm_read(struct mbg_tm_chip *chip, u16 addr, int *data)
> +{
> +	return regmap_read(chip->map, chip->base + addr, data);
> +}
> +
> +static int mbg_tm_write(struct mbg_tm_chip *chip, u16 addr, int data)
> +{
> +	return regmap_write(chip->map, chip->base + addr, data);
> +}
> +
> +static int mbg_tm_reg_update(struct mbg_tm_chip *chip, u16 addr, u8 mask, u8 val)
> +{
> +	return regmap_write_bits(chip->map, chip->base + addr, mask, val);
> +}

You're not saving much on code amount, or readability by adding these
accessors, r/w are used once and update is used twice.

> +
> +static int mbg_tm_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
> +	int ret, milli_celsius;
> +
> +	if (!temp)
> +		return -EINVAL;
> +
> +	if (chip->last_temp_set) {
> +		pr_debug("last_temp: %d\n", chip->last_temp);
> +		chip->last_temp_set = false;

last_temp_set -> last_thres_crossed?

> +		*temp = chip->last_temp;
> +		return 0;
> +	}

[...]

> +
> +	/* The HW has a limitation that the trip set must be above 25C */
> +	if (temp > MIN_TRIP_TEMP && temp < INT_MAX) {

INT_MAX -> MAX_SUPPORTED_TEMP?

> +		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
> +					 UP_THRESH_EN, UP_THRESH_EN);

regmap_set_bits

> +		vtemp = temp_to_vtemp(temp);
> +		ret = mbg_tm_write(chip, MON2_LVL1_UP_THRESH, vtemp);

regmap_write(...., temp_to_vtemp(temp))

> +		if (ret < 0) {
> +			mutex_unlock(&chip->lock);
> +			return ret;
> +		}
> +	} else {
> +		dev_dbg(chip->dev, "Setting %d failed, set trip between 25C and INT_MAX\n", temp);
> +		mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
> +					UP_THRESH_EN, 0);

regmap_clear_bits

[...]

> +	val &= MON_FAULT_STATUS_MASK;
> +	if ((val >> MON_FAULT_STATUS_SHIFT) & MON2_LVL1_ERR) {

FIELD_GET

> +		chip->last_temp_set = true;
> +		thermal_zone_device_update(chip->tz_dev,
> +					THERMAL_TRIP_VIOLATED);
> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
> +	} else {
> +		dev_dbg(chip->dev, "Lvl 1 upper threshold not violated, ignoring interrupt\n");

Would such spurious IRQs carry any meaning at all?

[...]

> +static struct platform_driver mbg_tm_driver = {
> +	.driver = {
> +		.name = "qcom-spmi-mbg-tm",
> +		.of_match_table = mbg_tm_match_table,
> +	},
> +	.probe  = mbg_tm_probe,

Double space

Konrad

[PATCH 4/5] ARM: dts: qcom: Add vadc support for pm8775 pmic on SA8775P

[Satya Priya Kakitapalli]

Add support for reading the adc channels of pm8775 on SA8775P platforms.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 90 +++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
index 1369c3d43f86..bd4f5f51e094 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: BSD-3-Clause
 /*
  * Copyright (c) 2023, Linaro Limited
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
+#include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/spmi/spmi.h>
 
@@ -105,6 +107,28 @@ pmm8654au_0: pmic@0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		pmm8654au_0_adc: vadc@8000 {
+			compatible = "qcom,spmi-adc5-gen3";
+			reg = <0x8000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts-extended = <&spmi_bus 0x0 0x80 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "adc-sdam0";
+			#io-channel-cells = <1>;
+
+			pmm8654au_0_die_temp {
+				reg = <PM8775_ADC5_GEN3_DIE_TEMP(0)>;
+				label = "pmm8654au_0_die_temp";
+				qcom,pre-scaling = <1 1>;
+			};
+
+			pmm8654au_0_vph_pwr {
+				reg = <PM8775_ADC5_GEN3_VPH_PWR(0)>;
+				label = "pmm8654au_0_vph_pwr";
+				qcom,pre-scaling = <1 3>;
+			};
+		};
+
 		pmm8654au_0_temp_alarm: temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
@@ -162,6 +186,28 @@ pmm8654au_1: pmic@2 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		pmm8654au_1_adc: vadc@8000 {
+			compatible = "qcom,spmi-adc5-gen3";
+			reg = <0x8000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts-extended = <&spmi_bus 0x2 0x80 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "adc-sdam0";
+			#io-channel-cells = <1>;
+
+			pmm8654au_1_die_temp {
+				reg = <PM8775_ADC5_GEN3_DIE_TEMP(2)>;
+				label = "pmm8654au_1_die_temp";
+				qcom,pre-scaling = <1 1>;
+			};
+
+			pmm8654au_1_vph_pwr {
+				reg = <PM8775_ADC5_GEN3_VPH_PWR(2)>;
+				label = "pmm8654au_1_vph_pwr";
+				qcom,pre-scaling = <1 3>;
+			};
+		};
+
 		pmm8654au_1_temp_alarm: temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
@@ -186,6 +232,28 @@ pmm8654au_2: pmic@4 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		pmm8654au_2_adc: vadc@8000 {
+			compatible = "qcom,spmi-adc5-gen3";
+			reg = <0x8000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts-extended = <&spmi_bus 0x4 0x80 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "adc-sdam0";
+			#io-channel-cells = <1>;
+
+			pmm8654au_2_die_temp {
+				reg = <PM8775_ADC5_GEN3_DIE_TEMP(4)>;
+				label = "pmm8654au_2_die_temp";
+				qcom,pre-scaling = <1 1>;
+			};
+
+			pmm8654au_2_vph_pwr {
+				reg = <PM8775_ADC5_GEN3_VPH_PWR(4)>;
+				label = "pmm8654au_2_vph_pwr";
+				qcom,pre-scaling = <1 3>;
+			};
+		};
+
 		pmm8654au_2_temp_alarm: temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
@@ -210,6 +278,28 @@ pmm8654au_3: pmic@6 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		pmm8654au_3_adc: vadc@8000 {
+			compatible = "qcom,spmi-adc5-gen3";
+			reg = <0x8000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts-extended = <&spmi_bus 0x6 0x80 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "adc-sdam0";
+			#io-channel-cells = <1>;
+
+			pmm8654au_3_die_temp {
+				reg = <PM8775_ADC5_GEN3_DIE_TEMP(6)>;
+				label = "pmm8654au_3_die_temp";
+				qcom,pre-scaling = <1 1>;
+			};
+
+			pmm8654au_3_vph_pwr {
+				reg = <PM8775_ADC5_GEN3_VPH_PWR(6)>;
+				label = "pmm8654au_3_vph_pwr";
+				qcom,pre-scaling = <1 3>;
+			};
+		};
+
 		pmm8654au_3_temp_alarm: temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;

-- 
2.25.1

Re: [PATCH 4/5] ARM: dts: qcom: Add vadc support for pm8775 pmic on SA8775P

[Krzysztof Kozlowski]

On 12/07/2024 14:43, Satya Priya Kakitapalli wrote:
> Add support for reading the adc channels of pm8775 on SA8775P platforms.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 90 +++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> index 1369c3d43f86..bd4f5f51e094 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> @@ -1,8 +1,10 @@
>  // SPDX-License-Identifier: BSD-3-Clause
>  /*
>   * Copyright (c) 2023, Linaro Limited
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
> +#include <dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8775.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/spmi/spmi.h>
>  
> @@ -105,6 +107,28 @@ pmm8654au_0: pmic@0 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> +		pmm8654au_0_adc: vadc@8000 {
> +			compatible = "qcom,spmi-adc5-gen3";
> +			reg = <0x8000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			interrupts-extended = <&spmi_bus 0x0 0x80 0x1 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "adc-sdam0";
> +			#io-channel-cells = <1>;
> +
> +			pmm8654au_0_die_temp {

Please read DTS coding style first and your internal guideline go/upstream.

Best regards,
Krzysztof

[PATCH 5/5] ARM: dts: qcom: Add support for MBG TM for pm8775 on SA8775P

[Satya Priya Kakitapalli]

Add support for MBG TM peripheral for pm8775 sail pmics on SA8775P.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 120 ++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
index bd4f5f51e094..69910306885e 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
@@ -89,6 +89,62 @@ trip1 {
 				};
 			};
 		};
+
+		pmm8654au_0_mbg_tm: pmm8654au_0_mbg_tz {
+			polling-delay-passive = <100>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmm8654au_0_tz>;
+
+			trips {
+				trip0 {
+					temperature = <115000>;
+					hysteresis = <5000>;
+					type = "passive";
+				};
+			};
+		};
+
+		pmm8654au_1_mbg_tm: pmm8654au_1_mbg_tz {
+			polling-delay-passive = <100>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmm8654au_1_tz>;
+
+			trips {
+				trip0 {
+					temperature = <115000>;
+					hysteresis = <5000>;
+					type = "passive";
+				};
+			};
+		};
+
+		pmm8654au_2_mbg_tm: pmm8654au_2_mbg_tz {
+			polling-delay-passive = <100>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmm8654au_2_tz>;
+
+			trips {
+				trip0 {
+					temperature = <115000>;
+					hysteresis = <5000>;
+					type = "passive";
+				};
+			};
+		};
+
+		pmm8654au_3_mbg_tm: pmm8654au_3_mbg_tz {
+			polling-delay-passive = <100>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmm8654au_3_tz>;
+
+			trips {
+				trip0 {
+					temperature = <115000>;
+					hysteresis = <5000>;
+					type = "passive";
+				};
+			};
+		};
 	};
 
 	reboot-mode {
@@ -180,6 +236,22 @@ reboot_reason: reboot-reason@48 {
 		};
 	};
 
+	pmm8654au_sail_0: pmic@1 {
+		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
+		reg = <0x1 SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pmm8654au_0_tz: qcom,mbg-tm@d700 {
+			compatible = "qcom,spmi-mgb-tm";
+			reg = <0xd700>;
+			io-channels = <&pmm8654au_0_adc PM8775_ADC5_GEN3_DIE_TEMP(0)>;
+			io-channel-names = "thermal";
+			interrupts-extended = <&spmi_bus 0x1 0xd7 0x0 IRQ_TYPE_EDGE_RISING>;
+			#thermal-sensor-cells = <0>;
+		};
+	};
+
 	pmm8654au_1: pmic@2 {
 		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
 		reg = <0x2 SPMI_USID>;
@@ -226,6 +298,22 @@ pmm8654au_1_gpios: gpio@8800 {
 		};
 	};
 
+	pmm8654au_sail_1: pmic@3 {
+		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
+		reg = <0x3 SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pmm8654au_1_tz: qcom,mbg-tm@d700 {
+			compatible = "qcom,spmi-mgb-tm";
+			reg = <0xd700>;
+			io-channels = <&pmm8654au_1_adc PM8775_ADC5_GEN3_DIE_TEMP(2)>;
+			io-channel-names = "thermal";
+			interrupts-extended = <&spmi_bus 0x3 0xd7 0x0 IRQ_TYPE_EDGE_RISING>;
+			#thermal-sensor-cells = <0>;
+		};
+	};
+
 	pmm8654au_2: pmic@4 {
 		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
 		reg = <0x4 SPMI_USID>;
@@ -272,6 +360,22 @@ pmm8654au_2_gpios: gpio@8800 {
 		};
 	};
 
+	pmm8654au_sail_2: pmic@5 {
+		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
+		reg = <0x5 SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pmm8654au_2_tz: qcom,mbg-tm@d700 {
+			compatible = "qcom,spmi-mgb-tm";
+			reg = <0xd700>;
+			io-channels = <&pmm8654au_2_adc PM8775_ADC5_GEN3_DIE_TEMP(4)>;
+			io-channel-names = "thermal";
+			interrupts-extended = <&spmi_bus 0x5 0xd7 0x0 IRQ_TYPE_EDGE_RISING>;
+			#thermal-sensor-cells = <0>;
+		};
+	};
+
 	pmm8654au_3: pmic@6 {
 		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
 		reg = <0x6 SPMI_USID>;
@@ -317,4 +421,20 @@ pmm8654au_3_gpios: gpio@8800 {
 			#interrupt-cells = <2>;
 		};
 	};
+
+	pmm8654au_sail_3: pmic@7 {
+		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
+		reg = <0x7 SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pmm8654au_3_tz: qcom,mbg-tm@d700 {
+			compatible = "qcom,spmi-mgb-tm";
+			reg = <0xd700>;
+			io-channels = <&pmm8654au_3_adc PM8775_ADC5_GEN3_DIE_TEMP(6)>;
+			io-channel-names = "thermal";
+			interrupts-extended = <&spmi_bus 0x7 0xd7 0x0 IRQ_TYPE_EDGE_RISING>;
+			#thermal-sensor-cells = <0>;
+		};
+	};
 };

-- 
2.25.1

Re: [PATCH 5/5] ARM: dts: qcom: Add support for MBG TM for pm8775 on SA8775P

[Krzysztof Kozlowski]

On 12/07/2024 14:43, Satya Priya Kakitapalli wrote:
> Add support for MBG TM peripheral for pm8775 sail pmics on SA8775P.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> ---
>  arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 120 ++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> index bd4f5f51e094..69910306885e 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> @@ -89,6 +89,62 @@ trip1 {
>  				};
>  			};
>  		};
> +
> +		pmm8654au_0_mbg_tm: pmm8654au_0_mbg_tz {

This wasn't ever tested and cannot work.

Please read your internal guideline before posting any more work. That
guideline you have is pretty detailed and offloads community from
pointing silly issues like - never testing.

> +			polling-delay-passive = <100>;
> +			polling-delay = <0>;
> +			thermal-sensors = <&pmm8654au_0_tz>;
> +

...

>  	pmm8654au_3: pmic@6 {
>  		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
>  		reg = <0x6 SPMI_USID>;
> @@ -317,4 +421,20 @@ pmm8654au_3_gpios: gpio@8800 {
>  			#interrupt-cells = <2>;
>  		};
>  	};
> +
> +	pmm8654au_sail_3: pmic@7 {
> +		compatible = "qcom,pmm8654au", "qcom,spmi-pmic";
> +		reg = <0x7 SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pmm8654au_3_tz: qcom,mbg-tm@d700 {

Again: do you see anywhere node called "qcom,foo-bar"?



Best regards,
Krzysztof

Re: [PATCH 5/5] ARM: dts: qcom: Add support for MBG TM for pm8775 on SA8775P

[Konrad Dybcio]

On 12.07.2024 2:43 PM, Satya Priya Kakitapalli wrote:
> Add support for MBG TM peripheral for pm8775 sail pmics on SA8775P.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi | 120 ++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> index bd4f5f51e094..69910306885e 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi
> @@ -89,6 +89,62 @@ trip1 {
>  				};
>  			};
>  		};
> +
> +		pmm8654au_0_mbg_tm: pmm8654au_0_mbg_tz {
> +			polling-delay-passive = <100>;
> +			polling-delay = <0>;

0 is the default polling delay, you can drop this

Konrad

Re: [PATCH 0/5] Add support for MBG Thermal monitoring device

[neil.armstrong]

Hi,

On 12/07/2024 14:43, Satya Priya Kakitapalli wrote:
> Add bindings, driver and DT for the Qualcomm's MBG thermal
> monitoring device.
> 
> Please note that this series is dependent on [1] which adds
> ADC5-GEN3 support.
> 
> [1] https://lore.kernel.org/linux-iio/20231231171237.3322376-1-quic_jprakash@quicinc.com/

Since this dependency was sent almost 7 months ago, and had plenty of changes requests,
this patchset should've been either delayed until a proper support of ADC5-GEN3
was accepted or marked as RFC.

> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
> Satya Priya Kakitapalli (5):
>        dt-bindings: iio: adc: Add ADC5 GEN3 Channel info for pm8775 PMIC
>        dt-bindings: thermal: qcom: Add MBG thermal monitor bindings
>        thermal: qcom: Add support for MBG Temp monitor
>        ARM: dts: qcom: Add vadc support for pm8775 pmic on SA8775P
>        ARM: dts: qcom: Add support for MBG TM for pm8775 on SA8775P

Those should be: "arm64: dts: qcom: sa8775p-pmics: ..."

> 
>   .../bindings/thermal/qcom-spmi-mbg-tm.yaml         |  63 +++++
>   arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi        | 210 ++++++++++++++++
>   drivers/thermal/qcom/Kconfig                       |  11 +
>   drivers/thermal/qcom/Makefile                      |   1 +
>   drivers/thermal/qcom/qcom-spmi-mbg-tm.c            | 269 +++++++++++++++++++++
>   .../iio/adc/qcom,spmi-adc5-gen3-pm8775.h           |  42 ++++
>   6 files changed, 596 insertions(+)
> ---
> base-commit: c27723304c1f6af79f7bece5edacace6a8d46167
> change-id: 20240627-mbg-tm-support-7bbf25c246e1
> 
> Best regards,

Neil

Re: [PATCH 0/5] Add support for MBG Thermal monitoring device

[Satya Priya Kakitapalli]

On 7/12/2024 8:09 PM, neil.armstrong@linaro.org wrote:
> Hi,
>
> On 12/07/2024 14:43, Satya Priya Kakitapalli wrote:
>> Add bindings, driver and DT for the Qualcomm's MBG thermal
>> monitoring device.
>>
>> Please note that this series is dependent on [1] which adds
>> ADC5-GEN3 support.
>>
>> [1] 
>> https://lore.kernel.org/linux-iio/20231231171237.3322376-1-quic_jprakash@quicinc.com/
>
> Since this dependency was sent almost 7 months ago, and had plenty of 
> changes requests,
> this patchset should've been either delayed until a proper support of 
> ADC5-GEN3
> was accepted or marked as RFC.
>

Sure, I'll re-base my patches on the recently posted version of 
ADC5-GEN3 patches and post my V2 as RFC.


>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
>> Satya Priya Kakitapalli (5):
>>        dt-bindings: iio: adc: Add ADC5 GEN3 Channel info for pm8775 PMIC
>>        dt-bindings: thermal: qcom: Add MBG thermal monitor bindings
>>        thermal: qcom: Add support for MBG Temp monitor
>>        ARM: dts: qcom: Add vadc support for pm8775 pmic on SA8775P
>>        ARM: dts: qcom: Add support for MBG TM for pm8775 on SA8775P
>
> Those should be: "arm64: dts: qcom: sa8775p-pmics: ..."
>

Okay.


>>
>>   .../bindings/thermal/qcom-spmi-mbg-tm.yaml         |  63 +++++
>>   arch/arm64/boot/dts/qcom/sa8775p-pmics.dtsi        | 210 
>> ++++++++++++++++
>>   drivers/thermal/qcom/Kconfig                       |  11 +
>>   drivers/thermal/qcom/Makefile                      |   1 +
>>   drivers/thermal/qcom/qcom-spmi-mbg-tm.c            | 269 
>> +++++++++++++++++++++
>>   .../iio/adc/qcom,spmi-adc5-gen3-pm8775.h           |  42 ++++
>>   6 files changed, 596 insertions(+)
>> ---
>> base-commit: c27723304c1f6af79f7bece5edacace6a8d46167
>> change-id: 20240627-mbg-tm-support-7bbf25c246e1
>>
>> Best regards,
>
> Neil
>