RE: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-vpfe-cap

2009-06-30 Thread Karicheri, Muralidharan
Mauro,

Thanks for looking into this.

>> - tvp514x: Migration to sub-device framework
>
>As already pointed, here we have those comments regression. Also, some
>functions were changed, but the comments still mentions the older
>parameters.
>Please fix it on a later patch.
>
I had sent a patch to fix the comment formatting. If you wish, you could apply 
it and wait for another that fix the old parameter descriptions.
>> - v4l: vpfe capture bridge driver for DM355 and DM6446
>> - vpfe_capture: add missing newlines and fix an incorrect error code.
>> - v4l: ccdc hw device header file for vpfe capture
>> - v4l: dm355 ccdc module for vpfe capture driver
>
>Hmm...
>
>+#define CCDC_CID_R_GAIN(V4L2_CID_PRIVATE_BASE + 0)
>+#define CCDC_CID_GR_GAIN   (V4L2_CID_PRIVATE_BASE + 1)
>+#define CCDC_CID_GB_GAIN   (V4L2_CID_PRIVATE_BASE + 2)
>+#define CCDC_CID_B_GAIN(V4L2_CID_PRIVATE_BASE + 3)
>
>This is not nice. Such gains are common to other webcam drivers, and should
>be
>part of the common part of V4L2 API.
>
>In fact, currently, we have it mapped as a general gain as red/blue balance.
>
>+#define CCDC_CID_OFFSET(V4L2_CID_PRIVATE_BASE + 4)
>
>What does it control?
This is to adjust the black level I suppose. But I will re-visit it when I do 
control IOCTL handling as per Hans comment against the patch.
>
>+#define CCDC_CID_MAX   (V4L2_CID_PRIVATE_BASE + 5)
>
>This one doesn't seem to be used.
>
This is part of my TODO list as per comments from Hans. So could we merge this 
and later fix it as part another patch that adds control ioctl handling?

>> - v4l: dm644x ccdc module for vpfe capture driver
>> - v4l: ccdc types used across ccdc modules for vpfe capture driver
>> - v4l: common vpss module for video drivers
>> - v4l: Makefile and config files for vpfe capture driver
>> - v4l: davinci drivers should only be compiled for kernels >= 2.6.31.
>
>The other patches are ok.
>
>>
>> Hopefully these changes can be merged into 2.6.31.
>
>After having the entire series committed, I'll see if it is still possible
>to
>submit it.
>>
>> There are two arch/arm patches that need to be applied to the git tree.
>> These patches should be applied last.
>>
>> They are:
>>
>> http://patchwork.kernel.org/patch/30968/
>> http://patchwork.kernel.org/patch/30974/
>>
>> Note that I had to move patch 9 (vpss) in the original patch series
>before
>> patch 6 (the Makefile changes) to make sure everything would still
>compile.
>>
>> Thanks,
>>
>> Hans
>>
>> diffstat:
>>  b/linux/drivers/media/video/davinci/Makefile   |9
>>  b/linux/drivers/media/video/davinci/ccdc_hw_device.h   |  110
>>  b/linux/drivers/media/video/davinci/dm355_ccdc.c   | 1163 +
>>  b/linux/drivers/media/video/davinci/dm355_ccdc_regs.h  |  310 ++
>>  b/linux/drivers/media/video/davinci/dm644x_ccdc.c  |  878 ++
>>  b/linux/drivers/media/video/davinci/dm644x_ccdc_regs.h |  145 +
>>  b/linux/drivers/media/video/davinci/vpfe_capture.c | 2136
>> +
>>  b/linux/drivers/media/video/davinci/vpss.c |  301 ++
>>  b/linux/include/media/davinci/ccdc_types.h |   43
>>  b/linux/include/media/davinci/dm355_ccdc.h |  336 ++
>>  b/linux/include/media/davinci/dm644x_ccdc.h|  184 +
>>  b/linux/include/media/davinci/vpfe_capture.h   |  188 +
>>  b/linux/include/media/davinci/vpfe_types.h |   51
>>  b/linux/include/media/davinci/vpss.h   |   69
>>  linux/drivers/media/video/Kconfig  |   49
>>  linux/drivers/media/video/Makefile |2
>>  linux/drivers/media/video/davinci/vpfe_capture.c   |   27
>>  linux/drivers/media/video/tvp514x.c|  879 ++
>>  linux/drivers/media/video/tvp514x_regs.h   |   10
>>  linux/include/media/tvp514x.h  |4
>>  v4l/versions.txt   |7
>>  21 files changed, 6346 insertions(+), 555 deletions(-)
>>
>
>
>
>
>Cheers,
>Mauro

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-vpfe-cap

2009-06-21 Thread Hans Verkuil
On Sunday 21 June 2009 19:33:11 Mauro Carvalho Chehab wrote:
> Em Fri, 19 Jun 2009 14:39:14 +0200
>
> Hans Verkuil  escreveu:
> > Hi Mauro,
> >
> > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-vpfe-cap
> > for the following:
> >
> > - tvp514x: Migration to sub-device framework
>
> Hmm... the kernel-doc format is wrong on all function description comment
> changes on tvp514x:

