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

Nicolas Pitre
2024-01-22T16:13:37Z

On Mon, 22 Jan 2024, Daniel Lezcano wrote:

> On 22/01/2024 16:23, Nicolas Pitre wrote:
> > On Mon, 22 Jan 2024, Daniel Lezcano wrote:
> >
> >>
> >> Hi Nico,
> >>
> >> On 19/01/2024 17:53, Nicolas Pitre wrote:
> >>
> >> [ ... ]
> >>
> >>>>> + 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 ?
> >>>
> >>> I'm not sure how I could make it simpler. Maybe a comment is in order
> >>> though?
> >>>
> >>> The sensor controller has 4 slots. Those slots are accessible either
> >>> through imm_regs[<slot_number>] oe msr_regs[<slot_number>]. If, say,
> >>> slot 0 is unpopulated then sensor 0 (i = 0) needs to address slot 1 (i =
> >>> 0, skip = 1), sensor 1 is in slot 2 (i = 1, skip = 1), etc. Does this
> >>> make sense?
> >>
> >> Why not keep the sensor id = slot id and declare the ones which are
> >> disabled
> >> with a mask?
> >
> > Then what do you do with the empty sensor 0? Do we want to present dead
> > sensor IDs to users?
>
> You can skip it somehow in lvts_ctrl_start(), right?

I see what you mean. Indeed that would be better.

Nicolas