Hi Sakari, Thanks for the review.
On Fri, Sep 7, 2012 at 11:50 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote: > Hi Prabhakar, > > Thanks for the patch! > > > Prabhakar Lad wrote: >> >> From: Lad, Prabhakar <prabhakar....@ti.com> >> >> add V4L2_CID_TEST_PATTERN of type menu, which determines >> the internal test pattern selected by the device. >> >> Signed-off-by: Lad, Prabhakar <prabhakar....@ti.com> >> Signed-off-by: Manjunath Hadli <manjunath.ha...@ti.com> >> Cc: Sakari Ailus <sakari.ai...@iki.fi> >> Cc: Hans Verkuil <hans.verk...@cisco.com> >> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> >> Cc: Mauro Carvalho Chehab <mche...@infradead.org> >> Cc: Sylwester Nawrocki <s.nawro...@samsung.com> >> Cc: Hans de Goede <hdego...@redhat.com> >> Cc: Kyungmin Park <kyungmin.p...@samsung.com> >> Cc: Rob Landley <r...@landley.net> >> --- >> This patches has one checkpatch warning for line over >> 80 characters altough it can be avoided I have kept it >> for consistency. >> >> Changes for v2: >> 1: Included display devices in the description for test pattern >> as pointed by Hans. >> 2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as >> pointed by Sylwester. >> 3: Removed the test patterns from menu as the are hardware specific >> as pointed by Sakari. >> >> Documentation/DocBook/media/v4l/controls.xml | 20 ++++++++++++++++++++ >> drivers/media/v4l2-core/v4l2-ctrls.c | 8 ++++++++ >> include/linux/videodev2.h | 4 ++++ >> 3 files changed, 32 insertions(+), 0 deletions(-) >> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml >> b/Documentation/DocBook/media/v4l/controls.xml >> index ad873ea..173934e 100644 >> --- a/Documentation/DocBook/media/v4l/controls.xml >> +++ b/Documentation/DocBook/media/v4l/controls.xml >> @@ -4311,6 +4311,26 @@ interface and may change in the future.</para> >> </tbody> >> </entrytbl> >> </row> >> + <row> >> + <entry >> spanname="id"><constant>V4L2_CID_TEST_PATTERN</constant></entry> >> + <entry>menu</entry> >> + </row> >> + <row id="v4l2-test-pattern"> >> + <entry spanname="descr"> The Capture/Display/Sensors have the >> capability >> + to generate internal test patterns and this are hardware >> specific. This >> + test patterns are used to test a device is properly working >> and can generate >> + the desired waveforms that it supports.</entry> >> + </row> >> + <row> >> + <entrytbl spanname="descr" cols="2"> >> + <tbody valign="top"> >> + <row> >> + >> <entry><constant>V4L2_TEST_PATTERN_DISABLED</constant></entry> >> + <entry>Test pattern generation is disabled</entry> >> + </row> >> + </tbody> >> + </entrytbl> >> + </row> >> <row><entry></entry></row> >> </tbody> >> </tgroup> >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c >> b/drivers/media/v4l2-core/v4l2-ctrls.c >> index 8f2f40b..d731422 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >> @@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >> "Advanced", >> NULL, >> }; >> + static const char * const test_pattern[] = { >> + "Disabled", >> + NULL, >> + }; >> >> switch (id) { >> case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: >> @@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) >> return jpeg_chroma_subsampling; >> case V4L2_CID_DPCM_PREDICTOR: >> return dpcm_predictor; >> + case V4L2_CID_TEST_PATTERN: >> + return test_pattern; > > > I think it's not necessary to define test_pattern (nor be prepared to return > it) since the menu is going to be device specific. So the driver is > responsible for all of the menu items. Such menus are created using > v4l2_ctrl_new_custom() instead of v4l2_ctrl_new_std_menu(). > Ok. Regrads, --Prabhakar Lad > Looks good to me otherwise. > > Kind regards, > > -- > Sakari Ailus > sakari.ai...@iki.fi > > _______________________________________________ > Davinci-linux-open-source mailing list > davinci-linux-open-sou...@linux.davincidsp.com > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source -- 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