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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/29/20 5:53 PM, Dmitry Osipenko wrote:
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.

Will have v6 and add additional patch at the end to do enable/disable separately.

Separating this out with additional patch before adding sensor support patch requires all patches to be updated.

So I am ok either ways. Please let me know if adding additional patch at the end to split tegra_channel_set_stream() and tegra_csi_s_stream() separately is ok?






[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux