Thanks, Vaibhav Hiremath > -----Original Message----- > From: David Brownell [mailto:david-b@xxxxxxxxxxx] > Sent: Friday, November 28, 2008 10:22 PM > To: Hiremath, Vaibhav > Cc: video4linux-list@xxxxxxxxxx; davinci-linux-open-source- > bounces@xxxxxxxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; Jadav, > Brijesh R; Shah, Hardik; Hadli, Manjunath; R, Sivaraj; Karicheri, > Muralidharan > Subject: Re: [PATCH 2/2] TVP514x Driver with Review comments fixed > > On Friday 28 November 2008, hvaibhav@xxxxxx wrote: > > + for (; next->token != TOK_TERM; next++) { > > + if (next->token == TOK_DELAY) { > > + schedule_timeout(msecs_to_jiffies(next- > >val)); > > msleep(next->val); > > would be clearer and more conventional. > > [Hiremath, Vaibhav] Yes, verified the implementation of msleep. It does the same thing what I am doing. Good to change. > > + continue; > > + } > > > > +static int > > +tvp514x_probe(struct i2c_client *client, const struct > i2c_device_id *id) > > +{ > > + struct tvp514x_decoder *decoder = &tvp514x_dev; > > + int err; > > + > > + if (i2c_get_clientdata(client)) > > + return -EBUSY; > > + > > When no driver is bound to the client, clientdata is undefined. > So just strike that ... you can rely on probe() only being > called on un-bound drivers. > > [Hiremath, Vaibhav] I am not an expert in I2C, will get back to you on this. > I still don't see any post-reset chip init being done. The data > sheets for the tvp5146 and tvp5147 say that after reset, some > commands must be sent ... that's not being done here. Are you > assuming perhaps tvp5146m2 and tvp5147m1, although they are not > listed as chips supported by this driver? (That's a repeat of > a previous review comment, which garnered no response, so the > $SUBJECT is inaccurate: at least some review comments have not > yet been addressed...) > [Hiremath, Vaibhav] Thanks pointing me to this, I was not aware of this thing. Two same ID chips (0x46) require different reset sequence. May be overlooked this part due to ID. Will have to now think how to differentiate between these two chips and handle this sequence. > - Dave > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
