Hi Laurent, On 09/19/2011 01:05 AM, Laurent Pinchart wrote: > On Saturday 17 September 2011 18:06:20 Sylwester Nawrocki wrote: >> On 09/17/2011 02:34 PM, Guennadi Liakhovetski wrote: >>> On Sat, 17 Sep 2011, Sylwester Nawrocki wrote: >>>> On 09/17/2011 12:54 PM, Laurent Pinchart wrote: >>>>> On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: >>>>>> HREF is a signal indicating valid data during single line >>>>>> transmission. Add corresponding flags for this signal to the set of >>>>>> mediabus signal polarity flags. >>>>> >>>>> So that's a data valid signal that gates the pixel data ? The OMAP3 ISP >>>>> has a >>>> >>>> Yes, it's "horizontal window reference" signal, it's well described in >>>> this datasheet: http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf >>>> >>>> AFAICS there can be also its vertical counterpart - VREF. >>>> >>>> Many devices seem to use this terminology. However, I realize, not all, >>>> as you're pointing out. So perhaps it's time for some naming contest >>>> now.. :-) >>> >>> No objections in principle, just one question though: can these signals >>> actually be used simultaneously with respective *SYNC signals or only as >>> an alternative? If the latter, maybe we could reuse same names by just >>> making them more generic? >> >> That's actually a good question. In my use cases only HREF is used as >> horizontal synchronization signal, i.e. physical bus interface has this >> signals: >> >> ->| PCLK >> ->| VSYNC >> ->| HREF >> ->| DATA[0:7] >> ->| FIELD >> >> For interlaced mode FIELD can be connected to the horizontal >> synchronization signal. For this case there is InvPolHSYNC bit in the host >> interface registers to indicate the polarity. There are 5 bits actually: >> >> InvPolPCLK >> InvPolVSYNC (vertical sychronization) >> InvPolHREF (horizontal synchronization) >> InvPolHSYNC (for interlaced mode only, FIELD port = horizontal sync. >> signal) InvPolFIELD (interlaced mode, FIELD port = FIELD signal) > > Shouldn't this be handled through platform data only ? Indeed, this is how it's done now and I didn't intend to change that. I just wanted to replace driver's private signal polarity flag definitions with the standard ones. Do you think I should rather keep these things in driver's public header? It's of course a way of less resistance :) To make things complete I thought about adding the FIELD flags, i.e. >From 9bd11f9b14dffe877f9c546e068b4b4027c9472a Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> Date: Sun, 18 Sep 2011 11:28:58 +0200 Subject: [PATCH 1/2] v4l2: Add the parallel bus HREF and FIELD signal polarity flags HREF is a signal gating valid data during single line transmission. FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 standard. Add corresponding flags for these signals to the set of media bus signal polarity flags. Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> --- include/media/v4l2-mediabus.h | 20 +++++++++++++------- 1 files changed, 13 insertions(+), 7 deletions(-) diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 6114007..1952d9f 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -23,15 +23,21 @@ #define V4L2_MBUS_MASTER (1 << 0) #define V4L2_MBUS_SLAVE (1 << 1) /* Which signal polarities it supports */ -/* Note: in BT.656 mode HSYNC and VSYNC are unused */ +/* Note: in BT.656 mode HSYNC, HREF, FIELD, and VSYNC are unused */ #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 5) -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 6) -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) -#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) +/* HREF is a horizontal window reference signal gating valid pixel data */ +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1 << 4) +#define V4L2_MBUS_HREF_ACTIVE_LOW (1 << 5) +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 6) +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 7) +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 8) +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 9) +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 10) +#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 11) +/* Field selection signal for interlaced scan mode */ +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 12) +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 13) /* Serial flags */ /* How many lanes the client can use */ -- 1.7.4.1 If there is more objection to the above changes then I'll drop the patch and stay with driver's private definitions. Thanks, -- Sylwester Nawrocki Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html