[RFC PATCH] adding support for setting bus parameters in sub device

[RFC PATCH] adding support for setting bus parameters in sub device

[m-karicheri2]

From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>

This patch adds support for setting bus parameters such as bus type
(Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field
etc) in sub device. This allows bridge driver to configure the sub device
bus for a specific set of bus parameters through s_bus() function call.
This also can be used to define platform specific bus parameters for
host and sub-devices.

Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
Applies to v4l-dvb repository

 include/media/v4l2-subdev.h |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 1785608..2f5ec98 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
 	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no service found */
 };
 
+/*
+ * Some sub-devices are connected to the host/bridge device through a bus that
+ * carries the clock, vsync, hsync and data. Some interfaces such as BT.656
+ * carries the sync embedded in the data where as others have separate line
+ * carrying the sync signals. The structure below is used to define bus 
+ * configuration parameters for host as well as sub-device
+ */
+enum v4l2_subdev_bus_type {
+	/* Raw YUV image data bus */
+	V4L2_SUBDEV_BUS_RAW_YUV,
+	/* Raw Bayer image data bus */
+	V4L2_SUBDEV_BUS_RAW_BAYER
+};
+
+struct v4l2_bus_settings {
+	/* yuv or bayer image data bus */
+	enum v4l2_subdev_bus_type type;
+	/* subdev bus width */
+	u8 subdev_width;
+	/* host bus width */
+	u8 host_width;
+	/* embedded sync, set this when sync is embedded in the data stream */
+	unsigned embedded_sync:1;
+	/* master or slave */
+	unsigned host_is_master:1;
+	/* 0 - active low, 1 - active high */
+	unsigned pol_vsync:1;
+	/* 0 - active low, 1 - active high */
+	unsigned pol_hsync:1;
+	/* 0 - low to high , 1 - high to low */
+	unsigned pol_field:1;
+	/* 0 - sample at falling edge , 1 - sample at rising edge */
+	unsigned pol_pclock:1;
+	/* 0 - active low , 1 - active high */
+	unsigned pol_data:1;
+};
+
 /* Sub-devices are devices that are connected somehow to the main bridge
    device. These devices are usually audio/video muxers/encoders/decoders or
    sensors and webcam controllers.
@@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
 
    s_routing: see s_routing in audio_ops, except this version is for video
 	devices.
+
+   s_bus: set bus parameters in sub device to configure the bus 
  */
 struct v4l2_subdev_video_ops {
 	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 config);
@@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
 	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
 	int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize);
 	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct v4l2_frmivalenum *fival);
+	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_bus_settings *bus);
 };
 
 struct v4l2_subdev_ops {
-- 
1.6.0.4

RE: [RFC PATCH] adding support for setting bus parameters in sub device

[Karicheri, Muralidharan]

Hans,

Let me know if this has all changes that you are expecting. This is just for review. I will send the final one against the latest v4l-dvb kernel tree.

Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
email: m-karicheri2@ti.com

>-----Original Message-----
>From: Karicheri, Muralidharan
>Sent: Wednesday, June 17, 2009 11:44 AM
>To: linux-media@vger.kernel.org
>Cc: davinci-linux-open-source@linux.davincidsp.com; Muralidharan Karicheri;
>Karicheri, Muralidharan
>Subject: [RFC PATCH] adding support for setting bus parameters in sub
>device
>
>From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>
>This patch adds support for setting bus parameters such as bus type
>(Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field
>etc) in sub device. This allows bridge driver to configure the sub device
>bus for a specific set of bus parameters through s_bus() function call.
>This also can be used to define platform specific bus parameters for
>host and sub-devices.
>
>Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>---
>Applies to v4l-dvb repository
>
> include/media/v4l2-subdev.h |   40
>++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 40 insertions(+), 0 deletions(-)
>
>diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>index 1785608..2f5ec98 100644
>--- a/include/media/v4l2-subdev.h
>+++ b/include/media/v4l2-subdev.h
>@@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
> 	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>service found */
> };
>
>+/*
>+ * Some sub-devices are connected to the host/bridge device through a bus
>that
>+ * carries the clock, vsync, hsync and data. Some interfaces such as
>BT.656
>+ * carries the sync embedded in the data where as others have separate
>line
>+ * carrying the sync signals. The structure below is used to define bus
>+ * configuration parameters for host as well as sub-device
>+ */
>+enum v4l2_subdev_bus_type {
>+	/* Raw YUV image data bus */
>+	V4L2_SUBDEV_BUS_RAW_YUV,
>+	/* Raw Bayer image data bus */
>+	V4L2_SUBDEV_BUS_RAW_BAYER
>+};
>+
>+struct v4l2_bus_settings {
>+	/* yuv or bayer image data bus */
>+	enum v4l2_subdev_bus_type type;
>+	/* subdev bus width */
>+	u8 subdev_width;
>+	/* host bus width */
>+	u8 host_width;
>+	/* embedded sync, set this when sync is embedded in the data stream
>*/
>+	unsigned embedded_sync:1;
>+	/* master or slave */
>+	unsigned host_is_master:1;
>+	/* 0 - active low, 1 - active high */
>+	unsigned pol_vsync:1;
>+	/* 0 - active low, 1 - active high */
>+	unsigned pol_hsync:1;
>+	/* 0 - low to high , 1 - high to low */
>+	unsigned pol_field:1;
>+	/* 0 - sample at falling edge , 1 - sample at rising edge */
>+	unsigned pol_pclock:1;
>+	/* 0 - active low , 1 - active high */
>+	unsigned pol_data:1;
>+};
>+
> /* Sub-devices are devices that are connected somehow to the main bridge
>    device. These devices are usually audio/video muxers/encoders/decoders
>or
>    sensors and webcam controllers.
>@@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>
>    s_routing: see s_routing in audio_ops, except this version is for video
> 	devices.
>+
>+   s_bus: set bus parameters in sub device to configure the bus
>  */
> struct v4l2_subdev_video_ops {
> 	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
>config);
>@@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
> 	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
> 	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>v4l2_frmsizeenum *fsize);
> 	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>v4l2_frmivalenum *fival);
>+	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_bus_settings
>*bus);
> };
>
> struct v4l2_subdev_ops {
>--
>1.6.0.4

Re: [RFC PATCH] adding support for setting bus parameters in sub device

[Hans Verkuil]

On Wednesday 17 June 2009 17:43:42 m-karicheri2@ti.com wrote:
> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>
> This patch adds support for setting bus parameters such as bus type
> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field
> etc) in sub device. This allows bridge driver to configure the sub device
> bus for a specific set of bus parameters through s_bus() function call.
> This also can be used to define platform specific bus parameters for
> host and sub-devices.
>
> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
> Applies to v4l-dvb repository
>
>  include/media/v4l2-subdev.h |   40
> ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 40
> insertions(+), 0 deletions(-)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 1785608..2f5ec98 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no service found
> */ };
>
> +/*
> + * Some sub-devices are connected to the host/bridge device through a
> bus that + * carries the clock, vsync, hsync and data. Some interfaces
> such as BT.656 + * carries the sync embedded in the data where as others
> have separate line + * carrying the sync signals. The structure below is
> used to define bus + * configuration parameters for host as well as
> sub-device
> + */
> +enum v4l2_subdev_bus_type {
> +	/* Raw YUV image data bus */
> +	V4L2_SUBDEV_BUS_RAW_YUV,
> +	/* Raw Bayer image data bus */
> +	V4L2_SUBDEV_BUS_RAW_BAYER
> +};

Remove the _subdev prefix from the enum above.

