[PATCH 0/2] thermal: qcom: Add Qualcomm SPMI MBG thermal monitor support

[PATCH 0/2] thermal: qcom: Add Qualcomm SPMI MBG thermal monitor support

[Sachin Gupta]

This series adds support for Qualcomm MBG thermal monitoring.

Adding support for:
- DT bindings for the MBG thermal monitor peripheral on PM8775
- A new Qualcomm SPMI MBG thermal monitor driver under `drivers/thermal/qcom/`
 
The driver monitors die temperature alarms, handles the MBG interrupt on
upper-threshold violation, reads the fault status, and reports events to the
thermal framework.
 
RFC patch:
https://lore.kernel.org/all/qq3cggafexwpdrv46eqijxfmrdbqusl2vpbuswqmcvshqueaiw@r4mrmap4nwkt/
 

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
---
Satya Priya Kakitapalli (2):
      dt-bindings: thermal: Add Qualcomm MBG thermal monitor support
      thermal: qcom: Add support for Qualcomm MBG thermal monitoring

 .../devicetree/bindings/mfd/qcom,spmi-pmic.yaml    |   4 +
 .../bindings/thermal/qcom-spmi-mbg-tm.yaml         |  72 ++++++
 drivers/thermal/qcom/Kconfig                       |  11 +
 drivers/thermal/qcom/Makefile                      |   1 +
 drivers/thermal/qcom/qcom-spmi-mbg-tm.c            | 254 +++++++++++++++++++++
 5 files changed, 342 insertions(+)
---
base-commit: 7da7f07112610a520567421dd2ffcb51beaefbcc
change-id: 20260601-spmi-mbg-driver-582aab5aa6a6

Best regards,
--  
Sachin Gupta <sachin.gupta@oss.qualcomm.com>

[PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support

[Sachin Gupta]

From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>

Add bindings for the Qualcomm MBG (Master Bandgap) temperature alarm peripheral
found on the PM8775 PMIC. Unlike the existing SPMI temp alarm peripheral,
the MBG peripheral supports both hot and cold threshold monitoring across
two programmable levels (LVL1 and LVL2), with interrupt status reported via
a fault status register over SPMI.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
---
 .../devicetree/bindings/mfd/qcom,spmi-pmic.yaml    |  4 ++
 .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 72 ++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
index 644c42b5e2e5..5f409fe700b2 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
@@ -193,6 +193,10 @@ patternProperties:
     type: object
     $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
 
+  "^temperature-sensor@[0-9a-f]+$":
+    type: object
+    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#
+
   "^typec@[0-9a-f]+$":
     type: object
     $ref: /schemas/usb/qcom,pmic-typec.yaml#
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..a0ecc9f35cf6
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml
@@ -0,0 +1,72 @@
+# 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's SPMI PMIC MBG Thermal Monitoring
+
+maintainers:
+  - Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
+  - Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
+
+description:
+  Qualcomm's MBG(Master Bandgap) temperature alarm monitors the die
+  temperature and generates an interrupt if the PMIC die temperature is
+  over a set of programmable temperature thresholds. It allows monitoring
+  for both hot and cold, LVL1 and LVL2 thresholds, which makes it different
+  from the existing temp alarm peripheral. The interrupt comes over SPMI
+  and the MBG's fault status register gives details to understand whether
+  it is a hot/cold and LVL1/LVL2 violation.
+
+properties:
+  compatible:
+    const: qcom,pm8775-mbg-tm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  io-channels:
+    items:
+      - description: ADC channel, which reports chip die temperature.
+
+  io-channel-names:
+    items:
+      - const: thermal
+
+  '#thermal-sensor-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - io-channels
+  - io-channel-names
+
+allOf:
+  - $ref: thermal-sensor.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    pmic {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        temperature-sensor@d700 {
+            compatible = "qcom,pm8775-mbg-tm";
+            reg = <0xd700>;
+            interrupts = <0x1 0xd7 0x0 IRQ_TYPE_EDGE_RISING>;
+            io-channels = <&pm8775_adc 0x3>;
+            io-channel-names = "thermal";
+            #thermal-sensor-cells = <0>;
+        };
+    };
+...

-- 
2.34.1

[PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring

[Sachin Gupta]

From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>

Add driver for the Qualcomm 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>
Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
---
 drivers/thermal/qcom/Kconfig            |  11 ++
 drivers/thermal/qcom/Makefile           |   1 +
 drivers/thermal/qcom/qcom-spmi-mbg-tm.c | 254 ++++++++++++++++++++++++++++++++
 3 files changed, 266 insertions(+)

diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
index a6bb01082ec6..f2fc24a3096f 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 SPMI PMIC MBG Temperature monitor"
+	depends on QCOM_SPMI_ADC5_GEN3
+	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 single trip point.
+	  It notifies the thermal framework when this trip 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..1bec2746b98d 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..60190b341fc7
--- /dev/null
+++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+#include <linux/iio/consumer.h>
+
+#define MBG_TEMP_MON2_FAULT_STATUS	0x50
+
+#define MON_FAULT_STATUS_MASK		GENMASK(7, 4)
+#define MON_FAULT_LVL1_UPR		0x5
+
+#define MON2_LVL1_UP_THRESH		0x59
+
+#define MBG_TEMP_MON2_MISC_CFG		0x5f
+#define MON2_UP_THRESH_EN		BIT(1)
+
+#define MBG_TEMP_STEP_MV		8
+#define MBG_TEMP_DEFAULT_TEMP_MV	600
+#define MBG_TEMP_CONSTANT		1000
+#define MBG_MIN_TRIP_TEMP		25000
+#define MBG_MAX_SUPPORTED_TEMP		160000
+
+/**
+ * struct mbg_tm_chip - MBG thermal monitor device data.
+ * @map: regmap for accessing MBG thermal registers.
+ * @dev: mbg_tm_chip device.
+ * @tz_dev: thermal zone device registered with the thermal framework.
+ * @lock: mbg_tm_chip lock for set trip temperature.
+ * @base: base register offset for this MBG instance
+ * @irq: interrupt line used to signal threshold events
+ * @last_temp: last measured temperature.
+ * @last_thres_crossed: indicates whether the last interrupt crossed a threshold
+ * @adc: IIO ADC channel used for temperature sensing
+ */
+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_thres_crossed;
+	struct iio_channel		*adc;
+};
+
+/**
+ * struct mbg_map_table - temperature to voltage mapping entry
+ * @min_temp: minimum temperature supported by this mapping entry
+ * @vtemp0: reference voltage or ADC code corresponding to the temperature
+ * @tc: temperature coefficient used for conversion calculations
+ */
+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_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
+	int ret, milli_celsius;
+
+	if (chip->last_thres_crossed) {
+		dev_dbg(chip->dev, "last_temp: %d\n", chip->last_temp);
+		chip->last_thres_crossed = 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 with %d\n", ret);
+		return ret;
+	}
+
+	*temp = milli_celsius;
+
+	return 0;
+}
+
+static int temp_to_vtemp_mv(int temp)
+{
+	int idx, vtemp, tc = 0, t0 = 0, vtemp0 = 0;
+
+	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;
+
+	/* step size is 8mV */
+	return abs(vtemp - MBG_TEMP_DEFAULT_TEMP_MV) / MBG_TEMP_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;
+
+	guard(mutex)(&chip->lock);
+
+	/* The HW has a limitation that the trip set must be above 25C */
+	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
+		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
+				      MON2_UP_THRESH_EN);
+		if (ret < 0)
+			return ret;
+
+		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
+				   temp_to_vtemp_mv(temp));
+		if (ret < 0)
+			return ret;
+	} else {
+		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");
+		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
+					MON2_UP_THRESH_EN);
+		return ret;
+	}
+
+	/*
+	 * 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, val;
+
+	scoped_guard(mutex, &chip->lock) {
+		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
+		if (ret < 0)
+			return IRQ_HANDLED;
+	}
+
+	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {
+		chip->last_thres_crossed = true;
+		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
+		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);
+	} else {
+		dev_dbg(chip->dev, "Lvl1 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;
+
+	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 = device_property_read_u32(chip->dev, "reg", &res);
+	if (ret < 0)
+		return dev_err_probe(chip->dev, ret, "Couldn't read reg property\n");
+
+	chip->base = res;
+
+	chip->irq = platform_get_irq(pdev, 0);
+	if (chip->irq < 0)
+		return dev_err_probe(chip->dev, chip->irq, "Failed to get irq\n");
+
+	chip->adc = devm_iio_channel_get(&pdev->dev, "thermal");
+	if (IS_ERR(chip->adc))
+		return dev_err_probe(chip->dev, PTR_ERR(chip->adc), "Failed to get adc channel\n");
+
+	chip->tz_dev = devm_thermal_of_zone_register(chip->dev, 0, chip, &mbg_tm_ops);
+	if (IS_ERR(chip->tz_dev))
+		return dev_err_probe(chip->dev, PTR_ERR(chip->tz_dev),
+				     "Failed to register sensor\n");
+
+	return devm_request_threaded_irq(&pdev->dev, chip->irq, NULL, mbg_tm_isr, IRQF_ONESHOT,
+					 node->name, chip);
+}
+
+static const struct of_device_id mbg_tm_match_table[] = {
+	{ .compatible = "qcom,pm8775-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.34.1

Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The PMIC schema restricts the standard 'temperature-sensor@' node name exclusively to the new MBG TM binding, causing false positive validation failures for standard-compliant legacy nodes.
--

commit 1c6b45c4d32f87b04fb868a977041acc92be3854
Author: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>

dt-bindings: thermal: Add Qualcomm MBG thermal monitor support
    
This commit adds device tree bindings for the Qualcomm Master Bandgap (MBG)
temperature alarm peripheral found on the PM8775 PMIC. It introduces a new
schema for the MBG thermal monitor and maps it in the SPMI PMIC bindings.

> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> @@ -193,6 +193,10 @@ patternProperties:
>      type: object
>      $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
>  
> +  "^temperature-sensor@[0-9a-f]+$":
> +    type: object
> +    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#

[Severity: Low]
Does mapping this generic pattern exclusively to the MBG thermal monitor
schema cause validation conflicts with other thermal sensors?

Since temperature-sensor is the standard generic node name, other thermal
sensors under the PMIC (like qcom,spmi-temp-alarm) might use it. If a node
uses temperature-sensor@a00 for a standard SPMI temp alarm, wouldn't it be
evaluated solely against this MBG-specific schema and fail with a compatible
string mismatch error?

Would it be better to structure this using a oneOf block to allow multiple
applicable thermal bindings to share the standard node name?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-spmi-mbg-driver-v1-0-b4892b55a17f@oss.qualcomm.com?part=1

Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Data race and TOCTOU window on `last_thres_crossed` leading to missed thermal violations.
- [High] Potential NULL pointer dereference accessing `node->name` on non-OF platforms.
- [High] Incorrect bitwise AND used to check a 4-bit fault status enumeration.
- [High] Silent disablement of supported thermal trips.
- [Medium] Hardware threshold interrupt is enabled before configuring the threshold value.
- [Low] Incompatible pointer type passed to `regmap_read`.
--

commit 59e6da95c48d5200129439570e254b1f9180acf4
Author: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
Subject: thermal: qcom: Add support for Qualcomm MBG thermal monitoring

This patch adds a driver for the Qualcomm MBG thermal monitoring device.
It monitors the die temperature and notifies the thermal framework when a 
level 1 upper threshold violation occurs via an SPMI interrupt.

> 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 0000000000000..60190b341fc7c
> --- /dev/null
> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c

[ ... ]

> +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 (chip->last_thres_crossed) {
> +		dev_dbg(chip->dev, "last_temp: %d\n", chip->last_temp);
> +		chip->last_thres_crossed = false;
> +		*temp = chip->last_temp;
> +		return 0;
> +	}

[Severity: High]
Could this lockless read and clear of chip->last_thres_crossed cause missed
thermal violations?

If a user-space sysfs read occurs concurrently with the thermal worker 
execution (which is triggered by the ISR), the sysfs read might consume the 
flag by setting it to false.

When the thermal worker subsequently calls mbg_tm_get_temp(), it would see 
false and read the raw ADC value. Because ADC readings can fluctuate and 
lag, it might read a value below the threshold, entirely missing the 
violation.

[ ... ]

> +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;
> +
> +	guard(mutex)(&chip->lock);
> +
> +	/* The HW has a limitation that the trip set must be above 25C */
> +	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
> +		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
> +				      MON2_UP_THRESH_EN);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
> +				   temp_to_vtemp_mv(temp));

