Hi David,

Thanks,
Vaibhav Hiremath

> -----Original Message-----
> From: David Brownell [mailto:[EMAIL PROTECTED]
> Sent: Saturday, November 29, 2008 1:24 AM
> To: Hiremath, Vaibhav
> Cc: [EMAIL PROTECTED]; davinci-linux-open-source-
> [EMAIL PROTECTED]; [email protected]; 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, David Brownell wrote:
> > On Friday 28 November 2008, Hiremath, Vaibhav wrote:
> > > Will have to now think how to differentiate between these
> > > two chips and handle this sequence.
> >
> > That's really easy, the "id" parameter to probe() tells you:
> >
> >     if (strcmp(id->name, "tvp5146") == 0)
> >             /* original '46 part ... */;
> >     else if (strmcp(id->name, "tvp5146m2") == 0)
> >             /* new '46m2 version ... */
> >     ... etc
> 
> ... although it's even easier to use id->driver_data to
> hold, for example, a bitmask telling various attributes
> of that particular device.  Examples here:
> 
>       - does it have the extra '46 registers?
>       - does it use the original '46 init sequence?
>       - or the new m2 one?
>       - or the original '47 init sequence?
>       - or the new m1 version?
>       - ...
> 
> Another common use of driver_data is to hold a pointer
> to a struct holding chip-specific data that doesn't fit
> into a simple bitmask.
> 
[Hiremath, Vaibhav] I am trying to use/save complete init sequence in 
id->driver_data -

static const struct i2c_device_id tvp514x_id[] = {
        {"tvp5146", (unsigned int)&tvp5146_init},
        {"tvp5146m2", (unsigned int)&tvp514xm_init},
        {"tvp5147", (unsigned int)&tvp5147_init},
        {"tvp5147m1", (unsigned int)&tvp514xm_init},
        {},
};

NOTE: Please note that init sequence for 46, 47 are different.

But I came to know that, client structure doesn't have any parameter which will 
provide me the index under this id table. The only differentiating parameter we 
have is "name" (decoder->client->driver->name).

I can use "id->driver_data" only in my probe function without any index. 

So left with only following options -

1) 
        if (strcmp(id->name, "tvp5146") == 0)
                /* original 46 init seq */;
        else if (strcmp(id->name, "tvp5147") == 0) 
                /* original 47 init seq */
        else if ((strmcp(id->name, "tvp5146m2") == 0) || 
                        (strmcp(id->name, "tvp5147m1") == 0))
                /* New 46/47 init seq */

2)

Driver specific structure must contain either of
        - Index of i2c_device_id table, use this to get the driver_data. (This 
also requires string compare to get the index.)
        - Pointer to init_reg_seq, which is pointer to array of structure for 
tvp514x_regs. This is little bit ugly, since will have to export tvp514x_regs 
structure.
        - Or have pointer to i2c_device_id itself. (Implemented and tested)


I prefer to use second option, instead of comparing the name string in s_power 
every time. And it will be very easy to add even more chips providing generic 
solution; we need to just add entry to i2c_device_id with expected init 
sequence and you are done.

Any suggestions or inputs appreciated???

> - Dave

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

Reply via email to