> +
> +struct v4l2_bus_settings {
> +	/* yuv or bayer image data bus */
> +	enum v4l2_subdev_bus_type type;
> +	/* subdev bus width */
> +	u8 subdev_width;
> +	/* host bus width */
> +	u8 host_width;
> +	/* embedded sync, set this when sync is embedded in the data stream */
> +	unsigned embedded_sync:1;
> +	/* master or slave */
> +	unsigned host_is_master:1;
> +	/* 0 - active low, 1 - active high */
> +	unsigned pol_vsync:1;
> +	/* 0 - active low, 1 - active high */
> +	unsigned pol_hsync:1;
> +	/* 0 - low to high , 1 - high to low */
> +	unsigned pol_field:1;
> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
> +	unsigned pol_pclock:1;

I'm not sure whether the pol_ prefix is correct here. Perhaps edge_pclock is 
a more appropriate name.

Regards,

	Hans

> +	/* 0 - active low , 1 - active high */
> +	unsigned pol_data:1;
> +};
> +
>  /* Sub-devices are devices that are connected somehow to the main bridge
>     device. These devices are usually audio/video
> muxers/encoders/decoders or sensors and webcam controllers.
> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>
>     s_routing: see s_routing in audio_ops, except this version is for
> video devices.
> +
> +   s_bus: set bus parameters in sub device to configure the bus
>   */
>  struct v4l2_subdev_video_ops {
>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
> config); @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum
> *fsize); int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
> v4l2_frmivalenum *fival); +	int (*s_bus)(struct v4l2_subdev *sd, const
> struct v4l2_bus_settings *bus); };
>
>  struct v4l2_subdev_ops {



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

Re: [RFC PATCH] adding support for setting bus parameters in sub device

[Jean-Philippe François]

Karicheri, Muralidharan a écrit :
>>> data9-data15 for MT9T031
>>> data11-data15 for MT9P031
>> But then you could set the host bus width accordingly for example :
>> MT9T031 MSB connected do data 9 : HOST Buswidth = 10
>> MT9T031 MSB connected to data 15: Host Buswdth = 16
> 
> How does the driver know which MSB of the sensor data line is connected to the host bus? This is to be configured in our hardware register. Without this, I need to hardcode it which doesn't seem right.
> 
I know you don't want to hardcode this, it is just not clear to me why 3 
parameters are needed. Let's take two different hardware setup : MT9T031 
with MSB connected on video pin 9 and 11

struct v4l2_bus_settings mt9t031_msb9 = {
	.subdev_width=10
	.host_width= ???
	.host_msb = 9
};

struct v4l2_bus_settings mt9t031_msb11 = {
	.subdev_width=10
	.host_width= ???
	.host_msb = 11
};

I can't see what is the use of those three parameters, and you have a 
cohabitation of width and bit number which is error prone.

Can you explain what is the meaning of host_width, or provide a 
realistic example where this 3 settings are useful ?



> Murali Karicheri
> Software Design Engineer
> Texas Instruments Inc.
> Germantown, MD 20874
> Phone : 301-515-3736
> email: m-karicheri2@ti.com
> 
>> -----Original Message-----
>> From: Jean-Philippe François [mailto:jp.francois@cynove.com]
>> Sent: Monday, June 29, 2009 12:23 PM
>> To: Karicheri, Muralidharan
>> Cc: Hans Verkuil; davinci-linux-open-source@linux.davincidsp.com; linux-
>> media@vger.kernel.org
>> Subject: Re: [RFC PATCH] adding support for setting bus parameters in sub
>> device
>>
>> Karicheri, Muralidharan a écrit :
>>> Hans,
>>>
>>> Had hit the send by mistake. Please ignore my previous reply...
>>>
>>> <snip>
>>>>> It seems very unlikely to me that someone would ever choose to e.g.
>> zero
>>>>> one or more MSB pins instead of the LSB pins in a case like this.
>>>>>
>>> No case in my experience...
>>>
>>> <snip>
>>>>> Or do you have examples where that actually happens?
>>>>>
>>> DM365 can work with 10 bit or 12 bit sensors. DM365 CCDC that receives
>> the
>>> image data needs to know which data lines have valid data. This is done
>> by
>>> specifying the MSB position. There is ccdc register to configure this
>> information. Ideally, you could connect the MSB of sensor to following
>> lines on host bus:-
>>> data9-data15 for MT9T031
>>> data11-data15 for MT9P031
>> But then you could set the host bus width accordingly for example :
>> MT9T031 MSB connected do data 9 : HOST Buswidth = 10
>> MT9T031 MSB connected to data 15: Host Buswdth = 16
>>
>> Since the host/sensor info are in the same structure, we can have
>> several subdev with different host buswidth.
>>
>> Jean-Philippe François
>>
>>>
>>> So it makes sense to specify this choice in the structure. I have not
>> added this earlier since we wanted to use the structure only for sub device
>> configuration. It has changed since then.
>>> I am also not sure if s_bus() is required since this will get set in the
>> platform data which could then be passed to the sub device using the new
>> api while loading it. When would host driver call s_bus()?
>>>>> If this never happens, then there is also no need for such a bitfield.
>>>>>
>>>>> I think I want to actually see someone using this before we add a field
>>>>> like that.
>>>>>
>>>>> Regards,
>>>>>
>>>>>       Hans
>>>>>
>>>>>
>>>>>> Murali Karicheri
>>>>>> Software Design Engineer
>>>>>> Texas Instruments Inc.
>>>>>> Germantown, MD 20874
>>>>>> email: m-karicheri2@ti.com
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>>>>>>> Sent: Monday, June 29, 2009 5:26 AM
>>>>>>> To: Karicheri, Muralidharan
>>>>>>> Cc: linux-media@vger.kernel.org; davinci-linux-open-
>>>>>>> source@linux.davincidsp.com
>>>>>>> Subject: Re: [RFC PATCH] adding support for setting bus parameters in
>>>> sub
>>>>>>> device
>>>>>>>
>>>>>>> Hi Murali,
>>>>>>>
>>>>>>>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>>>>>>>
>>>>>>>> This patch adds support for setting bus parameters such as bus type
>>>>>>>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>>>>>>>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync,
>>>>>>>> field
>>>>>>>> etc) in sub device. This allows bridge driver to configure the sub
>>>>>>>> device
>>>>>>>> bus for a specific set of bus parameters through s_bus() function
>> call.
>>>>>>>> This also can be used to define platform specific bus parameters for
>>>>>>>> host and sub-devices.
>>>>>>>>
>>>>>>>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>>>>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>>>>>> ---
>>>>>>>> Applies to v4l-dvb repository
>>>>>>>>
>>>>>>>>  include/media/v4l2-subdev.h |   40
>>>>>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>>>>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
>> subdev.h
>>>>>>>> index 1785608..2f5ec98 100644
>>>>>>>> --- a/include/media/v4l2-subdev.h
>>>>>>>> +++ b/include/media/v4l2-subdev.h
>>>>>>>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>>>>>>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>>>>>>> service found */
>>>>>>>>  };
>>>>>>>>
>>>>>>>> +/*
>>>>>>>> + * Some sub-devices are connected to the host/bridge device through
>> a
>>>>>>> bus
>>>>>>>> that
>>>>>>>> + * carries the clock, vsync, hsync and data. Some interfaces such
>> as
>>>>>>>> BT.656
>>>>>>>> + * carries the sync embedded in the data where as others have
>>>> separate
>>>>>>>> line
>>>>>>>> + * carrying the sync signals. The structure below is used to define
>>>>>>>> bus
>>>>>>>> + * configuration parameters for host as well as sub-device
>>>>>>>> + */
>>>>>>>> +enum v4l2_subdev_bus_type {
>>>>>>>> +	/* Raw YUV image data bus */
>>>>>>>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>>>>>>>> +	/* Raw Bayer image data bus */
>>>>>>>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +struct v4l2_bus_settings {
>>>>>>>> +	/* yuv or bayer image data bus */
>>>>>>>> +	enum v4l2_subdev_bus_type type;
>>>>>>>> +	/* subdev bus width */
>>>>>>>> +	u8 subdev_width;
>>>>>>>> +	/* host bus width */
>>>>>>>> +	u8 host_width;
>>>>>>>> +	/* embedded sync, set this when sync is embedded in the data
>>>> stream
>>>>>>> */
>>>>>>>> +	unsigned embedded_sync:1;
>>>>>>>> +	/* master or slave */
>>>>>>>> +	unsigned host_is_master:1;
>>>>>>>> +	/* 0 - active low, 1 - active high */
>>>>>>>> +	unsigned pol_vsync:1;
>>>>>>>> +	/* 0 - active low, 1 - active high */
>>>>>>>> +	unsigned pol_hsync:1;
>>>>>>>> +	/* 0 - low to high , 1 - high to low */
>>>>>>>> +	unsigned pol_field:1;
>>>>>>>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>>>>>>>> +	unsigned pol_pclock:1;
>>>>>>>> +	/* 0 - active low , 1 - active high */
>>>>>>>> +	unsigned pol_data:1;
>>>>>>>> +};
>>>>>>> I've been thinking about this for a while and I think this struct
>> should
>>>>>>> be extended with the host bus parameters as well:
>>>>>>>
>>>>>>> struct v4l2_bus_settings {
>>>>>>> 	/* yuv or bayer image data bus */
>>>>>>> 	enum v4l2_bus_type type;
>>>>>>> 	/* embedded sync, set this when sync is embedded in the data
>> stream
>>>>>>> */
>>>>>>> 	unsigned embedded_sync:1;
>>>>>>> 	/* master or slave */
>>>>>>> 	unsigned host_is_master:1;
>>>>>>>
>>>>>>> 	/* bus width */
>>>>>>> 	unsigned sd_width:8;
>>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>>> 	unsigned sd_pol_vsync:1;
>>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>>> 	unsigned sd_pol_hsync:1;
>>>>>>> 	/* 0 - low to high, 1 - high to low */
>>>>>>> 	unsigned sd_pol_field:1;
>>>>>>> 	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>>>> 	unsigned sd_edge_pclock:1;
>>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>>> 	unsigned sd_pol_data:1;
>>>>>>>
>>>>>>> 	/* host bus width */
>>>>>>> 	unsigned host_width:8;
>>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>>> 	unsigned host_pol_vsync:1;
>>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>>> 	unsigned host_pol_hsync:1;
>>>>>>> 	/* 0 - low to high, 1 - high to low */
>>>>>>> 	unsigned host_pol_field:1;
>>>>>>> 	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>>>> 	unsigned host_edge_pclock:1;
>>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>>> 	unsigned host_pol_data:1;
>>>>>>> };
>>>>>>>
>>>>>>> It makes sense since you need to setup both ends of the bus, and
>> having
>>>>>>> both ends defined in the same struct keeps everything together. I
>> have
>>>>>>> thought about having separate host and subdev structs, but part of
>> the
>>>>>>> bus
>>>>>>> description is always common (bus type, master/slave,
>> embedded/separate
>>>>>>> syncs), while another part can be different for each end of the bus.
>>>>>>>
>>>>>>> It's all bitfields, so it is a very compact representation.
>>>>>>>
>>>>>>> In addition, I think we need to require that at the start of the
>> s_bus
>>>>>>> implementation in the host or subdev there should be a standard
>> comment
>>>>>>> block describing the possible combinations supported by the hardware:
>>>>>>>
>>>>>>> /* Subdevice foo supports the following bus settings:
>>>>>>>
>>>>>>>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>>>>>>>          RAW_YUV (widths: 8/16, syncs: embedded)
>>>>>>>   bus master: slave
>>>>>>>   vsync polarity: 0/1
>>>>>>>   hsync polarity: 0/1
>>>>>>>   field polarity: not applicable
>>>>>>>   sampling edge pixelclock: 0/1
>>>>>>>   data polarity: 1
>>>>>>> */
>>>>>>>
>>>>>>> This should make it easy for implementers to pick a valid set of bus
>>>>>>> parameters.
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>>       Hans
>>>>>>>
>>>>>>>> +
>>>>>>>>  /* Sub-devices are devices that are connected somehow to the main
>>>>>>>> bridge
>>>>>>>>     device. These devices are usually audio/video
>>>>>>> muxers/encoders/decoders
>>>>>>>> or
>>>>>>>>     sensors and webcam controllers.
>>>>>>>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>>>>>>>
>>>>>>>>     s_routing: see s_routing in audio_ops, except this version is
>> for
>>>>>>>> video
>>>>>>>>  	devices.
>>>>>>>> +
>>>>>>>> +   s_bus: set bus parameters in sub device to configure the bus
>>>>>>>>   */
>>>>>>>>  struct v4l2_subdev_video_ops {
>>>>>>>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output,
>>>> u32
>>>>>>>> config);
>>>>>>>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>>>>>>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm
>>>> *param);
>>>>>>>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>>>>>>> v4l2_frmsizeenum
>>>>>>>> *fsize);
>>>>>>>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>>>>>>>> v4l2_frmivalenum *fival);
>>>>>>>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct
>>>> v4l2_bus_settings
>>>>>>>> *bus);
>>>>>>>>  };
>>>>>>>>
>>>>>>>>  struct v4l2_subdev_ops {
>>>>>>>> --
>>>>>>>> 1.6.0.4
>>>>>>>>
>>>>>>>> --
>>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-
>> media"
>>>>>>>> in
>>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>>>
>>>>>>> --
>>>>>>> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>>>>>
>>>>>>
>>>>> --
>>>>> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>>>
>>> _______________________________________________
>>> Davinci-linux-open-source mailing list
>>> Davinci-linux-open-source@linux.davincidsp.com
>>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>>>
> 
> 

RE: [RFC PATCH] adding support for setting bus parameters in sub device

[Karicheri, Muralidharan]

>> data9-data15 for MT9T031
>> data11-data15 for MT9P031
>But then you could set the host bus width accordingly for example :
>MT9T031 MSB connected do data 9 : HOST Buswidth = 10
>MT9T031 MSB connected to data 15: Host Buswdth = 16

How does the driver know which MSB of the sensor data line is connected to the host bus? This is to be configured in our hardware register. Without this, I need to hardcode it which doesn't seem right.

Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
Phone : 301-515-3736
email: m-karicheri2@ti.com

>-----Original Message-----
>From: Jean-Philippe François [mailto:jp.francois@cynove.com]
>Sent: Monday, June 29, 2009 12:23 PM
>To: Karicheri, Muralidharan
>Cc: Hans Verkuil; davinci-linux-open-source@linux.davincidsp.com; linux-
>media@vger.kernel.org
>Subject: Re: [RFC PATCH] adding support for setting bus parameters in sub
>device
>
>Karicheri, Muralidharan a écrit :
>> Hans,
>>
>> Had hit the send by mistake. Please ignore my previous reply...
>>
>> <snip>
>>>> It seems very unlikely to me that someone would ever choose to e.g.
>zero
>>>> one or more MSB pins instead of the LSB pins in a case like this.
>>>>
>>
>> No case in my experience...
>>
>> <snip>
>>>> Or do you have examples where that actually happens?
>>>>
>> DM365 can work with 10 bit or 12 bit sensors. DM365 CCDC that receives
>the
>> image data needs to know which data lines have valid data. This is done
>by
>> specifying the MSB position. There is ccdc register to configure this
>information. Ideally, you could connect the MSB of sensor to following
>lines on host bus:-
>>
>> data9-data15 for MT9T031
>> data11-data15 for MT9P031
>But then you could set the host bus width accordingly for example :
>MT9T031 MSB connected do data 9 : HOST Buswidth = 10
>MT9T031 MSB connected to data 15: Host Buswdth = 16
>
>Since the host/sensor info are in the same structure, we can have
>several subdev with different host buswidth.
>
>Jean-Philippe François
>
>>
>>
>> So it makes sense to specify this choice in the structure. I have not
>added this earlier since we wanted to use the structure only for sub device
>configuration. It has changed since then.
>>
>> I am also not sure if s_bus() is required since this will get set in the
>platform data which could then be passed to the sub device using the new
>api while loading it. When would host driver call s_bus()?
>>
>>>
>>>> If this never happens, then there is also no need for such a bitfield.
>>>>
>>>> I think I want to actually see someone using this before we add a field
>>>> like that.
>>>>
>>>> Regards,
>>>>
>>>>       Hans
>>>>
>>>>
>>>>>
>>>>> Murali Karicheri
>>>>> Software Design Engineer
>>>>> Texas Instruments Inc.
>>>>> Germantown, MD 20874
>>>>> email: m-karicheri2@ti.com
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>>>>>> Sent: Monday, June 29, 2009 5:26 AM
>>>>>> To: Karicheri, Muralidharan
>>>>>> Cc: linux-media@vger.kernel.org; davinci-linux-open-
>>>>>> source@linux.davincidsp.com
>>>>>> Subject: Re: [RFC PATCH] adding support for setting bus parameters in
>>> sub
>>>>>> device
>>>>>>
>>>>>> Hi Murali,
>>>>>>
>>>>>>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>>>>>>
>>>>>>> This patch adds support for setting bus parameters such as bus type
>>>>>>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>>>>>>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync,
>>>>>>> field
>>>>>>> etc) in sub device. This allows bridge driver to configure the sub
>>>>>>> device
>>>>>>> bus for a specific set of bus parameters through s_bus() function
>call.
>>>>>>> This also can be used to define platform specific bus parameters for
>>>>>>> host and sub-devices.
>>>>>>>
>>>>>>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>>>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>>>>> ---
>>>>>>> Applies to v4l-dvb repository
>>>>>>>
>>>>>>>  include/media/v4l2-subdev.h |   40
>>>>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>>>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
>subdev.h
>>>>>>> index 1785608..2f5ec98 100644
>>>>>>> --- a/include/media/v4l2-subdev.h
>>>>>>> +++ b/include/media/v4l2-subdev.h
>>>>>>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>>>>>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>>>>>> service found */
>>>>>>>  };
>>>>>>>
>>>>>>> +/*
>>>>>>> + * Some sub-devices are connected to the host/bridge device through
>a
>>>>>> bus
>>>>>>> that
>>>>>>> + * carries the clock, vsync, hsync and data. Some interfaces such
>as
>>>>>>> BT.656
>>>>>>> + * carries the sync embedded in the data where as others have
>>> separate
>>>>>>> line
>>>>>>> + * carrying the sync signals. The structure below is used to define
>>>>>>> bus
>>>>>>> + * configuration parameters for host as well as sub-device
>>>>>>> + */
>>>>>>> +enum v4l2_subdev_bus_type {
>>>>>>> +	/* Raw YUV image data bus */
>>>>>>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>>>>>>> +	/* Raw Bayer image data bus */
>>>>>>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct v4l2_bus_settings {
>>>>>>> +	/* yuv or bayer image data bus */
>>>>>>> +	enum v4l2_subdev_bus_type type;
>>>>>>> +	/* subdev bus width */
>>>>>>> +	u8 subdev_width;
>>>>>>> +	/* host bus width */
>>>>>>> +	u8 host_width;
>>>>>>> +	/* embedded sync, set this when sync is embedded in the data
>>> stream
>>>>>> */
>>>>>>> +	unsigned embedded_sync:1;
>>>>>>> +	/* master or slave */
>>>>>>> +	unsigned host_is_master:1;
>>>>>>> +	/* 0 - active low, 1 - active high */
>>>>>>> +	unsigned pol_vsync:1;
>>>>>>> +	/* 0 - active low, 1 - active high */
>>>>>>> +	unsigned pol_hsync:1;
>>>>>>> +	/* 0 - low to high , 1 - high to low */
>>>>>>> +	unsigned pol_field:1;
>>>>>>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>>>>>>> +	unsigned pol_pclock:1;
>>>>>>> +	/* 0 - active low , 1 - active high */
>>>>>>> +	unsigned pol_data:1;
>>>>>>> +};
>>>>>> I've been thinking about this for a while and I think this struct
>should
>>>>>> be extended with the host bus parameters as well:
>>>>>>
>>>>>> struct v4l2_bus_settings {
>>>>>> 	/* yuv or bayer image data bus */
>>>>>> 	enum v4l2_bus_type type;
>>>>>> 	/* embedded sync, set this when sync is embedded in the data
>stream
>>>>>> */
>>>>>> 	unsigned embedded_sync:1;
>>>>>> 	/* master or slave */
>>>>>> 	unsigned host_is_master:1;
>>>>>>
>>>>>> 	/* bus width */
>>>>>> 	unsigned sd_width:8;
>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>> 	unsigned sd_pol_vsync:1;
>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>> 	unsigned sd_pol_hsync:1;
>>>>>> 	/* 0 - low to high, 1 - high to low */
>>>>>> 	unsigned sd_pol_field:1;
>>>>>> 	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>>> 	unsigned sd_edge_pclock:1;
>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>> 	unsigned sd_pol_data:1;
>>>>>>
>>>>>> 	/* host bus width */
>>>>>> 	unsigned host_width:8;
>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>> 	unsigned host_pol_vsync:1;
>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>> 	unsigned host_pol_hsync:1;
>>>>>> 	/* 0 - low to high, 1 - high to low */
>>>>>> 	unsigned host_pol_field:1;
>>>>>> 	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>>> 	unsigned host_edge_pclock:1;
>>>>>> 	/* 0 - active low, 1 - active high */
>>>>>> 	unsigned host_pol_data:1;
>>>>>> };
>>>>>>
>>>>>> It makes sense since you need to setup both ends of the bus, and
>having
>>>>>> both ends defined in the same struct keeps everything together. I
>have
>>>>>> thought about having separate host and subdev structs, but part of
>the
>>>>>> bus
>>>>>> description is always common (bus type, master/slave,
>embedded/separate
>>>>>> syncs), while another part can be different for each end of the bus.
>>>>>>
>>>>>> It's all bitfields, so it is a very compact representation.
>>>>>>
>>>>>> In addition, I think we need to require that at the start of the
>s_bus
>>>>>> implementation in the host or subdev there should be a standard
>comment
>>>>>> block describing the possible combinations supported by the hardware:
>>>>>>
>>>>>> /* Subdevice foo supports the following bus settings:
>>>>>>
>>>>>>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>>>>>>          RAW_YUV (widths: 8/16, syncs: embedded)
>>>>>>   bus master: slave
>>>>>>   vsync polarity: 0/1
>>>>>>   hsync polarity: 0/1
>>>>>>   field polarity: not applicable
>>>>>>   sampling edge pixelclock: 0/1
>>>>>>   data polarity: 1
>>>>>> */
>>>>>>
>>>>>> This should make it easy for implementers to pick a valid set of bus
>>>>>> parameters.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>>       Hans
>>>>>>
>>>>>>> +
>>>>>>>  /* Sub-devices are devices that are connected somehow to the main
>>>>>>> bridge
>>>>>>>     device. These devices are usually audio/video
>>>>>> muxers/encoders/decoders
>>>>>>> or
>>>>>>>     sensors and webcam controllers.
>>>>>>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>>>>>>
>>>>>>>     s_routing: see s_routing in audio_ops, except this version is
>for
>>>>>>> video
>>>>>>>  	devices.
>>>>>>> +
>>>>>>> +   s_bus: set bus parameters in sub device to configure the bus
>>>>>>>   */
>>>>>>>  struct v4l2_subdev_video_ops {
>>>>>>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output,
>>> u32
>>>>>>> config);
>>>>>>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>>>>>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm
>>> *param);
>>>>>>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>>>>>> v4l2_frmsizeenum
>>>>>>> *fsize);
>>>>>>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>>>>>>> v4l2_frmivalenum *fival);
>>>>>>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct
>>> v4l2_bus_settings
>>>>>>> *bus);
>>>>>>>  };
>>>>>>>
>>>>>>>  struct v4l2_subdev_ops {
>>>>>>> --
>>>>>>> 1.6.0.4
>>>>>>>
>>>>>>> --
>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-
>media"
>>>>>>> in
>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>>
>>>>>> --
>>>>>> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>>
>>
>> _______________________________________________
>> Davinci-linux-open-source mailing list
>> Davinci-linux-open-source@linux.davincidsp.com
>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>>
>

Re: [RFC PATCH] adding support for setting bus parameters in sub device

[Jean-Philippe François]

Karicheri, Muralidharan a écrit :
> Hans,
> 
> Had hit the send by mistake. Please ignore my previous reply...
> 
> <snip>
>>> It seems very unlikely to me that someone would ever choose to e.g. zero
>>> one or more MSB pins instead of the LSB pins in a case like this.
>>>
> 
> No case in my experience...
> 
> <snip>
>>> Or do you have examples where that actually happens?
>>>
> DM365 can work with 10 bit or 12 bit sensors. DM365 CCDC that receives the
> image data needs to know which data lines have valid data. This is done by
> specifying the MSB position. There is ccdc register to configure this information. Ideally, you could connect the MSB of sensor to following lines on host bus:-
> 
> data9-data15 for MT9T031
> data11-data15 for MT9P031
But then you could set the host bus width accordingly for example :
MT9T031 MSB connected do data 9 : HOST Buswidth = 10
MT9T031 MSB connected to data 15: Host Buswdth = 16

Since the host/sensor info are in the same structure, we can have 
several subdev with different host buswidth.

Jean-Philippe François

> 
> 
> So it makes sense to specify this choice in the structure. I have not added this earlier since we wanted to use the structure only for sub device configuration. It has changed since then.
> 
> I am also not sure if s_bus() is required since this will get set in the platform data which could then be passed to the sub device using the new api while loading it. When would host driver call s_bus()?
> 
>>
>>> If this never happens, then there is also no need for such a bitfield.
>>>
>>> I think I want to actually see someone using this before we add a field
>>> like that.
>>>
>>> Regards,
>>>
>>>       Hans
>>>
>>>
>>>>
>>>> Murali Karicheri
>>>> Software Design Engineer
>>>> Texas Instruments Inc.
>>>> Germantown, MD 20874
>>>> email: m-karicheri2@ti.com
>>>>
>>>>> -----Original Message-----
>>>>> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>>>>> Sent: Monday, June 29, 2009 5:26 AM
>>>>> To: Karicheri, Muralidharan
>>>>> Cc: linux-media@vger.kernel.org; davinci-linux-open-
>>>>> source@linux.davincidsp.com
>>>>> Subject: Re: [RFC PATCH] adding support for setting bus parameters in
>> sub
>>>>> device
>>>>>
>>>>> Hi Murali,
>>>>>
>>>>>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>>>>>
>>>>>> This patch adds support for setting bus parameters such as bus type
>>>>>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>>>>>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync,
>>>>>> field
>>>>>> etc) in sub device. This allows bridge driver to configure the sub
>>>>>> device
>>>>>> bus for a specific set of bus parameters through s_bus() function call.
>>>>>> This also can be used to define platform specific bus parameters for
>>>>>> host and sub-devices.
>>>>>>
>>>>>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>>>> ---
>>>>>> Applies to v4l-dvb repository
>>>>>>
>>>>>>  include/media/v4l2-subdev.h |   40
>>>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>>>>>> index 1785608..2f5ec98 100644
>>>>>> --- a/include/media/v4l2-subdev.h
>>>>>> +++ b/include/media/v4l2-subdev.h
>>>>>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>>>>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>>>>> service found */
>>>>>>  };
>>>>>>
>>>>>> +/*
>>>>>> + * Some sub-devices are connected to the host/bridge device through a
>>>>> bus
>>>>>> that
>>>>>> + * carries the clock, vsync, hsync and data. Some interfaces such as
>>>>>> BT.656
>>>>>> + * carries the sync embedded in the data where as others have
>> separate
>>>>>> line
>>>>>> + * carrying the sync signals. The structure below is used to define
>>>>>> bus
>>>>>> + * configuration parameters for host as well as sub-device
>>>>>> + */
>>>>>> +enum v4l2_subdev_bus_type {
>>>>>> +	/* Raw YUV image data bus */
>>>>>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>>>>>> +	/* Raw Bayer image data bus */
>>>>>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>>>>>> +};
>>>>>> +
>>>>>> +struct v4l2_bus_settings {
>>>>>> +	/* yuv or bayer image data bus */
>>>>>> +	enum v4l2_subdev_bus_type type;
>>>>>> +	/* subdev bus width */
>>>>>> +	u8 subdev_width;
>>>>>> +	/* host bus width */
>>>>>> +	u8 host_width;
>>>>>> +	/* embedded sync, set this when sync is embedded in the data
>> stream
>>>>> */
>>>>>> +	unsigned embedded_sync:1;
>>>>>> +	/* master or slave */
>>>>>> +	unsigned host_is_master:1;
>>>>>> +	/* 0 - active low, 1 - active high */
>>>>>> +	unsigned pol_vsync:1;
>>>>>> +	/* 0 - active low, 1 - active high */
>>>>>> +	unsigned pol_hsync:1;
>>>>>> +	/* 0 - low to high , 1 - high to low */
>>>>>> +	unsigned pol_field:1;
>>>>>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>>>>>> +	unsigned pol_pclock:1;
>>>>>> +	/* 0 - active low , 1 - active high */
>>>>>> +	unsigned pol_data:1;
>>>>>> +};
>>>>> I've been thinking about this for a while and I think this struct should
>>>>> be extended with the host bus parameters as well:
>>>>>
>>>>> struct v4l2_bus_settings {
>>>>> 	/* yuv or bayer image data bus */
>>>>> 	enum v4l2_bus_type type;
>>>>> 	/* embedded sync, set this when sync is embedded in the data stream
>>>>> */
>>>>> 	unsigned embedded_sync:1;
>>>>> 	/* master or slave */
>>>>> 	unsigned host_is_master:1;
>>>>>
>>>>> 	/* bus width */
>>>>> 	unsigned sd_width:8;
>>>>> 	/* 0 - active low, 1 - active high */
>>>>> 	unsigned sd_pol_vsync:1;
>>>>> 	/* 0 - active low, 1 - active high */
>>>>> 	unsigned sd_pol_hsync:1;
>>>>> 	/* 0 - low to high, 1 - high to low */
>>>>> 	unsigned sd_pol_field:1;
>>>>> 	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>> 	unsigned sd_edge_pclock:1;
>>>>> 	/* 0 - active low, 1 - active high */
>>>>> 	unsigned sd_pol_data:1;
>>>>>
>>>>> 	/* host bus width */
>>>>> 	unsigned host_width:8;
>>>>> 	/* 0 - active low, 1 - active high */
>>>>> 	unsigned host_pol_vsync:1;
>>>>> 	/* 0 - active low, 1 - active high */
>>>>> 	unsigned host_pol_hsync:1;
>>>>> 	/* 0 - low to high, 1 - high to low */
>>>>> 	unsigned host_pol_field:1;
>>>>> 	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>> 	unsigned host_edge_pclock:1;
>>>>> 	/* 0 - active low, 1 - active high */
>>>>> 	unsigned host_pol_data:1;
>>>>> };
>>>>>
>>>>> It makes sense since you need to setup both ends of the bus, and having
>>>>> both ends defined in the same struct keeps everything together. I have
>>>>> thought about having separate host and subdev structs, but part of the
>>>>> bus
>>>>> description is always common (bus type, master/slave, embedded/separate
>>>>> syncs), while another part can be different for each end of the bus.
>>>>>
>>>>> It's all bitfields, so it is a very compact representation.
>>>>>
>>>>> In addition, I think we need to require that at the start of the s_bus
>>>>> implementation in the host or subdev there should be a standard comment
>>>>> block describing the possible combinations supported by the hardware:
>>>>>
>>>>> /* Subdevice foo supports the following bus settings:
>>>>>
>>>>>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>>>>>          RAW_YUV (widths: 8/16, syncs: embedded)
>>>>>   bus master: slave
>>>>>   vsync polarity: 0/1
>>>>>   hsync polarity: 0/1
>>>>>   field polarity: not applicable
>>>>>   sampling edge pixelclock: 0/1
>>>>>   data polarity: 1
>>>>> */
>>>>>
>>>>> This should make it easy for implementers to pick a valid set of bus
>>>>> parameters.
>>>>>
>>>>> Regards,
>>>>>
>>>>>       Hans
>>>>>
>>>>>> +
>>>>>>  /* Sub-devices are devices that are connected somehow to the main
>>>>>> bridge
>>>>>>     device. These devices are usually audio/video
>>>>> muxers/encoders/decoders
>>>>>> or
>>>>>>     sensors and webcam controllers.
>>>>>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>>>>>
>>>>>>     s_routing: see s_routing in audio_ops, except this version is for
>>>>>> video
>>>>>>  	devices.
>>>>>> +
>>>>>> +   s_bus: set bus parameters in sub device to configure the bus
>>>>>>   */
>>>>>>  struct v4l2_subdev_video_ops {
>>>>>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output,
>> u32
>>>>>> config);
>>>>>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>>>>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm
>> *param);
>>>>>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>>>>> v4l2_frmsizeenum
>>>>>> *fsize);
>>>>>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>>>>>> v4l2_frmivalenum *fival);
>>>>>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct
>> v4l2_bus_settings
>>>>>> *bus);
>>>>>>  };
>>>>>>
>>>>>>  struct v4l2_subdev_ops {
>>>>>> --
>>>>>> 1.6.0.4
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-media"
>>>>>> in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>
>>>>> --
>>>>> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>>>
>>>>
>>>>
>>>
>>> --
>>> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>
> 
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 

RE: [RFC PATCH] adding support for setting bus parameters in sub device

[Karicheri, Muralidharan]

Hans,

Had hit the send by mistake. Please ignore my previous reply...

<snip>
>>It seems very unlikely to me that someone would ever choose to e.g. zero
>>one or more MSB pins instead of the LSB pins in a case like this.
>>

No case in my experience...

<snip>
>>Or do you have examples where that actually happens?
>>
DM365 can work with 10 bit or 12 bit sensors. DM365 CCDC that receives the
image data needs to know which data lines have valid data. This is done by
specifying the MSB position. There is ccdc register to configure this information. Ideally, you could connect the MSB of sensor to following lines on host bus:-

data9-data15 for MT9T031
data11-data15 for MT9P031


So it makes sense to specify this choice in the structure. I have not added this earlier since we wanted to use the structure only for sub device configuration. It has changed since then.

I am also not sure if s_bus() is required since this will get set in the platform data which could then be passed to the sub device using the new api while loading it. When would host driver call s_bus()?

>
>
>>If this never happens, then there is also no need for such a bitfield.
>>
>>I think I want to actually see someone using this before we add a field
>>like that.
>>
>>Regards,
>>
>>       Hans
>>
>>
>>>
>>>
>>> Murali Karicheri
>>> Software Design Engineer
>>> Texas Instruments Inc.
>>> Germantown, MD 20874
>>> email: m-karicheri2@ti.com
>>>
>>>>-----Original Message-----
>>>>From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>>>>Sent: Monday, June 29, 2009 5:26 AM
>>>>To: Karicheri, Muralidharan
>>>>Cc: linux-media@vger.kernel.org; davinci-linux-open-
>>>>source@linux.davincidsp.com
>>>>Subject: Re: [RFC PATCH] adding support for setting bus parameters in
>sub
>>>>device
>>>>
>>>>Hi Murali,
>>>>
>>>>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>>>>
>>>>> This patch adds support for setting bus parameters such as bus type
>>>>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>>>>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync,
>>>>> field
>>>>> etc) in sub device. This allows bridge driver to configure the sub
>>>>> device
>>>>> bus for a specific set of bus parameters through s_bus() function call.
>>>>> This also can be used to define platform specific bus parameters for
>>>>> host and sub-devices.
>>>>>
>>>>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>>> ---
>>>>> Applies to v4l-dvb repository
>>>>>
>>>>>  include/media/v4l2-subdev.h |   40
>>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>>>>> index 1785608..2f5ec98 100644
>>>>> --- a/include/media/v4l2-subdev.h
>>>>> +++ b/include/media/v4l2-subdev.h
>>>>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>>>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>>>>service found */
>>>>>  };
>>>>>
>>>>> +/*
>>>>> + * Some sub-devices are connected to the host/bridge device through a
>>>>bus
>>>>> that
>>>>> + * carries the clock, vsync, hsync and data. Some interfaces such as
>>>>> BT.656
>>>>> + * carries the sync embedded in the data where as others have
>separate
>>>>> line
>>>>> + * carrying the sync signals. The structure below is used to define
>>>>> bus
>>>>> + * configuration parameters for host as well as sub-device
>>>>> + */
>>>>> +enum v4l2_subdev_bus_type {
>>>>> +	/* Raw YUV image data bus */
>>>>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>>>>> +	/* Raw Bayer image data bus */
>>>>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>>>>> +};
>>>>> +
>>>>> +struct v4l2_bus_settings {
>>>>> +	/* yuv or bayer image data bus */
>>>>> +	enum v4l2_subdev_bus_type type;
>>>>> +	/* subdev bus width */
>>>>> +	u8 subdev_width;
>>>>> +	/* host bus width */
>>>>> +	u8 host_width;
>>>>> +	/* embedded sync, set this when sync is embedded in the data
>stream
>>>>*/
>>>>> +	unsigned embedded_sync:1;
>>>>> +	/* master or slave */
>>>>> +	unsigned host_is_master:1;
>>>>> +	/* 0 - active low, 1 - active high */
>>>>> +	unsigned pol_vsync:1;
>>>>> +	/* 0 - active low, 1 - active high */
>>>>> +	unsigned pol_hsync:1;
>>>>> +	/* 0 - low to high , 1 - high to low */
>>>>> +	unsigned pol_field:1;
>>>>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>>>>> +	unsigned pol_pclock:1;
>>>>> +	/* 0 - active low , 1 - active high */
>>>>> +	unsigned pol_data:1;
>>>>> +};
>>>>
>>>>I've been thinking about this for a while and I think this struct should
>>>>be extended with the host bus parameters as well:
>>>>
>>>>struct v4l2_bus_settings {
>>>>	/* yuv or bayer image data bus */
>>>>	enum v4l2_bus_type type;
>>>>	/* embedded sync, set this when sync is embedded in the data stream
>>>>*/
>>>>	unsigned embedded_sync:1;
>>>>	/* master or slave */
>>>>	unsigned host_is_master:1;
>>>>
>>>>	/* bus width */
>>>>	unsigned sd_width:8;
>>>>	/* 0 - active low, 1 - active high */
>>>>	unsigned sd_pol_vsync:1;
>>>>	/* 0 - active low, 1 - active high */
>>>>	unsigned sd_pol_hsync:1;
>>>>	/* 0 - low to high, 1 - high to low */
>>>>	unsigned sd_pol_field:1;
>>>>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>	unsigned sd_edge_pclock:1;
>>>>	/* 0 - active low, 1 - active high */
>>>>	unsigned sd_pol_data:1;
>>>>
>>>>	/* host bus width */
>>>>	unsigned host_width:8;
>>>>	/* 0 - active low, 1 - active high */
>>>>	unsigned host_pol_vsync:1;
>>>>	/* 0 - active low, 1 - active high */
>>>>	unsigned host_pol_hsync:1;
>>>>	/* 0 - low to high, 1 - high to low */
>>>>	unsigned host_pol_field:1;
>>>>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>>	unsigned host_edge_pclock:1;
>>>>	/* 0 - active low, 1 - active high */
>>>>	unsigned host_pol_data:1;
>>>>};
>>>>
>>>>It makes sense since you need to setup both ends of the bus, and having
>>>>both ends defined in the same struct keeps everything together. I have
>>>>thought about having separate host and subdev structs, but part of the
>>>> bus
>>>>description is always common (bus type, master/slave, embedded/separate
>>>>syncs), while another part can be different for each end of the bus.
>>>>
>>>>It's all bitfields, so it is a very compact representation.
>>>>
>>>>In addition, I think we need to require that at the start of the s_bus
>>>>implementation in the host or subdev there should be a standard comment
>>>>block describing the possible combinations supported by the hardware:
>>>>
>>>>/* Subdevice foo supports the following bus settings:
>>>>
>>>>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>>>>          RAW_YUV (widths: 8/16, syncs: embedded)
>>>>   bus master: slave
>>>>   vsync polarity: 0/1
>>>>   hsync polarity: 0/1
>>>>   field polarity: not applicable
>>>>   sampling edge pixelclock: 0/1
>>>>   data polarity: 1
>>>> */
>>>>
>>>>This should make it easy for implementers to pick a valid set of bus
>>>>parameters.
>>>>
>>>>Regards,
>>>>
>>>>       Hans
>>>>
>>>>> +
>>>>>  /* Sub-devices are devices that are connected somehow to the main
>>>>> bridge
>>>>>     device. These devices are usually audio/video
>>>>muxers/encoders/decoders
>>>>> or
>>>>>     sensors and webcam controllers.
>>>>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>>>>
>>>>>     s_routing: see s_routing in audio_ops, except this version is for
>>>>> video
>>>>>  	devices.
>>>>> +
>>>>> +   s_bus: set bus parameters in sub device to configure the bus
>>>>>   */
>>>>>  struct v4l2_subdev_video_ops {
>>>>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output,
>u32
>>>>> config);
>>>>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>>>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm
>*param);
>>>>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>>>>v4l2_frmsizeenum
>>>>> *fsize);
>>>>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>>>>> v4l2_frmivalenum *fival);
>>>>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct
>v4l2_bus_settings
>>>>> *bus);
>>>>>  };
>>>>>
>>>>>  struct v4l2_subdev_ops {
>>>>> --
>>>>> 1.6.0.4
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-media"
>>>>> in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>
>>>>
>>>>--
>>>>Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>>
>>>
>>>
>>>
>>
>>
>>--
>>Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>

RE: [RFC PATCH] adding support for setting bus parameters in sub device

[Karicheri, Muralidharan]

>It seems very unlikely to me that someone would ever choose to e.g. zero
>one or more MSB pins instead of the LSB pins in a case like this.
>
No case in my experience...
>Or do you have examples where that actually happens?
>
DM365 can work with 10 bit or 12 bit sensors. DM365 CCDC that receives the image data needs to know which data lines have valid data. This is done by specifying the MSB position. Ideally, you could connect the MSB of sensor to following lines on host bus
9-15 for MT9T031
11-15 for MT9P031) 


