Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH v5 13/14] media: tegra-video: Add CSI MIPI pads
 calibration
To:     Sowjanya Komatineni <skomatineni@nvidia.com>,
        thierry.reding@gmail.com, jonathanh@nvidia.com, frankc@nvidia.com,
        hverkuil@xs4all.nl, sakari.ailus@iki.fi, robh+dt@kernel.org,
        helen.koike@collabora.com
Cc:     sboyd@kernel.org, gregkh@linuxfoundation.org,
        linux-media@vger.kernel.org, devicetree@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-i2c@vger.kernel.org
References: <1595883452-17343-1-git-send-email-skomatineni@nvidia.com>
 <1595883452-17343-14-git-send-email-skomatineni@nvidia.com>
 <c3d40261-9d77-3634-3e04-f20efad9d3d8@gmail.com>
 <01ee0805-3d57-d857-48e3-5c2245cd4500@nvidia.com>
 <2ec535c9-55e8-8834-6002-36c75aeb097c@gmail.com>
 <021707b9-5f72-6f8b-d125-43627ef64a6d@nvidia.com>
 <de2d6117-b1e5-14c0-52ca-ff46b444c866@nvidia.com>
 <ca18b93e-1f1c-50ae-e0c1-11758935ee89@gmail.com>
 <42f25a49-ad2b-4e52-fd99-cb0f52037988@nvidia.com>
 <f5440f57-3653-7cf0-9efe-e9a0c276f7cf@nvidia.com>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <6ae07f82-53f7-33fc-5892-340b0d9f12cf@gmail.com>
Date:   Thu, 30 Jul 2020 03:53:06 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <f5440f57-3653-7cf0-9efe-e9a0c276f7cf@nvidia.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.