RE: [PATCH - stable v3.2] omap3isp: ccdc: Fix crash in HS/VS interrupt handler
Hi Laurent, On Sun, Mar 11, 2012 at 17:20:08, Laurent Pinchart wrote: > The HS/VS interrupt handler needs to access the pipeline object. It > erronously tries to get it from the CCDC output video node, which isn't > necessarily included in the pipeline. This leads to a NULL pointer > dereference. > > Fix the bug by getting the pipeline object from the CCDC subdev entity. > > Reported-by: Gary Thomas > Signed-off-by: Laurent Pinchart > Acked-by: Sakari Ailus > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/video/omap3isp/ispccdc.c |3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > The patch fixes a v3.2 bug and has been included in v3.3-rc1. Could you please > add it to the stable v3.2 series ? AFAIK, sta...@kernel.org is down and the correct address is sta...@vger.kernel.org. Also, you need to include the upstream commit id in the commit message (See Documentation/stable_kernel_rules.txt) Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] davinci: vpif: add check for genuine interrupts in the isr
Hi Manju, On Wed, Jan 25, 2012 at 20:35:31, Hadli, Manjunath wrote: > add a condition to in the isr to check for interrupt ownership and "to" is misplaced here? > channel number to make sure we do not service wrong interrupts. > > Signed-off-by: Manjunath Hadli I think it will be nice to expand on the "why" this patch is required a little bit. What is the usage case where you can get wrong interrupts? What exactly happens if you service wrong interrupts? Explaining these in the commit message will help the maintainers take a call on the criticality of this patch (whether it should be queued in the current -rc cycle or not). Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4] davinci: da850: add build configuration for vpif drivers
Hi Manju, On Wed, Jan 25, 2012 at 20:35:34, Hadli, Manjunath wrote: > add build configuration for da850/omapl-138 for vpif > capture and display drivers. > > Signed-off-by: Manjunath Hadli > --- > drivers/media/video/davinci/Kconfig | 26 +- > drivers/media/video/davinci/Makefile |5 + > 2 files changed, 30 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/video/davinci/Kconfig > b/drivers/media/video/davinci/Kconfig > index 60a456e..a0b0fb3 100644 > --- a/drivers/media/video/davinci/Kconfig > +++ b/drivers/media/video/davinci/Kconfig > @@ -22,9 +22,33 @@ config CAPTURE_DAVINCI_DM646X_EVM > To compile this driver as a module, choose M here: the > module will be called vpif_capture. > > +config DISPLAY_DAVINCI_DA850_EVM > + tristate "DA850/OMAPL138 EVM Video Display" > + depends on DA850_UI_SD_VIDEO_PORT && VIDEO_DEV && MACH_DAVINCI_DA850_EVM > + select VIDEOBUF_DMA_CONTIG > + select VIDEO_DAVINCI_VPIF > + select VIDEO_ADV7343 > + select VIDEO_THS7303 Selecting these helper chips should be conditional on VIDEO_HELPER_CHIPS_AUTO. This is what I gather looking at existing Kconfig files like drivers/media/video/em28xx/Kconfig. This issue exists with existing code too, so that should be addressed separately. > + help > + Support for DA850/OMAP-L138/AM18xx based display device. > + > + To compile this driver as a module, choose M here: the > + module will be called vpif_display. > + > +config CAPTURE_DAVINCI_DA850_EVM > + tristate "DA850/OMAPL138 EVM Video Capture" > + depends on DA850_UI_SD_VIDEO_PORT && VIDEO_DEV && MACH_DAVINCI_DA850_EVM > + select VIDEOBUF_DMA_CONTIG > + select VIDEO_DAVINCI_VPIF > + help > + Support for DA850/OMAP-L138/AM18xx based capture device. > + > + To compile this driver as a module, choose M here: the > + module will be called vpif_capture. > + > config VIDEO_DAVINCI_VPIF > tristate "DaVinci VPIF Driver" > - depends on DISPLAY_DAVINCI_DM646X_EVM > + depends on DISPLAY_DAVINCI_DM646X_EVM || DISPLAY_DAVINCI_DA850_EVM > help > Support for DaVinci VPIF Driver. > > diff --git a/drivers/media/video/davinci/Makefile > b/drivers/media/video/davinci/Makefile > index ae7dafb..2c7cfb0 100644 > --- a/drivers/media/video/davinci/Makefile > +++ b/drivers/media/video/davinci/Makefile > @@ -10,6 +10,11 @@ obj-$(CONFIG_DISPLAY_DAVINCI_DM646X_EVM) += vpif_display.o > #DM646x EVM Capture driver > obj-$(CONFIG_CAPTURE_DAVINCI_DM646X_EVM) += vpif_capture.o > > +#DA850 EVM Display driver > +obj-$(CONFIG_DISPLAY_DAVINCI_DA850_EVM) += vpif_display.o > +#DA850 EVM Capture driver > +obj-$(CONFIG_CAPTURE_DAVINCI_DA850_EVM) += vpif_capture.o Repeating these lines in the makefile for every board that gets VPIF support seems certainly excessive. Instead why not convert the existing DM646x specific config variables to generic VPIF display and capture config variables and select these generic variables when someone chooses DA850 support. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v7 1/8] davinci: vpif: remove machine specific inclusion from driver
Hi Manju, On Wed, Dec 21, 2011 at 19:13:34, Hadli, Manjunath wrote: > remove machine specific inclusion from the driver which > comes in the way of platform code consolidation. I think it would be more readable to use the term "header file" here and in the headline. Just "machine specific inclusion" begs the question - "inclusion of what?" > currently was seen that these header inclusions were > not necessary. Sorry about nit-picking, but it is not good to talk in past tense in commit text. Past tense is natural for you to use since you write the text after making the changes, but for the reviewer it is not natural since he is seeing the commit text and the patch both at once. Also, usage of "currently" in above line is not necessary. It is assumed that commit text talks about current state of affairs. I would have made these changes myself after Mauro's ack, but.. > > Signed-off-by: Manjunath Hadli > Cc: Mauro Carvalho Chehab > Cc: LMML > --- > drivers/media/video/davinci/vpif.h |2 -- > drivers/media/video/davinci/vpif_display.c |2 -- > include/media/davinci/vpif_types.h |2 ++ > sound/soc/codecs/cq93vc.c |2 -- .. you clubbed this unrelated sound/soc/ change in this patch. First, the change is not related to VPIF in any way so it has no business being in this patch. Second, there is no way the sound/soc folks will have a look at this patch, so basically the change will end up bypassing the right maintainers if other reviewers fail to catch it. Please separate the change into another patch. You can just post the two patches alone copying the right maintainers in each instead of posting the entire series again. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v6 01/11] davinci: vpif: remove obsolete header file inclusion
Hi Manju, In the subject line, calling mach/dm646x.h obsolete is not right since as of this patch mach/dm646x.h still exists. On Thu, Dec 15, 2011 at 17:41:50, Hadli, Manjunath wrote: > remove inclusion of header files from vpif.h and vpif_dispaly.c > and add appropriate header file for building. The main purpose of the patch is to remove machine specific includes from driver files since that (among other things) comes in the way of platform code consolidation. This needs to come out in the description. Right now the description is just describing the change without answering the question - why? > > Signed-off-by: Manjunath Hadli > Cc: Mauro Carvalho Chehab > Cc: LMML > --- > drivers/media/video/davinci/vpif.h |2 +- > drivers/media/video/davinci/vpif_display.c |2 -- > 2 files changed, 1 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/video/davinci/vpif.h > b/drivers/media/video/davinci/vpif.h > index 25036cb..c96268a 100644 > --- a/drivers/media/video/davinci/vpif.h > +++ b/drivers/media/video/davinci/vpif.h > @@ -19,7 +19,7 @@ > #include > #include > #include It appears mach/hardware.h can be removed as well. Can you please check? > -#include > +#include I2C is actually needed by include/media/davinci/vpif_types.h so it should go there. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 0/5] ARM: davinci: re-arrange definitions to have a common davinci header
On Fri, Dec 02, 2011 at 11:59:49, Hadli, Manjunath wrote: > Sekhar, > > On Wed, Nov 30, 2011 at 17:07:21, Nori, Sekhar wrote: > > Hi Manju, > > > > On Thu, Nov 17, 2011 at 15:48:53, Hadli, Manjunath wrote: > > > Re-arrange definitions and remove unnecessary code so that we canx > > > > These are two different things and should be done in separate patches. > > Sergei has already pointed out couple of instances. > Ok, This is only subject for the cover letter and not individual patches. > The individual patches have separate modularized implementations. I will I am referring to the kind of issues Sergei pointed to here: http://linux.omap.com/pipermail/davinci-linux-open-source/2011-November/023524.html > change the cover letter subject to "remove private definitions from headers > and move to C files". Is that OK? > Current headline is fine by me. It doesn't become part of commit history anyway. > > > > > > have a common header for all davinci platforms. This will enable > > > > You mean all DMx platforms? DA8x and TNETVx will still have their own > > header files after this patch set. > > Yes, DMx platforms. I will also change the common "davinci.h" to dmx.h ? No, davinci.h is fine. > > > > > > us to share defines and enable common routines to be used without > > > polluting hardware.h. > > > This patch set forms the base for a later set of patches for having a > > > common system module base address (DAVINCI_SYSTEM_MODULE_BASE). > > > > > > Changes from previous version(As per Sergei's comments): > > > 1. Renamed davinci_common.h to davinci.h. > > > 2. Added extra line whereever appropriate. > > > 3. removed unnecessary header inclusion. > > > > > > Manjunath Hadli (5): > > > ARM: davinci: dm644x: remove the macros from the header to move to c > > > file > > > ARM: davinci: dm365: remove the macros from the header to move to c > > > file > > > ARM: davinci: dm646x: remove the macros from the header to move to c > > > file > > > > These headlines should describe the changes better. You are moving > > _private_ defines to C file (to reduce header file pollution). That should > > be clear from the headline. > > > > > ARM: davinci: create new common platform header for davinci > > > ARM: davinci: delete individual platform header files and use a > > > common header > > > > > > arch/arm/mach-davinci/board-dm355-evm.c |2 +- > > > arch/arm/mach-davinci/board-dm355-leopard.c |2 +- > > > arch/arm/mach-davinci/board-dm365-evm.c |2 +- > > > arch/arm/mach-davinci/board-dm644x-evm.c |2 +- > > > arch/arm/mach-davinci/board-dm646x-evm.c |2 +- > > > arch/arm/mach-davinci/board-neuros-osd2.c|2 +- > > > arch/arm/mach-davinci/board-sffsdr.c |2 +- > > > arch/arm/mach-davinci/dm355.c|2 +- > > > arch/arm/mach-davinci/dm365.c| 18 +- > > > arch/arm/mach-davinci/dm644x.c |9 +++- > > > arch/arm/mach-davinci/dm646x.c |9 +++- > > > arch/arm/mach-davinci/include/mach/davinci.h | 88 > > > ++ > > > > This file should be placed in arch/arm/mach-davinci itself since the > > definitions are private to arch/arm/mach-davinci. > > Russell has been complaining about placing unnecessary files in > > include/mach. > > I will just check if the file is needed from the main driver files. > If not, I will move it to mach-davinci. Driver files should not need to see machine private stuff. If that's the case, drivers will probably need some clean-up too. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] board-dm646x-evm.c: wrong register used in setup_vpif_input_channel_mode
Hi Hans, On Mon, Nov 14, 2011 at 23:50:49, Hans Verkuil wrote: > From: Hans Verkuil > > The function setup_vpif_input_channel_mode() used the VSCLKDIS register > instead of VIDCLKCTL. This meant that when in HD mode videoport channel 0 > used a different clock from channel 1. > > Clearly a copy-and-paste error. > > Signed-off-by: Hans Verkuil Queuing this for v3.2-rc. I changed the headline to match current convention Being used in ARM: ARM: davinci: dm646x evm: wrong register used in setup_vpif_input_channel_mode Also, the code in question has not changed for a long time, so added the stable tag. Regards, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 0/5] ARM: davinci: re-arrange definitions to have a common davinci header
Hi Manju, On Thu, Nov 17, 2011 at 15:48:53, Hadli, Manjunath wrote: > Re-arrange definitions and remove unnecessary code so that we canx These are two different things and should be done in separate patches. Sergei has already pointed out couple of instances. > have a common header for all davinci platforms. This will enable You mean all DMx platforms? DA8x and TNETVx will still have their own header files after this patch set. > us to share defines and enable common routines to be used without > polluting hardware.h. > This patch set forms the base for a later set of patches for having > a common system module base address (DAVINCI_SYSTEM_MODULE_BASE). > > Changes from previous version(As per Sergei's comments): > 1. Renamed davinci_common.h to davinci.h. > 2. Added extra line whereever appropriate. > 3. removed unnecessary header inclusion. > > Manjunath Hadli (5): > ARM: davinci: dm644x: remove the macros from the header to move to c > file > ARM: davinci: dm365: remove the macros from the header to move to c > file > ARM: davinci: dm646x: remove the macros from the header to move to c > file These headlines should describe the changes better. You are moving _private_ defines to C file (to reduce header file pollution). That should be clear from the headline. > ARM: davinci: create new common platform header for davinci > ARM: davinci: delete individual platform header files and use a > common header > > arch/arm/mach-davinci/board-dm355-evm.c |2 +- > arch/arm/mach-davinci/board-dm355-leopard.c |2 +- > arch/arm/mach-davinci/board-dm365-evm.c |2 +- > arch/arm/mach-davinci/board-dm644x-evm.c |2 +- > arch/arm/mach-davinci/board-dm646x-evm.c |2 +- > arch/arm/mach-davinci/board-neuros-osd2.c|2 +- > arch/arm/mach-davinci/board-sffsdr.c |2 +- > arch/arm/mach-davinci/dm355.c|2 +- > arch/arm/mach-davinci/dm365.c| 18 +- > arch/arm/mach-davinci/dm644x.c |9 +++- > arch/arm/mach-davinci/dm646x.c |9 +++- > arch/arm/mach-davinci/include/mach/davinci.h | 88 > ++ This file should be placed in arch/arm/mach-davinci itself since the definitions are private to arch/arm/mach-davinci. Russell has been complaining about placing unnecessary files in include/mach. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RESEND] davinci: dm646x: move vpif related code to driver core header from platform
On Fri, Nov 25, 2011 at 01:36:01, Mauro Carvalho Chehab wrote: > Em 24-11-2011 16:22, Nori, Sekhar escreveu: > > Hi Mauro, > > > > On Thu, Nov 24, 2011 at 23:35:24, Mauro Carvalho Chehab wrote: > >> Em 12-11-2011 13:06, Manjunath Hadli escreveu: > >>> move vpif related code for capture and display drivers > >>> from dm646x platform header file to vpif_types.h as these definitions > >>> are related to driver code more than the platform or board. > >>> > >>> Signed-off-by: Manjunath Hadli > >> > >> Manju, > >> > >> Why are you re-sending a patch? > >> > >> My understanding is that you're maintaining the davinci patches, so it is > >> up to you to put those patches on your tree and send me a pull request when > >> they're done. So, please, don't pollute the ML re-sending emails that > >> are for yourself to handle. > > > > Since this particular patch touches arch/arm/mach-davinci > > as well as drivers/media/video, the plan was to queue the > > patch through ARM tree with your Ack. We did not get your > > ack the last time around[1] so it was resent. > > > > Do let me know if your ack is not needed. > > > > Thanks, > > Sekhar > > > > [1] > > http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg21840.html > > Hmm.. I missed this email, but just re-sending it without request my ACK > doesn't help > much ;) > > If this ever happens again, next time the better is to forward me the patch > again, on > an email asking for my ack. > > With regards to the patch: > > Acked-by: Mauro Carvalho Chehab Thanks Mauro. Queuing this for v3.3 submission. Manju, while committing I changed the subject line to: "ARM: davinci: vpif: move code to driver core header from platform" to better match the current subject line conventions. Regards, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RESEND] davinci: dm646x: move vpif related code to driver core header from platform
Hi Mauro, On Thu, Nov 24, 2011 at 23:35:24, Mauro Carvalho Chehab wrote: > Em 12-11-2011 13:06, Manjunath Hadli escreveu: > > move vpif related code for capture and display drivers > > from dm646x platform header file to vpif_types.h as these definitions > > are related to driver code more than the platform or board. > > > > Signed-off-by: Manjunath Hadli > > Manju, > > Why are you re-sending a patch? > > My understanding is that you're maintaining the davinci patches, so it is > up to you to put those patches on your tree and send me a pull request when > they're done. So, please, don't pollute the ML re-sending emails that > are for yourself to handle. Since this particular patch touches arch/arm/mach-davinci as well as drivers/media/video, the plan was to queue the patch through ARM tree with your Ack. We did not get your ack the last time around[1] so it was resent. Do let me know if your ack is not needed. Thanks, Sekhar [1] http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg21840.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] davinci: dm646x: move vpif related code to driver core header from platform
Manju, On Fri, May 20, 2011 at 19:28:49, Hadli, Manjunath wrote: > move vpif related code for capture and display drivers > from dm646x platform header file to vpif.h as these definitions > are related to driver code more than the platform or board. > > Signed-off-by: Manjunath Hadli Can you rebase this patch to latest on my tree and repost this time CCing Mauro? Lets try and get his ack for the v3.2 merge. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] davinci: dm646x: move vpif related code to driver core header from platform
Hi Mauro, On Wed, Jun 22, 2011 at 09:35:58, Nori, Sekhar wrote: > On Thu, Jun 02, 2011 at 22:51:58, Nori, Sekhar wrote: > > Hi Mauro, > > > > On Fri, May 20, 2011 at 19:28:49, Hadli, Manjunath wrote: > > > move vpif related code for capture and display drivers > > > from dm646x platform header file to vpif.h as these definitions > > > are related to driver code more than the platform or board. > > > > > > Signed-off-by: Manjunath Hadli > > > > Will you be taking this patch through your tree? > > > > If not, with your ack, I can queue it for inclusion > > through the ARM tree. > > > > Ping :) Can you please provide your ack so that I can queue this for v3.1? Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] davinci: dm646x: move vpif related code to driver core header from platform
On Thu, Jun 02, 2011 at 22:51:58, Nori, Sekhar wrote: > Hi Mauro, > > On Fri, May 20, 2011 at 19:28:49, Hadli, Manjunath wrote: > > move vpif related code for capture and display drivers > > from dm646x platform header file to vpif.h as these definitions > > are related to driver code more than the platform or board. > > > > Signed-off-by: Manjunath Hadli > > Will you be taking this patch through your tree? > > If not, with your ack, I can queue it for inclusion > through the ARM tree. > Ping :) Thanks, Sekhar > > --- > > arch/arm/mach-davinci/include/mach/dm646x.h | 53 +--- > > drivers/media/video/davinci/vpif.h |1 + > > drivers/media/video/davinci/vpif_capture.h |2 +- > > drivers/media/video/davinci/vpif_display.h |1 + > > include/media/davinci/vpif.h| 73 > > +++ > > 5 files changed, 77 insertions(+), 53 deletions(-) > > create mode 100644 include/media/davinci/vpif.h -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] davinci: dm646x: move vpif related code to driver core header from platform
Hi Mauro,
On Fri, May 20, 2011 at 19:28:49, Hadli, Manjunath wrote:
> move vpif related code for capture and display drivers
> from dm646x platform header file to vpif.h as these definitions
> are related to driver code more than the platform or board.
>
> Signed-off-by: Manjunath Hadli
Will you be taking this patch through your tree?
If not, with your ack, I can queue it for inclusion
through the ARM tree.
Thanks,
Sekhar
> ---
> arch/arm/mach-davinci/include/mach/dm646x.h | 53 +---
> drivers/media/video/davinci/vpif.h |1 +
> drivers/media/video/davinci/vpif_capture.h |2 +-
> drivers/media/video/davinci/vpif_display.h |1 +
> include/media/davinci/vpif.h| 73
> +++
> 5 files changed, 77 insertions(+), 53 deletions(-)
> create mode 100644 include/media/davinci/vpif.h
>
> diff --git a/arch/arm/mach-davinci/include/mach/dm646x.h
> b/arch/arm/mach-davinci/include/mach/dm646x.h
> index 7a27f3f..245a1c0 100644
> --- a/arch/arm/mach-davinci/include/mach/dm646x.h
> +++ b/arch/arm/mach-davinci/include/mach/dm646x.h
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
>
> #define DM646X_EMAC_BASE (0x01C8)
> #define DM646X_EMAC_MDIO_BASE(DM646X_EMAC_BASE + 0x4000)
> @@ -36,58 +37,6 @@ int __init dm646x_init_edma(struct edma_rsv_info *rsv);
>
> void dm646x_video_init(void);
>
> -enum vpif_if_type {
> - VPIF_IF_BT656,
> - VPIF_IF_BT1120,
> - VPIF_IF_RAW_BAYER
> -};
> -
> -struct vpif_interface {
> - enum vpif_if_type if_type;
> - unsigned hd_pol:1;
> - unsigned vd_pol:1;
> - unsigned fid_pol:1;
> -};
> -
> -struct vpif_subdev_info {
> - const char *name;
> - struct i2c_board_info board_info;
> - u32 input;
> - u32 output;
> - unsigned can_route:1;
> - struct vpif_interface vpif_if;
> -};
> -
> -struct vpif_display_config {
> - int (*set_clock)(int, int);
> - struct vpif_subdev_info *subdevinfo;
> - int subdev_count;
> - const char **output;
> - int output_count;
> - const char *card_name;
> -};
> -
> -struct vpif_input {
> - struct v4l2_input input;
> - const char *subdev_name;
> -};
> -
> -#define VPIF_CAPTURE_MAX_CHANNELS2
> -
> -struct vpif_capture_chan_config {
> - const struct vpif_input *inputs;
> - int input_count;
> -};
> -
> -struct vpif_capture_config {
> - int (*setup_input_channel_mode)(int);
> - int (*setup_input_path)(int, const char *);
> - struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
> - struct vpif_subdev_info *subdev_info;
> - int subdev_count;
> - const char *card_name;
> -};
> -
> void dm646x_setup_vpif(struct vpif_display_config *,
> struct vpif_capture_config *);
>
> diff --git a/drivers/media/video/davinci/vpif.h
> b/drivers/media/video/davinci/vpif.h
> index 10550bd..e76dded 100644
> --- a/drivers/media/video/davinci/vpif.h
> +++ b/drivers/media/video/davinci/vpif.h
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> /* Maximum channel allowed */
> #define VPIF_NUM_CHANNELS(4)
> diff --git a/drivers/media/video/davinci/vpif_capture.h
> b/drivers/media/video/davinci/vpif_capture.h
> index 7a4196d..fa50b6b 100644
> --- a/drivers/media/video/davinci/vpif_capture.h
> +++ b/drivers/media/video/davinci/vpif_capture.h
> @@ -28,7 +28,7 @@
> #include
> #include
> #include
> -#include
> +#include
>
> #include "vpif.h"
>
> diff --git a/drivers/media/video/davinci/vpif_display.h
> b/drivers/media/video/davinci/vpif_display.h
> index b53aaa8..b531a01 100644
> --- a/drivers/media/video/davinci/vpif_display.h
> +++ b/drivers/media/video/davinci/vpif_display.h
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
>
> #include "vpif.h"
>
> diff --git a/include/media/davinci/vpif.h b/include/media/davinci/vpif.h
> new file mode 100644
> index 000..e4a4dc1
> --- /dev/null
> +++ b/include/media/davinci/vpif.h
> @@ -0,0 +1,73 @@
> +/*
> + * Copyright (C) 2011 Texas Instruments Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation version 2.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +#ifndef _VPIF_INC_H
> +#define _VPIF_INC_H
> +
> +#include
> +
> +#define VPIF_CAPTURE_MAX_CHANNELS2
> +
> +enum vpif_if_type {
> + VPIF_IF_B
RE: [PATCH 1/1] davinci: dm646x: move vpif related code to driver core header from platform
On Fri, May 20, 2011 at 19:28:49, Hadli, Manjunath wrote: > move vpif related code for capture and display drivers > from dm646x platform header file to vpif.h as these definitions > are related to driver code more than the platform or board. > > Signed-off-by: Manjunath Hadli > diff --git a/drivers/media/video/davinci/vpif.h > b/drivers/media/video/davinci/vpif.h > index 10550bd..e76dded 100644 > --- a/drivers/media/video/davinci/vpif.h > +++ b/drivers/media/video/davinci/vpif.h > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include mach/hardware.h and mach/dm646x.h can now be dropped. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v18 07/13] davinci: move DM64XX_VDD3P3V_PWDN to devices.c
On Sat, Apr 02, 2011 at 15:13:00, Hadli, Manjunath wrote:
> Move the definition of DM64XX_VDD3P3V_PWDN from hardware.h
> to devices.c since it is used only there.
>
> Signed-off-by: Manjunath Hadli
> Acked-by: Sekhar Nori
Applying this after updating the patch description to
point out that this also helps rid hardware.h of platform
private stuff..
> ---
> arch/arm/mach-davinci/devices.c |1 +
> arch/arm/mach-davinci/include/mach/hardware.h |3 ---
> 2 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
> index 22ebc64..4e1b663 100644
> --- a/arch/arm/mach-davinci/devices.c
> +++ b/arch/arm/mach-davinci/devices.c
> @@ -182,6 +182,7 @@ static struct platform_device davinci_mmcsd1_device = {
> .resource = mmcsd1_resources,
> };
>
> +#define DM64XX_VDD3P3V_PWDN 0x48
.. and moving this to the top of the file.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 5/6] davinci vpbe: Build infrastructure for VPBE driver
On Fri, May 20, 2011 at 20:02:08, Sergei Shtylyov wrote: > Hello. > > Manjunath Hadli wrote: > > > This patch adds the build infra-structure for Davinci > > VPBE dislay driver. > > > Signed-off-by: Manjunath Hadli > > Acked-by: Muralidharan Karicheri > > Acked-by: Hans Verkuil > [...] > > > diff --git a/drivers/media/video/davinci/Kconfig > > b/drivers/media/video/davinci/Kconfig > > index 6b19540..a7f11e7 100644 > > --- a/drivers/media/video/davinci/Kconfig > > +++ b/drivers/media/video/davinci/Kconfig > > @@ -91,3 +91,25 @@ config VIDEO_ISIF > > > >To compile this driver as a module, choose M here: the > >module will be called vpfe. > > + > > +config VIDEO_DM644X_VPBE > > + tristate "DM644X VPBE HW module" > > BTW, as this seems DM644x specific, shouldn't this depend on > CONFIG_ARCH_DAVINCI_DM644x? Since VENC/OSD etc are also applicable to other DaVinci devices, this KConfig entry should probably be split to refer to them individually and in a generic way. "depends on" can then be used to make sure only the relevant ones show up. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v18 08/13] davinci: eliminate use of IO_ADDRESS() on sysmod
Hi Manju, On Sat, Apr 02, 2011 at 15:13:17, Hadli, Manjunath wrote: > Current devices.c file has a number of instances where > IO_ADDRESS() is used for system module register > access. Eliminate this in favor of a ioremap() > based access. > > Consequent to this, a new global pointer davinci_sysmodbase > has been introduced which gets initialized during > the initialization of each relevant SoC > > Signed-off-by: Manjunath Hadli > Acked-by: Sekhar Nori > --- > diff --git a/arch/arm/mach-davinci/include/mach/hardware.h > b/arch/arm/mach-davinci/include/mach/hardware.h > index 414e0b9..2a6b560 100644 > --- a/arch/arm/mach-davinci/include/mach/hardware.h > +++ b/arch/arm/mach-davinci/include/mach/hardware.h > @@ -21,6 +21,12 @@ > */ > #define DAVINCI_SYSTEM_MODULE_BASE0x01C4 > > +#ifndef __ASSEMBLER__ > +extern void __iomem *davinci_sysmodbase; > +#define DAVINCI_SYSMODULE_VIRT(x)(davinci_sysmodbase + (x)) > +void davinci_map_sysmod(void); > +#endif Russell has posted[1] that the hardware.h file should not be polluted with platform private stuff like this. Your patch 7/13 actually helped towards that goal, but this one takes us back. This patch cannot be used in the current form. Currently there are separate header files for dm644x, dm355, dm646x and dm365. I would like to start by removing unnecessary code from these files and trying to consolidate them into a single file. Example, the EMAC base address definitions in dm365.h should be moved into dm365.c. Similarly, there is a lot of VPIF specific stuff in dm646x.h which is not really specific to dm646x.h and so should probably be moved to include/media/ or arch/arm/mach-davinci/include/mach/vpif.h Once consolidated into a single file, davinci_sysmodbase can be moved into that file. Also, Russell has said[2] that at least for this merge window only consolidation and bug fixes will go through his tree. This means that as far as mach-davinci is concerned, the clean-up part of this series can go to 2.6.40 - but not the stuff which adds new support. Thanks, Sekhar [1] http://www.spinics.net/lists/arm-kernel/msg120410.html [2] http://www.spinics.net/lists/arm-kernel/msg120606.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 08/13] davinci: eliminate use of IO_ADDRESS() on sysmod
On Tue, Mar 22, 2011 at 18:45:03, Arnd Bergmann wrote: > On Tuesday 22 March 2011, Nori, Sekhar wrote: > > .. but forgot to fix this. There is nothing wrong with > > using writel, but it doesn't fit into what the subject > > of this patch is. > > Well, to be more exact, the __raw_writel was actually > wrong here and it should be writel(), but it's certainly > better to mention the reason in the changelog, or to > make a separate patch for it. I wouldn't necessarily classify the use of __raw_writel as wrong (as in a bug) - since the specific code is question is only run on ARMv5 based SoCs. A lot of DaVinci code uses __raw_* variants, and perhaps should be changed to use readl/writel instead - at least it will help copying that code over for other uses. That should be the subject of separate (series of) patches. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 12/13] davinci: dm644x: add support for v4l2 video display
On Tue, Mar 15, 2011 at 19:29:40, Hadli, Manjunath wrote:
> Create platform devices for various video modules like venc,osd,
> vpbe and v4l2 driver for dm644x.
>
> Signed-off-by: Manjunath Hadli
> ---
> +struct venc_platform_data dm644x_venc_pdata = {
> + .venc_type = VPBE_VERSION_1,
> + .setup_clock= dm644x_venc_setup_clock,
> +};
Sparse pointed out that this symbol can
be static.
Can you please build the complete series with
C=1 on the command line? This will enable
sparse check on all files being re-compiled.
I also noticed some sparse warnings on vpbe_venc.c
Also, please build with CONFIG_DEBUG_SECTION_MISMATCH=y
on the command line. I noticed some section
mismatches as well with the new code.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 00/13] davinci vpbe: dm6446 v4l2 driver
Manju, On Tue, Mar 22, 2011 at 12:23:14, Hadli, Manjunath wrote: > Sekhar, Kevin, > These patches have gone through considerable reviews. > Could you please ACK from your end? I have some minor comments which I have already posted and once you fix those you can add: Acked-by: Sekhar Nori to the platform patches. > > On Tue, Mar 15, 2011 at 19:26:28, Hadli, Manjunath wrote: > > version17: > > The more important among the patch history from previous comments 1. > > Replacing _raw_readl() with readl(). This is not valid. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 13/13] davinci: dm644x EVM: add support for VPBE display
On Tue, Mar 15, 2011 at 19:29:55, Hadli, Manjunath wrote: > This patch adds support for V4L2 video display to DM6446 EVM. > Support for SD and ED modes is provided, along with Composite > and Component outputs. > > Signed-off-by: Manjunath Hadli > --- > arch/arm/mach-davinci/board-dm644x-evm.c| 108 > ++- > arch/arm/mach-davinci/dm644x.c |4 +- > arch/arm/mach-davinci/include/mach/dm644x.h |3 +- The changes adding the SoC support should be folded into patch 12/13. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 10/13] davinci: dm644x: change vpfe capture structure variables for consistency
On Tue, Mar 15, 2011 at 19:29:12, Hadli, Manjunath wrote: > change the vpfe capture related configuration structure variables from > to dm644xevm_ to make it consistent with the rest of > the file. This description is not fully accurate. You also have changes where you add SoC prefix to variable names. I guess you can just say "Add SoC and board prefixes to variable names so that.." Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v17 08/13] davinci: eliminate use of IO_ADDRESS() on sysmod
On Tue, Mar 15, 2011 at 19:28:43, Hadli, Manjunath wrote:
> Current devices.c file has a number of instances where
> IO_ADDRESS() is used for system module register
> access. Eliminate this in favor of a ioremap()
> based access.
>
> Consequent to this, a new global pointer davinci_sysmodbase
> has been introduced which gets initialized during
> the initialization of each relevant SoC.
>
> In this patch davinci_sysmodbase is used by davinci_setup_mmc
> but the later patches in the series use the same in different
> places using DAVINCI_SYSMODULE_VIRT.This patch lays the
> foundation for that.
>
> Signed-off-by: Manjunath Hadli
> ---
> arch/arm/mach-davinci/devices.c | 23 ++-
> arch/arm/mach-davinci/dm355.c |1 +
> arch/arm/mach-davinci/dm365.c |1 +
> arch/arm/mach-davinci/dm644x.c|1 +
> arch/arm/mach-davinci/dm646x.c|1 +
> arch/arm/mach-davinci/include/mach/hardware.h |6 ++
> 6 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
> index d3b2040..b7ef950 100644
> --- a/arch/arm/mach-davinci/devices.c
> +++ b/arch/arm/mach-davinci/devices.c
> @@ -33,6 +33,14 @@
> #define DM365_MMCSD0_BASE 0x01D11000
> #define DM365_MMCSD1_BASE 0x01D0
>
> +void __iomem *davinci_sysmodbase;
> +
> +void davinci_map_sysmod(void)
> +{
> + davinci_sysmodbase = ioremap_nocache(DAVINCI_SYSTEM_MODULE_BASE, 0x800);
> + WARN_ON(!davinci_sysmodbase);
> +}
> +
> static struct resource i2c_resources[] = {
> {
> .start = DAVINCI_I2C_BASE,
> @@ -210,12 +218,12 @@ void __init davinci_setup_mmc(int module, struct
> davinci_mmc_config *config)
> davinci_cfg_reg(DM355_SD1_DATA2);
> davinci_cfg_reg(DM355_SD1_DATA3);
> } else if (cpu_is_davinci_dm365()) {
> - void __iomem *pupdctl1 =
> - IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE + 0x7c);
> -
> /* Configure pull down control */
> - __raw_writel((__raw_readl(pupdctl1) & ~0xfc0),
> - pupdctl1);
> + void __iomem *pupdctl1 = DAVINCI_SYSMODULE_VIRT(0x7c);
> + unsigned v;
> +
> + v = __raw_readl(pupdctl1);
> + __raw_writel(v & ~0xfc0, pupdctl1);
You fixed this as Sergei requested...
>
> mmcsd1_resources[0].start = DM365_MMCSD1_BASE;
> mmcsd1_resources[0].end = DM365_MMCSD1_BASE +
> @@ -244,11 +252,8 @@ void __init davinci_setup_mmc(int module, struct
> davinci_mmc_config *config)
> mmcsd0_resources[2].start = IRQ_DM365_SDIOINT0;
> } else if (cpu_is_davinci_dm644x()) {
> /* REVISIT: should this be in board-init code? */
> - void __iomem *base =
> - IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE);
> -
> /* Power-on 3.3V IO cells */
> - __raw_writel(0, base + DM64XX_VDD3P3V_PWDN);
> + writel(0, DAVINCI_SYSMODULE_VIRT(DM64XX_VDD3P3V_PWDN));
.. but forgot to fix this. There is nothing wrong with
using writel, but it doesn't fit into what the subject
of this patch is.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/7] davinci: eliminate use of IO_ADDRESS() on sysmod
Hi Arnd, On Mon, Mar 14, 2011 at 21:51:51, Arnd Bergmann wrote: > On Monday 14 March 2011, Manjunath Hadli wrote: > > Current devices.c file has a number of instances where > > IO_ADDRESS() is used for system module register > > access. Eliminate this in favor of a ioremap() > > based access. > > > > Consequent to this, a new global pointer davinci_sysmodbase > > has been introduced which gets initialized during > > the initialization of each relevant SoC > > > > Signed-off-by: Manjunath Hadli > > The change looks good, it's definitely a step in the right > direction. > > Acked-by: Arnd Bergmann > > > I think you can go even further: > > * A straightforward change would be to move davinci_sysmodbase > into a local variable of the davinci_setup_mmc function, > which I believe is the only user. Then you can ioremap > and iounmap it directly there. This patch accesses sysmodule only in davinci_setup_mmc, but follow-on patches use it in other places. So, this patch sort of lays the foundation for that. This is not really evident in this patch so the patch description should have captured that. > * If you need to access sysmod in multiple places, a nicer > way would be to make the virtual address pointer static, > and export the accessor functions for it, rather than > having a global pointer. Seems like opinion is divided on this. A while back I submitted a patch with such an accessor function and was asked to do the opposite of what you are asking here. https://patchwork.kernel.org/patch/366501/ It can be changed to the way you are asking, but would like to know what is more universally acceptable (if at all there is such a thing). Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v16 1/3] davinci vpbe: changes to common files
Hi Manju,
You have got a wrong address for linux-arm-kernel ML.
The right address is: linux-arm-ker...@lists.infradead.org
Also, I think you need to subscribe to this list for your
messages to get posted automatically. Subscription information
is available here: http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
You can check that your patches are actually reaching ARM linux
mailing list by checking the archives here: http://marc.info/?l=linux-arm-kernel
On Tue, Jan 18, 2011 at 19:09:07, Hadli, Manjunath wrote:
> Implemented a common and single mapping for DAVINCI_SYSTEM_MODULE_BASE
> to be used by all davinci platforms.
>
> Signed-off-by: Manjunath Hadli
> Acked-by: Muralidharan Karicheri
> Acked-by: Hans Verkuil
> ---
> arch/arm/mach-davinci/common.c|4 +++-
> arch/arm/mach-davinci/devices.c | 10 --
> arch/arm/mach-davinci/include/mach/hardware.h |5 +
> 3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/common.c b/arch/arm/mach-davinci/common.c
> index 1d25573..949e615 100644
> --- a/arch/arm/mach-davinci/common.c
> +++ b/arch/arm/mach-davinci/common.c
> @@ -111,7 +111,9 @@ void __init davinci_common_init(struct davinci_soc_info
> *soc_info)
> if (ret != 0)
> goto err;
> }
> -
> + davinci_sysmodbase = ioremap_nocache(DAVINCI_SYSTEM_MODULE_BASE, 0x800);
> + if (!davinci_sysmodbase)
> + goto err;
This is actually not the right place to do this. davinci_common_init()
is called for all 7 supported SoCs. This system module base address
definitely not valid on the two DA8x SoCs. I suspect it is not valid on
TNETV as well. That makes this call unnecessary on 3 of the 7 supported
SoCs. I think the original approach of mapping it for each SoC that needed
it was fine.
> return;
>
> err:
> diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
> index 22ebc64..2bff2d6 100644
> --- a/arch/arm/mach-davinci/devices.c
> +++ b/arch/arm/mach-davinci/devices.c
> @@ -33,6 +33,8 @@
> #define DM365_MMCSD0_BASE 0x01D11000
> #define DM365_MMCSD1_BASE 0x01D0
>
> +void __iomem *davinci_sysmodbase;
> +
> static struct resource i2c_resources[] = {
> {
> .start = DAVINCI_I2C_BASE,
> @@ -209,9 +211,7 @@ void __init davinci_setup_mmc(int module, struct
> davinci_mmc_config *config)
> davinci_cfg_reg(DM355_SD1_DATA2);
> davinci_cfg_reg(DM355_SD1_DATA3);
> } else if (cpu_is_davinci_dm365()) {
> - void __iomem *pupdctl1 =
> - IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE + 0x7c);
> -
> + void __iomem *pupdctl1 = DAVINCI_SYSMODULE_VIRT(0x7c);
> /* Configure pull down control */
> __raw_writel((__raw_readl(pupdctl1) & ~0xfc0),
> pupdctl1);
> @@ -243,9 +243,7 @@ void __init davinci_setup_mmc(int module, struct
> davinci_mmc_config *config)
> mmcsd0_resources[2].start = IRQ_DM365_SDIOINT0;
> } else if (cpu_is_davinci_dm644x()) {
> /* REVISIT: should this be in board-init code? */
> - void __iomem *base =
> - IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE);
> -
> + void __iomem *base = DAVINCI_SYSMODULE_VIRT(0);
Please use DAVINCI_SYSMODULE_VIRT(DM64XX_VDD3P3V_PWDN) instead.
> /* Power-on 3.3V IO cells */
> __raw_writel(0, base + DM64XX_VDD3P3V_PWDN);
> /*Set up the pull regiter for MMC */
> diff --git a/arch/arm/mach-davinci/include/mach/hardware.h
> b/arch/arm/mach-davinci/include/mach/hardware.h
> index c45ba1f..5a105c4 100644
> --- a/arch/arm/mach-davinci/include/mach/hardware.h
> +++ b/arch/arm/mach-davinci/include/mach/hardware.h
> @@ -24,6 +24,11 @@
> /* System control register offsets */
> #define DM64XX_VDD3P3V_PWDN 0x48
>
> +#ifndef __ASSEMBLER__
> + extern void __iomem *davinci_sysmodbase;
> + #define DAVINCI_SYSMODULE_VIRT(x) (davinci_sysmodbase+(x))
Indenting the #defines is not required.
Also, this will need to be placed in individual .h file. The currently
defined DAVINCI_SYSTEM_MODULE_BASE and DM64XX_VDD3P3V_PWDN also violate the
guidance provided in comments just before those defines. They should be
moved to .h files too.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/8] davinci vpbe: platform specific additions
Hi Manju,
Please CC linux-arm-ker...@lists.infradead.org for mach-davinci
patches.
On Mon, Jan 10, 2011 at 18:57:37, Hadli, Manjunath wrote:
> This patch implements the overall device creation for the Video
> display driver.
>
> Signed-off-by: Manjunath Hadli
> Acked-by: Muralidharan Karicheri
> Acked-by: Hans Verkuil
> ---
> arch/arm/mach-davinci/dm644x.c | 172
> +--
> arch/arm/mach-davinci/include/mach/dm644x.h | 13 ++-
> 2 files changed, 172 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
> index 9a2376b..f2d24fb 100644
> --- a/arch/arm/mach-davinci/dm644x.c
> +++ b/arch/arm/mach-davinci/dm644x.c
> @@ -5,7 +5,7 @@
> *
> * 2007 (c) Deep Root Systems, LLC. This file is licensed under
> * the terms of the GNU General Public License version 2. This program
> - * is licensed "as is" without any warranty of any kind, whether express
> + * is licensed without any warranty of any kind, whether express
Please don't change the license text of existing licenses.
> * or implied.
> */
> #include
> @@ -590,8 +590,8 @@ static struct resource dm644x_vpss_resources[] = {
> {
> /* VPSS Base address */
> .name = "vpss",
> - .start = 0x01c73400,
> - .end= 0x01c73400 + 0xff,
> + .start = DM644X_VPSS_REG_BASE,
> + .end= DM644X_VPSS_REG_BASE + 0xff,
> .flags = IORESOURCE_MEM,
> },
> };
> @@ -618,6 +618,7 @@ static struct resource vpfe_resources[] = {
> };
>
> static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32);
> +
Random new line?
> static struct resource dm644x_ccdc_resource[] = {
> /* CCDC Base address */
> {
> @@ -654,6 +655,138 @@ void dm644x_set_vpfe_config(struct vpfe_config *cfg)
> vpfe_capture_dev.dev.platform_data = cfg;
> }
>
> +static struct resource dm644x_osd_resources[] = {
> + {
> + .start = DM644X_OSD_REG_BASE,
> + .end= DM644X_OSD_REG_BASE + 0x1ff,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +
> +static u64 dm644x_osd_dma_mask = DMA_BIT_MASK(32);
> +
> +static struct osd_platform_data osd_data = {
> + .vpbe_type = DM644X_VPBE,
> + .field_inv_wa_enable = 0,
No need of zero initialization.
> +};
> +
> +static struct platform_device dm644x_osd_dev = {
> + .name = VPBE_OSD_SUBDEV_NAME,
> + .id = -1,
> + .num_resources = ARRAY_SIZE(dm644x_osd_resources),
> + .resource = dm644x_osd_resources,
> + .dev = {
> + .dma_mask = &dm644x_osd_dma_mask,
> + .coherent_dma_mask = DMA_BIT_MASK(32),
> + .platform_data = &osd_data,
> + },
> +};
> +
> +static struct resource dm644x_venc_resources[] = {
> + /* venc registers io space */
> + {
> + .start = DM644X_VENC_REG_BASE,
> + .end= DM644X_VENC_REG_BASE + 0x17f,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +
> +static u64 dm644x_venc_dma_mask = DMA_BIT_MASK(32);
> +
> +#define VPSS_CLKCTL 0x01C40044
There is already a DAVINCI_SYSTEM_MODULE_BASE defined. This
should be defined as an offset from that base.
> +
> +static void __iomem *vpss_clkctl_reg;
> +
> +static int dm644x_venc_setup_clock(enum vpbe_enc_timings_type type, __u64
> mode)
> +{
> + int ret = 0;
> +
> + if (NULL == vpss_clkctl_reg)
> + return -EINVAL;
> + switch (type) {
> + case VPBE_ENC_STD:
> + writel(0x18, vpss_clkctl_reg);
> + break;
> + case VPBE_ENC_DV_PRESET:
> + switch ((unsigned int)mode) {
> + case V4L2_DV_480P59_94:
> + case V4L2_DV_576P50:
> + writel(0x19, vpss_clkctl_reg);
Additional space in indentation.
> + break;
> + case V4L2_DV_720P60:
> + case V4L2_DV_1080I60:
> + case V4L2_DV_1080P30:
> + /*
> + * For HD, use external clock source since
> + * HD requires higher clock rate
> + */
> + writel(0xa, vpss_clkctl_reg);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + }
> + return ret;
> +}
> +
> +static u64 vpbe_display_dma_mask = DMA_BIT_MASK(32);
> +
> +static struct resource dm644x_v4l2_disp_resources[] = {
> + {
> + .start = IRQ_VENCINT,
> + .end= IRQ_VENCINT,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device vpbe_v4l2_display = {
dm644x_vpbe_v4l2_display
> + .name = "vpbe-v4l2",
> + .id = -1,
>
RE: [PATCH v13 5/8] davinci vpbe: platform specific additions
On Mon, Jan 10, 2011 at 18:21:34, Hadli, Manjunath wrote: > On Mon, Jan 10, 2011 at 17:25:33, Nori, Sekhar wrote: > > On Mon, Jan 10, 2011 at 16:58:41, Sergei Shtylyov wrote: > > > > > > + > > > > +#define OSD_REG_SIZE 0x01ff > > > > +#define VENC_REG_SIZE 0x017f > > > > > > Well, actually that's not the size but "limit" -- sizes should be > > > 0x200 and 0x180 respectively... > > > > In most resource definitions on DaVinci, these are not even #defined. Just > > add the limit directly to the base to derive the .end > > > > Thanks, > > Sekhar > > > Ok. I shall keep the numbers as is. Thanks. You can look at some existing resource definitions in arch/arm/mach-davinci/devices.c to see the format being used. Regards, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v13 5/8] davinci vpbe: platform specific additions
On Mon, Jan 10, 2011 at 16:58:41, Sergei Shtylyov wrote: > > + > > +#define OSD_REG_SIZE 0x01ff > > +#define VENC_REG_SIZE 0x017f > > Well, actually that's not the size but "limit" -- sizes should be 0x200 > and 0x180 respectively... In most resource definitions on DaVinci, these are not even #defined. Just add the limit directly to the base to derive the .end Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.38] DaVinci VPIF: Support for DV preset and DV timings and core-assisted locking (v2)
Hi Mauro, Here is an updated pull request for DV preset and DV timing support on VPIF as well as core-assisted locking support. All the changes are local to DaVinci VPIF driver. The patches have been reviewed on linux-media and acked by TI developers. Thanks, Sekhar The following changes since commit aeb13b434d0953050306435cd3134d65547dbcf4: Mauro Carvalho Chehab (1): cx25821: Fix compilation breakage due to BKL dependency are available in the git repository at: git://arago-project.org/git/projects/linux-davinci.git for-mauro Hans Verkuil (2): davinci: convert vpif_capture to core-assisted locking davinci: convert vpif_display to core-assisted locking Mats Randgaard (5): vpif_cap/disp: Add debug functionality vpif: Consolidate formats from capture and display vpif_cap/disp: Add support for DV presets vpif_cap/disp: Added support for DV timings vpif_cap/disp: Cleanup, improved comments drivers/media/video/davinci/vpif.c | 177 +++ drivers/media/video/davinci/vpif.h | 18 +- drivers/media/video/davinci/vpif_capture.c | 451 -- drivers/media/video/davinci/vpif_capture.h |2 + drivers/media/video/davinci/vpif_display.c | 474 +--- drivers/media/video/davinci/vpif_display.h |2 + 6 files changed, 907 insertions(+), 217 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 0/2] davinci: convert to core-assisted locking
Hi Mauro, On Thu, Jan 06, 2011 at 12:10:07, Hadli, Manjunath wrote: > Tested for SD loopback and other IOCTLS. Reviewed the patches. > > Patch series Acked by: Manjunath Hadli Shall I add these two patches as well to the pull request I sent yesterday[1]? These changes are localized to the DaVinci VPIF driver and should be safe to take in. I can also send a separate pull request. Let me know and I will do that way. Thanks, Sekhar [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg26594.html > -Manju > > On Wed, Jan 05, 2011 at 22:12:38, Hans Verkuil wrote: > > > > These two patches convert vpif_capture and vpif_display to core-assisted > > locking and now use .unlocked_ioctl instead of .ioctl. > > > > These patches assume that the 'DaVinci VPIF: Support for DV preset and DV > > timings' patch series was applied first. See: > > > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg26594.html > > > > These patches are targeted for 2.6.38. > > > > Regards, > > > > Hans > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.38] DaVinci VPIF: Support for DV preset and DV timings
Hi Mauro, Can you please pull from the following tree for DV preset and DV timings support for DaVinci VPIF. Sorry for the late request, but we were waiting for an ack from Manju. The patches themselves have been reviewed on the list quite a while ago. The patches affect only DaVinci VPIF video and have been verified by Manju to not have broken anything. Thanks, Sekhar The following changes since commit 187134a5875df20356f4dca075db29f294115a47: David Henningsson (1): [media] DVB: IR support for TechnoTrend CT-3650 are available in the git repository at: git://arago-project.org/git/projects/linux-davinci.git for-mauro Mats Randgaard (5): vpif_cap/disp: Add debug functionality vpif: Consolidate formats from capture and display vpif_cap/disp: Add support for DV presets vpif_cap/disp: Added support for DV timings vpif_cap/disp: Cleanup, improved comments drivers/media/video/davinci/vpif.c | 177 drivers/media/video/davinci/vpif.h | 18 +- drivers/media/video/davinci/vpif_capture.c | 361 +++-- drivers/media/video/davinci/vpif_capture.h |2 + drivers/media/video/davinci/vpif_display.c | 400 +--- drivers/media/video/davinci/vpif_display.h |2 + 6 files changed, 884 insertions(+), 76 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 0/6] davinci vpbe: display driver for DM644X
Hi Murali, On Sat, Nov 27, 2010 at 20:14:46, Muralidharan Karicheri wrote: > Manjunath, > > Could you re-send the patch 1/6? I can't find it either at my inbox or > mailing list. I received the 1/6 in my inbox and also see it on patchwork and gmane. https://patchwork.kernel.org/patch/353081/ http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/25692 Can you please check your spam folder just in case... Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
On Wed, Dec 16, 2009 at 13:11:57, Hans Verkuil wrote:
> On Wednesday 16 December 2009 00:37:52 Karicheri, Muralidharan wrote:
> > Hans,
> >
> > I remember there was a comment against an earlier patch that asks
> > for combining such statements since it makes the function appear
> > as big. Not sure who had made that comment. That is the reason you
> > find code like this in this patch. It was initially done with multiple
> > OR statements to construct the value to be written to the register as you
> > stated below as
> >
> > >val = bc->horz.win_count_calc &
> > > ISIF_HORZ_BC_WIN_COUNT_MASK;
> > >val |= !!bc->horz.base_win_sel_calc <<
> > > ISIF_HORZ_BC_WIN_SEL_SHIFT;
> >
> > I have checked few other drivers such as bt819.c ir-kbd-i2c.c,
> > mt9m111.c etc, where similar statements are found, but they have used
> > hardcoded values masks which makes it appears less complex. But I
> > like to reduce magic numbers as much possible in the code.
>
> Personally I have mixed feelings about the use for symbolic names for things
> like this. The problem is that they do not help me understanding the code.
> The names tend to be long, leading to these broken up lines, and if I want
> to know how the bits are distributed in the value I continuously have to
> do the look up in the header containing these defines.
>
> Frankly, I have a similar problem with using symbolic names for registers.
> Every time I need to look up a register in the datasheet I first need to
> look up the register number the register name maps to. All datasheets seem
> to be organized around the register addresses and there rarely is a datasheet
> that has an index of symbolic names.
>
> Using hard-coded numbers together with a well written comment tends to be much
> more readable in my opinion. I don't really think there is anything magic
> about
> these numbers: these are exactly the numbers that I need to know whenever I
> have to deal with the datasheet. Having to go through a layer of obfuscation
> is just plain irritating to me.
>
> However, I seem to be a minority when it comes to this and I've given up
> arguing for this...
>
> Note that all this assumes that the datasheet is publicly available. If it
> is a reversed engineered driver or based on NDA datasheets only, then the
> symbolic names may be all there is to make you understand what is going on.
>
[...]
>
> That seems overkill. I actually think it can be improved a lot by visually
> grouping the lines:
>
> val = (bc->horz.win_count_calc &
> ISIF_HORZ_BC_WIN_COUNT_MASK) |
>((!!bc->horz.base_win_sel_calc) <<
> ISIF_HORZ_BC_WIN_SEL_SHIFT) |
>((!!bc->horz.clamp_pix_limit) <<
> ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
>((bc->horz.win_h_sz_calc &
> ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
> ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
>((bc->horz.win_v_sz_calc &
> ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
> ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);
>
> Now I can at least see the various values that are ORed.
>
I liked this piece of code from drivers/mtd/nand/s3c2410.c. Serves as
a good template to do this sort of thing.
#define S3C2410_NFCONF_TACLS(x)((x)<<8)
#define S3C2410_NFCONF_TWRPH0(x) ((x)<<4)
#define S3C2410_NFCONF_TWRPH1(x) ((x)<<0)
[Okay, spaces around '<<', please :)]
[...]
if (plat != NULL) {
tacls = s3c_nand_calc_rate(plat->tacls, clkrate, tacls_max);
twrph0 = s3c_nand_calc_rate(plat->twrph0, clkrate, 8);
twrph1 = s3c_nand_calc_rate(plat->twrph1, clkrate, 8);
}
[...]
mask = (S3C2410_NFCONF_TACLS(3) |
S3C2410_NFCONF_TWRPH0(7) |
S3C2410_NFCONF_TWRPH1(7));
set = S3C2410_NFCONF_EN;
set |= S3C2410_NFCONF_TACLS(tacls - 1);
set |= S3C2410_NFCONF_TWRPH0(twrph0 - 1);
set |= S3C2410_NFCONF_TWRPH1(twrph1 - 1);
[...]
cfg = readl(info->regs + S3C2410_NFCONF);
cfg &= ~mask;
cfg |= set;
writel(cfg, info->regs + S3C2410_NFCONF);
And Murali said:
> >Huh? That does not explain why apparently bc->horz.win_h_sz_calc can be
> >larger
> >than ISIF_HORZ_BC_WIN_H_SIZE_MASK.
> because the values come from the user and since we can't use the enum
> for the types, I have to make sure the value is within range. Other way
> to do is to check the value in the validate() function. I am inclined to
> do the validation so that the & statements with masks can be removed while
> setting it in
> the register.
Agree fully here. Either a separate validate function or
an if check before using the values. Results with masking
or without masking are both unpredictable.
Thanks,
Sekha
RE: [PATCH v0 1/2] V4L - vpfe capture - convert ccdc drivers to platform drivers
On Fri, Dec 04, 2009 at 01:21:43, Karicheri, Muralidharan wrote:
>
[...]
> >
> >> + if (!res) {
> >> + status = -EBUSY;
> >> + goto fail_nores;
> >> + }
> >> +
> >> + ccdc_base_addr = ioremap_nocache(res->start, res_len);
> >> + if (!ccdc_base_addr) {
> >> + status = -EBUSY;
> >[Hiremath, Vaibhav] Is -EBUSY right return value, I think it should be -
> >ENXIO or -ENOMEM.
> >
> I see -ENXIO & -ENOMEM being used by drivers. -ENXIO stands for "No such
> device or address". ENOMEM stands for "Out of memory" . Since we are trying
> to map the address here, -ENXIO looks reasonable to me. Same if
> request_mem_region() fails.
>
Sergei had posted on this earlier[1]. Quoting him here:
"
> What are the proper error codes when platform_get_resource,
-ENODEV.
> request_mem_region
-EBUSY.
> and ioremap functions fail?.
-ENOMEM.
"
Not sure if ioremap failure can relate to absence of a device.
Thanks,
Sekhar
[1]
http://www.mail-archive.com/davinci-linux-open-sou...@linux.davincidsp.com/msg14973.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/5 - v0] V4L-vpfe capture - adding CCDC driver for DM365
Hi Murali,
Here is a (styling related) review from an non-video person. The
review is neither complete nor exhaustive (the patch is huge!),
but I thought will send across whatever I have for you to take a look.
On Wed, Dec 02, 2009 at 03:08:49, Karicheri, Muralidharan wrote:
> From: Muralidharan Karicheri
>
> This patch is for adding support for DM365 CCDC. This will allow to
> capture YUV video frames from TVP5146 video decoder on DM365 EVM. The vpfe
> capture driver will use this module to configure ISIF (a.k.a CCDC)
> module to allow YUV data capture. This driver is written for ccdc_hw_device
> interface used by vpfe capture driver to configure the ccdc module.
> This patch is tested using NTSC & PAL video sources and verified for
> both formats.
>
> NOTE: This is the initial version for review.
Typically "RFC" is put instead of "PATCH" in subject line
to convey this.
>
> Signed-off-by: Muralidharan Karicheri
> ---
> drivers/media/video/davinci/dm365_ccdc.c | 1529
> +
> drivers/media/video/davinci/dm365_ccdc_regs.h | 293 +
> include/media/davinci/dm365_ccdc.h| 555 +
> 3 files changed, 2377 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/davinci/dm365_ccdc.c
> create mode 100644 drivers/media/video/davinci/dm365_ccdc_regs.h
> create mode 100644 include/media/davinci/dm365_ccdc.h
>
> diff --git a/drivers/media/video/davinci/dm365_ccdc.c
> b/drivers/media/video/davinci/dm365_ccdc.c
Hopefully it is possible to choose a "generic" name
instead of tying it to an SoC.
> new file mode 100644
> index 000..2f27696
> --- /dev/null
> +++ b/drivers/media/video/davinci/dm365_ccdc.c
> @@ -0,0 +1,1529 @@
[...]
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "dm365_ccdc_regs.h"
> +#include "ccdc_hw_device.h"
Typically the includes are grouped using empty lines
based on the folder "linux", "media" "mach" etc.
> +
> +static struct device *dev;
> +
> +/* Defauts for module configuation paramaters */
> +static struct ccdc_config_params_raw ccdc_config_defaults = {
> + .linearize = {
> + .en = 0,
> + .corr_shft = CCDC_NO_SHIFT,
> + .scale_fact = {1, 0},
> + },
> + .df_csc = {
> + .df_or_csc = 0,
> + .csc = {
> + .en = 0
Should use ',' at the end of line so adding
new members leads to adding just one line.
There are more of these in this static init
below.
> + },
> + },
> + .dfc = {
> + .en = 0
> + },
> + .bclamp = {
> + .en = 0
> + },
> + .gain_offset = {
> + .gain = {
> + .r_ye = {1, 0},
> + .gr_cy = {1, 0},
> + .gb_g = {1, 0},
> + .b_mg = {1, 0},
> + },
> + },
> + .culling = {
> + .hcpat_odd = 0xff,
> + .hcpat_even = 0xff,
> + .vcpat = 0xff
> + },
> + .compress = {
> + .alg = CCDC_ALAW,
> + },
> +};
> +
> +/* ISIF operation configuration */
> +struct ccdc_oper_config {
> + enum vpfe_hw_if_type if_type;
> + struct ccdc_ycbcr_config ycbcr;
> + struct ccdc_params_raw bayer;
> + enum ccdc_data_pack data_pack;
> + void *__iomem base_addr;
> + void *__iomem linear_tbl0_addr;
> + void *__iomem linear_tbl1_addr;
Usually it is void __iomem *foo;
> +};
> +
> +static struct ccdc_oper_config ccdc_cfg = {
> + .ycbcr = {
> + .pix_fmt = CCDC_PIXFMT_YCBCR_8BIT,
> + .frm_fmt = CCDC_FRMFMT_INTERLACED,
> + .win = CCDC_WIN_NTSC,
> + .fid_pol = VPFE_PINPOL_POSITIVE,
> + .vd_pol = VPFE_PINPOL_POSITIVE,
> + .hd_pol = VPFE_PINPOL_POSITIVE,
> + .pix_order = CCDC_PIXORDER_CBYCRY,
> + .buf_type = CCDC_BUFTYPE_FLD_INTERLEAVED,
> + },
> + .bayer = {
> + .pix_fmt = CCDC_PIXFMT_RAW,
> + .frm_fmt = CCDC_FRMFMT_PROGRESSIVE,
> + .win = CCDC_WIN_VGA,
> + .fid_pol = VPFE_PINPOL_POSITIVE,
> + .vd_pol = VPFE_PINPOL_POSITIVE,
> + .hd_pol = VPFE_PINPOL_POSITIVE,
> + .gain = {
> + .r_ye = {1, 0},
> + .gr_cy = {1, 0},
> + .gb_g = {1, 0},
> + .b_mg = {1, 0},
> + },
> + .cfa_pat = CCDC_CFA_PAT_MOSAIC,
> + .data_msb = CCDC_BIT_MSB_11,
> + .config_params = {
> + .data_shift = CCDC_NO_SHIFT,
> + .col_pat_field0 = {
> + .olop = CCDC_GREEN_BLUE,
> + .olep = CCDC_BLUE,
> + .elop = CCDC_RED,
> + .elep = CCDC_GREEN_RED,
> + },
> + .col_pat_field1 = {
> +
RE: [PATCH 2/6 v5] Support for TVP7002 in dm365 board
On Fri, Oct 16, 2009 at 00:17:46, Kevin Hilman wrote:
> writes:
>
> > From: Santiago Nunez-Corrales
> >
> > This patch provides support for TVP7002 in architecture definitions
> > within DM365.
> >
> > Signed-off-by: Santiago Nunez-Corrales
> > ---
> > 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
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> >
> >
> > 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.
But this wouldn't work when TVP7002 is built as a module. Correct?
The current patch does not take care of the module case as well.
Patch 6/6 of this series does seem to make the TVP7002 driver available
as module.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