[Severity: Medium]
Is it safe to enable the threshold comparator before programming the new
threshold value?

If the hardware retains a stale threshold from a prior state that is lower
than the current temperature, enabling the comparator first could
immediately trigger a spurious interrupt.

Would it be better to write the threshold value before enabling the
interrupt?

> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");
> +		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
> +					MON2_UP_THRESH_EN);
> +		return ret;
> +	}

[Severity: High]
When the requested temperature falls outside the hardware's supported bounds,
the driver disables the threshold interrupt and returns 0.

By returning 0, does the driver deceptively signal to the thermal core that
the trip has been successfully configured?

Because the interrupt is disabled, the core will never be notified when the
temperature violates the trip point. Should this return an error code like
-ERANGE instead?

[ ... ]

> +static irqreturn_t mbg_tm_isr(int irq, void *data)
> +{
> +	struct mbg_tm_chip *chip = data;
> +	int ret, val;

[Severity: Low]
The regmap_read() function expects an unsigned int pointer for the value
parameter, but val is declared as a signed int here.

> +
> +	scoped_guard(mutex, &chip->lock) {
> +		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
> +		if (ret < 0)
> +			return IRQ_HANDLED;
> +	}
> +
> +	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {

[Severity: High]
Does this bitwise AND correctly check the 4-bit fault status enumeration?

MON_FAULT_LVL1_UPR is defined as 0x5 (binary 0101). A bitwise AND against
0x5 will evaluate to true if either bit 0 or bit 2 is set, matching
unrelated fault states like 0x1, 0x4, or 0x7.

Should this use an exact equality check (==) instead?

> +		chip->last_thres_crossed = true;

[Severity: High]
This sets chip->last_thres_crossed to true outside of the mutex. As
mentioned earlier in mbg_tm_get_temp(), this flag is read and cleared
locklessly.

Could this lead to a data race if mbg_tm_get_temp() executes concurrently?

> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
> +		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);
> +	} else {
> +		dev_dbg(chip->dev, "Lvl1 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;

[ ... ]

> +	chip->tz_dev = devm_thermal_of_zone_register(chip->dev, 0, chip, &mbg_tm_ops);
> +	if (IS_ERR(chip->tz_dev))
> +		return dev_err_probe(chip->dev, PTR_ERR(chip->tz_dev),
> +				     "Failed to register sensor\n");
> +
> +	return devm_request_threaded_irq(&pdev->dev, chip->irq, NULL, mbg_tm_isr, IRQF_ONESHOT,
> +					 node->name, chip);
> +}

[Severity: High]
Will node->name cause a NULL pointer dereference on non-OF platforms?

Because the driver uses device_property_read_u32(), it can be instantiated
via ACPI or MFD without a device tree node, meaning pdev->dev.of_node
could be NULL.

Would it be safer to use dev_name(&pdev->dev) instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-spmi-mbg-driver-v1-0-b4892b55a17f@oss.qualcomm.com?part=2

Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support

[Krzysztof Kozlowski]

On Mon, Jun 01, 2026 at 04:31:18PM +0530, Sachin Gupta wrote:
> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> 
> Add bindings for the Qualcomm MBG (Master Bandgap) temperature alarm peripheral

Feels unwrapped.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597


> found on the PM8775 PMIC. Unlike the existing SPMI temp alarm peripheral,
> the MBG peripheral supports both hot and cold threshold monitoring across
> two programmable levels (LVL1 and LVL2), with interrupt status reported via
> a fault status register over SPMI.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/mfd/qcom,spmi-pmic.yaml    |  4 ++
>  .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 72 ++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> index 644c42b5e2e5..5f409fe700b2 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> @@ -193,6 +193,10 @@ patternProperties:
>      type: object
>      $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
>  
> +  "^temperature-sensor@[0-9a-f]+$":
> +    type: object
> +    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#
> +
>    "^typec@[0-9a-f]+$":
>      type: object
>      $ref: /schemas/usb/qcom,pmic-typec.yaml#
> 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..a0ecc9f35cf6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml

Filename must match compatible.

> @@ -0,0 +1,72 @@
> +# 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's SPMI PMIC MBG Thermal Monitoring

Best regards,
Krzysztof

Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring

[Konrad Dybcio]

On 6/1/26 1:01 PM, Sachin Gupta wrote:
> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> 
> Add driver for the Qualcomm 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>
> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
> ---

[...]

> +static const struct mbg_map_table map_table[] = {
> +	/* minT	vtemp0	tc */

The struct is defined 2 lines above, the reader can tell the names
of the fields
> +	{ -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 },
> +};

Please add a comment stating this map is not PMIC-specific

[...]

> +	/* The HW has a limitation that the trip set must be above 25C */
> +	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
> +		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
> +				      MON2_UP_THRESH_EN);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
> +				   temp_to_vtemp_mv(temp));
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");

