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

Daniel Lezcano
2024-01-22T16:03:49Z

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?

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