From: Balsam CHIHI <[email protected]>
The LVTS (Low Voltage Thermal Sensor) driver is capable of monitoring
multiple hot points. For that, it contains 7 thermal control blocks
dedicated to specific devices on the die. Each control block can handle
up to 4 sensors.
The thermal controller supports several interrupts. One for the cold
trip point, the hot trip point, the return to the normal trip point,
and a specific programmable trip point to monitor the temperature
dynamically.
The temperature measurement can be done in two ways, the immediate mode
where the temperature read is instantaneous and the filtered mode where
the controller uses, by configuration, an average of a set of values
removing the minimum and the maximum.
Finally, it is composed of 2 finite-state machines responsible for
the state of the temperature (cold, hot, hot 2 normal, hot hot),
the triggering of the interrupts, and the monitoring of the temperature.
As requested, the thermal driver has been reworked to reduce
the complexity of the code. At this time, the 4 little CPUs and
the 4 big CPUs are supported by the thermal driver.They are described
in a data structure and more devices can be added later.
The calibration routine has been simplified also.
The series provide the following changes:
- Move the Mediatek drivers inside a dedicated folder as their number
is increasing
- Add the DT bindings for the controller
- Add the efuse node for the mt8195
- The LVTS driver
- The thermal zones description in the DT
Changelog:
v10:
- Rebase on top of "thermal/linux-next"
- Rework the LVTS driver
- Add the thermal trip temperature and cooling devices
for the sensors supported by the driver
v9:
- Rebase on top of 6.0.0-rc1
- Fix coding style issues
- Fix commit titles and commit messages
- Update dt-bindings :
- Add "allOf:if:then:"
- Use mt8192 as example (instead of mt8195)
- Fix dt-binding errors
- Fix DTS errors
v8:
- Fix coding style issues
- Rebase on top of next-20220803
- Add multi-instance support :
- Rewrite DT-binding and DTS :
- Add DT-binding and DTS for LVTS_v4 (MT8192 and MT8195)
- One LVTS node for each HW Domain (AP and MCU)
- One SW Instance for each HW Domain
- Add a Kconfig sub-menu entry for LVTS and LVTS_v4 SoCs
- Replace platform_get_resource by platform_get_mem_or_io to get
Base Address
- Replace platform_get_resource by platform_get_irq to get
Interrupt Number
- Add "lvts_" prefix to functions and structs
v7:
- Fix coding style issues
- Rewrite dt bindings
- was not accurate
- Use mt8195 for example (instead of mt8192)
- Rename mt6873 to mt8192
- Remove clock name
- Rebased on top of to series:
- patchwork.kernel.org/project/linux-mediatek/list/?series=637849
- patchwork.kernel.org/project/linux-pm/list/?series=639386
v6:
- Remove temperature aggregation (it will be added in another
series)
- Update the way to read the temperature (read one sensor
instead of all)
- Add support of mt8195
v5:
- Use 'git mv' for the relocated file.
v4:
- Rebase to kernel-v5.13-rc1
v3:
- change the expression in the lvts_temp_to_raw to dev_s64.
v2:
- Rebase to kernel-5.11-rc1.
- sort headers
- remove initial value 0 of msr_raw in the lvts_temp_to_raw.
- disconstruct the api of lvts_read_tc_msr_raw.
- add the initial value max_temp = 0 and compare e.q.
in the lvts_read_all_tc_temperature.
- add the return with an invalid number in the lvts_init.
Balsam CHIHI (6):
thermal/drivers/mediatek: Relocate driver to mediatek folder
dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal
controllers
arm64/dts/mt8195: Add efuse node to mt8195
thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver
arm64/dts/mt8195: Add thermal zones and thermal nodes
arm64/dts/mt8195: Add temperature mitigation threshold
.../thermal/mediatek,lvts-thermal.yaml | 140 ++
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 256 ++++
drivers/thermal/Kconfig | 14 +-
drivers/thermal/Makefile | 2 +-
drivers/thermal/mediatek/Kconfig | 36 +
drivers/thermal/mediatek/Makefile | 2 +
.../auxadc_thermal.c} | 2 +-
drivers/thermal/mediatek/lvts_thermal.c | 1244 +++++++++++++++++
include/dt-bindings/thermal/mediatek-lvts.h | 19 +
9 files changed, 1703 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
create mode 100644 drivers/thermal/mediatek/Kconfig
create mode 100644 drivers/thermal/mediatek/Makefile
rename drivers/thermal/{mtk_thermal.c => mediatek/auxadc_thermal.c} (99%)
create mode 100644 drivers/thermal/mediatek/lvts_thermal.c
create mode 100644 include/dt-bindings/thermal/mediatek-lvts.h
--
2.34.1
From: Balsam CHIHI <[email protected]>
The Low Voltage Thermal Sensor (LVTS) is a multiple sensors, multi
controllers contained in a thermal domain.
A thermal domains can be the MCU or the AP.
Each thermal domains contain up to seven controllers, each thermal
controller handle up to four thermal sensors.
The LVTS has two Finite State Machines (FSM), one to handle the
functionin temperatures range like hot or cold temperature and another
one to handle monitoring trip point. The FSM notifies via interrupts
when a trip point is crossed.
The interrupt is managed at the thermal controller level, so when an
interrupt occurs, the driver has to find out which sensor triggered
such an interrupt.
The sampling of the thermal can be filtered or immediate. For the
former, the LVTS measures several points and applies a low pass
filter.
Signed-off-by: Balsam CHIHI <[email protected]>
---
drivers/thermal/mediatek/Kconfig | 15 +
drivers/thermal/mediatek/Makefile | 1 +
drivers/thermal/mediatek/lvts_thermal.c | 1244 +++++++++++++++++++
include/dt-bindings/thermal/mediatek-lvts.h | 19 +
4 files changed, 1279 insertions(+)
create mode 100644 drivers/thermal/mediatek/lvts_thermal.c
create mode 100644 include/dt-bindings/thermal/mediatek-lvts.h
diff --git a/drivers/thermal/mediatek/Kconfig b/drivers/thermal/mediatek/Kconfig
index 7558a847d4e9..99597d7b9890 100644
--- a/drivers/thermal/mediatek/Kconfig
+++ b/drivers/thermal/mediatek/Kconfig
@@ -18,4 +18,19 @@ config MTK_SOC_THERMAL
This driver configures thermal controllers to collect
temperature via AUXADC interface.
+config MTK_LVTS_THERMAL
+ tristate "LVTS Thermal Driver for MediaTek SoCs"
+ depends on HAS_IOMEM
+ help
+ Enable this option if you want to get SoC temperature
+ information for supported MediaTek platforms.
+ This driver configures LVTS (Low Voltage Thermal Sensor)
+ thermal controllers to collect temperatures via ASIF
+ (Analog Serial Interface).
+
+config MTK_LVTS_THERMAL_DEBUGFS
+ bool "LVTS thermal debugfs"
+ depends on MTK_LVTS_THERMAL && DEBUG_FS
+ help
+ Enable this option to debug the internals of the device driver.
endif
diff --git a/drivers/thermal/mediatek/Makefile b/drivers/thermal/mediatek/Makefile
index 53e86e30b26f..1c6daa1e644b 100644
--- a/drivers/thermal/mediatek/Makefile
+++ b/drivers/thermal/mediatek/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_MTK_SOC_THERMAL) += auxadc_thermal.o
+obj-$(CONFIG_MTK_LVTS_THERMAL) += lvts_thermal.o
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
new file mode 100644
index 000000000000..a8fe64beb3c4
--- /dev/null
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -0,0 +1,1244 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Balsam CHIHI <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/debugfs.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/thermal.h>
+#include <dt-bindings/thermal/mediatek-lvts.h>
+
+#define LVTS_MONCTL0(__base) (__base + 0x0000)
+#define LVTS_MONCTL1(__base) (__base + 0x0004)
+#define LVTS_MONCTL2(__base) (__base + 0x0008)
+#define LVTS_MONINT(__base) (__base + 0x000C)
+#define LVTS_MONINTSTS(__base) (__base + 0x0010)
+#define LVTS_MONIDET0(__base) (__base + 0x0014)
+#define LVTS_MONIDET1(__base) (__base + 0x0018)
+#define LVTS_MONIDET2(__base) (__base + 0x001C)
+#define LVTS_MONIDET3(__base) (__base + 0x0020)
+#define LVTS_H2NTHRE(__base) (__base + 0x0024)
+#define LVTS_HTHRE(__base) (__base + 0x0028)
+#define LVTS_OFFSETH(__base) (__base + 0x0030)
+#define LVTS_OFFSETL(__base) (__base + 0x0034)
+#define LVTS_MSRCTL0(__base) (__base + 0x0038)
+#define LVTS_MSRCTL1(__base) (__base + 0x003C)
+#define LVTS_TSSEL(__base) (__base + 0x0040)
+#define LVTS_CALSCALE(__base) (__base + 0x0048)
+#define LVTS_ID(__base) (__base + 0x004C)
+#define LVTS_CONFIG(__base) (__base + 0x0050)
+#define LVTS_EDATA00(__base) (__base + 0x0054)
+#define LVTS_EDATA01(__base) (__base + 0x0058)
+#define LVTS_EDATA02(__base) (__base + 0x005C)
+#define LVTS_EDATA03(__base) (__base + 0x0060)
+#define LVTS_MSR0(__base) (__base + 0x0090)
+#define LVTS_MSR1(__base) (__base + 0x0094)
+#define LVTS_MSR2(__base) (__base + 0x0098)
+#define LVTS_MSR3(__base) (__base + 0x009C)
+#define LVTS_IMMD0(__base) (__base + 0x00A0)
+#define LVTS_IMMD1(__base) (__base + 0x00A4)
+#define LVTS_IMMD2(__base) (__base + 0x00A8)
+#define LVTS_IMMD3(__base) (__base + 0x00AC)
+#define LVTS_PROTCTL(__base) (__base + 0x00C0)
+#define LVTS_PROTTA(__base) (__base + 0x00C4)
+#define LVTS_PROTTB(__base) (__base + 0x00C8)
+#define LVTS_PROTTC(__base) (__base + 0x00CC)
+#define LVTS_CLKEN(__base) (__base + 0x00E4)
+
+#define LVTS_SENSOR_MAX 4
+#define LVTS_GOLDEN_TEMP_MAX 62
+#define LVTS_GOLDEN_TEMP_DEFAULT 50
+#define LVTS_COEFF_A -250460
+#define LVTS_COEFF_B 250460
+
+#define LVTS_MSR_IMMEDIATE_MODE 0
+#define LVTS_MSR_FILTERED_MODE 1
+
+#define LVTS_HW_SHUTDOWN_MT8195 105000
+
+static int golden_temp = LVTS_GOLDEN_TEMP_DEFAULT;
+static int coeff_b = LVTS_COEFF_B;
+
+struct lvts_sensor_data {
+ int dt_id;
+};
+
+struct lvts_ctrl_data {
+ struct lvts_sensor_data lvts_sensor[LVTS_SENSOR_MAX];
+ int cal_offset[LVTS_SENSOR_MAX];
+ int hw_tshut_temp;
+ int num_lvts_sensor;
+ int offset;
+ int mode;
+};
+
+struct lvts_data {
+ struct lvts_ctrl_data *lvts_ctrl;
+ int num_lvts_ctrl;
+};
+
+struct lvts_sensor {
+ struct thermal_zone_device *tz;
+ void __iomem *msr;
+ void __iomem *base;
+ int id;
+ int dt_id;
+};
+
+struct lvts_ctrl {
+ struct lvts_sensor sensors[LVTS_SENSOR_MAX];
+ u32 calibration[LVTS_SENSOR_MAX];
+ u32 hw_tshut_raw_temp;
+ int num_lvts_sensor;
+ int mode;
+ void __iomem *base;
+};
+
+struct lvts_domain {
+ struct lvts_ctrl *lvts_ctrl;
+ struct reset_control *reset;
+ struct clk *clk;
+ int num_lvts_ctrl;
+ void __iomem *base;
+ size_t calib_len;
+ u8 *calib;
+};
+
+#ifdef CONFIG_MTK_LVTS_THERMAL_DEBUGFS
+
+static struct dentry *root;
+
+#define LVTS_DEBUG_FS_REGS(__reg) \
+{ \
+ .name = __stringify(__reg), \
+ .offset = __reg(0), \
+}
+
+static const struct debugfs_reg32 lvts_regs[] = {
+ LVTS_DEBUG_FS_REGS(LVTS_MONCTL0),
+ LVTS_DEBUG_FS_REGS(LVTS_MONCTL1),
+ LVTS_DEBUG_FS_REGS(LVTS_MONCTL2),
+ LVTS_DEBUG_FS_REGS(LVTS_MONINT),
+ LVTS_DEBUG_FS_REGS(LVTS_MONINTSTS),
+ LVTS_DEBUG_FS_REGS(LVTS_MONIDET0),
+ LVTS_DEBUG_FS_REGS(LVTS_MONIDET1),
+ LVTS_DEBUG_FS_REGS(LVTS_MONIDET2),
+ LVTS_DEBUG_FS_REGS(LVTS_MONIDET3),
+ LVTS_DEBUG_FS_REGS(LVTS_H2NTHRE),
+ LVTS_DEBUG_FS_REGS(LVTS_HTHRE),
+ LVTS_DEBUG_FS_REGS(LVTS_OFFSETH),
+ LVTS_DEBUG_FS_REGS(LVTS_OFFSETL),
+ LVTS_DEBUG_FS_REGS(LVTS_MSRCTL0),
+ LVTS_DEBUG_FS_REGS(LVTS_MSRCTL1),
+ LVTS_DEBUG_FS_REGS(LVTS_TSSEL),
+ LVTS_DEBUG_FS_REGS(LVTS_CALSCALE),
+ LVTS_DEBUG_FS_REGS(LVTS_ID),
+ LVTS_DEBUG_FS_REGS(LVTS_CONFIG),
+ LVTS_DEBUG_FS_REGS(LVTS_EDATA00),
+ LVTS_DEBUG_FS_REGS(LVTS_EDATA01),
+ LVTS_DEBUG_FS_REGS(LVTS_EDATA02),
+ LVTS_DEBUG_FS_REGS(LVTS_EDATA03),
+ LVTS_DEBUG_FS_REGS(LVTS_MSR0),
+ LVTS_DEBUG_FS_REGS(LVTS_MSR1),
+ LVTS_DEBUG_FS_REGS(LVTS_MSR2),
+ LVTS_DEBUG_FS_REGS(LVTS_MSR3),
+ LVTS_DEBUG_FS_REGS(LVTS_IMMD0),
+ LVTS_DEBUG_FS_REGS(LVTS_IMMD1),
+ LVTS_DEBUG_FS_REGS(LVTS_IMMD2),
+ LVTS_DEBUG_FS_REGS(LVTS_IMMD3),
+ LVTS_DEBUG_FS_REGS(LVTS_PROTCTL),
+ LVTS_DEBUG_FS_REGS(LVTS_PROTTA),
+ LVTS_DEBUG_FS_REGS(LVTS_PROTTB),
+ LVTS_DEBUG_FS_REGS(LVTS_PROTTC),
+ LVTS_DEBUG_FS_REGS(LVTS_CLKEN),
+};
+
+static int lvts_debugfs_init(struct device *dev,
+ struct lvts_domain *lvts_td)
+{
+ struct debugfs_regset32 *regset;
+ struct lvts_ctrl *lvts_ctrl;
+ struct dentry *dentry;
+ struct dentry *dom_dentry;
+ char name[64];
+ int i;
+
+ if (!root)
+ root = debugfs_create_dir("lvts", NULL);
+
+ dom_dentry = debugfs_create_dir(dev_name(dev), root);
+ if (!dom_dentry)
+ return 0;
+
+ for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {
+
+ lvts_ctrl = &lvts_td->lvts_ctrl[i];
+
+ sprintf(name, "controller%d", i);
+ dentry = debugfs_create_dir(name, dom_dentry);
+ if (!dentry)
+ continue;
+
+ regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
+ if (!regset)
+ continue;
+
+ regset->base = lvts_ctrl->base;
+ regset->regs = lvts_regs;
+ regset->nregs = ARRAY_SIZE(lvts_regs);
+
+ debugfs_create_regset32("registers", 0400, dentry, regset);
+ }
+
+ return 0;
+}
+
+static void lvts_debugfs_exit(void)
+{
+ debugfs_remove_recursive(root);
+}
+
+#else
+
+static inline int lvts_debugfs_init(struct device *dev,
+ struct lvts_domain *lvts_td)
+{
+ return 0;
+}
+
+static void lvts_debugfs_exit(void) { }
+
+#endif
+
+static int lvts_raw_to_temp(u32 raw_temp)
+{
+ int temperature;
+
+ temperature = ((s64)(raw_temp & 0xFFFF) * LVTS_COEFF_A) >> 14;
+ temperature += coeff_b;
+
+ return temperature;
+}
+
+static u32 lvts_temp_to_raw(int temperature)
+{
+ u32 raw_temp = ((s64)(coeff_b - temperature)) << 14;
+
+ raw_temp = div_s64(raw_temp, -LVTS_COEFF_A);
+
+ return raw_temp;
+}
+
+static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+ struct lvts_sensor *lvts_sensor = tz->devdata;
+ void __iomem *msr = lvts_sensor->msr;
+ u32 value;
+
+ /*
+ * Measurement registers:
+ *
+ * LVTS_MSR[0-3] / LVTS_IMMD[0-3]
+ *
+ * Bits:
+ *
+ * 32-17: Unused
+ * 16 : Valid temperature
+ * 15-0 : Raw temperature
+ */
+ value = readl(msr);
+
+ /*
+ * As the thermal zone temperature will read before the
+ * hardware sensor is fully initialized, we have to check the
+ * validity of the temperature returned when reading the
+ * measurement register. The thermal controller will set the
+ * valid bit temperature only when it is totally initialized.
+ *
+ * Otherwise, we may end up with garbage values out of the
+ * functionning temperature and directly jump to a system
+ * shutdown.
+ */
+ if (!(value & BIT(16)))
+ return -EAGAIN;
+
+ *temp = lvts_raw_to_temp(value & 0xFFFF);
+
+ return 0;
+}
+
+static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
+{
+ struct lvts_sensor *lvts_sensor = tz->devdata;
+ void __iomem *base = lvts_sensor->base;
+ u32 raw_low = lvts_temp_to_raw(low);
+ u32 raw_high = lvts_temp_to_raw(high);
+
+ /*
+ * Hot to normal temperature threshold
+ *
+ * LVTS_H2NTHRE
+ *
+ * Bits:
+ *
+ * 14-0 : Raw temperature for threshold
+ */
+ if (low != -INT_MAX) {
+ dev_dbg(&tz->device, "Setting low limit temperature interrupt: %d\n", low);
+ writel(raw_low, LVTS_H2NTHRE(base));
+ }
+
+ /*
+ * Hot temperature threshold
+ *
+ * LVTS_HTHRE
+ *
+ * Bits:
+ *
+ * 14-0 : Raw temperature for threshold
+ */
+ dev_dbg(&tz->device, "Setting high limit temperature interrupt: %d\n", high);
+ writel(raw_high, LVTS_HTHRE(base));
+
+ return 0;
+}
+
+static irqreturn_t lvts_ctrl_irq_handler(struct lvts_ctrl *lvts_ctrl)
+{
+ irqreturn_t iret = IRQ_NONE;
+ u32 value, masks[] = { 0x0009001F, 0X000881F0, 0x00247C00, 0x1FC00000 };
+ int i;
+
+ /*
+ * Interrupt monitoring status
+ *
+ * LVTS_MONINTST
+ *
+ * Bits:
+ *
+ * 31 : Interrupt for stage 3
+ * 30 : Interrupt for stage 2
+ * 29 : Interrupt for state 1
+ * 28 : Interrupt using filter on sensor 3
+ *
+ * 27 : Interrupt using immediate on sensor 3
+ * 26 : Interrupt normal to hot on sensor 3
+ * 25 : Interrupt high offset on sensor 3
+ * 24 : Interrupt low offset on sensor 3
+ *
+ * 23 : Interrupt hot threshold on sensor 3
+ * 22 : Interrupt cold threshold on sensor 3
+ * 21 : Interrupt using filter on sensor 2
+ * 20 : Interrupt using filter on sensor 1
+ *
+ * 19 : Interrupt using filter on sensor 0
+ * 18 : Interrupt using immediate on sensor 2
+ * 17 : Interrupt using immediate on sensor 1
+ * 16 : Interrupt using immediate on sensor 0
+ *
+ * 15 : Interrupt device access timeout interrupt
+ * 14 : Interrupt normal to hot on sensor 2
+ * 13 : Interrupt high offset interrupt on sensor 2
+ * 12 : Interrupt low offset interrupt on sensor 2
+ *
+ * 11 : Interrupt hot threshold on sensor 2
+ * 10 : Interrupt cold threshold on sensor 2
+ * 9 : Interrupt normal to hot on sensor 1
+ * 8 : Interrupt high offset interrupt on sensor 1
+ *
+ * 7 : Interrupt low offset interrupt on sensor 1
+ * 6 : Interrupt hot threshold on sensor 1
+ * 5 : Interrupt cold threshold on sensor 1
+ * 4 : Interrupt normal to hot on sensor 0
+ *
+ * 3 : Interrupt high offset interrupt on sensor 0
+ * 2 : Interrupt low offset interrupt on sensor 0
+ * 1 : Interrupt hot threshold on sensor 0
+ * 0 : Interrupt cold threshold on sensor 0
+ *
+ * We are interested in the sensor(s) responsible of the
+ * interrupt event. We update the thermal framework with the
+ * thermal zone associated with the sensor. The framework will
+ * take care of the rest whatever the kind of interrupt, we
+ * are only interested in which sensor raised the interrupt.
+ *
+ * sensor 3 interrupt: 0001 1111 1100 0000 0000 0000 0000 0000
+ * => 0x1FC00000
+ * sensor 2 interrupt: 0000 0000 0010 0100 0111 1100 0000 0000
+ * => 0x00247C00
+ * sensor 1 interrupt: 0000 0000 0001 0001 0000 0011 1110 0000
+ * => 0X000881F0
+ * sensor 0 interrupt: 0000 0000 0000 1001 0000 0000 0001 1111
+ * => 0x0009001F
+ */
+ value = readl(LVTS_MONINTSTS(lvts_ctrl->base));
+
+ /*
+ * Let's figure out which sensors raised the interrupt
+ *
+ * NOTE: the masks array must be ordered with the index
+ * corresponding to the sensor id eg. index=0, mask for
+ * sensor0.
+ */
+ for (i = 0; i < ARRAY_SIZE(masks); i++) {
+
+ if (!(value & masks[i]))
+ continue;
+
+ thermal_zone_device_update(lvts_ctrl->sensors[i].tz,
+ THERMAL_TRIP_VIOLATED);
+ iret |= IRQ_HANDLED;
+ }
+
+ /*
+ * Write back to clear the interrupt status (W1C)
+ */
+ writel(value, LVTS_MONINTSTS(lvts_ctrl->base));
+
+ return iret;
+}
+
+/*
+ * Temperature interrupt handler. Even if the driver supports more
+ * interrupt modes, we use the interrupt when the temperature crosses
+ * the hot threshold the way up and the way down (modulo the
+ * hysteresis).
+ *
+ * Each thermal domain has a couple of interrupts, one for hardware
+ * reset and another one for all the thermal events happening on the
+ * different sensors.
+ *
+ * The interrupt is configured for thermal events when crossing the
+ * hot temperature limit. At each interrupt, we check in every
+ * controller if there is an interrupt pending.
+ */
+static irqreturn_t lvts_irq_handler(int irq, void *data)
+{
+ struct lvts_domain *lvts_td = data;
+ irqreturn_t iret = IRQ_NONE;
+ int i;
+
+ for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
+ iret |= lvts_ctrl_irq_handler(lvts_td->lvts_ctrl);
+
+ return iret;
+}
+
+static struct thermal_zone_device_ops lvts_ops = {
+ .get_temp = lvts_get_temp,
+ .set_trips = lvts_set_trips,
+};
+
+static int __init lvts_sensor_init(struct device *dev,
+ struct lvts_ctrl *lvts_ctrl,
+ struct lvts_ctrl_data *lvts_ctrl_data)
+{
+ struct lvts_sensor *lvts_sensor = lvts_ctrl->sensors;
+ void __iomem *msr_regs[] = {
+ LVTS_MSR0(lvts_ctrl->base),
+ LVTS_MSR1(lvts_ctrl->base),
+ LVTS_MSR2(lvts_ctrl->base),
+ LVTS_MSR3(lvts_ctrl->base)
+ };
+
+ void __iomem *imm_regs[] = {
+ LVTS_IMMD0(lvts_ctrl->base),
+ LVTS_IMMD1(lvts_ctrl->base),
+ LVTS_IMMD2(lvts_ctrl->base),
+ LVTS_IMMD3(lvts_ctrl->base)
+ };
+
+ int i;
+
+ for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++) {
+
+ int dt_id = lvts_ctrl_data->lvts_sensor[i].dt_id;
+
+ /*
+ * At this point, we don't know which id matches which
+ * sensor. Let's set arbitrally the id from the index.
+ */
+ lvts_sensor[i].id = i;
+
+ /*
+ * The thermal zone registration will set the trip
+ * point interrupt in the thermal controller
+ * register. But this one will be reset in the
+ * initialization after. So we need to post pone the
+ * thermal zone creation after the controller is
+ * setup. For this reason, we store the device tree
+ * node id from the data in the sensor structure
+ */
+ lvts_sensor[i].dt_id = dt_id;
+
+ /*
+ * We assign the base address of the thermal
+ * controller as a back pointer. So it will be
+ * accessible from the different thermal framework ops
+ * as we pass the lvts_sensor pointer as thermal zone
+ * private data.
+ */
+ lvts_sensor[i].base = lvts_ctrl->base;
+
+ /*
+ * Each sensor has its own register address to read from.
+ */
+ lvts_sensor[i].msr = lvts_ctrl_data->mode == LVTS_MSR_IMMEDIATE_MODE ?
+ imm_regs[i] : msr_regs[i];
+ };
+
+ lvts_ctrl->num_lvts_sensor = lvts_ctrl_data->num_lvts_sensor;
+
+ return 0;
+}
+
+/*
+ * The efuse blob values follows the sensor enumeration per thermal
+ * controller. The decoding of the stream is as follow:
+ *
+ * <--?-> <----big0 ???---> <-sensor0-> <-0->
+ * ------------------------------------------
+ * index in the stream: : | 0x0 | 0x1 | 0x2 | 0x3 | 0x4 | 0x5 | 0x6 |
+ * ------------------------------------------
+ *
+ * <--sensor1--><-0-> <----big1 ???---> <-sen
+ * ------------------------------------------
+ * | 0x7 | 0x8 | 0x9 | 0xA | 0xB | OxC | OxD |
+ * ------------------------------------------
+ *
+ * sor0-> <-0-> <-sensor1-> <-0-> ..........
+ * ------------------------------------------
+ * | 0x7 | 0x8 | 0x9 | 0xA | 0xB | OxC | OxD |
+ * ------------------------------------------
+ *
+ * And so on ...
+ *
+ * The data description gives the offset of the calibration data in
+ * this bytes stream for each sensor.
+ *
+ * Each thermal controller can handle up to 4 sensors max, we don't
+ * care if there are less as the array of calibration is sized to 4
+ * anyway. The unused sensor slot will be zeroed.
+ */
+static int __init lvts_calibration_init(struct device *dev,
+ struct lvts_ctrl *lvts_ctrl,
+ struct lvts_ctrl_data *lvts_ctrl_data,
+ u8 *efuse_calibration)
+{
+ int i;
+
+ for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++)
+ memcpy(&lvts_ctrl->calibration[i],
+ efuse_calibration + lvts_ctrl_data->cal_offset[i], 2);
+
+ return 0;
+}
+
+/*
+ * The efuse bytes stream can be split into different chunk of
+ * nvmems. This function reads and concatenate those into a single
+ * buffer so it can be read sequentially when initializing the
+ * calibration data.
+ */
+static int lvts_calibration_read(struct device *dev, struct lvts_domain *lvts_td,
+ struct lvts_data *lvts_data)
+{
+ struct device_node *np = dev_of_node(dev);
+ struct nvmem_cell *cell;
+ struct property *prop;
+ const char *cell_name;
+
+ of_property_for_each_string(np, "nvmem-cell-names", prop, cell_name) {
+ size_t len;
+ u8 *efuse;
+
+ cell = of_nvmem_cell_get(np, cell_name);
+ if (IS_ERR(cell)) {
+ dev_dbg(dev, "Failed to get cell '%s'\n", cell_name);
+ return PTR_ERR(cell);
+ }
+
+ efuse = nvmem_cell_read(cell, &len);
+
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(efuse)) {
+ dev_dbg(dev, "Failed to read cell '%s'\n", cell_name);
+ return PTR_ERR(efuse);
+ }
+
+ lvts_td->calib = devm_krealloc(dev, lvts_td->calib,
+ lvts_td->calib_len + len, GFP_KERNEL);
+ if (!lvts_td->calib)
+ return -ENOMEM;
+
+ memcpy(lvts_td->calib + lvts_td->calib_len, efuse, len);
+
+ lvts_td->calib_len += len;
+
+ kfree(efuse);
+ }
+
+ return 0;
+}
+
+static int __init lvts_golden_temp_init(struct device *dev, u32 *value)
+{
+ u32 gt;
+
+ gt = (*value) >> 24;
+
+ if (gt && gt < LVTS_GOLDEN_TEMP_MAX)
+ golden_temp = gt;
+
+ coeff_b = golden_temp * 500 + LVTS_COEFF_B;
+
+ return 0;
+}
+
+static int __init lvts_ctrl_init(struct device *dev,
+ struct lvts_domain *lvts_td,
+ struct lvts_data *lvts_data)
+{
+ size_t size = sizeof(*lvts_td->lvts_ctrl) * lvts_data->num_lvts_ctrl;
+ struct lvts_ctrl *lvts_ctrl;
+ int i, ret;
+
+ /*
+ * Create the calibration bytes stream from efuse data
+ */
+ ret = lvts_calibration_read(dev, lvts_td, lvts_data);
+ if (ret)
+ return ret;
+
+ /*
+ * The golden temp information is contained in the first chunk
+ * of efuse data.
+ */
+ ret = lvts_golden_temp_init(dev, (u32 *)lvts_td->calib);
+ if (ret)
+ return ret;
+
+ lvts_ctrl = devm_kzalloc(dev, size, GFP_KERNEL);
+ if (!lvts_ctrl)
+ return -ENOMEM;
+
+ for (i = 0; i < lvts_data->num_lvts_ctrl; i++) {
+
+ lvts_ctrl[i].base = lvts_td->base + lvts_data->lvts_ctrl[i].offset;
+
+ ret = lvts_sensor_init(dev, &lvts_ctrl[i],
+ &lvts_data->lvts_ctrl[i]);
+ if (ret)
+ return ret;
+
+ ret = lvts_calibration_init(dev, &lvts_ctrl[i],
+ &lvts_data->lvts_ctrl[i],
+ lvts_td->calib);
+ if (ret)
+ return ret;
+
+ /*
+ * The mode the ctrl will use to read the temperature
+ * (filtered or immediate)
+ */
+ lvts_ctrl[i].mode = lvts_data->lvts_ctrl[i].mode;
+
+ /*
+ * The temperature to raw temperature must be done
+ * after initializing the calibration.
+ */
+ lvts_ctrl[i].hw_tshut_raw_temp =
+ lvts_temp_to_raw(lvts_data->lvts_ctrl[i].hw_tshut_temp);
+ }
+
+ /*
+ * We no longer need the efuse bytes stream, let's free it
+ */
+ devm_kfree(dev, lvts_td->calib);
+
+ lvts_td->lvts_ctrl = lvts_ctrl;
+ lvts_td->num_lvts_ctrl = lvts_data->num_lvts_ctrl;
+
+ return 0;
+}
+
+/*
+ * At this point the configuration register is the only place in the
+ * driver where we write multiple values. Per hardware constraint,
+ * each write in the configuration register must be separated by a
+ * delay of 2 us.
+ */
+static void lvts_write_config(struct lvts_ctrl *lvts_ctrl, u32 *cmds, int nr_cmds)
+{
+ int i;
+
+ /*
+ * Configuration register
+ */
+ for (i = 0; i < nr_cmds; i++) {
+ writel(cmds[i], LVTS_CONFIG(lvts_ctrl->base));
+ usleep_range(2, 4);
+ }
+}
+
+static int lvts_irq_init(struct lvts_ctrl *lvts_ctrl)
+{
+ u32 value;
+
+ /*
+ * LVTS_PROTCTL : Thermal Protection Sensor Selection
+ *
+ * Bits:
+ *
+ * 19-18 : Sensor to base the protection on
+ * 17-16 : Strategy:
+ * 00 : Average of 4 sensors
+ * 01 : Max of 4 sensors
+ * 10 : Selected sensor with bits 19-18
+ * 11 : Reserved
+ */
+ value = BIT(16);
+ writel(value, LVTS_PROTCTL(lvts_ctrl->base));
+
+ /*
+ * LVTS_PROTTA : Stage 1 temperature threshold
+ * LVTS_PROTTB : Stage 2 temperature threshold
+ * LVTS_PROTTC : Stage 3 temperature threshold
+ *
+ * Bits:
+ *
+ * 14-0: Raw temperature threshold
+ *
+ * writel(0x0, LVTS_PROTTA(lvts_ctrl->base));
+ * writel(0x0, LVTS_PROTTB(lvts_ctrl->base));
+ */
+ writel(lvts_ctrl->hw_tshut_raw_temp, LVTS_PROTTC(lvts_ctrl->base));
+
+ /*
+ * LVTS_MONINT : Interrupt configuration register
+ *
+ * The LVTS_MONINT register layout is the same as the LVTS_MONINTSTS
+ * register, except we set the bits to enable the interrupt.
+ */
+ value = 0x9FBF7BDE;
+ writel(value, LVTS_MONINT(lvts_ctrl->base));
+
+ return 0;
+}
+
+static int lvts_domain_reset(struct device *dev, struct reset_control *reset)
+{
+ int ret;
+
+ ret = reset_control_assert(reset);
+ if (ret)
+ return ret;
+
+ return reset_control_deassert(reset);
+}
+
+/*
+ * Enable or disable the clocks of a specified thermal controller
+ */
+static int lvts_ctrl_set_enable(struct lvts_ctrl *lvts_ctrl, int enable)
+{
+ /*
+ * LVTS_CLKEN : Internal LVTS clock
+ *
+ * Bits:
+ *
+ * 0 : enable / disable clock
+ */
+ writel(enable, LVTS_CLKEN(lvts_ctrl->base));
+
+ return 0;
+}
+
+static inline int lvts_ctrl_enable(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ return lvts_ctrl_set_enable(lvts_ctrl, 1);
+}
+
+static inline int lvts_ctrl_disable(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ return lvts_ctrl_set_enable(lvts_ctrl, 0);
+}
+
+static int lvts_ctrl_connect(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ u32 id, cmds[] = { 0xC103FFFF, 0xC502FF55 };
+
+ lvts_write_config(lvts_ctrl, cmds, ARRAY_SIZE(cmds));
+
+ /*
+ * LVTS_ID : Get ID and status of the thermal controller
+ *
+ * Bits:
+ *
+ * 0-5 : thermal controller id
+ * 7 : thermal controller connection is valid
+ */
+ id = readl(LVTS_ID(lvts_ctrl->base));
+ if (!(id & BIT(7)))
+ return -EIO;
+
+ return 0;
+}
+
+static int lvts_ctrl_initialize(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ /*
+ * Write device mask: 0xC1030000
+ */
+ u32 cmds[] = {
+ 0xC1030E01, 0xC1030CFC, 0xC1030A8C, 0xC103098D, 0xC10308F1,
+ 0xC10307A6, 0xC10306B8, 0xC1030500, 0xC1030420, 0xC1030300,
+ 0xC1030030, 0xC10300F6, 0xC1030050, 0xC1030060, 0xC10300AC,
+ 0xC10300FC, 0xC103009D, 0xC10300F1, 0xC10300E1
+ };
+
+ lvts_write_config(lvts_ctrl, cmds, ARRAY_SIZE(cmds));
+
+ return 0;
+}
+
+static int lvts_ctrl_calibrate(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ int i;
+ void __iomem *lvts_edata[] = {
+ LVTS_EDATA00(lvts_ctrl->base),
+ LVTS_EDATA01(lvts_ctrl->base),
+ LVTS_EDATA02(lvts_ctrl->base),
+ LVTS_EDATA03(lvts_ctrl->base)
+ };
+
+ /*
+ * LVTS_EDATA0X : Efuse calibration reference value for sensor X
+ *
+ * Bits:
+ *
+ * 20-0 : Efuse value for normalization data
+ */
+ for (i = 0; i < LVTS_SENSOR_MAX; i++)
+ writel(lvts_ctrl->calibration[i], lvts_edata[i]);
+
+ return 0;
+}
+
+static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ u32 period_unit = (118 * 1000) / (256 * 38);
+ u32 grp_interval = 1;
+ u32 flt_interval = 1;
+ u32 sensor_interval = 1;
+ u32 hw_filter = 0x2;
+ u32 value;
+
+ /*
+ * LVTS_TSSEL : Sensing point index numbering
+ *
+ * Bits:
+ *
+ * 31-24: ADC Sense 3
+ * 23-16: ADC Sense 2
+ * 15-8 : ADC Sense 1
+ * 7-0 : ADC Sense 0
+ */
+ value = 0x13121110;
+ writel(value, LVTS_TSSEL(lvts_ctrl->base));
+
+ /*
+ * LVTS_CALSCALE : ADC voltage round
+ */
+ value = 0x300;
+ writel(value, LVTS_CALSCALE(lvts_ctrl->base));
+
+ /*
+ * LVTS_MSRCTL0 : Sensor filtering strategy
+ *
+ * Filters:
+ *
+ * 000 : One sample
+ * 001 : Avg 2 samples
+ * 010 : 4 samples, drop min and max, avg 2 samples
+ * 011 : 6 samples, drop min and max, avg 4 samples
+ * 100 : 10 samples, drop min and max, avg 8 samples
+ * 101 : 18 samples, drop min and max, avg 16 samples
+ *
+ * Bits:
+ *
+ * 0-2 : Sensor0 filter
+ * 3-5 : Sensor1 filter
+ * 6-8 : Sensor2 filter
+ * 9-11 : Sensor3 filter
+ */
+ value = hw_filter << 9 | hw_filter << 6 | hw_filter << 3 | hw_filter;
+ writel(value, LVTS_MSRCTL0(lvts_ctrl->base));
+
+ /*
+ * LVTS_MSRCTL1 : Measurement control
+ *
+ * Bits:
+ *
+ * 9: Ignore MSRCTL0 config and do immediate measurement on sensor3
+ * 6: Ignore MSRCTL0 config and do immediate measurement on sensor2
+ * 5: Ignore MSRCTL0 config and do immediate measurement on sensor1
+ * 4: Ignore MSRCTL0 config and do immediate measurement on sensor0
+ *
+ * That configuration will ignore the filtering and the delays
+ * introduced below in MONCTL1 and MONCTL2
+ */
+ if (lvts_ctrl->mode == LVTS_MSR_IMMEDIATE_MODE) {
+ value = BIT(9) | BIT(6) | BIT(5) | BIT(4);
+ writel(value, LVTS_MSRCTL1(lvts_ctrl->base));
+ }
+
+ /*
+ * LVTS_MONCTL1 : Period unit and group interval configuration
+ *
+ * The clock source of LVTS thermal controller is 26MHz.
+ *
+ * The period unit is a time base for all the interval delays
+ * specified in the registers. By default we use 12. The time
+ * conversion is done by multiplying by 256 and 1/26.10^6
+ *
+ * An interval delay multiplied by the period unit gives the
+ * duration in seconds.
+ *
+ * - Filter interval delay is a delay between two samples of
+ * the same sensor.
+ *
+ * - Sensor interval delay is a delay between two samples of
+ * different sensors.
+ *
+ * - Group interval delay is a delay between different rounds.
+ *
+ * For example:
+ * If Period unit = C, filter delay = 1, sensor delay = 2, group delay = 1,
+ * and two sensors, TS1 and TS2, are in a LVTS thermal controller
+ * and then
+ * Period unit time = C * 1/26M * 256 = 12 * 38.46ns * 256 = 118.149us
+ * Filter interval delay = 1 * Period unit = 118.149us
+ * Sensor interval delay = 2 * Period unit = 236.298us
+ * Group interval delay = 1 * Period unit = 118.149us
+ *
+ * TS1 TS1 ... TS1 TS2 TS2 ... TS2 TS1...
+ * <--> Filter interval delay
+ * <--> Sensor interval delay
+ * <--> Group interval delay
+ * Bits:
+ * 29 - 20 : Group interval
+ * 16 - 13 : Send a single interrupt when crossing the hot threshold (1)
+ * or an interrupt everytime the hot threshold is crossed (0)
+ * 9 - 0 : Period unit
+ *
+ */
+ value = grp_interval << 20 | period_unit;
+ writel(value, LVTS_MONCTL1(lvts_ctrl->base));
+
+ /*
+ * LVTS_MONCTL2 : Filtering and sensor interval
+ *
+ * Bits:
+ *
+ * 25-16 : Interval unit in PERIOD_UNIT between sample on
+ * the same sensor, filter interval
+ * 9-0 : Interval unit in PERIOD_UNIT between each sensor
+ *
+ */
+ value = flt_interval << 16 | sensor_interval;
+ writel(value, LVTS_MONCTL2(lvts_ctrl->base));
+
+ return lvts_irq_init(lvts_ctrl);
+}
+
+static int lvts_ctrl_start(struct device *dev, struct lvts_ctrl *lvts_ctrl)
+{
+ struct lvts_sensor *lvts_sensors = lvts_ctrl->sensors;
+ struct thermal_zone_device *tz;
+ u32 sensor_map = 0;
+ int i;
+
+ for (i = 0; i < lvts_ctrl->num_lvts_sensor; i++) {
+
+ int dt_id = lvts_sensors[i].dt_id;
+
+ tz = devm_thermal_of_zone_register(dev, dt_id, &lvts_sensors[i],
+ &lvts_ops);
+ if (IS_ERR(tz)) {
+ /*
+ * This thermal zone is not described in the
+ * device tree. It is not an error from the
+ * thermal OF code POV, we just continue.
+ */
+ if (PTR_ERR(tz) == -ENODEV)
+ continue;
+
+ return PTR_ERR(tz);
+ }
+
+ /*
+ * The thermal zone pointer will be needed in the
+ * interrupt handler, we store it in the sensor
+ * structure. The thermal domain structure will be
+ * passed to the interrupt handler private data as the
+ * interrupt is shared for all the controller
+ * belonging to the thermal domain.
+ */
+ lvts_sensors[i].tz = tz;
+
+ /*
+ * This sensor was correctly associated with a thermal
+ * zone, let's set the corresponding bit in the sensor
+ * map, so we can enable the temperature monitoring in
+ * the hardware thermal controller.
+ */
+ sensor_map |= BIT(i);
+ }
+
+ /*
+ * Bits:
+ * 9: Single point access flow
+ * 0-3: Enable sensing point 0-3
+ *
+ * The initialization of the thermal zones give us
+ * which sensor point to enable. If any thermal zone
+ * was not described in the device tree, it won't be
+ * enabled here in the sensor map.
+ */
+ writel(sensor_map | BIT(9), LVTS_MONCTL0(lvts_ctrl->base));
+
+ return 0;
+}
+
+static int lvts_domain_init(struct device *dev, struct lvts_domain *lvts_td,
+ struct lvts_data *lvts_data)
+{
+ struct lvts_ctrl *lvts_ctrl;
+ int i, ret;
+
+ ret = lvts_ctrl_init(dev, lvts_td, lvts_data);
+ if (ret)
+ return ret;
+
+ ret = lvts_domain_reset(dev, lvts_td->reset);
+ if (ret) {
+ dev_dbg(dev, "Failed to reset domain");
+ return ret;
+ }
+
+ for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {
+
+ lvts_ctrl = &lvts_td->lvts_ctrl[i];
+
+ /*
+ * Initialization steps:
+ *
+ * - Enable the clock
+ * - Connect to the LVTS
+ * - Initialize the LVTS
+ * - Prepare the calibration data
+ * - Select monitored sensors
+ * [ Configure sampling ]
+ * [ Configure the interrupt ]
+ * - Start measurement
+ */
+ ret = lvts_ctrl_enable(dev, lvts_ctrl);
+ if (ret) {
+ dev_dbg(dev, "Failed to enable LVTS clock");
+ return ret;
+ }
+
+ ret = lvts_ctrl_connect(dev, lvts_ctrl);
+ if (ret) {
+ dev_dbg(dev, "Failed to connect to LVTS controller");
+ return ret;
+ }
+
+ ret = lvts_ctrl_initialize(dev, lvts_ctrl);
+ if (ret) {
+ dev_dbg(dev, "Failed to initialize controller");
+ return ret;
+ }
+
+ ret = lvts_ctrl_calibrate(dev, lvts_ctrl);
+ if (ret) {
+ dev_dbg(dev, "Failed to calibrate controller");
+ return ret;
+ }
+
+ ret = lvts_ctrl_configure(dev, lvts_ctrl);
+ if (ret) {
+ dev_dbg(dev, "Failed to configure controller");
+ return ret;
+ }
+
+ ret = lvts_ctrl_start(dev, lvts_ctrl);
+ if (ret) {
+ dev_dbg(dev, "Failed to start controller");
+ return ret;
+ }
+ }
+
+ return lvts_debugfs_init(dev, lvts_td);
+}
+
+static int lvts_probe(struct platform_device *pdev)
+{
+ struct lvts_data *lvts_data;
+ struct lvts_domain *lvts_td;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int irq, ret;
+
+ lvts_td = devm_kzalloc(dev, sizeof(*lvts_td), GFP_KERNEL);
+ if (!lvts_td)
+ return -ENOMEM;
+
+ lvts_data = (struct lvts_data *)of_device_get_match_data(dev);
+ if (!lvts_data) {
+ dev_dbg(dev, "No platforme data");
+ return -ENODATA;
+ };
+
+ lvts_td->clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(lvts_td->clk)) {
+ dev_dbg(dev, "Failed to retrieve clock\n");
+ return PTR_ERR(lvts_td->clk);
+ }
+
+ res = platform_get_mem_or_io(pdev, 0);
+ if (!res) {
+ dev_dbg(dev, "No IO resource\n");
+ return -ENXIO;
+ }
+
+ lvts_td->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(lvts_td->base)) {
+ dev_dbg(dev, "Failed to map io resource\n");
+ return PTR_ERR(lvts_td->base);
+ }
+
+ lvts_td->reset = devm_reset_control_get_by_index(dev, 0);
+ if (IS_ERR(lvts_td->reset)) {
+ dev_dbg(dev, "Failed to get reset control\n");
+ return PTR_ERR(lvts_td->reset);
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_dbg(dev, "No irq resource\n");
+ return irq;
+ }
+
+ ret = lvts_domain_init(dev, lvts_td, lvts_data);
+ if (ret) {
+ dev_dbg(dev, "Failed to initialize the lvts domain\n");
+ return ret;
+ }
+
+ /*
+ * At this point the LVTS is initialized and enabled. We can
+ * safely enable the interrupt.
+ */
+ ret = devm_request_threaded_irq(dev, irq, NULL, lvts_irq_handler,
+ IRQF_ONESHOT, dev_name(dev), lvts_td);
+ if (ret) {
+ dev_dbg(dev, "Failed to request interrupt\n");
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, lvts_td);
+
+ return 0;
+}
+
+static int lvts_remove(struct platform_device *pdev)
+{
+ struct lvts_domain *lvts_td;
+ struct device *dev = &pdev->dev;
+ int i;
+
+ lvts_td = platform_get_drvdata(pdev);
+
+ for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
+ lvts_ctrl_disable(dev, &lvts_td->lvts_ctrl[i]);
+
+ lvts_debugfs_exit();
+
+ return 0;
+}
+
+static struct lvts_ctrl_data mt819x_lvts_data_ctrl[] = {
+ {
+ .cal_offset = { 0x4, 0x7 },
+ .lvts_sensor = {
+ { .dt_id = MT819x_MCU_BIG_CPU0 },
+ { .dt_id = MT819x_MCU_BIG_CPU1 }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x0,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
+ },
+
+ {
+ .cal_offset = { 0xd, 0x10 },
+ .lvts_sensor = {
+ { .dt_id = MT819x_MCU_BIG_CPU2 },
+ { .dt_id = MT819x_MCU_BIG_CPU3 }
+ },
+ .num_lvts_sensor = 2,
+ .offset = 0x100,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
+ },
+
+ {
+ .cal_offset = { 0x16, 0x19, 0x1c, 0x1f },
+ .lvts_sensor = {
+ { .dt_id = MT819x_MCU_LITTLE_CPU0 },
+ { .dt_id = MT819x_MCU_LITTLE_CPU1 },
+ { .dt_id = MT819x_MCU_LITTLE_CPU2 },
+ { .dt_id = MT819x_MCU_LITTLE_CPU3 }
+ },
+ .num_lvts_sensor = 4,
+ .offset = 0x200,
+ .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
+ }
+};
+
+static struct lvts_data mt819x_lvts_mcu_data = {
+ .lvts_ctrl = mt819x_lvts_data_ctrl,
+ .num_lvts_ctrl = ARRAY_SIZE(mt819x_lvts_data_ctrl),
+};
+
+static const struct of_device_id lvts_of_match[] = {
+ { .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt819x_lvts_mcu_data },
+ {},
+};
+MODULE_DEVICE_TABLE(of, lvts_of_match);
+
+static struct platform_driver lvts_driver = {
+ .probe = lvts_probe,
+ .remove = lvts_remove,
+ .driver = {
+ .name = "mtk-lvts-thermal",
+ .of_match_table = lvts_of_match,
+ },
+};
+module_platform_driver(lvts_driver);
+
+MODULE_AUTHOR("Balsam CHIHI <[email protected]>");
+MODULE_DESCRIPTION("MediaTek LVTS Thermal Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/dt-bindings/thermal/mediatek-lvts.h b/include/dt-bindings/thermal/mediatek-lvts.h
new file mode 100644
index 000000000000..80d060400236
--- /dev/null
+++ b/include/dt-bindings/thermal/mediatek-lvts.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Balsam CHIHI <[email protected]>
+ */
+
+#ifndef __MEDIATEK_LVTS_DT_H
+#define __MEDIATEK_LVTS_DT_H
+
+#define MT819x_MCU_BIG_CPU0 0
+#define MT819x_MCU_BIG_CPU1 1
+#define MT819x_MCU_BIG_CPU2 2
+#define MT819x_MCU_BIG_CPU3 3
+#define MT819x_MCU_LITTLE_CPU0 4
+#define MT819x_MCU_LITTLE_CPU1 5
+#define MT819x_MCU_LITTLE_CPU2 6
+#define MT819x_MCU_LITTLE_CPU3 7
+
+#endif /* __MEDIATEK_LVTS_DT_H */
--
2.34.1
From: Balsam CHIHI <[email protected]>
Add MediaTek proprietary folder to upstream more thermal zone and cooler
drivers, relocate the original thermal controller driver to it, and rename it
as "auxadc_thermal.c" to show its purpose more clearly.
Signed-off-by: Balsam CHIHI <[email protected]>
---
drivers/thermal/Kconfig | 14 ++++---------
drivers/thermal/Makefile | 2 +-
drivers/thermal/mediatek/Kconfig | 21 +++++++++++++++++++
drivers/thermal/mediatek/Makefile | 1 +
.../auxadc_thermal.c} | 2 +-
5 files changed, 28 insertions(+), 12 deletions(-)
create mode 100644 drivers/thermal/mediatek/Kconfig
create mode 100644 drivers/thermal/mediatek/Makefile
rename drivers/thermal/{mtk_thermal.c => mediatek/auxadc_thermal.c} (99%)
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index e052dae614eb..d35f63daca3b 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -412,16 +412,10 @@ config DA9062_THERMAL
zone.
Compatible with the DA9062 and DA9061 PMICs.
-config MTK_THERMAL
- tristate "Temperature sensor driver for mediatek SoCs"
- depends on ARCH_MEDIATEK || COMPILE_TEST
- depends on HAS_IOMEM
- depends on NVMEM || NVMEM=n
- depends on RESET_CONTROLLER
- default y
- help
- Enable this option if you want to have support for thermal management
- controller present in Mediatek SoCs
+menu "Mediatek thermal drivers"
+depends on ARCH_MEDIATEK || COMPILE_TEST
+source "drivers/thermal/mediatek/Kconfig"
+endmenu
config AMLOGIC_THERMAL
tristate "Amlogic Thermal Support"
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 2506c6c8ca83..766ce38ff4f3 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -55,7 +55,7 @@ obj-y += st/
obj-y += qcom/
obj-y += tegra/
obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
-obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o
+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
diff --git a/drivers/thermal/mediatek/Kconfig b/drivers/thermal/mediatek/Kconfig
new file mode 100644
index 000000000000..7558a847d4e9
--- /dev/null
+++ b/drivers/thermal/mediatek/Kconfig
@@ -0,0 +1,21 @@
+config MTK_THERMAL
+ tristate "MediaTek thermal drivers"
+ depends on THERMAL_OF
+ help
+ This is the option for MediaTek thermal software solutions.
+ Please enable corresponding options to get temperature
+ information from thermal sensors or turn on throttle
+ mechaisms for thermal mitigation.
+
+if MTK_THERMAL
+
+config MTK_SOC_THERMAL
+ tristate "AUXADC temperature sensor driver for MediaTek SoCs"
+ depends on HAS_IOMEM
+ help
+ Enable this option if you want to get SoC temperature
+ information for MediaTek platforms.
+ This driver configures thermal controllers to collect
+ temperature via AUXADC interface.
+
+endif
diff --git a/drivers/thermal/mediatek/Makefile b/drivers/thermal/mediatek/Makefile
new file mode 100644
index 000000000000..53e86e30b26f
--- /dev/null
+++ b/drivers/thermal/mediatek/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MTK_SOC_THERMAL) += auxadc_thermal.o
diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mediatek/auxadc_thermal.c
similarity index 99%
rename from drivers/thermal/mtk_thermal.c
rename to drivers/thermal/mediatek/auxadc_thermal.c
index 8440692e3890..b4ef57fa9183 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mediatek/auxadc_thermal.c
@@ -23,7 +23,7 @@
#include <linux/reset.h>
#include <linux/types.h>
-#include "thermal_hwmon.h"
+#include "../thermal_hwmon.h"
/* AUXADC Registers */
#define AUXADC_CON1_SET_V 0x008
--
2.34.1
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rafael-pm/thermal]
[also build test WARNING on robh/for-next linus/master v6.2-rc3 next-20230112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/bchihi-baylibre-com/thermal-drivers-mediatek-Relocate-driver-to-mediatek-folder/20230112-235557
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal
patch link: https://lore.kernel.org/r/20230112152855.216072-2-bchihi%40baylibre.com
patch subject: [PATCH v10 1/6] thermal/drivers/mediatek: Relocate driver to mediatek folder
config: sh-allmodconfig
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1892ee82b9ea354e3e733c2a76fdf5467dc9b00e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review bchihi-baylibre-com/thermal-drivers-mediatek-Relocate-driver-to-mediatek-folder/20230112-235557
git checkout 1892ee82b9ea354e3e733c2a76fdf5467dc9b00e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/thermal/mediatek/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/thermal/mediatek/auxadc_thermal.c:562: warning: expecting prototype for raw_to_mcelsius(). Prototype was for raw_to_mcelsius_v1() instead
vim +562 drivers/thermal/mediatek/auxadc_thermal.c
a4ffe6b52d27f4 drivers/thermal/mtk_thermal.c Michael Kao 2019-02-01 551
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 552 /**
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 553 * raw_to_mcelsius - convert a raw ADC value to mcelsius
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 554 * @mt: The thermal controller
3772bb422072d4 drivers/thermal/mtk_thermal.c Amit Kucheria 2019-11-20 555 * @sensno: sensor number
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 556 * @raw: raw ADC value
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 557 *
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 558 * This converts the raw ADC value to mcelsius using the SoC specific
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 559 * calibration constants
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 560 */
54bf1e5a629dfb drivers/thermal/mtk_thermal.c Henry Yen 2020-04-30 561 static int raw_to_mcelsius_v1(struct mtk_thermal *mt, int sensno, s32 raw)
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 @562 {
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 563 s32 tmp;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 564
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 565 raw &= 0xfff;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 566
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 567 tmp = 203450520 << 3;
f84514766985d3 drivers/thermal/mtk_thermal.c Michael Kao 2019-02-01 568 tmp /= mt->conf->cali_val + mt->o_slope;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 569 tmp /= 10000 + mt->adc_ge;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 570 tmp *= raw - mt->vts[sensno] - 3350;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 571 tmp >>= 3;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 572
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 573 return mt->degc_cali * 500 - tmp;
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 574 }
a92db1c8089e82 drivers/thermal/mtk_thermal.c Sascha Hauer 2015-11-30 575
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On 12/01/2023 21:08, kernel test robot wrote:
[ ... ]
> All warnings (new ones prefixed by >>):
>
>>> drivers/thermal/mediatek/auxadc_thermal.c:562: warning: expecting prototype for raw_to_mcelsius(). Prototype was for raw_to_mcelsius_v1() instead
That has been fixed by:
https://lore.kernel.org/all/[email protected]/
and applied in the thermal/linux-next branch
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Il 12/01/23 16:28, [email protected] ha scritto:
> From: Balsam CHIHI <[email protected]>
>
> The Low Voltage Thermal Sensor (LVTS) is a multiple sensors, multi
> controllers contained in a thermal domain.
>
> A thermal domains can be the MCU or the AP.
>
> Each thermal domains contain up to seven controllers, each thermal
> controller handle up to four thermal sensors.
>
> The LVTS has two Finite State Machines (FSM), one to handle the
> functionin temperatures range like hot or cold temperature and another
> one to handle monitoring trip point. The FSM notifies via interrupts
> when a trip point is crossed.
>
> The interrupt is managed at the thermal controller level, so when an
> interrupt occurs, the driver has to find out which sensor triggered
> such an interrupt.
>
> The sampling of the thermal can be filtered or immediate. For the
> former, the LVTS measures several points and applies a low pass
> filter.
>
> Signed-off-by: Balsam CHIHI <[email protected]>
> ---
> drivers/thermal/mediatek/Kconfig | 15 +
> drivers/thermal/mediatek/Makefile | 1 +
> drivers/thermal/mediatek/lvts_thermal.c | 1244 +++++++++++++++++++
> include/dt-bindings/thermal/mediatek-lvts.h | 19 +
> 4 files changed, 1279 insertions(+)
> create mode 100644 drivers/thermal/mediatek/lvts_thermal.c
> create mode 100644 include/dt-bindings/thermal/mediatek-lvts.h
>
..snip..
> +
> +static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
> +{
> + struct lvts_sensor *lvts_sensor = tz->devdata;
> + void __iomem *base = lvts_sensor->base;
> + u32 raw_low = lvts_temp_to_raw(low);
> + u32 raw_high = lvts_temp_to_raw(high);
> +
> + /*
> + * Hot to normal temperature threshold
> + *
> + * LVTS_H2NTHRE
> + *
> + * Bits:
> + *
> + * 14-0 : Raw temperature for threshold
> + */
> + if (low != -INT_MAX) {
> + dev_dbg(&tz->device, "Setting low limit temperature interrupt: %d\n", low);
> + writel(raw_low, LVTS_H2NTHRE(base));
> + }
> +
> + /*
> + * Hot temperature threshold
> + *
> + * LVTS_HTHRE
> + *
> + * Bits:
> + *
> + * 14-0 : Raw temperature for threshold
> + */
> + dev_dbg(&tz->device, "Setting high limit temperature interrupt: %d\n", high);
> + writel(raw_high, LVTS_HTHRE(base));
> +
> + return 0;
> +}
> +
> +static irqreturn_t lvts_ctrl_irq_handler(struct lvts_ctrl *lvts_ctrl)
> +{
> + irqreturn_t iret = IRQ_NONE;
> + u32 value, masks[] = { 0x0009001F, 0X000881F0, 0x00247C00, 0x1FC00000 };
Please, no magic numbers around.
> + int i;
> +
> + /*
> + * Interrupt monitoring status
> + *
> + * LVTS_MONINTST
> + *
> + * Bits:
You're describing the register with nice words, but there's another way to do
the same that will be even more effective.
/*
* LVTS MONINT: Interrupt Monitoring register
* Each bit describes the enable status of per-sensor interrupts.
*/
#define LVTS_MONINT_THRES_COLD BIT(0) /* Cold threshold */
#define LVTS_MONINT_THRES_HOT BIT(1) /* Hot threshold */
#define LVTS_MONINT_OFFST_LOW BIT(2) /* Low offset */
#define LVTS_MONINT_OFFST_HIGH BIT(3) /* High offset */
#define LVTS_MONINT_OFFST_NTH BIT(4) /* Normal To Hot */
#define EVERYTHING_ELSE ........................
#define LVTS_MONINT_SNS0_MASK GENMASK( ... )
#define LVTS_MONINT_SNS1_MASK GENMASK .....
/* Find a better name for this one */
#define LVTS_MONINT_EN_IRQS ( LVTS_MONINT_THRES_COLD | LVTS_MONINT_THRES_HOT |
LVTS_MONINT_OFFST_LOW ..... etc etc)
> + *
> + * 31 : Interrupt for stage 3
> + * 30 : Interrupt for stage 2
> + * 29 : Interrupt for state 1
> + * 28 : Interrupt using filter on sensor 3
> + *
> + * 27 : Interrupt using immediate on sensor 3
> + * 26 : Interrupt normal to hot on sensor 3
> + * 25 : Interrupt high offset on sensor 3
> + * 24 : Interrupt low offset on sensor 3
> + *
> + * 23 : Interrupt hot threshold on sensor 3
> + * 22 : Interrupt cold threshold on sensor 3
> + * 21 : Interrupt using filter on sensor 2
> + * 20 : Interrupt using filter on sensor 1
> + *
> + * 19 : Interrupt using filter on sensor 0
> + * 18 : Interrupt using immediate on sensor 2
> + * 17 : Interrupt using immediate on sensor 1
> + * 16 : Interrupt using immediate on sensor 0
> + *
> + * 15 : Interrupt device access timeout interrupt
> + * 14 : Interrupt normal to hot on sensor 2
> + * 13 : Interrupt high offset interrupt on sensor 2
> + * 12 : Interrupt low offset interrupt on sensor 2
> + *
> + * 11 : Interrupt hot threshold on sensor 2
> + * 10 : Interrupt cold threshold on sensor 2
> + * 9 : Interrupt normal to hot on sensor 1
> + * 8 : Interrupt high offset interrupt on sensor 1
> + *
> + * 7 : Interrupt low offset interrupt on sensor 1
> + * 6 : Interrupt hot threshold on sensor 1
> + * 5 : Interrupt cold threshold on sensor 1
> + * 4 : Interrupt normal to hot on sensor 0
> + *
> + * 3 : Interrupt high offset interrupt on sensor 0
> + * 2 : Interrupt low offset interrupt on sensor 0
> + * 1 : Interrupt hot threshold on sensor 0
> + * 0 : Interrupt cold threshold on sensor 0
> + *
> + * We are interested in the sensor(s) responsible of the
> + * interrupt event. We update the thermal framework with the
> + * thermal zone associated with the sensor. The framework will
> + * take care of the rest whatever the kind of interrupt, we
> + * are only interested in which sensor raised the interrupt.
> + *
> + * sensor 3 interrupt: 0001 1111 1100 0000 0000 0000 0000 0000
> + * => 0x1FC00000
> + * sensor 2 interrupt: 0000 0000 0010 0100 0111 1100 0000 0000
> + * => 0x00247C00
> + * sensor 1 interrupt: 0000 0000 0001 0001 0000 0011 1110 0000
> + * => 0X000881F0
> + * sensor 0 interrupt: 0000 0000 0000 1001 0000 0000 0001 1111
> + * => 0x0009001F
> + */
> + value = readl(LVTS_MONINTSTS(lvts_ctrl->base));
> +
> + /*
> + * Let's figure out which sensors raised the interrupt
> + *
> + * NOTE: the masks array must be ordered with the index
> + * corresponding to the sensor id eg. index=0, mask for
> + * sensor0.
> + */
> + for (i = 0; i < ARRAY_SIZE(masks); i++) {
> +
> + if (!(value & masks[i]))
Questions for you:
1. Are the masks always the same for all SoCs?
2. Do they correspond to what we set in lvts_irq_init()?
I'd expect future new SoCs to have different masks... and since lvts_irq_init() is
actually "playing with" interrupts register(s), with one of them (LVTS_MONINT)
having the same layout as this one, I would place all of the initialization in
that function instead.
This means that we'd initialize those masks at lvts_irq_init() time, in a struct
member, and read it back in this interrupt handler: like that, we get that a bit
more ordered and generally more readable.
> + continue;
> +
> + thermal_zone_device_update(lvts_ctrl->sensors[i].tz,
> + THERMAL_TRIP_VIOLATED);
> + iret |= IRQ_HANDLED;
> + }
> +
> + /*
> + * Write back to clear the interrupt status (W1C)
> + */
> + writel(value, LVTS_MONINTSTS(lvts_ctrl->base));
> +
> + return iret;
> +}
> +
> +/*
> + * Temperature interrupt handler. Even if the driver supports more
> + * interrupt modes, we use the interrupt when the temperature crosses
> + * the hot threshold the way up and the way down (modulo the
> + * hysteresis).
> + *
> + * Each thermal domain has a couple of interrupts, one for hardware
> + * reset and another one for all the thermal events happening on the
> + * different sensors.
> + *
> + * The interrupt is configured for thermal events when crossing the
> + * hot temperature limit. At each interrupt, we check in every
> + * controller if there is an interrupt pending.
> + */
> +static irqreturn_t lvts_irq_handler(int irq, void *data)
> +{
> + struct lvts_domain *lvts_td = data;
> + irqreturn_t iret = IRQ_NONE;
> + int i;
> +
> + for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
> + iret |= lvts_ctrl_irq_handler(lvts_td->lvts_ctrl);
Please do *not* OR your function calls! While this is surely fine here in
this function and for this particular case, it's generally bad practice
and shall be avoided.
> +
> + return iret;
> +}
> +
> +static struct thermal_zone_device_ops lvts_ops = {
> + .get_temp = lvts_get_temp,
> + .set_trips = lvts_set_trips,
> +};
> +
..snip..
> +
> +static int lvts_irq_init(struct lvts_ctrl *lvts_ctrl)
> +{
> + u32 value;
> +
> + /*
> + * LVTS_PROTCTL : Thermal Protection Sensor Selection
> + *
> + * Bits:
> + *
> + * 19-18 : Sensor to base the protection on
> + * 17-16 : Strategy:
> + * 00 : Average of 4 sensors
> + * 01 : Max of 4 sensors
> + * 10 : Selected sensor with bits 19-18
> + * 11 : Reserved
> + */
> + value = BIT(16);
> + writel(value, LVTS_PROTCTL(lvts_ctrl->base));
> +
> + /*
> + * LVTS_PROTTA : Stage 1 temperature threshold
> + * LVTS_PROTTB : Stage 2 temperature threshold
> + * LVTS_PROTTC : Stage 3 temperature threshold
> + *
> + * Bits:
> + *
> + * 14-0: Raw temperature threshold
> + *
> + * writel(0x0, LVTS_PROTTA(lvts_ctrl->base));
> + * writel(0x0, LVTS_PROTTB(lvts_ctrl->base));
> + */
> + writel(lvts_ctrl->hw_tshut_raw_temp, LVTS_PROTTC(lvts_ctrl->base));
> +
> + /*
> + * LVTS_MONINT : Interrupt configuration register
> + *
> + * The LVTS_MONINT register layout is the same as the LVTS_MONINTSTS
> + * register, except we set the bits to enable the interrupt.
> + */
> + value = 0x9FBF7BDE;
u32 val;
val = FIELD_PREP(LVTS_MONINT_SNS0_MASK, LVTS_MONINT_EN_IRQS);
val |= FIELD_PREP(LVTS_MONINT_SNS1_MASK, LVTS_MONINT_EN_IRQS);
... etc
writel(val, ...... )
> + writel(value, LVTS_MONINT(lvts_ctrl->base));
> +
> + return 0;
> +}
> +
..snip..
> +
> +static int lvts_ctrl_initialize(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> +{
> + /*
> + * Write device mask: 0xC1030000
> + */
> + u32 cmds[] = {
> + 0xC1030E01, 0xC1030CFC, 0xC1030A8C, 0xC103098D, 0xC10308F1,
> + 0xC10307A6, 0xC10306B8, 0xC1030500, 0xC1030420, 0xC1030300,
> + 0xC1030030, 0xC10300F6, 0xC1030050, 0xC1030060, 0xC10300AC,
> + 0xC10300FC, 0xC103009D, 0xC10300F1, 0xC10300E1
> + };
...what is this long list of commands?
Why 0xC103_0000? Describe that please.
Also, why is this not a platform data constant?
Example:
struct lvts_plat {
const struct lvts_ctrl_data *ctrl_data;
u8 num_ctrl_data;
const u16 device_mask;
const u16 *init_cmds;
u8 num_init_cmds;
}
where device_mask gets set to 0xc103 and init_cmds is an array containing
the low-16 (0x0e01, 0x0cfc, ...), and where this function would simply do
something like
lvts_write_config(lvts_ctrl, plat->device_mask, init_cmds, num_init_cmds);
... and where lvts_write_config() does something like:
for (i = 0; i < num_cmds; i++) {
u32 val = device_mask | init_cmds[i];
writel(val, LVTS_CONFIG ...)
}
> +
> + lvts_write_config(lvts_ctrl, cmds, ARRAY_SIZE(cmds));
> +
> + return 0;
> +}
> +
> +static int lvts_ctrl_calibrate(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> +{
> + int i;
> + void __iomem *lvts_edata[] = {
Can we constify this?
> + LVTS_EDATA00(lvts_ctrl->base),
> + LVTS_EDATA01(lvts_ctrl->base),
> + LVTS_EDATA02(lvts_ctrl->base),
> + LVTS_EDATA03(lvts_ctrl->base)
> + };
> +
> + /*
> + * LVTS_EDATA0X : Efuse calibration reference value for sensor X
> + *
> + * Bits:
> + *
> + * 20-0 : Efuse value for normalization data
> + */
> + for (i = 0; i < LVTS_SENSOR_MAX; i++)
> + writel(lvts_ctrl->calibration[i], lvts_edata[i]);
> +
> + return 0;
> +}
> +
> +static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> +{
> + u32 period_unit = (118 * 1000) / (256 * 38);
#define SOMETHING 118
#define SOMETHING_ELSE 1000
#define ....
const u32 period_unit = (SOMETHING * SOMETHING_ELSE) / ....
> + u32 grp_interval = 1;
> + u32 flt_interval = 1;
> + u32 sensor_interval = 1;
> + u32 hw_filter = 0x2;
> + u32 value;
> +
...snip...
> +
> +static struct lvts_ctrl_data mt819x_lvts_data_ctrl[] = {
No wildcards. Please, rename this to give the name of the oldest SoC
that uses these values. Assuming that it is MT8192.... mt8192_lvts_data_ctrl[]
> + {
> + .cal_offset = { 0x4, 0x7 },
> + .lvts_sensor = {
> + { .dt_id = MT819x_MCU_BIG_CPU0 },
> + { .dt_id = MT819x_MCU_BIG_CPU1 }
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x0,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
> + },
> +
> + {
> + .cal_offset = { 0xd, 0x10 },
> + .lvts_sensor = {
> + { .dt_id = MT819x_MCU_BIG_CPU2 },
> + { .dt_id = MT819x_MCU_BIG_CPU3 }
> + },
> + .num_lvts_sensor = 2,
> + .offset = 0x100,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
> + },
> +
> + {
> + .cal_offset = { 0x16, 0x19, 0x1c, 0x1f },
> + .lvts_sensor = {
> + { .dt_id = MT819x_MCU_LITTLE_CPU0 },
> + { .dt_id = MT819x_MCU_LITTLE_CPU1 },
> + { .dt_id = MT819x_MCU_LITTLE_CPU2 },
> + { .dt_id = MT819x_MCU_LITTLE_CPU3 }
> + },
> + .num_lvts_sensor = 4,
> + .offset = 0x200,
> + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
> + }
> +};
> +
> +static struct lvts_data mt819x_lvts_mcu_data = {
Same here.
> + .lvts_ctrl = mt819x_lvts_data_ctrl,
> + .num_lvts_ctrl = ARRAY_SIZE(mt819x_lvts_data_ctrl),
> +};
> +
> +static const struct of_device_id lvts_of_match[] = {
> + { .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt819x_lvts_mcu_data },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, lvts_of_match);
> +
> +static struct platform_driver lvts_driver = {
> + .probe = lvts_probe,
> + .remove = lvts_remove,
> + .driver = {
> + .name = "mtk-lvts-thermal",
> + .of_match_table = lvts_of_match,
> + },
> +};
> +module_platform_driver(lvts_driver);
> +
> +MODULE_AUTHOR("Balsam CHIHI <[email protected]>");
> +MODULE_DESCRIPTION("MediaTek LVTS Thermal Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/dt-bindings/thermal/mediatek-lvts.h b/include/dt-bindings/thermal/mediatek-lvts.h
> new file mode 100644
> index 000000000000..80d060400236
> --- /dev/null
> +++ b/include/dt-bindings/thermal/mediatek-lvts.h
Bindings go in a different commit: add this in your patch [2/6], where you are
adding the yaml binding.
Also, please follow binding names: rename this file to mediatek,mt8192-lvts.h.
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2023 MediaTek Inc.
> + * Author: Balsam CHIHI <[email protected]>
> + */
> +
> +#ifndef __MEDIATEK_LVTS_DT_H
> +#define __MEDIATEK_LVTS_DT_H
> +
> +#define MT819x_MCU_BIG_CPU0 0
No wildcards: MT8192_MCU_BIG_CPU0
> +#define MT819x_MCU_BIG_CPU1 1
> +#define MT819x_MCU_BIG_CPU2 2
> +#define MT819x_MCU_BIG_CPU3 3
> +#define MT819x_MCU_LITTLE_CPU0 4
> +#define MT819x_MCU_LITTLE_CPU1 5
> +#define MT819x_MCU_LITTLE_CPU2 6
> +#define MT819x_MCU_LITTLE_CPU3 7
> +
> +#endif /* __MEDIATEK_LVTS_DT_H */
Regards,
Angelo
On 16/01/2023 11:50, AngeloGioacchino Del Regno wrote:
> Il 12/01/23 16:28, [email protected] ha scritto:
>> From: Balsam CHIHI <[email protected]>
>>
>> The Low Voltage Thermal Sensor (LVTS) is a multiple sensors, multi
>> controllers contained in a thermal domain.
>>
>> A thermal domains can be the MCU or the AP.
>>
>> Each thermal domains contain up to seven controllers, each thermal
>> controller handle up to four thermal sensors.
>>
>> The LVTS has two Finite State Machines (FSM), one to handle the
>> functionin temperatures range like hot or cold temperature and another
>> one to handle monitoring trip point. The FSM notifies via interrupts
>> when a trip point is crossed.
>>
>> The interrupt is managed at the thermal controller level, so when an
>> interrupt occurs, the driver has to find out which sensor triggered
>> such an interrupt.
>>
>> The sampling of the thermal can be filtered or immediate. For the
>> former, the LVTS measures several points and applies a low pass
>> filter.
>>
>> Signed-off-by: Balsam CHIHI <[email protected]>
>> ---
>> drivers/thermal/mediatek/Kconfig | 15 +
>> drivers/thermal/mediatek/Makefile | 1 +
>> drivers/thermal/mediatek/lvts_thermal.c | 1244 +++++++++++++++++++
>> include/dt-bindings/thermal/mediatek-lvts.h | 19 +
>> 4 files changed, 1279 insertions(+)
>> create mode 100644 drivers/thermal/mediatek/lvts_thermal.c
>> create mode 100644 include/dt-bindings/thermal/mediatek-lvts.h
>>
>
> ..snip..
>
>> +
>> +static int lvts_set_trips(struct thermal_zone_device *tz, int low,
>> int high)
>> +{
>> + struct lvts_sensor *lvts_sensor = tz->devdata;
>> + void __iomem *base = lvts_sensor->base;
>> + u32 raw_low = lvts_temp_to_raw(low);
>> + u32 raw_high = lvts_temp_to_raw(high);
>> +
>> + /*
>> + * Hot to normal temperature threshold
>> + *
>> + * LVTS_H2NTHRE
>> + *
>> + * Bits:
>> + *
>> + * 14-0 : Raw temperature for threshold
>> + */
>> + if (low != -INT_MAX) {
>> + dev_dbg(&tz->device, "Setting low limit temperature
>> interrupt: %d\n", low);
>> + writel(raw_low, LVTS_H2NTHRE(base));
>> + }
>> +
>> + /*
>> + * Hot temperature threshold
>> + *
>> + * LVTS_HTHRE
>> + *
>> + * Bits:
>> + *
>> + * 14-0 : Raw temperature for threshold
>> + */
>> + dev_dbg(&tz->device, "Setting high limit temperature interrupt:
>> %d\n", high);
>> + writel(raw_high, LVTS_HTHRE(base));
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t lvts_ctrl_irq_handler(struct lvts_ctrl *lvts_ctrl)
>> +{
>> + irqreturn_t iret = IRQ_NONE;
>> + u32 value, masks[] = { 0x0009001F, 0X000881F0, 0x00247C00,
>> 0x1FC00000 };
>
> Please, no magic numbers around.
>
>> + int i;
>> +
>> + /*
>> + * Interrupt monitoring status
>> + *
>> + * LVTS_MONINTST
>> + *
>> + * Bits:
>
> You're describing the register with nice words, but there's another way
> to do
> the same that will be even more effective.
>
> /*
> * LVTS MONINT: Interrupt Monitoring register
> * Each bit describes the enable status of per-sensor interrupts.
> */
> #define LVTS_MONINT_THRES_COLD BIT(0) /* Cold threshold */
> #define LVTS_MONINT_THRES_HOT BIT(1) /* Hot threshold */
> #define LVTS_MONINT_OFFST_LOW BIT(2) /* Low offset */
> #define LVTS_MONINT_OFFST_HIGH BIT(3) /* High offset */
> #define LVTS_MONINT_OFFST_NTH BIT(4) /* Normal To Hot */
> #define EVERYTHING_ELSE ........................
I don't see how this is more effective than describing the register
layout. If someone wants to hack the driver, it is much better to have
the layout than this long list of defines for every bits of every registers.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Il 16/01/23 12:08, Daniel Lezcano ha scritto:
> On 16/01/2023 11:50, AngeloGioacchino Del Regno wrote:
>> Il 12/01/23 16:28, [email protected] ha scritto:
>>> From: Balsam CHIHI <[email protected]>
>>>
>>> The Low Voltage Thermal Sensor (LVTS) is a multiple sensors, multi
>>> controllers contained in a thermal domain.
>>>
>>> A thermal domains can be the MCU or the AP.
>>>
>>> Each thermal domains contain up to seven controllers, each thermal
>>> controller handle up to four thermal sensors.
>>>
>>> The LVTS has two Finite State Machines (FSM), one to handle the
>>> functionin temperatures range like hot or cold temperature and another
>>> one to handle monitoring trip point. The FSM notifies via interrupts
>>> when a trip point is crossed.
>>>
>>> The interrupt is managed at the thermal controller level, so when an
>>> interrupt occurs, the driver has to find out which sensor triggered
>>> such an interrupt.
>>>
>>> The sampling of the thermal can be filtered or immediate. For the
>>> former, the LVTS measures several points and applies a low pass
>>> filter.
>>>
>>> Signed-off-by: Balsam CHIHI <[email protected]>
>>> ---
>>> drivers/thermal/mediatek/Kconfig | 15 +
>>> drivers/thermal/mediatek/Makefile | 1 +
>>> drivers/thermal/mediatek/lvts_thermal.c | 1244 +++++++++++++++++++
>>> include/dt-bindings/thermal/mediatek-lvts.h | 19 +
>>> 4 files changed, 1279 insertions(+)
>>> create mode 100644 drivers/thermal/mediatek/lvts_thermal.c
>>> create mode 100644 include/dt-bindings/thermal/mediatek-lvts.h
>>>
>>
>> ..snip..
>>
>>> +
>>> +static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
>>> +{
>>> + struct lvts_sensor *lvts_sensor = tz->devdata;
>>> + void __iomem *base = lvts_sensor->base;
>>> + u32 raw_low = lvts_temp_to_raw(low);
>>> + u32 raw_high = lvts_temp_to_raw(high);
>>> +
>>> + /*
>>> + * Hot to normal temperature threshold
>>> + *
>>> + * LVTS_H2NTHRE
>>> + *
>>> + * Bits:
>>> + *
>>> + * 14-0 : Raw temperature for threshold
>>> + */
>>> + if (low != -INT_MAX) {
>>> + dev_dbg(&tz->device, "Setting low limit temperature interrupt: %d\n",
>>> low);
>>> + writel(raw_low, LVTS_H2NTHRE(base));
>>> + }
>>> +
>>> + /*
>>> + * Hot temperature threshold
>>> + *
>>> + * LVTS_HTHRE
>>> + *
>>> + * Bits:
>>> + *
>>> + * 14-0 : Raw temperature for threshold
>>> + */
>>> + dev_dbg(&tz->device, "Setting high limit temperature interrupt: %d\n", high);
>>> + writel(raw_high, LVTS_HTHRE(base));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static irqreturn_t lvts_ctrl_irq_handler(struct lvts_ctrl *lvts_ctrl)
>>> +{
>>> + irqreturn_t iret = IRQ_NONE;
>>> + u32 value, masks[] = { 0x0009001F, 0X000881F0, 0x00247C00, 0x1FC00000 };
>>
>> Please, no magic numbers around.
>>
>>> + int i;
>>> +
>>> + /*
>>> + * Interrupt monitoring status
>>> + *
>>> + * LVTS_MONINTST
>>> + *
>>> + * Bits:
>>
>> You're describing the register with nice words, but there's another way to do
>> the same that will be even more effective.
>>
>> /*
>> * LVTS MONINT: Interrupt Monitoring register
>> * Each bit describes the enable status of per-sensor interrupts.
>> */
>> #define LVTS_MONINT_THRES_COLD BIT(0) /* Cold threshold */
>> #define LVTS_MONINT_THRES_HOT BIT(1) /* Hot threshold */
>> #define LVTS_MONINT_OFFST_LOW BIT(2) /* Low offset */
>> #define LVTS_MONINT_OFFST_HIGH BIT(3) /* High offset */
>> #define LVTS_MONINT_OFFST_NTH BIT(4) /* Normal To Hot */
>> #define EVERYTHING_ELSE ........................
>
> I don't see how this is more effective than describing the register layout. If
> someone wants to hack the driver, it is much better to have the layout than this
> long list of defines for every bits of every registers.
>
This also describes the register layout, with the difference that we can use those
definitions with bitfield macros to avoid writing magic numbers around.
Anyway, I'm not against the long comment that is describing the layout with nice
words - my suggestion was just only about one of the ways to replace the magic
numbers with actual definitions.
Regards,
Angelo
Sorry for forgetting this comment.
> > +static int lvts_ctrl_initialize(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> > +{
> > + /*
> > + * Write device mask: 0xC1030000
> > + */
> > + u32 cmds[] = {
> > + 0xC1030E01, 0xC1030CFC, 0xC1030A8C, 0xC103098D, 0xC10308F1,
> > + 0xC10307A6, 0xC10306B8, 0xC1030500, 0xC1030420, 0xC1030300,
> > + 0xC1030030, 0xC10300F6, 0xC1030050, 0xC1030060, 0xC10300AC,
> > + 0xC10300FC, 0xC103009D, 0xC10300F1, 0xC10300E1
> > + };
> ...what is this long list of commands?
>
I can not give an answer for that.
The LVTS programming guide is short in explanation.
It just gives this code sequence to configure it without any explanation.
Hi Angelo,
On Mon, Jan 16, 2023 at 11:50 AM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 12/01/23 16:28, [email protected] ha scritto:
> > From: Balsam CHIHI <[email protected]>
> >
> > The Low Voltage Thermal Sensor (LVTS) is a multiple sensors, multi
> > controllers contained in a thermal domain.
> >
> > A thermal domains can be the MCU or the AP.
> >
> > Each thermal domains contain up to seven controllers, each thermal
> > controller handle up to four thermal sensors.
> >
> > The LVTS has two Finite State Machines (FSM), one to handle the
> > functionin temperatures range like hot or cold temperature and another
> > one to handle monitoring trip point. The FSM notifies via interrupts
> > when a trip point is crossed.
> >
> > The interrupt is managed at the thermal controller level, so when an
> > interrupt occurs, the driver has to find out which sensor triggered
> > such an interrupt.
> >
> > The sampling of the thermal can be filtered or immediate. For the
> > former, the LVTS measures several points and applies a low pass
> > filter.
> >
> > Signed-off-by: Balsam CHIHI <[email protected]>
> > ---
> > drivers/thermal/mediatek/Kconfig | 15 +
> > drivers/thermal/mediatek/Makefile | 1 +
> > drivers/thermal/mediatek/lvts_thermal.c | 1244 +++++++++++++++++++
> > include/dt-bindings/thermal/mediatek-lvts.h | 19 +
> > 4 files changed, 1279 insertions(+)
> > create mode 100644 drivers/thermal/mediatek/lvts_thermal.c
> > create mode 100644 include/dt-bindings/thermal/mediatek-lvts.h
> >
..snip..
> > +
> > +static irqreturn_t lvts_ctrl_irq_handler(struct lvts_ctrl *lvts_ctrl)
> > +{
> > + irqreturn_t iret = IRQ_NONE;
> > + u32 value, masks[] = { 0x0009001F, 0X000881F0, 0x00247C00, 0x1FC00000 };
>
> Please, no magic numbers around.
>
These number are explained in the comment bellow.
This is part of it :
* sensor 3 interrupt: 0001 1111 1100 0000 0000 0000 0000 0000
* => 0x1FC00000
* sensor 2 interrupt: 0000 0000 0010 0100 0111 1100 0000 0000
* => 0x00247C00
* sensor 1 interrupt: 0000 0000 0001 0001 0000 0011 1110 0000
* => 0X000881F0
* sensor 0 interrupt: 0000 0000 0000 1001 0000 0000 0001 1111
* => 0x0009001F
> > + int i;
> > +
> > + /*
> > + * Interrupt monitoring status
> > + *
> > + * LVTS_MONINTST
> > + *
> > + * Bits:
>
> You're describing the register with nice words, but there's another way to do
> the same that will be even more effective.
>
> /*
> * LVTS MONINT: Interrupt Monitoring register
> * Each bit describes the enable status of per-sensor interrupts.
> */
> #define LVTS_MONINT_THRES_COLD BIT(0) /* Cold threshold */
> #define LVTS_MONINT_THRES_HOT BIT(1) /* Hot threshold */
> #define LVTS_MONINT_OFFST_LOW BIT(2) /* Low offset */
> #define LVTS_MONINT_OFFST_HIGH BIT(3) /* High offset */
> #define LVTS_MONINT_OFFST_NTH BIT(4) /* Normal To Hot */
> #define EVERYTHING_ELSE ........................
>
> #define LVTS_MONINT_SNS0_MASK GENMASK( ... )
> #define LVTS_MONINT_SNS1_MASK GENMASK .....
>
> /* Find a better name for this one */
> #define LVTS_MONINT_EN_IRQS ( LVTS_MONINT_THRES_COLD | LVTS_MONINT_THRES_HOT |
> LVTS_MONINT_OFFST_LOW ..... etc etc)
>
Given the complexity of the controller and the number of registers,
if we create a define per bits, we will end up with a huge list of
defines (~300).
I don't think that will help for the readability.
> > + *
> > + * 31 : Interrupt for stage 3
> > + * 30 : Interrupt for stage 2
> > + * 29 : Interrupt for state 1
> > + * 28 : Interrupt using filter on sensor 3
> > + *
..snip..
> > + *
> > + * 3 : Interrupt high offset interrupt on sensor 0
> > + * 2 : Interrupt low offset interrupt on sensor 0
> > + * 1 : Interrupt hot threshold on sensor 0
> > + * 0 : Interrupt cold threshold on sensor 0
> > + *
> > + * We are interested in the sensor(s) responsible of the
> > + * interrupt event. We update the thermal framework with the
> > + * thermal zone associated with the sensor. The framework will
> > + * take care of the rest whatever the kind of interrupt, we
> > + * are only interested in which sensor raised the interrupt.
> > + *
> > + * sensor 3 interrupt: 0001 1111 1100 0000 0000 0000 0000 0000
> > + * => 0x1FC00000
> > + * sensor 2 interrupt: 0000 0000 0010 0100 0111 1100 0000 0000
> > + * => 0x00247C00
> > + * sensor 1 interrupt: 0000 0000 0001 0001 0000 0011 1110 0000
> > + * => 0X000881F0
> > + * sensor 0 interrupt: 0000 0000 0000 1001 0000 0000 0001 1111
> > + * => 0x0009001F
> > + */
> > + value = readl(LVTS_MONINTSTS(lvts_ctrl->base));
> > +
> > + /*
> > + * Let's figure out which sensors raised the interrupt
> > + *
> > + * NOTE: the masks array must be ordered with the index
> > + * corresponding to the sensor id eg. index=0, mask for
> > + * sensor0.
> > + */
> > + for (i = 0; i < ARRAY_SIZE(masks); i++) {
> > +
> > + if (!(value & masks[i]))
>
> Questions for you:
>
> 1. Are the masks always the same for all SoCs?
The LVTS controller is not SoC specific.
The mask is controller specific whatever the SoC version.
> 2. Do they correspond to what we set in lvts_irq_init()?
Not exactly, we set LVTS_MONINT and the controller sets the LVTS_MONINTSTS.
The content will be different with what we set and what we get.
>
> I'd expect future new SoCs to have different masks... and since lvts_irq_init() is
> actually "playing with" interrupts register(s), with one of them (LVTS_MONINT)
> having the same layout as this one, I would place all of the initialization in
> that function instead.
>
> This means that we'd initialize those masks at lvts_irq_init() time, in a struct
> member, and read it back in this interrupt handler: like that, we get that a bit
> more ordered and generally more readable.
>
No. Actually, what will change is on which sensor a thermal zone is tie to,
and that is handled already by the device tree configuration.
> > + continue;
> > +
> > + thermal_zone_device_update(lvts_ctrl->sensors[i].tz,
> > + THERMAL_TRIP_VIOLATED);
> > + iret |= IRQ_HANDLED;
> > + }
> > +
> > + /*
> > + * Write back to clear the interrupt status (W1C)
> > + */
> > + writel(value, LVTS_MONINTSTS(lvts_ctrl->base));
> > +
> > + return iret;
> > +}
> > +
> > +/*
> > + * Temperature interrupt handler. Even if the driver supports more
> > + * interrupt modes, we use the interrupt when the temperature crosses
> > + * the hot threshold the way up and the way down (modulo the
> > + * hysteresis).
> > + *
> > + * Each thermal domain has a couple of interrupts, one for hardware
> > + * reset and another one for all the thermal events happening on the
> > + * different sensors.
> > + *
> > + * The interrupt is configured for thermal events when crossing the
> > + * hot temperature limit. At each interrupt, we check in every
> > + * controller if there is an interrupt pending.
> > + */
> > +static irqreturn_t lvts_irq_handler(int irq, void *data)
> > +{
> > + struct lvts_domain *lvts_td = data;
> > + irqreturn_t iret = IRQ_NONE;
> > + int i;
> > +
> > + for (i = 0; i < lvts_td->num_lvts_ctrl; i++)
> > + iret |= lvts_ctrl_irq_handler(lvts_td->lvts_ctrl);
>
> Please do *not* OR your function calls! While this is surely fine here in
> this function and for this particular case, it's generally bad practice
> and shall be avoided.
>
I understand that could be prone to errors.
I can propose this alternative but it looks less elegant than OR'ing the result.
Do you have a suggestion to improve this code snippet?
> > +
> > + return iret;
> > +}
> > +
> > +static struct thermal_zone_device_ops lvts_ops = {
> > + .get_temp = lvts_get_temp,
> > + .set_trips = lvts_set_trips,
> > +};
> > +
>
> ..snip..
>
> > +
> > +static int lvts_irq_init(struct lvts_ctrl *lvts_ctrl)
> > +{
> > + u32 value;
> > +
> > + /*
> > + * LVTS_PROTCTL : Thermal Protection Sensor Selection
> > + *
> > + * Bits:
> > + *
> > + * 19-18 : Sensor to base the protection on
> > + * 17-16 : Strategy:
> > + * 00 : Average of 4 sensors
> > + * 01 : Max of 4 sensors
> > + * 10 : Selected sensor with bits 19-18
> > + * 11 : Reserved
> > + */
> > + value = BIT(16);
> > + writel(value, LVTS_PROTCTL(lvts_ctrl->base));
> > +
> > + /*
> > + * LVTS_PROTTA : Stage 1 temperature threshold
> > + * LVTS_PROTTB : Stage 2 temperature threshold
> > + * LVTS_PROTTC : Stage 3 temperature threshold
> > + *
> > + * Bits:
> > + *
> > + * 14-0: Raw temperature threshold
> > + *
> > + * writel(0x0, LVTS_PROTTA(lvts_ctrl->base));
> > + * writel(0x0, LVTS_PROTTB(lvts_ctrl->base));
> > + */
> > + writel(lvts_ctrl->hw_tshut_raw_temp, LVTS_PROTTC(lvts_ctrl->base));
> > +
> > + /*
> > + * LVTS_MONINT : Interrupt configuration register
> > + *
> > + * The LVTS_MONINT register layout is the same as the LVTS_MONINTSTS
> > + * register, except we set the bits to enable the interrupt.
> > + */
> > + value = 0x9FBF7BDE;
>
> u32 val;
>
> val = FIELD_PREP(LVTS_MONINT_SNS0_MASK, LVTS_MONINT_EN_IRQS);
> val |= FIELD_PREP(LVTS_MONINT_SNS1_MASK, LVTS_MONINT_EN_IRQS);
>
> ... etc
>
> writel(val, ...... )
>
OK, I'll change it accordingly.
> > + writel(value, LVTS_MONINT(lvts_ctrl->base));
> > +
> > + return 0;
> > +}
> > +
>
> ..snip..
>
> > +
> > +static int lvts_ctrl_initialize(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> > +{
> > + /*
> > + * Write device mask: 0xC1030000
> > + */
> > + u32 cmds[] = {
> > + 0xC1030E01, 0xC1030CFC, 0xC1030A8C, 0xC103098D, 0xC10308F1,
> > + 0xC10307A6, 0xC10306B8, 0xC1030500, 0xC1030420, 0xC1030300,
> > + 0xC1030030, 0xC10300F6, 0xC1030050, 0xC1030060, 0xC10300AC,
> > + 0xC10300FC, 0xC103009D, 0xC10300F1, 0xC10300E1
> > + };
> ...what is this long list of commands?
>
> Why 0xC103_0000? Describe that please.
>
AFAIU, based on the documentation, the configuration register can be
read or write.
When we write it, we set the different bits corresponding to a write sequence
which is 0xC1030000.
The documentation gives the register layout but does not explain how it works.
> Also, why is this not a platform data constant?
>
It is not a platform data, it is a controller data.
Whatever the SoC the configuration sequence will be the same.
> Example:
> struct lvts_plat {
> const struct lvts_ctrl_data *ctrl_data;
> u8 num_ctrl_data;
> const u16 device_mask;
> const u16 *init_cmds;
> u8 num_init_cmds;
> }
>
> where device_mask gets set to 0xc103 and init_cmds is an array containing
> the low-16 (0x0e01, 0x0cfc, ...), and where this function would simply do
> something like
>
> lvts_write_config(lvts_ctrl, plat->device_mask, init_cmds, num_init_cmds);
>
> ... and where lvts_write_config() does something like:
>
> for (i = 0; i < num_cmds; i++) {
> u32 val = device_mask | init_cmds[i];
> writel(val, LVTS_CONFIG ...)
> }
> > +
> > + lvts_write_config(lvts_ctrl, cmds, ARRAY_SIZE(cmds));
> > +
> > + return 0;
> > +}
> > +
> > +static int lvts_ctrl_calibrate(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> > +{
> > + int i;
> > + void __iomem *lvts_edata[] = {
>
> Can we constify this?
>
Constifying "void __iomem *lvts_edata[]" generates the following
compilation warning :
drivers/thermal/mediatek/lvts_thermal.c:835:47: warning: passing
argument 2 of ‘writel’ discards ‘const’ qualifier from pointer target
type [-Wdiscarded-qualifiers]
835 | writel(lvts_ctrl->calibration[i], lvts_edata[i]);
| ~~~~~~~~~~^~~
> > + LVTS_EDATA00(lvts_ctrl->base),
> > + LVTS_EDATA01(lvts_ctrl->base),
> > + LVTS_EDATA02(lvts_ctrl->base),
> > + LVTS_EDATA03(lvts_ctrl->base)
> > + };
> > +
> > + /*
> > + * LVTS_EDATA0X : Efuse calibration reference value for sensor X
> > + *
> > + * Bits:
> > + *
> > + * 20-0 : Efuse value for normalization data
> > + */
> > + for (i = 0; i < LVTS_SENSOR_MAX; i++)
> > + writel(lvts_ctrl->calibration[i], lvts_edata[i]);
> > +
> > + return 0;
> > +}
> > +
> > +static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> > +{
> > + u32 period_unit = (118 * 1000) / (256 * 38);
>
> #define SOMETHING 118
> #define SOMETHING_ELSE 1000
> #define ....
>
> const u32 period_unit = (SOMETHING * SOMETHING_ELSE) / ....
>
Constifying "u32 period_unit" generates the following compilation warning :
./include/asm-generic/io.h:273:61: note: expected ‘volatile void *’
but argument is of type ‘const void *’
273 | static inline void writel(u32 value, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
> > + u32 grp_interval = 1;
> > + u32 flt_interval = 1;
> > + u32 sensor_interval = 1;
> > + u32 hw_filter = 0x2;
> > + u32 value;
> > +
>
> ...snip...
>
> > +
> > +static struct lvts_ctrl_data mt819x_lvts_data_ctrl[] = {
>
> No wildcards. Please, rename this to give the name of the oldest SoC
> that uses these values. Assuming that it is MT8192.... mt8192_lvts_data_ctrl[]
>
OK, it Will be mt8195_lvts_data_ctrl[].
> > + {
> > + .cal_offset = { 0x4, 0x7 },
> > + .lvts_sensor = {
> > + { .dt_id = MT819x_MCU_BIG_CPU0 },
> > + { .dt_id = MT819x_MCU_BIG_CPU1 }
> > + },
> > + .num_lvts_sensor = 2,
> > + .offset = 0x0,
> > + .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8195,
> > + },
> > +
..snip..
> > +static struct lvts_data mt819x_lvts_mcu_data = {
>
> Same here.
>
OK, it Will be mt8195_lvts_mcu_data.
> > + .lvts_ctrl = mt819x_lvts_data_ctrl,
> > + .num_lvts_ctrl = ARRAY_SIZE(mt819x_lvts_data_ctrl),
> > +};
> > +
> > +static const struct of_device_id lvts_of_match[] = {
> > + { .compatible = "mediatek,mt8195-lvts-mcu", .data = &mt819x_lvts_mcu_data },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, lvts_of_match);
> > +
> > +static struct platform_driver lvts_driver = {
> > + .probe = lvts_probe,
> > + .remove = lvts_remove,
> > + .driver = {
> > + .name = "mtk-lvts-thermal",
> > + .of_match_table = lvts_of_match,
> > + },
> > +};
> > +module_platform_driver(lvts_driver);
> > +
> > +MODULE_AUTHOR("Balsam CHIHI <[email protected]>");
> > +MODULE_DESCRIPTION("MediaTek LVTS Thermal Driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/dt-bindings/thermal/mediatek-lvts.h b/include/dt-bindings/thermal/mediatek-lvts.h
> > new file mode 100644
> > index 000000000000..80d060400236
> > --- /dev/null
> > +++ b/include/dt-bindings/thermal/mediatek-lvts.h
>
> Bindings go in a different commit: add this in your patch [2/6], where you are
> adding the yaml binding.
>
OK, it will be moved to :
[2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS
thermal controllers
> Also, please follow binding names: rename this file to mediatek,mt8192-lvts.h.
>
LVTS is SoC independent (only available on MT8192 and MT8195).
Should not we leave this file name SoC indemendent too "mediatek-lvts.h",
just like "mediatek,lvts-thermal.yaml"?
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (c) 2023 MediaTek Inc.
> > + * Author: Balsam CHIHI <[email protected]>
> > + */
> > +
> > +#ifndef __MEDIATEK_LVTS_DT_H
> > +#define __MEDIATEK_LVTS_DT_H
> > +
> > +#define MT819x_MCU_BIG_CPU0 0
>
> No wildcards: MT8192_MCU_BIG_CPU0
>
OK, it Will be MT8195_MCU_BIG_CPU0.
>
> > +#define MT819x_MCU_BIG_CPU1 1
> > +#define MT819x_MCU_BIG_CPU2 2
> > +#define MT819x_MCU_BIG_CPU3 3
> > +#define MT819x_MCU_LITTLE_CPU0 4
> > +#define MT819x_MCU_LITTLE_CPU1 5
> > +#define MT819x_MCU_LITTLE_CPU2 6
> > +#define MT819x_MCU_LITTLE_CPU3 7
> > +
> > +#endif /* __MEDIATEK_LVTS_DT_H */
>
> Regards,
> Angelo
Best regards,
Balsam
Hi Balsam,
On 18/01/2023 14:58, Balsam CHIHI wrote:
[ ... ]
>> You're describing the register with nice words, but there's another way to do
>> the same that will be even more effective.
>>
>> /*
>> * LVTS MONINT: Interrupt Monitoring register
>> * Each bit describes the enable status of per-sensor interrupts.
>> */
>> #define LVTS_MONINT_THRES_COLD BIT(0) /* Cold threshold */
>> #define LVTS_MONINT_THRES_HOT BIT(1) /* Hot threshold */
>> #define LVTS_MONINT_OFFST_LOW BIT(2) /* Low offset */
>> #define LVTS_MONINT_OFFST_HIGH BIT(3) /* High offset */
>> #define LVTS_MONINT_OFFST_NTH BIT(4) /* Normal To Hot */
>> #define EVERYTHING_ELSE ........................
>>
>> #define LVTS_MONINT_SNS0_MASK GENMASK( ... )
>> #define LVTS_MONINT_SNS1_MASK GENMASK .....
>>
>> /* Find a better name for this one */
>> #define LVTS_MONINT_EN_IRQS ( LVTS_MONINT_THRES_COLD | LVTS_MONINT_THRES_HOT |
>> LVTS_MONINT_OFFST_LOW ..... etc etc)
>>
>
> Given the complexity of the controller and the number of registers,
> if we create a define per bits, we will end up with a huge list of
> defines (~300).
Yeah, that is too much for a little gain.
However, a few can be added for the interrupt only.
Instead of LVTS_MONINT_THRES ..., it could be LVTS_INT_THRES_... and
reused for LVTS_MONINTSTS and LVTS_MONINT setup as the bits position are
the same?
[ ... ]
>>> +static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
>>> +{
>>> + u32 period_unit = (118 * 1000) / (256 * 38);
>>
>> #define SOMETHING 118
>> #define SOMETHING_ELSE 1000
>> #define ....
>>
>> const u32 period_unit = (SOMETHING * SOMETHING_ELSE) / ....
>>
>
> Constifying "u32 period_unit" generates the following compilation warning :
> ./include/asm-generic/io.h:273:61: note: expected ‘volatile void *’
> but argument is of type ‘const void *’
> 273 | static inline void writel(u32 value, volatile void __iomem *addr)
> | ~~~~~~~~~~~~~~~~~~~~~~~^~~~
That is strange. period_unit is the 'value', not the 'addr'. Are you
sure about the warning?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Hi Daniel,
On Wed, Jan 18, 2023 at 3:30 PM Daniel Lezcano
<[email protected]> wrote:
>
>
> Hi Balsam,
>
> On 18/01/2023 14:58, Balsam CHIHI wrote:
>
> [ ... ]
>
> >> You're describing the register with nice words, but there's another way to do
> >> the same that will be even more effective.
> >>
> >> /*
> >> * LVTS MONINT: Interrupt Monitoring register
> >> * Each bit describes the enable status of per-sensor interrupts.
> >> */
> >> #define LVTS_MONINT_THRES_COLD BIT(0) /* Cold threshold */
> >> #define LVTS_MONINT_THRES_HOT BIT(1) /* Hot threshold */
> >> #define LVTS_MONINT_OFFST_LOW BIT(2) /* Low offset */
> >> #define LVTS_MONINT_OFFST_HIGH BIT(3) /* High offset */
> >> #define LVTS_MONINT_OFFST_NTH BIT(4) /* Normal To Hot */
> >> #define EVERYTHING_ELSE ........................
> >>
> >> #define LVTS_MONINT_SNS0_MASK GENMASK( ... )
> >> #define LVTS_MONINT_SNS1_MASK GENMASK .....
> >>
> >> /* Find a better name for this one */
> >> #define LVTS_MONINT_EN_IRQS ( LVTS_MONINT_THRES_COLD | LVTS_MONINT_THRES_HOT |
> >> LVTS_MONINT_OFFST_LOW ..... etc etc)
> >>
> >
> > Given the complexity of the controller and the number of registers,
> > if we create a define per bits, we will end up with a huge list of
> > defines (~300).
>
> Yeah, that is too much for a little gain.
>
> However, a few can be added for the interrupt only.
>
> Instead of LVTS_MONINT_THRES ..., it could be LVTS_INT_THRES_... and
> reused for LVTS_MONINTSTS and LVTS_MONINT setup as the bits position are
> the same?
>
OK, I will add a few for the interrupts.
> [ ... ]
>
>
> >>> +static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> >>> +{
> >>> + u32 period_unit = (118 * 1000) / (256 * 38);
> >>
> >> #define SOMETHING 118
> >> #define SOMETHING_ELSE 1000
> >> #define ....
> >>
> >> const u32 period_unit = (SOMETHING * SOMETHING_ELSE) / ....
> >>
> >
> > Constifying "u32 period_unit" generates the following compilation warning :
> > ./include/asm-generic/io.h:273:61: note: expected ‘volatile void *’
> > but argument is of type ‘const void *’
> > 273 | static inline void writel(u32 value, volatile void __iomem *addr)
> > | ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>
> That is strange. period_unit is the 'value', not the 'addr'. Are you
> sure about the warning?
I double-checked and you are right.
The warning is not related to "const u32 period_unit".
It is related to the previous one, which is "const void __iomem *lvts_edata".
And this is the full log :
drivers/thermal/mediatek/lvts_thermal.c: In function ‘lvts_ctrl_calibrate’:
drivers/thermal/mediatek/lvts_thermal.c:835:47: warning: passing
argument 2 of ‘writel’ discards ‘const’ qualifier from pointer target
type [-Wdiscarded-qualifiers]
835 | writel(lvts_ctrl->calibration[i], lvts_edata[i]);
| ~~~~~~~~~~^~~
In file included from ./arch/arm64/include/asm/io.h:163,
from ./include/linux/io.h:13,
from ./include/linux/irq.h:20,
from ./include/asm-generic/hardirq.h:17,
from ./arch/arm64/include/asm/hardirq.h:17,
from ./include/linux/hardirq.h:11,
from ./include/linux/interrupt.h:11,
from drivers/thermal/mediatek/lvts_thermal.c:12:
./include/asm-generic/io.h:273:61: note: expected ‘volatile void *’
but argument is of type ‘const void *’
273 | static inline void writel(u32 value, volatile void __iomem *addr)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
>
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
Best regards,
Balsam