Hi Ramiro,

On Thu, Feb 09, 2017 at 10:13:02AM +0000, Ramiro Oliveira wrote:

Hi Sakari

On 2/9/2017 10:02 AM, Sakari Ailus wrote:

quoted

Hi Ramiro,

On Wed, Feb 08, 2017 at 09:56:12AM +0000, Ramiro Oliveira wrote:

quoted

Hi Sakari

On 2/7/2017 5:31 PM, Sakari Ailus wrote:

quoted

Hi Ramiro,

On Mon, Feb 06, 2017 at 11:38:28AM +0000, Ramiro Oliveira wrote:
...

quoted

quoted

quoted

+	ret = ov5647_write_array(sd, ov5647_640x480,
+					ARRAY_SIZE(ov5647_640x480));
+	if (ret < 0) {
+		dev_err(&client->dev, "write sensor_default_regs error\n");
+		return ret;
+	}
+
+	ov5647_set_virtual_channel(sd, 0);
+
+	ov5647_read(sd, 0x0100, &resetval);
+	if (!(resetval & 0x01)) {

Can this ever happen? Streaming start is at the end of the register list.

I'm not sure it can happen. It was just a safeguard, but I can remove it if you
think it's not necessary

You're not reading back the other registers either, albeit I'd check that
the I2C accesses actually succeed. Generally the return values are ignored.

So you're recommending I perform a random I2C access after power on to check the
system, and discard the read value? Or just drop this piece of code entirely?

I'm not. What I'm saying that you're mostly not checking whether I2C
accesses succeed or not.

I think I'm understanding what you're saying now. You want me to check more
often the return value from write/read functions?

That makes sense. I'll add more checks to the code

Please do. That's generally a better approach with I2C than reading back
register values.  :-)

-- 
Sakari Ailus
sakari.ailus@linux.intel.com