public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Jie Gan <jie.gan@oss.qualcomm.com>
To: Shuwei Wu <shuwei.wu@mailbox.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@kernel.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Lukasz Luba <lukasz.luba@arm.com>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Yixun Lan <dlan@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>
Cc: linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-riscv@lists.infradead.org, spacemit@lists.linux.dev,
	linux-kernel@vger.kernel.org, Anand Moon <linux.amoon@gmail.com>,
	Troy Mitchell <troy.mitchell@linux.spacemit.com>,
	Yao Zi <me@ziyao.cc>, Vincent Legoll <legoll@online.fr>,
	Gong Shuai <gsh517025@gmail.com>
Subject: Re: [PATCH v4 2/3] thermal: spacemit: k1: Add thermal sensor support
Date: Fri, 10 Apr 2026 16:25:08 +0800	[thread overview]
Message-ID: <38088fd6-1db6-4591-b489-33b52f727549@oss.qualcomm.com> (raw)
In-Reply-To: <20260410-k1-thermal-v1-2-12c87dd063c3@mailbox.org>



On 4/10/2026 11:31 AM, Shuwei Wu wrote:
> The thermal sensor on K1 supports monitoring five temperature zones.
> The driver registers these sensors with the thermal framework
> and supports standard operations:
> - Reading temperature (millidegree Celsius)
> - Setting high/low thresholds for interrupts
> 
> Signed-off-by: Shuwei Wu <shuwei.wu@mailbox.org>
> Reviewed-by: Anand Moon <linux.amoon@gmail.com>
> Tested-by: Anand Moon <linux.amoon@gmail.com>
> Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> Reviewed-by: Yao Zi <me@ziyao.cc>
> Tested-by: Vincent Legoll <legoll@online.fr> # OrangePi-RV2
> Tested-by: Gong Shuai <gsh517025@gmail.com>
> 
> ---
> Changes in v4:
> - Add 'depends on THERMAL_OF' in drivers/thermal/spacemit/Kconfig
> 
> Changes in v3:
> - Align multi-line assignments as suggested by reviewer
> - Remove unnecessary variable definitions
> 
> Changes in v2:
> - Rename k1_thermal.c to k1_tsensor.c for better hardware alignment
> - Move driver to drivers/thermal/spacemit/
> - Add Kconfig/Makefile for spacemit and update top-level build files
> - Refactor names, style, code alignment, and comments
> - Simplify probe and error handling
> ---
>   drivers/thermal/Kconfig               |   2 +
>   drivers/thermal/Makefile              |   1 +
>   drivers/thermal/spacemit/Kconfig      |  19 +++
>   drivers/thermal/spacemit/Makefile     |   3 +
>   drivers/thermal/spacemit/k1_tsensor.c | 281 ++++++++++++++++++++++++++++++++++
>   5 files changed, 306 insertions(+)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index b10080d61860..1c4a5cd5a23e 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -472,6 +472,8 @@ endmenu
>   
>   source "drivers/thermal/renesas/Kconfig"
>   
> +source "drivers/thermal/spacemit/Kconfig"
> +
>   source "drivers/thermal/tegra/Kconfig"
>   
>   config GENERIC_ADC_THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index bb21e7ea7fc6..3b249195c088 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -65,6 +65,7 @@ obj-y				+= mediatek/
>   obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
>   obj-$(CONFIG_UNIPHIER_THERMAL)	+= uniphier_thermal.o
>   obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
> +obj-y				+= spacemit/
>   obj-$(CONFIG_SPRD_THERMAL)	+= sprd_thermal.o
>   obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL)	+= khadas_mcu_fan.o
>   obj-$(CONFIG_LOONGSON2_THERMAL)	+= loongson2_thermal.o
> diff --git a/drivers/thermal/spacemit/Kconfig b/drivers/thermal/spacemit/Kconfig
> new file mode 100644
> index 000000000000..de7b5ece5af2
> --- /dev/null
> +++ b/drivers/thermal/spacemit/Kconfig
> @@ -0,0 +1,19 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menu "SpacemiT thermal drivers"
> +depends on ARCH_SPACEMIT || COMPILE_TEST
> +
> +config SPACEMIT_K1_TSENSOR
> +	tristate "SpacemiT K1 thermal sensor driver"
> +	depends on THERMAL_OF
> +	help
> +	  This driver provides support for the thermal sensor
> +	  integrated in the SpacemiT K1 SoC.
> +
> +	  The thermal sensor monitors temperatures for five thermal zones:
> +	  soc, package, gpu, cluster0, and cluster1. It supports reporting
> +	  temperature values and handling high/low threshold interrupts.
> +
> +	  Say Y here if you want to enable thermal monitoring on SpacemiT K1.
> +	  If compiled as a module, it will be called k1_tsensor.
> +
> +endmenu
> diff --git a/drivers/thermal/spacemit/Makefile b/drivers/thermal/spacemit/Makefile
> new file mode 100644
> index 000000000000..82b30741e4ec
> --- /dev/null
> +++ b/drivers/thermal/spacemit/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_SPACEMIT_K1_TSENSOR)	+= k1_tsensor.o
> diff --git a/drivers/thermal/spacemit/k1_tsensor.c b/drivers/thermal/spacemit/k1_tsensor.c
> new file mode 100644
> index 000000000000..b742739e9019
> --- /dev/null
> +++ b/drivers/thermal/spacemit/k1_tsensor.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Thermal sensor driver for SpacemiT K1 SoC
> + *
> + * Copyright (C) 2026 Shuwei Wu <shuwei.wu@mailbox.org>
> + */
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#include "../thermal_hwmon.h"
> +
> +#define K1_TSENSOR_PCTRL_REG		0x00
> +#define K1_TSENSOR_PCTRL_ENABLE		BIT(0)
> +#define K1_TSENSOR_PCTRL_TEMP_MODE	BIT(3)
> +#define K1_TSENSOR_PCTRL_RAW_SEL	BIT(7)
> +
> +#define K1_TSENSOR_PCTRL_CTUNE		GENMASK(11, 8)
> +#define K1_TSENSOR_PCTRL_SW_CTRL	GENMASK(21, 18)
> +#define K1_TSENSOR_PCTRL_HW_AUTO_MODE	BIT(23)
> +
> +#define K1_TSENSOR_EN_REG		0x08
> +#define K1_TSENSOR_EN_ALL		GENMASK(MAX_SENSOR_NUMBER - 1, 0)
> +
> +#define K1_TSENSOR_TIME_REG		0x0C
> +#define K1_TSENSOR_TIME_WAIT_REF_CNT	GENMASK(3, 0)
> +#define K1_TSENSOR_TIME_ADC_CNT_RST	GENMASK(7, 4)
> +#define K1_TSENSOR_TIME_FILTER_PERIOD	GENMASK(21, 20)
> +#define K1_TSENSOR_TIME_MASK		GENMASK(23, 0)
> +
> +#define K1_TSENSOR_INT_CLR_REG		0x10
> +#define K1_TSENSOR_INT_EN_REG		0x14
> +#define K1_TSENSOR_INT_STA_REG		0x18
> +
> +#define K1_TSENSOR_INT_EN_MASK		BIT(0)
> +#define K1_TSENSOR_INT_MASK(x)		(GENMASK(2, 1) << ((x) * 2))
> +
> +#define K1_TSENSOR_DATA_BASE_REG	0x20
> +#define K1_TSENSOR_DATA_REG(x)		(K1_TSENSOR_DATA_BASE_REG + ((x) / 2) * 4)
> +#define K1_TSENSOR_DATA_LOW_MASK	GENMASK(15, 0)
> +#define K1_TSENSOR_DATA_HIGH_MASK	GENMASK(31, 16)
> +
> +#define K1_TSENSOR_THRSH_BASE_REG	0x40
> +#define K1_TSENSOR_THRSH_REG(x)		(K1_TSENSOR_THRSH_BASE_REG + ((x) * 4))
> +#define K1_TSENSOR_THRSH_LOW_MASK	GENMASK(15, 0)
> +#define K1_TSENSOR_THRSH_HIGH_MASK	GENMASK(31, 16)
> +
> +#define MAX_SENSOR_NUMBER		5
> +
> +/* Hardware offset value required for temperature calculation */
> +#define TEMPERATURE_OFFSET		278
> +
> +struct k1_tsensor_channel {
> +	struct k1_tsensor *ts;
> +	struct thermal_zone_device *tzd;
> +	int id;
> +};
> +
> +struct k1_tsensor {
> +	void __iomem *base;
> +	struct k1_tsensor_channel ch[MAX_SENSOR_NUMBER];
> +};
> +
> +static void k1_tsensor_init(struct k1_tsensor *ts)
> +{
> +	u32 val;
> +
> +	/* Disable all the interrupts */
> +	writel(0xffffffff, ts->base + K1_TSENSOR_INT_EN_REG);
> +
> +	/* Configure ADC sampling time and filter period */
> +	val = readl(ts->base + K1_TSENSOR_TIME_REG);
> +	val &= ~K1_TSENSOR_TIME_MASK;
> +	val |= K1_TSENSOR_TIME_FILTER_PERIOD |
> +	       K1_TSENSOR_TIME_ADC_CNT_RST |
> +	       K1_TSENSOR_TIME_WAIT_REF_CNT;
> +	writel(val, ts->base + K1_TSENSOR_TIME_REG);
> +
> +	/*
> +	 * Enable all sensors' auto mode, enable dither control,
> +	 * consecutive mode, and power up sensor.
> +	 */
> +	val = readl(ts->base + K1_TSENSOR_PCTRL_REG);
> +	val &= ~K1_TSENSOR_PCTRL_SW_CTRL;
> +	val &= ~K1_TSENSOR_PCTRL_CTUNE;
> +	val |= K1_TSENSOR_PCTRL_RAW_SEL |
> +	       K1_TSENSOR_PCTRL_TEMP_MODE |
> +	       K1_TSENSOR_PCTRL_HW_AUTO_MODE |
> +	       K1_TSENSOR_PCTRL_ENABLE;
> +	writel(val, ts->base + K1_TSENSOR_PCTRL_REG);
> +
> +	/* Enable thermal interrupt */
> +	val = readl(ts->base + K1_TSENSOR_INT_EN_REG);
> +	val |= K1_TSENSOR_INT_EN_MASK;
> +	writel(val, ts->base + K1_TSENSOR_INT_EN_REG);
> +
> +	/* Enable each sensor */
> +	val = readl(ts->base + K1_TSENSOR_EN_REG);
> +	val |= K1_TSENSOR_EN_ALL;
> +	writel(val, ts->base + K1_TSENSOR_EN_REG);
> +}
> +
> +static void k1_tsensor_enable_irq(struct k1_tsensor_channel *ch)
> +{
> +	struct k1_tsensor *ts = ch->ts;
> +	u32 val;
> +
> +	val = readl(ts->base + K1_TSENSOR_INT_CLR_REG);
> +	val |= K1_TSENSOR_INT_MASK(ch->id);
> +	writel(val, ts->base + K1_TSENSOR_INT_CLR_REG);
> +
> +	val = readl(ts->base + K1_TSENSOR_INT_EN_REG);
> +	val &= ~K1_TSENSOR_INT_MASK(ch->id);
> +	writel(val, ts->base + K1_TSENSOR_INT_EN_REG);
> +}
> +
> +/*
> + * The conversion formula used is:
> + * T(m°C) = (((raw_value & mask) >> shift) - TEMPERATURE_OFFSET) * 1000
> + */
> +static int k1_tsensor_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct k1_tsensor_channel *ch = thermal_zone_device_priv(tz);
> +	struct k1_tsensor *ts = ch->ts;
> +	u32 val;
> +
> +	val = readl(ts->base + K1_TSENSOR_DATA_REG(ch->id));
> +	if (ch->id % 2)
> +		*temp = FIELD_GET(K1_TSENSOR_DATA_HIGH_MASK, val);
> +	else
> +		*temp = FIELD_GET(K1_TSENSOR_DATA_LOW_MASK, val);
> +
> +	*temp -= TEMPERATURE_OFFSET;
> +	*temp *= 1000;
> +
> +	return 0;
> +}
> +
> +/*
> + * For each sensor, the hardware threshold register is 32 bits:
> + * - Lower 16 bits [15:0] configure the low threshold temperature.
> + * - Upper 16 bits [31:16] configure the high threshold temperature.
> + */
> +static int k1_tsensor_set_trips(struct thermal_zone_device *tz, int low, int high)
> +{
> +	struct k1_tsensor_channel *ch = thermal_zone_device_priv(tz);
> +	struct k1_tsensor *ts = ch->ts;
> +	u32 val;
> +
> +	if (low >= high)
> +		return -EINVAL;
> +
> +	if (low < 0)
> +		low = 0;
> +
> +	high = high / 1000 + TEMPERATURE_OFFSET;

Consider passes high = INT_MAX here:

high = INT/1000 + TEMPERATURE_OFFSET == 2147761;

> +	low = low / 1000 + TEMPERATURE_OFFSET;
> +
> +	val = readl(ts->base + K1_TSENSOR_THRSH_REG(ch->id));
> +	val &= ~K1_TSENSOR_THRSH_HIGH_MASK;
> +	val |= FIELD_PREP(K1_TSENSOR_THRSH_HIGH_MASK, high);

K1_TSENSOR_THRSH_HIGH_MASK is a 16-bit MASK:
FIELD_PREP(K1_TSENSOR_THRSH_HIGH_MASK, 2147761); <- overflow happened

the maximum value here will be changed to 50609 from 65536.

We should add a check here and limit the 'high' value here to avoid 
overflow:

if (high > (int)((0xFFFF - TEMPERATURE_OFFSET) * 1000))
	high = (0Xffff - TEMPERATURE_OFFSET) * 1000;

high = high / 1000 + TEMPERATURE_OFFSET;

...
	
> +
> +	val &= ~K1_TSENSOR_THRSH_LOW_MASK;
> +	val |= FIELD_PREP(K1_TSENSOR_THRSH_LOW_MASK, low);
> +	writel(val, ts->base + K1_TSENSOR_THRSH_REG(ch->id));
> +
> +	return 0;
> +}
> +
> +static const struct thermal_zone_device_ops k1_tsensor_ops = {
> +	.get_temp = k1_tsensor_get_temp,
> +	.set_trips = k1_tsensor_set_trips,
> +};
> +
> +static irqreturn_t k1_tsensor_irq_thread(int irq, void *data)
> +{
> +	struct k1_tsensor *ts = (struct k1_tsensor *)data;
> +	int mask, status, i;
> +
> +	status = readl(ts->base + K1_TSENSOR_INT_STA_REG);
> +
> +	for (i = 0; i < MAX_SENSOR_NUMBER; i++) {
> +		if (status & K1_TSENSOR_INT_MASK(i)) {
> +			mask = readl(ts->base + K1_TSENSOR_INT_CLR_REG);
> +			mask |= K1_TSENSOR_INT_MASK(i);
> +			writel(mask, ts->base + K1_TSENSOR_INT_CLR_REG);
> +			thermal_zone_device_update(ts->ch[i].tzd, THERMAL_EVENT_UNSPECIFIED);
> +		}
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int k1_tsensor_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct k1_tsensor *ts;
> +	struct reset_control *reset;
> +	struct clk *clk;
> +	int i, irq, ret;
> +
> +	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> +	if (!ts)
> +		return -ENOMEM;
> +
> +	ts->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(ts->base))
> +		return dev_err_probe(dev, PTR_ERR(ts->base), "Failed to get reg\n");
> +
> +	reset = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> +	if (IS_ERR(reset))
> +		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get/deassert reset control\n");
> +
> +	clk = devm_clk_get_enabled(dev, "core");
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk), "Failed to get core clock\n");
> +
> +	clk = devm_clk_get_enabled(dev, "bus");
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk), "Failed to get bus clock\n");
> +
> +	k1_tsensor_init(ts);
> +
> +	for (i = 0; i < MAX_SENSOR_NUMBER; ++i) {
> +		ts->ch[i].id = i;
> +		ts->ch[i].ts = ts;
> +		ts->ch[i].tzd = devm_thermal_of_zone_register(dev, i, ts->ch + i, &k1_tsensor_ops);
> +		if (IS_ERR(ts->ch[i].tzd))
> +			return PTR_ERR(ts->ch[i].tzd);
> +
> +		/* Attach sysfs hwmon attributes for userspace monitoring */
> +		ret = devm_thermal_add_hwmon_sysfs(dev, ts->ch[i].tzd);
> +		if (ret)
> +			dev_warn(dev, "Failed to add hwmon sysfs attributes\n");
> +
> +		k1_tsensor_enable_irq(ts->ch + i);

should call after the devm_request_threaded_irq succeeds;

Thanks,
Jie

> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	ret = devm_request_threaded_irq(dev, irq, NULL,
> +					k1_tsensor_irq_thread,
> +					IRQF_ONESHOT, "k1_tsensor", ts);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, ts);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id k1_tsensor_dt_ids[] = {
> +	{ .compatible = "spacemit,k1-tsensor" },
> +	{ /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, k1_tsensor_dt_ids);
> +
> +static struct platform_driver k1_tsensor_driver = {
> +	.driver = {
> +		.name		= "k1_tsensor",
> +		.of_match_table = k1_tsensor_dt_ids,
> +	},
> +	.probe	= k1_tsensor_probe,
> +};
> +module_platform_driver(k1_tsensor_driver);
> +
> +MODULE_DESCRIPTION("SpacemiT K1 Thermal Sensor Driver");
> +MODULE_AUTHOR("Shuwei Wu <shuwei.wu@mailbox.org>");
> +MODULE_LICENSE("GPL");
> 


  reply	other threads:[~2026-04-10  8:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-10  3:31 [PATCH v4 0/3] thermal: spacemit: Add support for SpacemiT K1 SoC thermal sensor Shuwei Wu
2026-04-10  3:31 ` [PATCH v4 1/3] dt-bindings: thermal: Add SpacemiT K1 " Shuwei Wu
2026-04-10  7:22   ` Krzysztof Kozlowski
2026-04-11  1:36     ` Shuwei Wu
2026-04-10  3:31 ` [PATCH v4 2/3] thermal: spacemit: k1: Add thermal sensor support Shuwei Wu
2026-04-10  8:25   ` Jie Gan [this message]
2026-04-11  1:56     ` Shuwei Wu
2026-04-10  3:31 ` [PATCH v4 3/3] riscv: dts: spacemit: Add thermal sensor for K1 SoC Shuwei Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=38088fd6-1db6-4591-b489-33b52f727549@oss.qualcomm.com \
    --to=jie.gan@oss.qualcomm.com \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor+dt@kernel.org \
    --cc=daniel.lezcano@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlan@kernel.org \
    --cc=gsh517025@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=legoll@online.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux.amoon@gmail.com \
    --cc=lukasz.luba@arm.com \
    --cc=me@ziyao.cc \
    --cc=p.zabel@pengutronix.de \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=shuwei.wu@mailbox.org \
    --cc=spacemit@lists.linux.dev \
    --cc=troy.mitchell@linux.spacemit.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox