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

Daniel Lezcano
2024-01-19T16:29:26Z

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 |