Re: [PATCH v3] media: imx: mipi csi-2: Don't fail if initial state times-out

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Ezequiel,

On Wed, Aug 07, 2019 at 10:59:22AM -0300, Ezequiel Garcia wrote:
> Hi Sakari,
> 
> Thanks for reviewing the patch.
> 
> On Wed, 2019-08-07 at 15:06 +0300, Sakari Ailus wrote:
> > On Tue, Jul 30, 2019 at 05:14:24AM -0300, Ezequiel Garcia wrote:
> > > Hey Hans,
> > > 
> > > On Mon, 2019-07-01 at 08:48 +0200, Philipp Zabel wrote:
> > > > On Thu, 2019-06-27 at 19:29 -0300, Fabio Estevam wrote:
> > > > > From: Ezequiel Garcia <ezequiel@collabora.com>
> > > > > 
> > > > > Not all sensors will be able to guarantee a proper initial state.
> > > > > This may be either because the driver is not properly written,
> > > > > or (probably unlikely) because the hardware won't support it.
> > > > > 
> > > > > While the right solution in the former case is to fix the sensor
> > > > > driver, the real world not always allows right solutions, due to lack
> > > > > of available documentation and support on these sensors.
> > > > > 
> > > > > Let's relax this requirement, and allow the driver to support stream start,
> > > > > even if the sensor initial sequence wasn't the expected.
> > > > > 
> > > > > Also improve the warning message to better explain the problem and provide
> > > > > a hint that the sensor driver needs to be fixed.
> > > > > 
> > > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > > > Signed-off-by: Fabio Estevam <festevam@gmail.com>
> > > > 
> > > > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > > 
> > > 
> > > This seems ready to pick and it has Philipp's and Steve's RB.
> > 
> > Hi Ezequiel,
> > 
> > In general the LP-11 condition should be detected by hardware (or firmware)
> > in such a way that it's detected even if a transmitter that holds the state
> > just a short period of time. In other words, software is not supposed to be
> > even testing for it.
> > 
> > Have you checked how it works if you simply leave out this test?
> > 
> 
> The current change relaxes a condition, which we observed was too strict.
> Some drivers might be unable to enter LP-11 state, but I don't think
> that's a reason to fail capture.

Some devices can be commanded to enter LP-11 state but some will just
briefly visit that state. The LP-11 state is mandatory but software should
not be involved in detecting it if at all possible.

So if the hardware does not require further initialisation to be done in
LP-11 state, you should remove the check.

> 
> We had to fix at least OV5645 and OV5640 recently because of this,
> and I can imagine more drivers will have the same issue.

This is actually an issue in the IMX driver (or hardware), not in the
sensor driver. It may be that sometimes it's easier to work around it in
the sensor driver.

So, I'd like to know whether the check itself is a driver bug, or something
that the hardware requires. The fact that you're sending this patch
suggests the former.

-- 
Regards,

Sakari Ailus