Re: [PATCH 8/9] thermal/drivers/mediatek/lvts_thermal: allow early empty sensor slots

[Daniel Lezcano <daniel.lezcano@linaro.org>]

Hi Nico,

On 11/01/2024 23:30, Nicolas Pitre wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
> 
> Some systems don't always populate sensor controller slots starting
> at slot 0.
> 
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> ---
>   drivers/thermal/mediatek/lvts_thermal.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index b20b70fd36..473ef91ea3 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -112,6 +112,7 @@ struct lvts_ctrl_data {
>   	struct lvts_sensor_data lvts_sensor[LVTS_SENSOR_MAX];
>   	int hw_tshut_temp;
>   	int num_lvts_sensor;
> +	int skipped_sensors;
>   	int offset;
>   	int mode;
>   };
> @@ -555,6 +556,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
>   					const 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),
> @@ -569,7 +571,7 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
>   		LVTS_IMMD3(lvts_ctrl->base)
>   	};
>   
> -	int i;
> +	int i, skip;
>   
>   	for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++) {
>   
> @@ -604,8 +606,9 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
>   		/*
>   		 * Each sensor has its own register address to read from.
>   		 */
> +		skip = lvts_ctrl_data->skipped_sensors;
>   		lvts_sensor[i].msr = lvts_ctrl_data->mode == LVTS_MSR_IMMEDIATE_MODE ?
> -			imm_regs[i] : msr_regs[i];
> +			imm_regs[i + skip] : msr_regs[i + skip];

Overall the series look ok but this changes is hard to understand.

Could you propose a different approach to have the resulting code easier 
to understand ?


>   		lvts_sensor[i].low_thresh = INT_MIN;
>   		lvts_sensor[i].high_thresh = INT_MIN;

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