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

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

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 |
<http://www.linaro.org/linaro-blog/> Blog