lkml.org 
[lkml]   [2023]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v2 2/6] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode
From
On 04/05/2023 02:48, Nícolas F. R. A. Prado wrote:
> Each controller can be configured to operate on immediate or filtered
> mode. On filtered mode, the sensors are enabled by setting the
> corresponding bits in MONCTL0, while on immediate mode, by setting
> MSRCTL1.
>
> Previously, the code would set MSRCTL1 for all four sensors when
> configured to immediate mode, but given that the controller might not
> have all four sensors connected, this would cause interrupts to trigger
> for non-existent sensors. Fix this by handling the MSRCTL1 register
> analogously to the MONCTL0: only enable the sensors that were declared.
>
> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>
> (no changes since v1)
>
> drivers/thermal/mediatek/lvts_thermal.c | 50 +++++++++++++------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 2988f201633a..f7d998a45ea0 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -922,24 +922,6 @@ static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> LVTS_HW_FILTER << 3 | LVTS_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
> *
> @@ -1004,6 +986,8 @@ 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;
> + u32 sensor_map_bits[][4] = {{BIT(4), BIT(5), BIT(6), BIT(9)},
> + {BIT(0), BIT(1), BIT(2), BIT(3)}};

Even correct, this initialization sounds prone to error and a bit
obfuscating (eg. it assumes LVTS_MSR_IMMEDIATE_MODE is 1).

What about?

/*
* A description
*/
u32 sensor_imm_bitmap[] = { BIT(0), BIT(1), BIT(2), BIT(3) };
u32 sensor_filt_bitmap[] = { BIT(4), BIT(5), BIT(6), BIT(9) };

u32 *sensor_bitmap = lvts_ctrl->mode ? LVTS_MSR_IMMEDIATE_MODE :
sensor_imm_bitmap : sensor_filt_bitmap;

> int i;
>
> for (i = 0; i < lvts_ctrl->num_lvts_sensor; i++) {
> @@ -1040,20 +1024,38 @@ static int lvts_ctrl_start(struct device *dev, struct lvts_ctrl *lvts_ctrl)
> * map, so we can enable the temperature monitoring in
> * the hardware thermal controller.
> */
> - sensor_map |= BIT(i);
> + sensor_map |= sensor_map_bits[lvts_ctrl->mode][i];

sensor_map |= sensor_bitmap[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));
> + if (lvts_ctrl->mode == LVTS_MSR_IMMEDIATE_MODE) {
> + /*
> + * 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 in MONCTL1 and MONCTL2
> + */
> + writel(sensor_map, LVTS_MSRCTL1(lvts_ctrl->base));
> + } else {
> + /*
> + * Bits:
> + * 9: Single point access flow
> + * 0-3: Enable sensing point 0-3
> + */
> + writel(sensor_map | BIT(9), LVTS_MONCTL0(lvts_ctrl->base));
> + }
>
> return 0;
> }

--
<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

\
 
 \ /
  Last update: 2023-06-02 10:49    [W:0.348 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site