Re: [RFC PATCH v5 13/14] media: tegra-video: Add CSI MIPI pads calibration

[Dmitry Osipenko <digetx@gmail.com>]

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.