Should this be an error print, returning an error condition?

> +		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
> +					MON2_UP_THRESH_EN);
> +		return ret;
> +	}
> +
> +	/*
> +	 * 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;

Will this work fine if the user tries to set the max temp supported
by the hardware (i.e. is there headroom for max+1)?

> +
> +	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, val;
> +
> +	scoped_guard(mutex, &chip->lock) {
> +		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
> +		if (ret < 0)
> +			return IRQ_HANDLED;
> +	}
> +
> +	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {
> +		chip->last_thres_crossed = true;
> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
> +		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);

Should the assignment and this call also be guarded by the mutex?

Konrad

Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support

[Sachin Gupta]

On 6/1/2026 5:00 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The PMIC schema restricts the standard 'temperature-sensor@' node name exclusively to the new MBG TM binding, causing false positive validation failures for standard-compliant legacy nodes.
> --
> 
> commit 1c6b45c4d32f87b04fb868a977041acc92be3854
> Author: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> 
> dt-bindings: thermal: Add Qualcomm MBG thermal monitor support
>      
> This commit adds device tree bindings for the Qualcomm Master Bandgap (MBG)
> temperature alarm peripheral found on the PM8775 PMIC. It introduces a new
> schema for the MBG thermal monitor and maps it in the SPMI PMIC bindings.
> 
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> @@ -193,6 +193,10 @@ patternProperties:
>>       type: object
>>       $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
>>   
>> +  "^temperature-sensor@[0-9a-f]+$":
>> +    type: object
>> +    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#
> 
> [Severity: Low]
> Does mapping this generic pattern exclusively to the MBG thermal monitor
> schema cause validation conflicts with other thermal sensors?
> 
> Since temperature-sensor is the standard generic node name, other thermal
> sensors under the PMIC (like qcom,spmi-temp-alarm) might use it. If a node
> uses temperature-sensor@a00 for a standard SPMI temp alarm, wouldn't it be
> evaluated solely against this MBG-specific schema and fail with a compatible
> string mismatch error?
> 
> Would it be better to structure this using a oneOf block to allow multiple
> applicable thermal bindings to share the standard node name?
> 

The reported case does not seem valid for this binding update.

temperature-sensor@... is intended for the MBG TM block 
(qcom-spmi-mbg-tm), while the existing SPMI temp-alarm block uses 
temp-alarm@... and its own binding (qcom,spmi-temp-alarm). So in 
practice they are distinct node names for distinct peripherals, and a 
normal temp-alarm node is
not expected to be described as temperature-sensor@....

Because of that, this should not cause a real conflict for standard PMIC 
DTS usage. A qcom,spmi-temp-alarm node named temperature-sensor@... 
would be a misnamed node rather than a valid legacy form we need to 
preserve.

Thanks,
Sachin

Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring

[Sachin Gupta]

On 6/1/2026 5:10 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
> - [High] Data race and TOCTOU window on `last_thres_crossed` leading to missed thermal violations.
> - [High] Potential NULL pointer dereference accessing `node->name` on non-OF platforms.
> - [High] Incorrect bitwise AND used to check a 4-bit fault status enumeration.
> - [High] Silent disablement of supported thermal trips.
> - [Medium] Hardware threshold interrupt is enabled before configuring the threshold value.
> - [Low] Incompatible pointer type passed to `regmap_read`.
> --
> 
> commit 59e6da95c48d5200129439570e254b1f9180acf4
> Author: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> Subject: thermal: qcom: Add support for Qualcomm MBG thermal monitoring
> 
> This patch adds a driver for the Qualcomm MBG thermal monitoring device.
> It monitors the die temperature and notifies the thermal framework when a
> level 1 upper threshold violation occurs via an SPMI interrupt.
> 
>> 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 0000000000000..60190b341fc7c
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> 
> [ ... ]
> 
>> +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 (chip->last_thres_crossed) {
>> +		dev_dbg(chip->dev, "last_temp: %d\n", chip->last_temp);
>> +		chip->last_thres_crossed = false;
>> +		*temp = chip->last_temp;
>> +		return 0;
>> +	}
> 
> [Severity: High]
> Could this lockless read and clear of chip->last_thres_crossed cause missed
> thermal violations?
> 
> If a user-space sysfs read occurs concurrently with the thermal worker
> execution (which is triggered by the ISR), the sysfs read might consume the
> flag by setting it to false.
> 
> When the thermal worker subsequently calls mbg_tm_get_temp(), it would see
> false and read the raw ADC value. Because ADC readings can fluctuate and
> lag, it might read a value below the threshold, entirely missing the
> violation.
> 

I will address this by adding proper locking for this state in the next 
patch series.

> [ ... ]
> 
>> +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;
>> +
>> +	guard(mutex)(&chip->lock);
>> +
>> +	/* The HW has a limitation that the trip set must be above 25C */
>> +	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
>> +		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +				      MON2_UP_THRESH_EN);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
>> +				   temp_to_vtemp_mv(temp));
> 
> [Severity: Medium]
> Is it safe to enable the threshold comparator before programming the new
> threshold value?
> 
> If the hardware retains a stale threshold from a prior state that is lower
> than the current temperature, enabling the comparator first could
> immediately trigger a spurious interrupt.
> 
> Would it be better to write the threshold value before enabling the
> interrupt?
> 

You are right that enabling the comparator before programming 
MON2_LVL1_UP_THRESH can allow a stale threshold to trigger a spurious 
interrupt. I will update the programming sequence to write the threshold 
first and enable the comparator afterward in the next patch series.

>> +		if (ret < 0)
>> +			return ret;
>> +	} else {
>> +		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");
>> +		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +					MON2_UP_THRESH_EN);
>> +		return ret;
>> +	}
> 
> [Severity: High]
> When the requested temperature falls outside the hardware's supported bounds,
> the driver disables the threshold interrupt and returns 0.
> 
> By returning 0, does the driver deceptively signal to the thermal core that
> the trip has been successfully configured?
> 
> Because the interrupt is disabled, the core will never be notified when the
> temperature violates the trip point. Should this return an error code like
> -ERANGE instead?
> 

I agree returning 0 here is not correct for an out-of-range request, 
since the trip is not actually configured. I will return -ERANGE for 
unsupported trip temperatures and update this in the next patch series.

> [ ... ]
> 
>> +static irqreturn_t mbg_tm_isr(int irq, void *data)
>> +{
>> +	struct mbg_tm_chip *chip = data;
>> +	int ret, val;
> 
> [Severity: Low]
> The regmap_read() function expects an unsigned int pointer for the value
> parameter, but val is declared as a signed int here.
> 

We can keep this as-is for now to stay consistent with the surrounding 
driver style, where int is commonly used for temporary readback 
variables with regmap_read(). Since the value is only used for bit 
checks and not sign-sensitive arithmetic, this does not cause a 
functional issue. I plan to keep it unchanged in this series for 
consistency.

>> +
>> +	scoped_guard(mutex, &chip->lock) {
>> +		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
>> +		if (ret < 0)
>> +			return IRQ_HANDLED;
>> +	}
>> +
>> +	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {
> 
> [Severity: High]
> Does this bitwise AND correctly check the 4-bit fault status enumeration?
> 
> MON_FAULT_LVL1_UPR is defined as 0x5 (binary 0101). A bitwise AND against
> 0x5 will evaluate to true if either bit 0 or bit 2 is set, matching
> unrelated fault states like 0x1, 0x4, or 0x7.
> 
> Should this use an exact equality check (==) instead?
> 

I’ll correct this to use == in the next patch series.

>> +		chip->last_thres_crossed = true;
> 
> [Severity: High]
> This sets chip->last_thres_crossed to true outside of the mutex. As
> mentioned earlier in mbg_tm_get_temp(), this flag is read and cleared
> locklessly.
> 
> Could this lead to a data race if mbg_tm_get_temp() executes concurrently?
> 

Agree updating last_thres_crossed outside the lock can race with 
concurrent reads in mbg_tm_get_temp(). I will move this update under the 
lock in the next patch series.

>> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
>> +		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);
>> +	} else {
>> +		dev_dbg(chip->dev, "Lvl1 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;
> 
> [ ... ]
> 
>> +	chip->tz_dev = devm_thermal_of_zone_register(chip->dev, 0, chip, &mbg_tm_ops);
>> +	if (IS_ERR(chip->tz_dev))
>> +		return dev_err_probe(chip->dev, PTR_ERR(chip->tz_dev),
>> +				     "Failed to register sensor\n");
>> +
>> +	return devm_request_threaded_irq(&pdev->dev, chip->irq, NULL, mbg_tm_isr, IRQF_ONESHOT,
>> +					 node->name, chip);
>> +}
> 
> [Severity: High]
> Will node->name cause a NULL pointer dereference on non-OF platforms?
> 
> Because the driver uses device_property_read_u32(), it can be instantiated
> via ACPI or MFD without a device tree node, meaning pdev->dev.of_node
> could be NULL.
> 
> Would it be safer to use dev_name(&pdev->dev) instead?
> 

This driver is intended to support DT-only instantiation, so of_node is 
expected to be present for valid platforms. In that context, using 
node->name is consistent with the intended usage model.

Thanks,
Sachin Gupta

Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support

[Sachin Gupta]

On 6/8/2026 3:37 PM, Krzysztof Kozlowski wrote:
> On Mon, Jun 01, 2026 at 04:31:18PM +0530, Sachin Gupta wrote:
>> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>>
>> Add bindings for the Qualcomm MBG (Master Bandgap) temperature alarm peripheral
> 
> Feels unwrapped.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
> 

I will re-wrap the commit message in the next revision.

> 
>> found on the PM8775 PMIC. Unlike the existing SPMI temp alarm peripheral,
>> the MBG peripheral supports both hot and cold threshold monitoring across
>> two programmable levels (LVL1 and LVL2), with interrupt status reported via
>> a fault status register over SPMI.
>>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> ---
>>   .../devicetree/bindings/mfd/qcom,spmi-pmic.yaml    |  4 ++
>>   .../bindings/thermal/qcom-spmi-mbg-tm.yaml         | 72 ++++++++++++++++++++++
>>   2 files changed, 76 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> index 644c42b5e2e5..5f409fe700b2 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> @@ -193,6 +193,10 @@ patternProperties:
>>       type: object
>>       $ref: /schemas/thermal/qcom,spmi-temp-alarm.yaml#
>>   
>> +  "^temperature-sensor@[0-9a-f]+$":
>> +    type: object
>> +    $ref: /schemas/thermal/qcom-spmi-mbg-tm.yaml#
>> +
>>     "^typec@[0-9a-f]+$":
>>       type: object
>>       $ref: /schemas/usb/qcom,pmic-typec.yaml#
>> 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..a0ecc9f35cf6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml
> 
> Filename must match compatible.
> 

Thanks for the review. I can rename this to match the compatible naming, 
but wanted your preference on scope:

Should I use a generic naming scheme (qcom,spmi-mbg-tm.yaml with 
matching compatible), or make it PMIC-specific (qcom,pm8775-mbg-tm.yaml).

Thanks,
Sachin

>> @@ -0,0 +1,72 @@
>> +# 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's SPMI PMIC MBG Thermal Monitoring
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring

[Sachin Gupta]

On 6/16/2026 3:40 PM, Konrad Dybcio wrote:
> On 6/1/26 1:01 PM, Sachin Gupta wrote:
>> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>>
>> Add driver for the Qualcomm 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>
>> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static const struct mbg_map_table map_table[] = {
>> +	/* minT	vtemp0	tc */
> 
> The struct is defined 2 lines above, the reader can tell the names
> of the fields

I will remove this in the next patch series.

>> +	{ -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 },
>> +};
> 
> Please add a comment stating this map is not PMIC-specific
> 

This table is PMIC-specific and applies only to the PM8775 MBG block.
MBG is used only on PM8775 PMIC.

> [...]
> 
>> +	/* The HW has a limitation that the trip set must be above 25C */
>> +	if (temp > MBG_MIN_TRIP_TEMP && temp < MBG_MAX_SUPPORTED_TEMP) {
>> +		ret = regmap_set_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +				      MON2_UP_THRESH_EN);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ret = regmap_write(chip->map, chip->base + MON2_LVL1_UP_THRESH,
>> +				   temp_to_vtemp_mv(temp));
>> +		if (ret < 0)
>> +			return ret;
>> +	} else {
>> +		dev_dbg(chip->dev, "Set trip b/w 25C and 160C\n");
> 
> Should this be an error print, returning an error condition?
> 

Yes, this should be treated as an error path. For out-of-range trip 
requests, I will return -ERANGE and update the log to an error-level 
message in the next patch series.

>> +		ret = regmap_clear_bits(chip->map, chip->base + MBG_TEMP_MON2_MISC_CFG,
>> +					MON2_UP_THRESH_EN);
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * 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;
> 
> Will this work fine if the user tries to set the max temp supported
> by the hardware (i.e. is there headroom for max+1)?
> 

In the current implementation, temp == MBG_MAX_SUPPORTED_TEMP is not 
accepted (temp < MBG_MAX_SUPPORTED_TEMP), so the last_temp = temp + 
MBG_TEMP_CONSTANT path is never taken at absolute max. For accepted 
trips (strictly below max), there is headroom for the +1C adjustment.

>> +
>> +	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, val;
>> +
>> +	scoped_guard(mutex, &chip->lock) {
>> +		ret = regmap_read(chip->map, chip->base + MBG_TEMP_MON2_FAULT_STATUS, &val);
>> +		if (ret < 0)
>> +			return IRQ_HANDLED;
>> +	}
>> +
>> +	if (FIELD_GET(MON_FAULT_STATUS_MASK, val) & MON_FAULT_LVL1_UPR) {
>> +		chip->last_thres_crossed = true;
>> +		dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
>> +		thermal_zone_device_update(chip->tz_dev, THERMAL_TRIP_VIOLATED);
> 
> Should the assignment and this call also be guarded by the mutex?
> 
> Konrad

Yes, agreed. I will update this locking in the next patch series.

Thanks,
Sachin

Re: [PATCH 2/2] thermal: qcom: Add support for Qualcomm MBG thermal monitoring

[Konrad Dybcio]

On 6/19/26 8:45 AM, Sachin Gupta wrote:
> 
> 
> On 6/16/2026 3:40 PM, Konrad Dybcio wrote:
>> On 6/1/26 1:01 PM, Sachin Gupta wrote:
>>> From: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>>>
>>> Add driver for the Qualcomm 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>
>>> Co-developed-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>>> Signed-off-by: Sachin Gupta <sachin.gupta@oss.qualcomm.com>
>>> ---

[...]

>>> +    /*
>>> +     * 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;
>>
>> Will this work fine if the user tries to set the max temp supported
>> by the hardware (i.e. is there headroom for max+1)?
>>
> 
> In the current implementation, temp == MBG_MAX_SUPPORTED_TEMP is not accepted (temp < MBG_MAX_SUPPORTED_TEMP), so the last_temp = temp + MBG_TEMP_CONSTANT path is never taken at absolute max. For accepted trips (strictly below max), there is headroom for the +1C adjustment.

You check for `temp < MBG_MAX_SUPPORTED_TEMP` and there's:

#define MBG_MAX_SUPPORTED_TEMP 160000,

so passing temp=159999 is "valid" and after the addition it becomes 160999,
which in my understanding is outside the range

Konrad

Re: [PATCH 1/2] dt-bindings: thermal: Add Qualcomm MBG thermal monitor support

[Krzysztof Kozlowski]

On 19/06/2026 08:44, Sachin Gupta wrote:
>>> index 000000000000..a0ecc9f35cf6
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-mbg-tm.yaml
>>
>> Filename must match compatible.
>>
> 
> Thanks for the review. I can rename this to match the compatible naming, 
> but wanted your preference on scope:
> 
> Should I use a generic naming scheme (qcom,spmi-mbg-tm.yaml with 
> matching compatible), or make it PMIC-specific (qcom,pm8775-mbg-tm.yaml).

The second one, please.



Best regards,
Krzysztof