>If this never happens, then there is also no need for such a bitfield.
>
>I think I want to actually see someone using this before we add a field
>like that.
>
>Regards,
>
>       Hans
>
>
>>
>>
>> Murali Karicheri
>> Software Design Engineer
>> Texas Instruments Inc.
>> Germantown, MD 20874
>> email: m-karicheri2@ti.com
>>
>>>-----Original Message-----
>>>From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>>>Sent: Monday, June 29, 2009 5:26 AM
>>>To: Karicheri, Muralidharan
>>>Cc: linux-media@vger.kernel.org; davinci-linux-open-
>>>source@linux.davincidsp.com
>>>Subject: Re: [RFC PATCH] adding support for setting bus parameters in sub
>>>device
>>>
>>>Hi Murali,
>>>
>>>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>>>
>>>> This patch adds support for setting bus parameters such as bus type
>>>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>>>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync,
>>>> field
>>>> etc) in sub device. This allows bridge driver to configure the sub
>>>> device
>>>> bus for a specific set of bus parameters through s_bus() function call.
>>>> This also can be used to define platform specific bus parameters for
>>>> host and sub-devices.
>>>>
>>>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>> ---
>>>> Applies to v4l-dvb repository
>>>>
>>>>  include/media/v4l2-subdev.h |   40
>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>>>> index 1785608..2f5ec98 100644
>>>> --- a/include/media/v4l2-subdev.h
>>>> +++ b/include/media/v4l2-subdev.h
>>>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>>>service found */
>>>>  };
>>>>
>>>> +/*
>>>> + * Some sub-devices are connected to the host/bridge device through a
>>>bus
>>>> that
>>>> + * carries the clock, vsync, hsync and data. Some interfaces such as
>>>> BT.656
>>>> + * carries the sync embedded in the data where as others have separate
>>>> line
>>>> + * carrying the sync signals. The structure below is used to define
>>>> bus
>>>> + * configuration parameters for host as well as sub-device
>>>> + */
>>>> +enum v4l2_subdev_bus_type {
>>>> +	/* Raw YUV image data bus */
>>>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>>>> +	/* Raw Bayer image data bus */
>>>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>>>> +};
>>>> +
>>>> +struct v4l2_bus_settings {
>>>> +	/* yuv or bayer image data bus */
>>>> +	enum v4l2_subdev_bus_type type;
>>>> +	/* subdev bus width */
>>>> +	u8 subdev_width;
>>>> +	/* host bus width */
>>>> +	u8 host_width;
>>>> +	/* embedded sync, set this when sync is embedded in the data stream
>>>*/
>>>> +	unsigned embedded_sync:1;
>>>> +	/* master or slave */
>>>> +	unsigned host_is_master:1;
>>>> +	/* 0 - active low, 1 - active high */
>>>> +	unsigned pol_vsync:1;
>>>> +	/* 0 - active low, 1 - active high */
>>>> +	unsigned pol_hsync:1;
>>>> +	/* 0 - low to high , 1 - high to low */
>>>> +	unsigned pol_field:1;
>>>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>>>> +	unsigned pol_pclock:1;
>>>> +	/* 0 - active low , 1 - active high */
>>>> +	unsigned pol_data:1;
>>>> +};
>>>
>>>I've been thinking about this for a while and I think this struct should
>>>be extended with the host bus parameters as well:
>>>
>>>struct v4l2_bus_settings {
>>>	/* yuv or bayer image data bus */
>>>	enum v4l2_bus_type type;
>>>	/* embedded sync, set this when sync is embedded in the data stream
>>>*/
>>>	unsigned embedded_sync:1;
>>>	/* master or slave */
>>>	unsigned host_is_master:1;
>>>
>>>	/* bus width */
>>>	unsigned sd_width:8;
>>>	/* 0 - active low, 1 - active high */
>>>	unsigned sd_pol_vsync:1;
>>>	/* 0 - active low, 1 - active high */
>>>	unsigned sd_pol_hsync:1;
>>>	/* 0 - low to high, 1 - high to low */
>>>	unsigned sd_pol_field:1;
>>>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>	unsigned sd_edge_pclock:1;
>>>	/* 0 - active low, 1 - active high */
>>>	unsigned sd_pol_data:1;
>>>
>>>	/* host bus width */
>>>	unsigned host_width:8;
>>>	/* 0 - active low, 1 - active high */
>>>	unsigned host_pol_vsync:1;
>>>	/* 0 - active low, 1 - active high */
>>>	unsigned host_pol_hsync:1;
>>>	/* 0 - low to high, 1 - high to low */
>>>	unsigned host_pol_field:1;
>>>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>>	unsigned host_edge_pclock:1;
>>>	/* 0 - active low, 1 - active high */
>>>	unsigned host_pol_data:1;
>>>};
>>>
>>>It makes sense since you need to setup both ends of the bus, and having
>>>both ends defined in the same struct keeps everything together. I have
>>>thought about having separate host and subdev structs, but part of the
>>> bus
>>>description is always common (bus type, master/slave, embedded/separate
>>>syncs), while another part can be different for each end of the bus.
>>>
>>>It's all bitfields, so it is a very compact representation.
>>>
>>>In addition, I think we need to require that at the start of the s_bus
>>>implementation in the host or subdev there should be a standard comment
>>>block describing the possible combinations supported by the hardware:
>>>
>>>/* Subdevice foo supports the following bus settings:
>>>
>>>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>>>          RAW_YUV (widths: 8/16, syncs: embedded)
>>>   bus master: slave
>>>   vsync polarity: 0/1
>>>   hsync polarity: 0/1
>>>   field polarity: not applicable
>>>   sampling edge pixelclock: 0/1
>>>   data polarity: 1
>>> */
>>>
>>>This should make it easy for implementers to pick a valid set of bus
>>>parameters.
>>>
>>>Regards,
>>>
>>>       Hans
>>>
>>>> +
>>>>  /* Sub-devices are devices that are connected somehow to the main
>>>> bridge
>>>>     device. These devices are usually audio/video
>>>muxers/encoders/decoders
>>>> or
>>>>     sensors and webcam controllers.
>>>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>>>
>>>>     s_routing: see s_routing in audio_ops, except this version is for
>>>> video
>>>>  	devices.
>>>> +
>>>> +   s_bus: set bus parameters in sub device to configure the bus
>>>>   */
>>>>  struct v4l2_subdev_video_ops {
>>>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
>>>> config);
>>>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
>>>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>>>v4l2_frmsizeenum
>>>> *fsize);
>>>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>>>> v4l2_frmivalenum *fival);
>>>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_bus_settings
>>>> *bus);
>>>>  };
>>>>
>>>>  struct v4l2_subdev_ops {
>>>> --
>>>> 1.6.0.4
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-media"
>>>> in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>>--
>>>Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>>
>>
>>
>>
>
>
>--
>Hans Verkuil - video4linux developer - sponsored by TANDBERG
>

RE: [RFC PATCH] adding support for setting bus parameters in sub device

[Hans Verkuil]

> Hans,
>
> When connecting a sensor like mt9t031 to SoC like DM355, DM6446 etc,
> driver also need to know which MSB of the sensor data bus connected to
> which host bus. For example, on DM365, we have following connection:-
>
> data 9 (MSB) of the sensor is connected to data 11 of the host bus. For 10
> bit sensor, this means, the lower 2 bits of the received data are zeros
> and for a 12 bit sensor, it has valid data.
>
> So I suggest including another field for this.
>
> unsigned host_msb:5; (8 - 15) ??

It seems very unlikely to me that someone would ever choose to e.g. zero
one or more MSB pins instead of the LSB pins in a case like this.

Or do you have examples where that actually happens?

If this never happens, then there is also no need for such a bitfield.

I think I want to actually see someone using this before we add a field
like that.

Regards,

       Hans


>
>
> Murali Karicheri
> Software Design Engineer
> Texas Instruments Inc.
> Germantown, MD 20874
> email: m-karicheri2@ti.com
>
>>-----Original Message-----
>>From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>>Sent: Monday, June 29, 2009 5:26 AM
>>To: Karicheri, Muralidharan
>>Cc: linux-media@vger.kernel.org; davinci-linux-open-
>>source@linux.davincidsp.com
>>Subject: Re: [RFC PATCH] adding support for setting bus parameters in sub
>>device
>>
>>Hi Murali,
>>
>>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>>
>>> This patch adds support for setting bus parameters such as bus type
>>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync,
>>> field
>>> etc) in sub device. This allows bridge driver to configure the sub
>>> device
>>> bus for a specific set of bus parameters through s_bus() function call.
>>> This also can be used to define platform specific bus parameters for
>>> host and sub-devices.
>>>
>>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> ---
>>> Applies to v4l-dvb repository
>>>
>>>  include/media/v4l2-subdev.h |   40
>>> ++++++++++++++++++++++++++++++++++++++++
>>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>>> index 1785608..2f5ec98 100644
>>> --- a/include/media/v4l2-subdev.h
>>> +++ b/include/media/v4l2-subdev.h
>>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>>service found */
>>>  };
>>>
>>> +/*
>>> + * Some sub-devices are connected to the host/bridge device through a
>>bus
>>> that
>>> + * carries the clock, vsync, hsync and data. Some interfaces such as
>>> BT.656
>>> + * carries the sync embedded in the data where as others have separate
>>> line
>>> + * carrying the sync signals. The structure below is used to define
>>> bus
>>> + * configuration parameters for host as well as sub-device
>>> + */
>>> +enum v4l2_subdev_bus_type {
>>> +	/* Raw YUV image data bus */
>>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>>> +	/* Raw Bayer image data bus */
>>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>>> +};
>>> +
>>> +struct v4l2_bus_settings {
>>> +	/* yuv or bayer image data bus */
>>> +	enum v4l2_subdev_bus_type type;
>>> +	/* subdev bus width */
>>> +	u8 subdev_width;
>>> +	/* host bus width */
>>> +	u8 host_width;
>>> +	/* embedded sync, set this when sync is embedded in the data stream
>>*/
>>> +	unsigned embedded_sync:1;
>>> +	/* master or slave */
>>> +	unsigned host_is_master:1;
>>> +	/* 0 - active low, 1 - active high */
>>> +	unsigned pol_vsync:1;
>>> +	/* 0 - active low, 1 - active high */
>>> +	unsigned pol_hsync:1;
>>> +	/* 0 - low to high , 1 - high to low */
>>> +	unsigned pol_field:1;
>>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>>> +	unsigned pol_pclock:1;
>>> +	/* 0 - active low , 1 - active high */
>>> +	unsigned pol_data:1;
>>> +};
>>
>>I've been thinking about this for a while and I think this struct should
>>be extended with the host bus parameters as well:
>>
>>struct v4l2_bus_settings {
>>	/* yuv or bayer image data bus */
>>	enum v4l2_bus_type type;
>>	/* embedded sync, set this when sync is embedded in the data stream
>>*/
>>	unsigned embedded_sync:1;
>>	/* master or slave */
>>	unsigned host_is_master:1;
>>
>>	/* bus width */
>>	unsigned sd_width:8;
>>	/* 0 - active low, 1 - active high */
>>	unsigned sd_pol_vsync:1;
>>	/* 0 - active low, 1 - active high */
>>	unsigned sd_pol_hsync:1;
>>	/* 0 - low to high, 1 - high to low */
>>	unsigned sd_pol_field:1;
>>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>	unsigned sd_edge_pclock:1;
>>	/* 0 - active low, 1 - active high */
>>	unsigned sd_pol_data:1;
>>
>>	/* host bus width */
>>	unsigned host_width:8;
>>	/* 0 - active low, 1 - active high */
>>	unsigned host_pol_vsync:1;
>>	/* 0 - active low, 1 - active high */
>>	unsigned host_pol_hsync:1;
>>	/* 0 - low to high, 1 - high to low */
>>	unsigned host_pol_field:1;
>>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>>	unsigned host_edge_pclock:1;
>>	/* 0 - active low, 1 - active high */
>>	unsigned host_pol_data:1;
>>};
>>
>>It makes sense since you need to setup both ends of the bus, and having
>>both ends defined in the same struct keeps everything together. I have
>>thought about having separate host and subdev structs, but part of the
>> bus
>>description is always common (bus type, master/slave, embedded/separate
>>syncs), while another part can be different for each end of the bus.
>>
>>It's all bitfields, so it is a very compact representation.
>>
>>In addition, I think we need to require that at the start of the s_bus
>>implementation in the host or subdev there should be a standard comment
>>block describing the possible combinations supported by the hardware:
>>
>>/* Subdevice foo supports the following bus settings:
>>
>>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>>          RAW_YUV (widths: 8/16, syncs: embedded)
>>   bus master: slave
>>   vsync polarity: 0/1
>>   hsync polarity: 0/1
>>   field polarity: not applicable
>>   sampling edge pixelclock: 0/1
>>   data polarity: 1
>> */
>>
>>This should make it easy for implementers to pick a valid set of bus
>>parameters.
>>
>>Regards,
>>
>>       Hans
>>
>>> +
>>>  /* Sub-devices are devices that are connected somehow to the main
>>> bridge
>>>     device. These devices are usually audio/video
>>muxers/encoders/decoders
>>> or
>>>     sensors and webcam controllers.
>>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>>
>>>     s_routing: see s_routing in audio_ops, except this version is for
>>> video
>>>  	devices.
>>> +
>>> +   s_bus: set bus parameters in sub device to configure the bus
>>>   */
>>>  struct v4l2_subdev_video_ops {
>>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
>>> config);
>>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
>>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>>v4l2_frmsizeenum
>>> *fsize);
>>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>>> v4l2_frmivalenum *fival);
>>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_bus_settings
>>> *bus);
>>>  };
>>>
>>>  struct v4l2_subdev_ops {
>>> --
>>> 1.6.0.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-media"
>>> in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>--
>>Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>
>
>
>


-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

RE: [RFC PATCH] adding support for setting bus parameters in sub device

[Karicheri, Muralidharan]

Hans,

When connecting a sensor like mt9t031 to SoC like DM355, DM6446 etc, driver also need to know which MSB of the sensor data bus connected to which host bus. For example, on DM365, we have following connection:-

data 9 (MSB) of the sensor is connected to data 11 of the host bus. For 10 bit sensor, this means, the lower 2 bits of the received data are zeros and for a 12 bit sensor, it has valid data. 

So I suggest including another field for this. 

unsigned host_msb:5; (8 - 15) ??


Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
email: m-karicheri2@ti.com

>-----Original Message-----
>From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
>Sent: Monday, June 29, 2009 5:26 AM
>To: Karicheri, Muralidharan
>Cc: linux-media@vger.kernel.org; davinci-linux-open-
>source@linux.davincidsp.com
>Subject: Re: [RFC PATCH] adding support for setting bus parameters in sub
>device
>
>Hi Murali,
>
>> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>>
>> This patch adds support for setting bus parameters such as bus type
>> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
>> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field
>> etc) in sub device. This allows bridge driver to configure the sub device
>> bus for a specific set of bus parameters through s_bus() function call.
>> This also can be used to define platform specific bus parameters for
>> host and sub-devices.
>>
>> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>> ---
>> Applies to v4l-dvb repository
>>
>>  include/media/v4l2-subdev.h |   40
>> ++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index 1785608..2f5ec98 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no
>service found */
>>  };
>>
>> +/*
>> + * Some sub-devices are connected to the host/bridge device through a
>bus
>> that
>> + * carries the clock, vsync, hsync and data. Some interfaces such as
>> BT.656
>> + * carries the sync embedded in the data where as others have separate
>> line
>> + * carrying the sync signals. The structure below is used to define bus
>> + * configuration parameters for host as well as sub-device
>> + */
>> +enum v4l2_subdev_bus_type {
>> +	/* Raw YUV image data bus */
>> +	V4L2_SUBDEV_BUS_RAW_YUV,
>> +	/* Raw Bayer image data bus */
>> +	V4L2_SUBDEV_BUS_RAW_BAYER
>> +};
>> +
>> +struct v4l2_bus_settings {
>> +	/* yuv or bayer image data bus */
>> +	enum v4l2_subdev_bus_type type;
>> +	/* subdev bus width */
>> +	u8 subdev_width;
>> +	/* host bus width */
>> +	u8 host_width;
>> +	/* embedded sync, set this when sync is embedded in the data stream
>*/
>> +	unsigned embedded_sync:1;
>> +	/* master or slave */
>> +	unsigned host_is_master:1;
>> +	/* 0 - active low, 1 - active high */
>> +	unsigned pol_vsync:1;
>> +	/* 0 - active low, 1 - active high */
>> +	unsigned pol_hsync:1;
>> +	/* 0 - low to high , 1 - high to low */
>> +	unsigned pol_field:1;
>> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
>> +	unsigned pol_pclock:1;
>> +	/* 0 - active low , 1 - active high */
>> +	unsigned pol_data:1;
>> +};
>
>I've been thinking about this for a while and I think this struct should
>be extended with the host bus parameters as well:
>
>struct v4l2_bus_settings {
>	/* yuv or bayer image data bus */
>	enum v4l2_bus_type type;
>	/* embedded sync, set this when sync is embedded in the data stream
>*/
>	unsigned embedded_sync:1;
>	/* master or slave */
>	unsigned host_is_master:1;
>
>	/* bus width */
>	unsigned sd_width:8;
>	/* 0 - active low, 1 - active high */
>	unsigned sd_pol_vsync:1;
>	/* 0 - active low, 1 - active high */
>	unsigned sd_pol_hsync:1;
>	/* 0 - low to high, 1 - high to low */
>	unsigned sd_pol_field:1;
>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>	unsigned sd_edge_pclock:1;
>	/* 0 - active low, 1 - active high */
>	unsigned sd_pol_data:1;
>
>	/* host bus width */
>	unsigned host_width:8;
>	/* 0 - active low, 1 - active high */
>	unsigned host_pol_vsync:1;
>	/* 0 - active low, 1 - active high */
>	unsigned host_pol_hsync:1;
>	/* 0 - low to high, 1 - high to low */
>	unsigned host_pol_field:1;
>	/* 0 - sample at falling edge, 1 - sample at rising edge */
>	unsigned host_edge_pclock:1;
>	/* 0 - active low, 1 - active high */
>	unsigned host_pol_data:1;
>};
>
>It makes sense since you need to setup both ends of the bus, and having
>both ends defined in the same struct keeps everything together. I have
>thought about having separate host and subdev structs, but part of the bus
>description is always common (bus type, master/slave, embedded/separate
>syncs), while another part can be different for each end of the bus.
>
>It's all bitfields, so it is a very compact representation.
>
>In addition, I think we need to require that at the start of the s_bus
>implementation in the host or subdev there should be a standard comment
>block describing the possible combinations supported by the hardware:
>
>/* Subdevice foo supports the following bus settings:
>
>   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
>          RAW_YUV (widths: 8/16, syncs: embedded)
>   bus master: slave
>   vsync polarity: 0/1
>   hsync polarity: 0/1
>   field polarity: not applicable
>   sampling edge pixelclock: 0/1
>   data polarity: 1
> */
>
>This should make it easy for implementers to pick a valid set of bus
>parameters.
>
>Regards,
>
>       Hans
>
>> +
>>  /* Sub-devices are devices that are connected somehow to the main bridge
>>     device. These devices are usually audio/video
>muxers/encoders/decoders
>> or
>>     sensors and webcam controllers.
>> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>>
>>     s_routing: see s_routing in audio_ops, except this version is for
>> video
>>  	devices.
>> +
>> +   s_bus: set bus parameters in sub device to configure the bus
>>   */
>>  struct v4l2_subdev_video_ops {
>>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
>> config);
>> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
>>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct
>v4l2_frmsizeenum
>> *fsize);
>>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
>> v4l2_frmivalenum *fival);
>> +	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_bus_settings
>> *bus);
>>  };
>>
>>  struct v4l2_subdev_ops {
>> --
>> 1.6.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>--
>Hans Verkuil - video4linux developer - sponsored by TANDBERG
>

Re: [RFC PATCH] adding support for setting bus parameters in sub device

[Hans Verkuil]

Hi Murali,

> From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>
>
> This patch adds support for setting bus parameters such as bus type
> (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
> image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field
> etc) in sub device. This allows bridge driver to configure the sub device
> bus for a specific set of bus parameters through s_bus() function call.
> This also can be used to define platform specific bus parameters for
> host and sub-devices.
>
> Reviewed by: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
> Applies to v4l-dvb repository
>
>  include/media/v4l2-subdev.h |   40
> ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 1785608..2f5ec98 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -37,6 +37,43 @@ struct v4l2_decode_vbi_line {
>  	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no service found */
>  };
>
> +/*
> + * Some sub-devices are connected to the host/bridge device through a bus
> that
> + * carries the clock, vsync, hsync and data. Some interfaces such as
> BT.656
> + * carries the sync embedded in the data where as others have separate
> line
> + * carrying the sync signals. The structure below is used to define bus
> + * configuration parameters for host as well as sub-device
> + */
> +enum v4l2_subdev_bus_type {
> +	/* Raw YUV image data bus */
> +	V4L2_SUBDEV_BUS_RAW_YUV,
> +	/* Raw Bayer image data bus */
> +	V4L2_SUBDEV_BUS_RAW_BAYER
> +};
> +
> +struct v4l2_bus_settings {
> +	/* yuv or bayer image data bus */
> +	enum v4l2_subdev_bus_type type;
> +	/* subdev bus width */
> +	u8 subdev_width;
> +	/* host bus width */
> +	u8 host_width;
> +	/* embedded sync, set this when sync is embedded in the data stream */
> +	unsigned embedded_sync:1;
> +	/* master or slave */
> +	unsigned host_is_master:1;
> +	/* 0 - active low, 1 - active high */
> +	unsigned pol_vsync:1;
> +	/* 0 - active low, 1 - active high */
> +	unsigned pol_hsync:1;
> +	/* 0 - low to high , 1 - high to low */
> +	unsigned pol_field:1;
> +	/* 0 - sample at falling edge , 1 - sample at rising edge */
> +	unsigned pol_pclock:1;
> +	/* 0 - active low , 1 - active high */
> +	unsigned pol_data:1;
> +};

I've been thinking about this for a while and I think this struct should
be extended with the host bus parameters as well:

struct v4l2_bus_settings {
	/* yuv or bayer image data bus */
	enum v4l2_bus_type type;
	/* embedded sync, set this when sync is embedded in the data stream */
	unsigned embedded_sync:1;
	/* master or slave */
	unsigned host_is_master:1;

	/* bus width */
	unsigned sd_width:8;
	/* 0 - active low, 1 - active high */
	unsigned sd_pol_vsync:1;
	/* 0 - active low, 1 - active high */
	unsigned sd_pol_hsync:1;
	/* 0 - low to high, 1 - high to low */
	unsigned sd_pol_field:1;
	/* 0 - sample at falling edge, 1 - sample at rising edge */
	unsigned sd_edge_pclock:1;
	/* 0 - active low, 1 - active high */
	unsigned sd_pol_data:1;

	/* host bus width */
	unsigned host_width:8;
	/* 0 - active low, 1 - active high */
	unsigned host_pol_vsync:1;
	/* 0 - active low, 1 - active high */
	unsigned host_pol_hsync:1;
	/* 0 - low to high, 1 - high to low */
	unsigned host_pol_field:1;
	/* 0 - sample at falling edge, 1 - sample at rising edge */
	unsigned host_edge_pclock:1;
	/* 0 - active low, 1 - active high */
	unsigned host_pol_data:1;
};

It makes sense since you need to setup both ends of the bus, and having
both ends defined in the same struct keeps everything together. I have
thought about having separate host and subdev structs, but part of the bus
description is always common (bus type, master/slave, embedded/separate
syncs), while another part can be different for each end of the bus.

It's all bitfields, so it is a very compact representation.

In addition, I think we need to require that at the start of the s_bus
implementation in the host or subdev there should be a standard comment
block describing the possible combinations supported by the hardware:

/* Subdevice foo supports the following bus settings:

   types: RAW_BAYER (widths: 8/10/12, syncs: embedded/separate)
          RAW_YUV (widths: 8/16, syncs: embedded)
   bus master: slave
   vsync polarity: 0/1
   hsync polarity: 0/1
   field polarity: not applicable
   sampling edge pixelclock: 0/1
   data polarity: 1
 */

This should make it easy for implementers to pick a valid set of bus
parameters.

Regards,

       Hans

> +
>  /* Sub-devices are devices that are connected somehow to the main bridge
>     device. These devices are usually audio/video muxers/encoders/decoders
> or
>     sensors and webcam controllers.
> @@ -199,6 +236,8 @@ struct v4l2_subdev_audio_ops {
>
>     s_routing: see s_routing in audio_ops, except this version is for
> video
>  	devices.
> +
> +   s_bus: set bus parameters in sub device to configure the bus
>   */
>  struct v4l2_subdev_video_ops {
>  	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
> config);
> @@ -219,6 +258,7 @@ struct v4l2_subdev_video_ops {
>  	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
>  	int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum
> *fsize);
>  	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct
> v4l2_frmivalenum *fival);
> +	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_bus_settings
> *bus);
>  };
>
>  struct v4l2_subdev_ops {
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

[RFC PATCH] adding support for setting bus parameters in sub device

[m-karicheri2]

From: Muralidharan Karicheri <a0868495@gt516km11.gt.design.ti.com>

This patch adds support for setting bus parameters such as bus type
(Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw
image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field
etc) in sub device. This allows bridge driver to configure the sub device
interface for a specific set of bus parameters through s_bus() function call.

Reviewed By "Hans Verkuil".
Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
Applies to v4l-dvb repository

 include/media/v4l2-subdev.h |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 1785608..8e719c4 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -37,6 +37,39 @@ struct v4l2_decode_vbi_line {
 	u32 type;		/* VBI service type (V4L2_SLICED_*). 0 if no service found */
 };
 
+/*
+ * Some sub-devices are connected to the bridge device through a bus that
+ * carries * the clock, vsync, hsync and data. Some interfaces such as BT.656
+ * carries the sync embedded in the data where as others have separate line
+ * carrying the sync signals. The structure below is used by bridge driver to
+ * set the desired bus parameters in the sub device to work with it.
+ */
+enum v4l2_subdev_bus_type {
+	/* Raw YUV image data bus */
+	V4L2_SUBDEV_BUS_RAW_YUV,
+	/* Raw Bayer image data bus */
+	V4L2_SUBDEV_BUS_RAW_BAYER
+};
+
+struct v4l2_subdev_bus	{
+	/* yuv or bayer image data bus */
+	enum v4l2_subdev_bus_type type;
+	/* bus width */
+	u8 width;
+	/* embedded sync, set this when sync is embedded in the data stream */
+	unsigned embedded_sync:1;
+	/* 0 - active low, 1 - active high */
+	unsigned pol_vsync:1;
+	/* 0 - active low, 1 - active high */
+	unsigned pol_hsync:1;
+	/* 0 - low to high , 1 - high to low */
+	unsigned pol_field:1;
+	/* 0 - sample at falling edge , 1 - sample at rising edge */
+	unsigned pol_pclock:1;
+	/* 0 - active low , 1 - active high */
+	unsigned pol_data:1;
+};
+
 /* Sub-devices are devices that are connected somehow to the main bridge
    device. These devices are usually audio/video muxers/encoders/decoders or
    sensors and webcam controllers.
@@ -199,6 +232,8 @@ struct v4l2_subdev_audio_ops {
 
    s_routing: see s_routing in audio_ops, except this version is for video
 	devices.
+
+   s_bus: set bus parameters in sub device to configure the interface
  */
 struct v4l2_subdev_video_ops {
 	int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 config);
@@ -219,6 +254,7 @@ struct v4l2_subdev_video_ops {
 	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
 	int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize);
 	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct v4l2_frmivalenum *fival);
+	int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_subdev_bus *bus);
 };
 
 struct v4l2_subdev_ops {
-- 
1.6.0.4