I'm assuming this can be corrected in a separate patch later? I don't think 
this is a blocking issue.

Muralidharan, can you take a look at this?

Regards,

Hans

>
> -/**
> +/*
>   * struct tvp514x_decoder - TVP5146/47 decoder object
> - * @v4l2_int_device: Slave handle
> - * @tvp514x_slave: Slave pointer which is used by @v4l2_int_device
> + * @sd: Subdevice Slave handle
>   * @tvp514x_regs: copy of hw's regs with preset values.
>   * @pdata: Board specific
> - * @client: I2C client data
> - * @id: Entry from I2C table
>   * @ver: Chip version
> - * @state: TVP5146/47 decoder state - detected or not-detected
> + * @streaming: TVP5146/47 decoder streaming - enabled or disabled.
>   * @pix: Current pixel format
>   * @num_fmts: Number of formats
>   * @fmt_list: Format list
>   * @current_std: Current standard
>   * @num_stds: Number of standards
>   * @std_list: Standards list
> - * @route: input and output routing at chip level
> + * @input: Input routing at chip level
> + * @output: Output routing at chip level
>   */
>
> Please read Documentation/kernel-doc-nano-HOWTO.txt for the proper
> format. Basically, it should be like this example:
>
> /**
>  * foobar() - short function description of foobar
>  * @arg1:   Describe the first argument to foobar.
>  * @arg2:   Describe the second argument to foobar.
>  *  One can provide multiple line descriptions
>  *  for arguments.
>  *
>  * A longer description, with more discussion of the function foobar()
>  * that might be useful to those using or modifying it.  Begins with
>  * empty comment line, and may include additional embedded empty
>  * comment lines.
>  *
>  * The longer description can have multiple paragraphs.
>  **/
>
> > - v4l: vpfe capture bridge driver for DM355 and DM6446
> > - vpfe_capture: add missing newlines and fix an incorrect error code.
> > - v4l: ccdc hw device header file for vpfe capture
> > - v4l: dm355 ccdc module for vpfe capture driver
> > - v4l: dm644x ccdc module for vpfe capture driver
> > - v4l: ccdc types used across ccdc modules for vpfe capture driver
> > - v4l: common vpss module for video drivers
> > - v4l: Makefile and config files for vpfe capture driver
> > - v4l: davinci drivers should only be compiled for kernels >= 2.6.31.
>
> I'll try to analyze those remaining patches later today. Unfortunately,
> I'm very busy this weekend finishing some pending work.
>
> > Hopefully these changes can be merged into 2.6.31.
> >
> > There are two arch/arm patches that need to be applied to the git tree.
> > These patches should be applied last.
> >
> > They are:
> >
> > http://patchwork.kernel.org/patch/30968/
> > http://patchwork.kernel.org/patch/30974/
> >
> > Note that I had to move patch 9 (vpss) in the original patch series
> > before patch 6 (the Makefile changes) to make sure everything would
> > still compile.
> >
> > Thanks,
> >
> > Hans
> >
> > diffstat:
> >  b/linux/drivers/media/video/davinci/Makefile   |9
> >  b/linux/drivers/media/video/davinci/ccdc_hw_device.h   |  110
> >  b/linux/drivers/media/video/davinci/dm355_ccdc.c   | 1163
> > + b/linux/drivers/media/video/davinci/dm355_ccdc_regs.h  |  310
> > ++ b/linux/drivers/media/video/davinci/dm644x_ccdc.c  |  878 ++
> > b/linux/drivers/media/video/davinci/dm644x_ccdc_regs.h |  145 +
> > b/linux/drivers/media/video/davinci/vpfe_capture.c | 2136
> > +
> >  b/linux/drivers/media/video/davinci/vpss.c |  301 ++
> >  b/linux/include/media/davinci/ccdc_types.h |   43
> >  b/linux/include/media/davinci/dm355_ccdc.h |  336 ++
> >  b/linux/include/media/davinci/dm644x_ccdc.h|  184 +
> >  b/linux/include/media/davinci/vpfe_capture.h   |  188 +
> >  b/linux/include/media/davinci/vpfe_types.h |   51
> >  b/linux/include/media/davinci/vpss.h   |   69
> >  linux/drivers/media/video/Kconfig  |   49
> >  linux/drivers/media/video/Makefile |2
> >  linux/drivers/media/video/davinci/vpfe_capture.c   |   27
> >  linux/drivers/media/video/tvp514x.c|  879 ++
> >  linux/drivers/media/video/tvp514x_regs.h   |   10
> >  linux/include/media/tvp514x.h  |4
> >  v4l/versions.txt   |7
> >  21 files changed, 6346 insertions(+), 555 deletions(-)



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

___
Davinci-lin