Re: [PATCH 2/6 v5] Support for TVP7002 in dm365 board

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

 



Understood. I will send my refactoring according to community revisions I have up to now and wait for integration with Murali. I have some questions before fully using his DV interfaces.


Regards,

Narnakaje, Snehaprabha wrote:
-----Original Message-----
From: Santiago Nunez-Corrales [mailto:snunez@xxxxxxxxxxxx]
Sent: Tuesday, October 27, 2009 11:54 AM
To: Narnakaje, Snehaprabha
Cc: Kevin Hilman; davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx;
todd.fischer@xxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 2/6 v5] Support for TVP7002 in dm365 board

Sneha,


So, if I got it right, have_tvp7002 is unnecessary since that
initialization is done via the CPLD interface. Therefore, all I need to
do is actually remove the logic for the device from board-dm365-evm.h.
Am I right? If so, should I also remove the logic for tvp5146?


Santiago,

Yes, have_tvp7002 is not necessary. In fact evm_init_cpld() is also not necessary from video input source selection perspective. At this point, you can remove have_tvp7002() and keep the default as TVP5146. The evm_init_cpld() requires some cleanup for have_imager() as well. I am expecting Murali to provide patches for this sometime soon.

This brings up another concern. Your board setup related patches are dependent on one of the old patches from Neil Sikka - to add TVP5146 capture support. That patch is not in the mainline yet and requires re-submission by Murali.

I suggest you complete the TVP7002 sub-device driver (tvp7002.c and tvp7002.h), submit patches and follow up with the approval in the V4L2 community.

Patches corresponding to VPFE and DM365 board setup can be handled by Murali, working with you.

Thanks
Sneha

Regards,

Narnakaje, Snehaprabha wrote:
Santiago, Kevin,


-----Original Message-----
From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx]
Sent: Monday, October 26, 2009 5:35 PM
To: santiago.nunez@xxxxxxxxxxxx
Cc: Narnakaje, Snehaprabha; davinci-linux-open-
source@xxxxxxxxxxxxxxxxxxxx; todd.fischer@xxxxxxxxxxxx; linux-
media@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 2/6 v5] Support for TVP7002 in dm365 board

Santiago Nunez-Corrales <snunez@xxxxxxxxxxxx> writes:


Kevin Hilman wrote:

<santiago.nunez@xxxxxxxxxxxx> writes:



From: Santiago Nunez-Corrales <santiago.nunez@xxxxxxxxxxxx>

This patch provides support for TVP7002 in architecture definitions
within DM365.

Signed-off-by: Santiago Nunez-Corrales <santiago.nunez@xxxxxxxxxxxx>
---
 arch/arm/mach-davinci/board-dm365-evm.c |  170

++++++++++++++++++++++++++++++-

 1 files changed, 166 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
b/arch/arm/mach-
davinci/board-dm365-evm.c

index a1d5e7d..6c544d3 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -38,6 +38,11 @@
 #include <mach/common.h>
 #include <mach/mmc.h>
 #include <mach/nand.h>
+#include <mach/gpio.h>
+#include <linux/videodev2.h>
+#include <media/tvp514x.h>
+#include <media/tvp7002.h>
+#include <media/davinci/videohd.h>
   static inline int have_imager(void)
@@ -48,8 +53,11 @@ static inline int have_imager(void)
  static inline int have_tvp7002(void)
 {
-	/* REVISIT when it's supported, trigger via Kconfig */
+#ifdef CONFIG_VIDEO_TVP7002
+	return 1;
+#else
 	return 0;
+#endif


I've said this before, but I'll say it again.  I don't like the
#ifdef-on-Kconfig-option here.

Can you add a probe hook to the platform_data so that when the
tvp7002
is found it can call pdata->probe() which could then set a flag
for use by have_tvp7002().

This will have he same effect without the ifdef since if the driver
is not compiled in, its probe can never be triggered.

Kevin



Kevin,

I've been working on this particular implementation. This
board-dm365-evm.c is specific to the board, therefore I don't still
get the point of not having those values wired to the board file, but
I know it'd be nice to have the CPLD configuration triggered upon
TVP7002 detection. I see two options:

Having them in the board file is appropriate, what I object to is the
selection by Kconfig.  Run-time detection is always preferred when
possible.

We have this CPLD init API - evm_init_cpld() called from the
dm365_evm_init() function. The CPLD init API was trying to initialize the
CPLD, based on the default configuration. I believe David Brownell had
this placeholder for have_imager() and have_tvp7002() APIs since, we have
different CPLD settings for the imager, tvp7002 and tvp5146 for the video
input source.
1. Do the callback function inside pdata and initialize it at driver
load time (tvp7002_probe). Set tvp5146 as default and override when
driver loads (and restore when unloads).

This is the preferred option to me.

We can decide on the default video input source to be TVP5146. However
we do not need a new callback function. We already have the VPFE
.setup_input callback API dm365evm_setup_video_input() for the same
purpose. The VPFE .setup_input API is called when each of the decoders
(sub-devices) registered with VPFE capture driver. It is also called when
the application decides on an input source and switches between the input
sources.
So, TVP5146 remains as the default video input source, only until VPFE
probe is called, which registers the all decoders (sub-devices) defined in
the VPFE platform data. This also means that the last decoder in the VPFE
platform data remains active after boot-up. One can change the order in
the VPFE platform data if a particular input source needs to remain active
after boot-up. Note that application can always switch the input-source at
run time.
I quickly tried removing the have_imager() construct in the
evm_init_cpld() and here is the output -
Starting kernel ...

Uncompressing
Linux.....................................................................
..........................
........................................ done, booting the kernel.
Linux version 2.6.32-rc2-davinci1-dirty
...
CPU: Testing write buffer coherency: ok
DaVinci: 8 gpio irqs
NET: Registered protocol family 16
EVM: tvp5146 SD video input
bio: create slab <bio-0> at 0
SCSI subsystem initialized
...
vpfe_init
vpfe-capture: vpss clock vpss_master enabled
vpfe-capture vpfe-capture: v4l2 device registered
vpfe-capture vpfe-capture: video device registered
EVM: switch to tvp5146 SD video input
tvp514x 1-005d: tvp514x 1-005d decoder driver registered !!
vpfe-capture vpfe-capture: v4l2 sub device tvp5146 registered
EVM: switch to tvp7002 HD video input
tvp7002 1-005c: tvp7002 1-005c decoder driver registered !!
vpfe-capture vpfe-capture: v4l2 sub device tvp7002 registered
ths7353 1-002e: chip found @ 0x5c (DaVinci I2C adapter)
vpfe-capture vpfe-capture: v4l2 sub device ths7353 registered
...

As you can see, the default video input source is TVP5146 and it has
been video input source has been switched to TVP7002.
Thanks
Sneha


2. Add an entry to sysfs such that it can be user-configurable whether
to activate one of the other regardless of whether tvp5156 or tvp7002
are actually there (the only result would be fail to access the
device).

Why do you need sysfs options for switching?  Wouldn't building as
modules and loading/unloading the needed modules serve the same
purpose?

Remeber that the 'probe' isn't going to be called until the
platform_driver
is registered, and that will (usually) happen at module load time.


Sneha, do you have any suggestions on this one?

Kevin


--
Santiago Nunez-Corrales, Eng.
RidgeRun Engineering, LLC

Guayabos, Curridabat
San Jose, Costa Rica
+(506) 2271 1487
+(506) 8313 0536
http://www.ridgerun.com





--
Santiago Nunez-Corrales, Eng.
RidgeRun Engineering, LLC

Guayabos, Curridabat
San Jose, Costa Rica
+(506) 2271 1487
+(506) 8313 0536
http://www.ridgerun.com


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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux