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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

Thanks,
Vaibhav Hiremath

> -----Original Message-----
> From: David Brownell [mailto:david-b@xxxxxxxxxxx]
> Sent: Saturday, November 29, 2008 1:24 AM
> 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, 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux