Hello Laurent,

On 01/06/2016 10:48 AM, Laurent Pinchart wrote:

[snip]

quoted

quoted

quoted

@@ -940,6 +948,16 @@ static int tvp5150_cropcap(struct v4l2_subdev *sd,

struct v4l2_cropcap *a)
 static int tvp5150_g_mbus_config(struct v4l2_subdev *sd,
 				 struct v4l2_mbus_config *cfg)
 {
+	struct tvp5150_platform_data *pdata = to_tvp5150(sd)->pdata;
+
+	if (pdata) {
+		cfg->type = pdata->bus_type;
+		cfg->flags = pdata->parallel_flags;

The clock and sync signals polarity don't seem configurable, shouldn't
they just be hardcoded as currently done ?

That's a very good question, I added the flags because according to
Documentation/devicetree/bindings/media/video-interfaces.txt, the way
to define that the output format will be BT.656 is to avoid defining
{hsync,vsync,field-even}-active properties.

IOW, if parallel sync is used, then these properties have to be defined
and it felt strange to not use in the driver what is defined in the DT.

In that case we should restrict the values of the properties to what the 
hardware actually supports. I would hardcode the flags here, and check them 
when parsing the endpoint to make sure they're valid.

That's a good idea, I'll also mention the supported values in the binding doc.
 

If you find a register I have missed in the documentation with which 
polarities could be configured then please also feel free to prove me wrong 
:-)

I didn't find either when reading the datasheet to prepare this patch-set
so I think you are correct on that.

[snip]

quoted

quoted

quoted

+
+		pdata->bus_type = bus_cfg.bus_type;
+		pdata->parallel_flags = bus_cfg.bus.parallel.flags;

The V4L2_MBUS_DATA_ACTIVE_HIGH flags set returned by
tvp5150_g_mbus_config() when pdata is NULL is never set by
v4l2_of_parse_endpoint(), should you add it unconditionally ?

But v4l2_of_parse_endpoint() calls v4l2_of_parse_parallel_bus() which does
it or did I read the code incorrectly?

No, you're right, I had overlooked the V4L2_MBUS_DATA_ACTIVE_HIGH flag when 
reading v4l2_of_parse_parallel_bus(), probably a typo when searching. Please 
ignore that comment.

Ok, thanks for the clarification.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html