Em Thu, 25 Apr 2013 22:33:20 +0200
Jon Arne Jørgensen <jona...@jonarne.no> escreveu:
> On Thu, Apr 25, 2013 at 05:13:28PM -0300, Mauro Carvalho Chehab wrote:
> > Em Thu, 25 Apr 2013 21:10:18 +0200
> > Jon Arne Jørgensen <jona...@jonarne.no> escreveu:
> >
> > > /* Check whether this chip is part of the saa711x series */
> > > - if (memcmp(name + 1, "f711", 4)) {
> > > + if (memcmp(id->name + 1, "gm7113c", 7)) {
> > > + chip_id = 'c';
> >
> > There are several issues on the above:
> > 1) "id" may be NULL on autodetect mode;
> >
> > 2) Why are you adding 1 here?
> > It should be, instead id->name
> >
> > 3) memcmp returns 0 if matches. So, the test is wrong.
> > So, It should be instead:
> > if (!memcmp(id->name, "gm7113c", 7)) {
> >
> > 4) Also, while that works, it seems a little hackish...
> >
>
> Oh, this is embarrassing.
> I just tried to change as little as possible in this module to make the
> device work.
No problems. On my experience, quick hacks like that are the
ones that generally have more troubles ;)
> > Something like:
> >
> > static int saa711x_detect_chip(struct i2c_client *client,
> > struct saa711x_state *state,
> > const struct i2c_device_id *id)
> > {
> > int i;
> > char chip_id, name[16];
> >
> > /*
> > * Check for gm7113c (a saa7113 clone). Currently, there's no
> > * known way to autodetect it, so boards that use will need to
> > * explicitly fill the id->name field.
> > */
> > if (id && !memcmp(id->name, "gm7113c", 7)) {
> > state->ident = V4L2_IDENT_GM7113C;
> > snprintf(client->name, sizeof(client->name), "%s", id->name);
> > return 0;
> > }
> >
> > /* Check for Philips/NXP original chips */
> > for (i = 0; i < sizeof(name); i++) {
> > i2c_smbus_write_byte_data(client, 0, i);
> > name[i] = (i2c_smbus_read_byte_data(client, 0) & 0x0f) + '0';
> > if (name[i] > '9')
> > name[i] += 'a' - '9' - 1;
> > }
> > name[i] = '\0';
> >
> > if (memcmp(name + 1, "f711", 4))
> > return -ENODEV;
> >
> > chip_id = name[5];
> >
> > snprintf(client->name, sizeof(client->name), "saa711%c", chip_id);
> >
> > /*
> > * Put here the code that fills state->ident for Philips/NXP chips
> > */
> > ...
> >
> > return 0;
>
> Yes this seems to be a much better way to do it.
> I will fix my code.
Thanks!
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
