Re: [PATCH 2/2] TVP514x Driver with Review comments fixed

[David Brownell <david-b@pacbell.net>]

On Friday 28 November 2008, hvaibhav@ti.com 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.


> +                       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.


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...)

- Dave


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html