Re: [PATCH v10 4/6] thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver
From: Daniel Lezcano
Date: Mon Jan 16 2023 - 06:08:45 EST
On 16/01/2023 11:50, AngeloGioacchino Del Regno wrote:
Il 12/01/23 16:28, bchihi@xxxxxxxxxxxx ha scritto:
From: Balsam CHIHI <bchihi@xxxxxxxxxxxx>
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 <bchihi@xxxxxxxxxxxx>
---
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