30.07.2020 03:55, Sowjanya Komatineni пишет:
>
> On 7/29/20 5:52 PM, Sowjanya Komatineni wrote:
>>
>> On 7/29/20 5:43 PM, Dmitry Osipenko wrote:
>>> 30.07.2020 03:27, Sowjanya Komatineni пишет:
>>> ...
>>>>>> Secondly, perhaps a failed calibration isn't a very critical error?
>>>>>> Hence just printing a warning message should be enough.
>>>>> Using dev_err to report calibration failure. Are you suggesting to use
>>>>> dev_warn instead of dev_err?
>>> I meant that failing s_stream might be unnecessary.
>>>
>>> The dev_warn should be more appropriate for a non-critical errors.
>>>
>>>>>> Could you please make a patch that factors all ON/OFF code paths
>>>>>> into a
>>>>>> separate functions? It's a bit difficult to follow the combined code,
>>>>>> especially partial changes in the patches. Thanks in advance!
>>>>> what do you mean by partial changes in patches?
>>>>>
>>>>> Can you please be more clear?
>>>> Also please specify what ON/OFF code paths you are referring to when
>>>> you
>>>> say to move into separate functions?
>>> I meant to change all the code like this:
>>>
>>> set(on) {
>>> if (on) {
>>> ...
>>> return;
>>> }
>>>
>>> if (!on)
>>> ...
>>>
>>> return;
>>> }
>>>
>>> to somewhat like this:
>>>
>>> set(on) {
>>> if (on)
>>> ret = enable();
>>> else
>>> ret = disable();
>>>
>>> return ret;
>>> }
>>
>> You mean to change tegra_channel_set_stream() ?
Yes, and tegra_csi_s_stream().
> changing tegra_channel_set_stream() to have like below will have
> redundant calls as most of the code b/w enable and disable is same
> except calling them in reverse order based on on/off and doing MIPI
> calibration only during ON
>
>
> if (on)
> ret = enable()
> else
> ret = disable()
> return ret;
Readability should be more important than number of lines.
