RE: [PATCH 5/7] OMAP: DSS: Adding initialization routine to picodlp panel

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Thu, 2011-04-21 at 16:47 +0530, Janorkar, Mayuresh wrote:
> 
> > -----Original Message-----
> > From: Valkeinen, Tomi
> > Sent: Tuesday, April 19, 2011 5:23 PM
> > To: Janorkar, Mayuresh
> > Cc: linux-omap@vger.kernel.org; K, Mythri P
> > Subject: Re: [PATCH 5/7] OMAP: DSS: Adding initialization routine to
> > picodlp panel
> > 
> > On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:

<snip>

> > >  static int picodlp_i2c_probe(struct i2c_client *client,
> > >  		const struct i2c_device_id *id)
> > >  {
> > > @@ -134,6 +446,11 @@ static int picodlp_panel_power_on(struct
> > omap_dss_device *dssdev)
> > >  	picodlp_i2c_data =
> > >  		i2c_get_clientdata(picod->picodlp_i2c_client);
> > >
> > > +	msleep(700); /* sleep till panel is settled */
> > 
> > And another huge sleep. This, unlike the other sleeps, make some sense,
> > as there's an i2c transaction done below.
> > 
> > Somehow I get the feeling that you've just put big sleeps here and there
> > until the driver started working... Can you point me to the
> > documentation that describes the delays required?
> 
> Except msleep(5) in
> /* transfer control to flash controller */
> r = picodlp_i2c_write(client, PBC_CONTROL, 1);
>         msleep(5);
> r = picodlp_i2c_write(client, PBC_CONTROL, 0); 
> 
> Other delays are not documented. But it is practically observed that we need to wait otherwise the i2c_packet does not succeed.

Then we need to contact the HW guys developing picodlp. They are TIers,
aren't they? We can't just put sleeps here and there and be sure it'll
work properly.

What if your sleep of 700ms has, by luck, caused a 900ms sleep in your
tests (sleeps can take considerably longer than the given value), and
the hardware requires 800ms sleep? Your tests show it works, but for
some other user the sleep may cause a 750ms sleep, causing picodlp to
fail to start.

 Tomi