[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76ac592a-c2a9-413e-848e-a15a36173cf5@linaro.org>
Date: Mon, 15 Jul 2024 10:26:19 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Satya Priya Kakitapalli <quic_skakitap@...cinc.com>,
Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>,
Lukasz Luba <lukasz.luba@....com>, Amit Kucheria <amitk@...nel.org>,
Thara Gopinath <thara.gopinath@...il.com>,
Bjorn Andersson <andersson@...nel.org>
Cc: Kamal Wadhwa <quic_kamalw@...cinc.com>, Taniya Das
<quic_tdas@...cinc.com>, Jishnu Prakash <quic_jprakash@...cinc.com>,
linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-pm@...r.kernel.org, Ajit Pandey <quic_ajipan@...cinc.com>,
Imran Shaik <quic_imrashai@...cinc.com>,
Jagadeesh Kona <quic_jkona@...cinc.com>
Subject: Re: [PATCH 3/5] thermal: qcom: Add support for MBG Temp monitor
On 12.07.2024 2:43 PM, Satya Priya Kakitapalli wrote:
> Add driver for the MBG thermal monitoring device. It monitors
> the die temperature, and when there is a level 1 upper threshold
> violation, it receives an interrupt over spmi. The driver reads
> the fault status register and notifies thermal accordingly.
>
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@...cinc.com>
> ---
[...]
> drivers/thermal/qcom/Kconfig | 11 ++
> drivers/thermal/qcom/Makefile | 1 +
> drivers/thermal/qcom/qcom-spmi-mbg-tm.c | 269 ++++++++++++++++++++++++++++++++
> 3 files changed, 281 insertions(+)
>
> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index 2c7f3f9a26eb..46045094020c 100644
> --- a/drivers/thermal/qcom/Kconfig
> +++ b/drivers/thermal/qcom/Kconfig
> @@ -21,6 +21,17 @@ config QCOM_SPMI_ADC_TM5
> Thermal client sets threshold temperature for both warm and cool and
> gets updated when a threshold is reached.
>
> +config QCOM_SPMI_MBG_TM
> + tristate "Qualcomm Technologies, Inc. SPMI PMIC MBG Temperature monitor"
> + depends on OF && SPMI && IIO
> + select REGMAP_SPMI
> + help
> + This enables a thermal driver for the MBG thermal monitoring device.
> + It shows up in sysfs as a thermal sensor with two trip points.
> + It notifies the thermal framework when level 1 high threshold is
> + violated. The temperature reported by the thermal sensor reflects
> + the real time die temperature through ADC channel.
> +
> config QCOM_SPMI_TEMP_ALARM
> tristate "Qualcomm SPMI PMIC Temperature Alarm"
> depends on OF && SPMI && IIO
> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> index 0fa2512042e7..bc18e08ee3e2 100644
> --- a/drivers/thermal/qcom/Makefile
> +++ b/drivers/thermal/qcom/Makefile
> @@ -4,5 +4,6 @@ obj-$(CONFIG_QCOM_TSENS) += qcom_tsens.o
> qcom_tsens-y += tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
> tsens-8960.o
> obj-$(CONFIG_QCOM_SPMI_ADC_TM5) += qcom-spmi-adc-tm5.o
> +obj-$(CONFIG_QCOM_SPMI_MBG_TM) += qcom-spmi-mbg-tm.o
> obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM) += qcom-spmi-temp-alarm.o
> obj-$(CONFIG_QCOM_LMH) += lmh.o
> diff --git a/drivers/thermal/qcom/qcom-spmi-mbg-tm.c b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> new file mode 100644
> index 000000000000..70964ea5a48d
> --- /dev/null
> +++ b/drivers/thermal/qcom/qcom-spmi-mbg-tm.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/thermal.h>
> +#include <linux/iio/consumer.h>
> +
> +#include "../thermal_core.h"
> +
> +#define MBG_TEMP_MON_MM_MON2_FAULT_STATUS 0x50
> +
> +#define MON_FAULT_STATUS_MASK GENMASK(7, 6)
> +#define MON_FAULT_STATUS_SHIFT 6
> +#define MON2_LVL1_ERR 0x1
> +
> +#define MON2_LVL1_UP_THRESH 0x59
> +
> +#define MBG_TEMP_MON_MM_MON2_MISC_CFG 0x5f
> +#define UP_THRESH_EN BIT(1)
> +
> +#define STEP_MV 8
> +#define MBG_DEFAULT_TEMP_MV 600
> +#define MBG_TEMP_CONSTANT 1000
> +#define MIN_TRIP_TEMP 25000
> +#define MAX_SUPPORTED_TEMP 160000
> +
> +struct mbg_tm_chip {
> + struct regmap *map;
> + struct device *dev;
> + struct thermal_zone_device *tz_dev;
> + struct mutex lock;
> + unsigned int base;
> + int irq;
> + int last_temp;
> + bool last_temp_set;
> + struct iio_channel *adc;
> +};
> +
> +struct mbg_map_table {
> + int min_temp;
> + int vtemp0;
> + int tc;
> +};
> +
> +static const struct mbg_map_table map_table[] = {
> + /* minT vtemp0 tc */
> + { -60000, 4337, 1967 },
> + { -40000, 4731, 1964 },
> + { -20000, 5124, 1957 },
> + { 0, 5515, 1949 },
> + { 20000, 5905, 1940 },
> + { 40000, 6293, 1930 },
> + { 60000, 6679, 1921 },
> + { 80000, 7064, 1910 },
> + { 100000, 7446, 1896 },
> + { 120000, 7825, 1878 },
> + { 140000, 8201, 1859 },
> +};
> +
> +static int mbg_tm_read(struct mbg_tm_chip *chip, u16 addr, int *data)
> +{
> + return regmap_read(chip->map, chip->base + addr, data);
> +}
> +
> +static int mbg_tm_write(struct mbg_tm_chip *chip, u16 addr, int data)
> +{
> + return regmap_write(chip->map, chip->base + addr, data);
> +}
> +
> +static int mbg_tm_reg_update(struct mbg_tm_chip *chip, u16 addr, u8 mask, u8 val)
> +{
> + return regmap_write_bits(chip->map, chip->base + addr, mask, val);
> +}
You're not saving much on code amount, or readability by adding these
accessors, r/w are used once and update is used twice.
> +
> +static int mbg_tm_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> + struct mbg_tm_chip *chip = thermal_zone_device_priv(tz);
> + int ret, milli_celsius;
> +
> + if (!temp)
> + return -EINVAL;
> +
> + if (chip->last_temp_set) {
> + pr_debug("last_temp: %d\n", chip->last_temp);
> + chip->last_temp_set = false;
last_temp_set -> last_thres_crossed?
> + *temp = chip->last_temp;
> + return 0;
> + }
[...]
> +
> + /* The HW has a limitation that the trip set must be above 25C */
> + if (temp > MIN_TRIP_TEMP && temp < INT_MAX) {
INT_MAX -> MAX_SUPPORTED_TEMP?
> + mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
> + UP_THRESH_EN, UP_THRESH_EN);
regmap_set_bits
> + vtemp = temp_to_vtemp(temp);
> + ret = mbg_tm_write(chip, MON2_LVL1_UP_THRESH, vtemp);
regmap_write(...., temp_to_vtemp(temp))
> + if (ret < 0) {
> + mutex_unlock(&chip->lock);
> + return ret;
> + }
> + } else {
> + dev_dbg(chip->dev, "Setting %d failed, set trip between 25C and INT_MAX\n", temp);
> + mbg_tm_reg_update(chip, MBG_TEMP_MON_MM_MON2_MISC_CFG,
> + UP_THRESH_EN, 0);
regmap_clear_bits
[...]
> + val &= MON_FAULT_STATUS_MASK;
> + if ((val >> MON_FAULT_STATUS_SHIFT) & MON2_LVL1_ERR) {
FIELD_GET
> + chip->last_temp_set = true;
> + thermal_zone_device_update(chip->tz_dev,
> + THERMAL_TRIP_VIOLATED);
> + dev_dbg(chip->dev, "Notifying Thermal, fault status=%d\n", val);
> + } else {
> + dev_dbg(chip->dev, "Lvl 1 upper threshold not violated, ignoring interrupt\n");
Would such spurious IRQs carry any meaning at all?
[...]
> +static struct platform_driver mbg_tm_driver = {
> + .driver = {
> + .name = "qcom-spmi-mbg-tm",
> + .of_match_table = mbg_tm_match_table,
> + },
> + .probe = mbg_tm_probe,
Double space
Konrad
Powered by blists - more mailing lists