On 12/05/2022 14:24, Alexandre Bailon wrote:
> From: Michael Kao <michael.kao@xxxxxxxxxxxx>
>
> Add a LVTS (Low voltage thermal sensor) driver to report junction
> temperatures in Mediatek SoC and register the maximum temperature
> of sensors and each sensor as a thermal zone.
>
> Signed-off-by: Yu-Chia Chang <ethan.chang@xxxxxxxxxxxx>
> Signed-off-by: Michael Kao <michael.kao@xxxxxxxxxxxx>
> Signed-off-by: Ben Tseng <ben.tseng@xxxxxxxxxxxx>
> Signed-off-by: Alexandre Bailon <abailon@xxxxxxxxxxxx>
> + .domain_index = MT6873_AP_DOMAIN,
> + .addr_offset = 0x200,
> + .num_sensor = 2,
> + .sensor_map = {MT6873_TS6_0, MT6873_TS6_1},
> + .tc_speed = SET_TC_SPEED_IN_US(118, 118, 118, 118),
> + .hw_filter = LVTS_FILTER_2_OF_4,
> + .dominator_sensing_point = SENSING_POINT1,
> + .hw_reboot_trip_point = 117000,
> + .irq_bit = BIT(5),
> + },
> + [6] = {
> + .domain_index = MT6873_AP_DOMAIN,
> + .addr_offset = 0x300,
> + .num_sensor = 3,
> + .sensor_map = {MT6873_TS7_0, MT6873_TS7_1, MT6873_TS7_2},
> + .tc_speed = SET_TC_SPEED_IN_US(118, 118, 118, 118),
> + .hw_filter = LVTS_FILTER_2_OF_4,
> + .dominator_sensing_point = SENSING_POINT2,
> + .hw_reboot_trip_point = 117000,
> + .irq_bit = BIT(6),
> + }
> +};
> +
> +static struct lvts_data mt6873_lvts_data = {
Most of your structures should be const.
> + .num_domain = MT6873_NUM_DOMAIN,
> + .num_tc = MT6873_NUM_LVTS,
> + .tc = mt6873_tc_settings,
> + .num_sensor = MT6873_NUM_TS,
> + .ops = {
> + .efuse_to_cal_data = mt6873_efuse_to_cal_data,
> + .device_enable_and_init = device_enable_and_init_v4,
> + .device_enable_auto_rck = device_enable_auto_rck_v4,
> + .device_read_count_rc_n = device_read_count_rc_n_v4,
> + .set_cal_data = set_calibration_data_v4,
> + .init_controller = init_controller_v4,
> + },
> + .feature_bitmap = FEATURE_DEVICE_AUTO_RCK,
> + .num_efuse_addr = 22,
> + .num_efuse_block = 1,
> + .cal_data = {
> + .default_golden_temp = 50,
> + .default_count_r = 35000,
> + .default_count_rc = 2750,
> + },
> + .coeff = {
> + .a = -250460,
> + .b = 250460,
> + },
> +};
> +
> +/*==================================================
> + *==================================================
> + * Support chips
> + *==================================================
> + */
This is not a comment in Linux coding style. Please skip all such
headers. Code should have pointers to const when not modifying...
> +static const struct of_device_id lvts_of_match[] = {
> + {
> + .compatible = "mediatek,mt6873-lvts",
> + .data = (void *)&mt6873_lvts_data,
> + },
> + {
> + },
> +};
> +MODULE_DEVICE_TABLE(of, lvts_of_match);
> +/*==================================================*/
> +static struct platform_driver soc_temp_lvts = {
> + .probe = lvts_probe,
> + .remove = lvts_remove,
> + .suspend = lvts_suspend,
> + .resume = lvts_resume,
> + .driver = {
> + .name = "mtk-soc-temp-lvts",
> + .of_match_table = lvts_of_match,
> + },
> +};
> +
> +module_platform_driver(soc_temp_lvts);
> +MODULE_AUTHOR("Yu-Chia Chang <ethan.chang@xxxxxxxxxxxx>");
> +MODULE_AUTHOR("Michael Kao <michael.kao@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Mediatek soc temperature driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/thermal/mediatek/soc_temp_lvts.h b/drivers/thermal/mediatek/soc_temp_lvts.h
> new file mode 100644
> index 000000000000..77c64145aa17
> --- /dev/null
> +++ b/drivers/thermal/mediatek/soc_temp_lvts.h
> @@ -0,0 +1,312 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_SOC_TEMP_LVTS_H__
> +#define __MTK_SOC_TEMP_LVTS_H__
> +
> +/* LVTS HW filter settings
> + * 000: Get one sample
> + * 001: Get 2 samples and average them
> + * 010: Get 4 samples, drop max and min, then average the rest of 2 samples
> + * 011: Get 6 samples, drop max and min, then average the rest of 4 samples
> + * 100: Get 10 samples, drop max and min, then average the rest of 8 samples
> + * 101: Get 18 samples, drop max and min, then average the rest of 16 samples
> + */
> +enum lvts_hw_filter {
> + LVTS_FILTER_1,
> + LVTS_FILTER_2,
> + LVTS_FILTER_2_OF_4,
> + LVTS_FILTER_4_OF_6,
> + LVTS_FILTER_8_OF_10,
> + LVTS_FILTER_16_OF_18
> +};
> +
> +enum lvts_sensing_point {
> + SENSING_POINT0,
> + SENSING_POINT1,
> + SENSING_POINT2,
> + SENSING_POINT3,
> + ALL_SENSING_POINTS
> +};
> +
> +/*==================================================
> + * Data structure
> + *==================================================
> + */
> +struct lvts_data;
> +
> +struct speed_settings {
> + unsigned int period_unit;
> + unsigned int group_interval_delay;
> + unsigned int filter_interval_delay;
> + unsigned int sensor_interval_delay;
> +};
> +
> +struct tc_settings {
> + unsigned int domain_index;
> + unsigned int addr_offset;
> + unsigned int num_sensor;
> + unsigned int sensor_map[ALL_SENSING_POINTS]; /* In sensor ID */
> + struct speed_settings tc_speed;
> + /* HW filter setting
Missing /* in starting line, it's not a network subsystem. Same in other
places.
> + * 000: Get one sample
> + * 001: Get 2 samples and average them
> + * 010: Get 4 samples, drop max and min, then average the rest of 2 samples
> + * 011: Get 6 samples, drop max and min, then average the rest of 4 samples
> + * 100: Get 10 samples, drop max and min, then average the rest of 8 samples
> + * 101: Get 18 samples, drop max and min, then average the rest of 16 samples
> + */
> + unsigned int hw_filter;
> + /* Dominator_sensing point is used to select a sensing point
> + * and reference its temperature to trigger Thermal HW Reboot
> + * When it is ALL_SENSING_POINTS, it will select all sensing points
> + */
> + int dominator_sensing_point;
> + int hw_reboot_trip_point; /* -274000: Disable HW reboot */
> + unsigned int irq_bit;
> +};
> +
> +struct formula_coeff {
> + int a;
> + int b;
> + unsigned int golden_temp;
> +};
> +
> +struct sensor_cal_data {
> + int use_fake_efuse; /* 1: Use fake efuse, 0: Use real efuse */
> + unsigned int golden_temp;
> + unsigned int *count_r;
> + unsigned int *count_rc;
> + unsigned int *count_rc_now;
> +
> + unsigned int default_golden_temp;
> + unsigned int default_count_r;
> + unsigned int default_count_rc;
> +};
> +
> +struct platform_ops {
> + void (*efuse_to_cal_data)(struct lvts_data *lvts_data);
> + void (*device_enable_and_init)(struct lvts_data *lvts_data);
> + void (*device_enable_auto_rck)(struct lvts_data *lvts_data);
> + int (*device_read_count_rc_n)(struct lvts_data *lvts_data);
> + void (*set_cal_data)(struct lvts_data *lvts_data);
> + void (*init_controller)(struct lvts_data *lvts_data);
> +};
> +
> +struct power_domain {
> + void __iomem *base; /* LVTS base addresses */
> + unsigned int irq_num; /* LVTS interrupt numbers */
> + struct reset_control *reset;
> +};
> +
> +struct sensor_data {
> + void __iomem *base; /* Sensor base address */
> + int offset; /* Sensor offset */
> +};
> +
> +struct lvts_data {
> + struct device *dev;
> + struct clk *clk;
> + unsigned int num_domain;
> + struct power_domain *domain;
> +
> + int num_tc; /* Number of LVTS thermal controllers */
> + struct tc_settings *tc;
> + int counting_window_us; /* LVTS device counting window */
> +
> + int num_sensor; /* Number of sensors in this platform */
> + void __iomem **reg;
> +
> + struct platform_ops ops;
> + int feature_bitmap; /* Show what features are enabled */
> +
> + unsigned int num_efuse_addr;
> + unsigned int *efuse;
> + unsigned int num_efuse_block; /* Number of contiguous efuse indexes */
> + struct sensor_cal_data cal_data;
> + struct formula_coeff coeff;
> +};
> +
> +struct soc_temp_tz {
> + unsigned int id; /* if id is 0, get max temperature of all sensors */
> + struct lvts_data *lvts_data;
> +};
> +
> +struct match_entry {
> + char chip[32];
> + struct lvts_data *lvts_data;
Please do not mix up indentation of members. In one place you use it, in
other not. Entire code should be consistent, not a mixup of ten
different coding styles.
> +};
Best regards,
Krzysztof
