RE: [PATCH - stable v3.2] omap3isp: ccdc: Fix crash in HS/VS interrupt handler

2012-03-11 Thread Nori, Sekhar
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 g...@mlbassoc.com
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Acked-by: Sakari Ailus sakari.ai...@iki.fi
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 ---
  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

2012-02-28 Thread Nori, Sekhar
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 manjunath.ha...@ti.com

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

2012-02-22 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 ---
  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

2011-12-22 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: LMML linux-media@vger.kernel.org
 ---
  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 v3 0/5] ARM: davinci: re-arrange definitions to have a common davinci header

2011-12-02 Thread Nori, Sekhar
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 RESEND] davinci: dm646x: move vpif related code to driver core header from platform

2011-11-30 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 
  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 mche...@redhat.com

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 v3 0/5] ARM: davinci: re-arrange definitions to have a common davinci header

2011-11-30 Thread Nori, Sekhar
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] board-dm646x-evm.c: wrong register used in setup_vpif_input_channel_mode

2011-11-30 Thread Nori, Sekhar
Hi Hans,

On Mon, Nov 14, 2011 at 23:50:49, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 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 hans.verk...@cisco.com

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 RESEND] davinci: dm646x: move vpif related code to driver core header from platform

2011-11-24 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 
 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

2011-08-03 Thread Nori, Sekhar
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 manjunath.ha...@ti.com

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

2011-07-06 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
  
  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

2011-06-21 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 
 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

2011-06-02 Thread Nori, Sekhar
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 manjunath.ha...@ti.com

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 linux/videodev2.h
  #include linux/clk.h
  #include linux/davinci_emac.h
 +#include media/davinci/vpif.h
  
  #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 linux/videodev2.h
  #include mach/hardware.h
  #include mach/dm646x.h
 +#include media/davinci/vpif.h
  
  /* 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 media/v4l2-device.h
  #include media/videobuf-core.h
  #include media/videobuf-dma-contig.h
 -#include mach/dm646x.h
 +#include media/davinci/vpif.h
  
  #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 media/v4l2-device.h
  #include media/videobuf-core.h
  #include media/videobuf-dma-contig.h
 +#include media/davinci/vpif.h
  
  #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 

RE: [PATCH v17 5/6] davinci vpbe: Build infrastructure for VPBE driver

2011-05-23 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
  Acked-by: Muralidharan Karicheri m-kariche...@ti.com
  Acked-by: Hans Verkuil hverk...@xs4all.nl
 [...]
 
  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 07/13] davinci: move DM64XX_VDD3P3V_PWDN to devices.c

2011-05-23 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 Acked-by: Sekhar Nori nsek...@ti.com

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 1/1] davinci: dm646x: move vpif related code to driver core header from platform

2011-05-23 Thread Nori, Sekhar
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 manjunath.ha...@ti.com

 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 linux/videodev2.h
  #include mach/hardware.h
  #include mach/dm646x.h
 +#include media/davinci/vpif.h

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 08/13] davinci: eliminate use of IO_ADDRESS() on sysmod

2011-04-05 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 Acked-by: Sekhar Nori nsek...@ti.com
 ---

 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

2011-03-23 Thread Nori, Sekhar
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 08/13] davinci: eliminate use of IO_ADDRESS() on sysmod

2011-03-22 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 ---
  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 v17 10/13] davinci: dm644x: change vpfe capture structure variables for consistency

2011-03-22 Thread Nori, Sekhar
On Tue, Mar 15, 2011 at 19:29:12, Hadli, Manjunath wrote:
 change the vpfe capture related configuration structure variables from
 config to dm644xevm_config 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 13/13] davinci: dm644x EVM: add support for VPBE display

2011-03-22 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 ---
  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 00/13] davinci vpbe: dm6446 v4l2 driver

2011-03-22 Thread Nori, Sekhar
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 nsek...@ti.com

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 12/13] davinci: dm644x: add support for v4l2 video display

2011-03-22 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 ---

 +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 2/7] davinci: eliminate use of IO_ADDRESS() on sysmod

2011-03-15 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 
 The change looks good, it's definitely a step in the right
 direction.
 
 Acked-by: Arnd Bergmann a...@arndb.de
 
 
 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

2011-01-18 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 Acked-by: Muralidharan Karicheri m-kariche...@ti.com
 Acked-by: Hans Verkuil hverk...@xs4all.nl
 ---
  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 soc.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 soc.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 v13 5/8] davinci vpbe: platform specific additions

2011-01-10 Thread Nori, Sekhar
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


RE: [PATCH v13 5/8] davinci vpbe: platform specific additions

2011-01-10 Thread Nori, Sekhar
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 5/8] davinci vpbe: platform specific additions

2011-01-10 Thread Nori, Sekhar
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 manjunath.ha...@ti.com
 Acked-by: Muralidharan Karicheri m-kariche...@ti.com
 Acked-by: Hans Verkuil hverk...@xs4all.nl
 ---
  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 linux/init.h
 @@ -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,
 + .num_resources  = ARRAY_SIZE(dm644x_v4l2_disp_resources),
 + 

RE: [RFC PATCH 0/2] davinci: convert to core-assisted locking

2011-01-06 Thread Nori, Sekhar
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 manjunath.ha...@ti.com   

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 and core-assisted locking (v2)

2011-01-06 Thread Nori, Sekhar
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


[GIT PATCHES FOR 2.6.38] DaVinci VPIF: Support for DV preset and DV timings

2011-01-05 Thread Nori, Sekhar
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

2010-11-28 Thread Nori, Sekhar
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

2009-12-17 Thread Nori, Sekhar
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,
Sekhar

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More 

RE: [PATCH v0 1/2] V4L - vpfe capture - convert ccdc drivers to platform drivers

2009-12-03 Thread Nori, Sekhar
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