* [PATCH v4 0/6] media: rcar-vin: Brush endpoint properties @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Hello, 4th round for the VIN endpoint brushing series. Slightly enlarged the linux-media receiver list, as this new version introduces a common property in 'video-interfaces.txt'. Compared to v3 I have dropped the last controversial parts: - The custom 'renesas,hsync-as-de' property has been dropped: do not handle CHS bit for the moment. - Do not remove properties not parsed by the driver and not listed in the interface bindings from Gen2 boards. As this lead to a long discussion, I have now proposed a patch to clearly state that properties not listed in the interface bindings can be optionally specified, but they don't affect the interface behaviour. To avoid more discussions this patch may be dropped and things will stay the way they are now. For the common 'data-enable-active' property, I guess with Rob's ack we should be fine there and I hope the rest of the series won't slow down its acceptance. Thanks j Jacopo Mondi (6): dt-bindings: media: rcar-vin: Describe optional ep properties dt-bindings: media: Document data-enable-active property media: v4l2-fwnode: parse 'data-enable-active' prop dt-bindings: media: rcar-vin: Add 'data-enable-active' media: rcar-vin: Handle data-enable polarity dt-bindings: media: rcar-vin: Clarify optional props Documentation/devicetree/bindings/media/rcar_vin.txt | 18 ++++++++++++++++++ .../devicetree/bindings/media/video-interfaces.txt | 2 ++ drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++ include/media/v4l2-mediabus.h | 2 ++ 5 files changed, 31 insertions(+) -- 2.7.4 ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 0/6] media: rcar-vin: Brush endpoint properties @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Hello, 4th round for the VIN endpoint brushing series. Slightly enlarged the linux-media receiver list, as this new version introduces a common property in 'video-interfaces.txt'. Compared to v3 I have dropped the last controversial parts: - The custom 'renesas,hsync-as-de' property has been dropped: do not handle CHS bit for the moment. - Do not remove properties not parsed by the driver and not listed in the interface bindings from Gen2 boards. As this lead to a long discussion, I have now proposed a patch to clearly state that properties not listed in the interface bindings can be optionally specified, but they don't affect the interface behaviour. To avoid more discussions this patch may be dropped and things will stay the way they are now. For the common 'data-enable-active' property, I guess with Rob's ack we should be fine there and I hope the rest of the series won't slow down its acceptance. Thanks j Jacopo Mondi (6): dt-bindings: media: rcar-vin: Describe optional ep properties dt-bindings: media: Document data-enable-active property media: v4l2-fwnode: parse 'data-enable-active' prop dt-bindings: media: rcar-vin: Add 'data-enable-active' media: rcar-vin: Handle data-enable polarity dt-bindings: media: rcar-vin: Clarify optional props Documentation/devicetree/bindings/media/rcar_vin.txt | 18 ++++++++++++++++++ .../devicetree/bindings/media/video-interfaces.txt | 2 ++ drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++ include/media/v4l2-mediabus.h | 2 ++ 5 files changed, 31 insertions(+) -- 2.7.4 ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 0/6] media: rcar-vin: Brush endpoint properties @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hello, 4th round for the VIN endpoint brushing series. Slightly enlarged the linux-media receiver list, as this new version introduces a common property in 'video-interfaces.txt'. Compared to v3 I have dropped the last controversial parts: - The custom 'renesas,hsync-as-de' property has been dropped: do not handle CHS bit for the moment. - Do not remove properties not parsed by the driver and not listed in the interface bindings from Gen2 boards. As this lead to a long discussion, I have now proposed a patch to clearly state that properties not listed in the interface bindings can be optionally specified, but they don't affect the interface behaviour. To avoid more discussions this patch may be dropped and things will stay the way they are now. For the common 'data-enable-active' property, I guess with Rob's ack we should be fine there and I hope the rest of the series won't slow down its acceptance. Thanks j Jacopo Mondi (6): dt-bindings: media: rcar-vin: Describe optional ep properties dt-bindings: media: Document data-enable-active property media: v4l2-fwnode: parse 'data-enable-active' prop dt-bindings: media: rcar-vin: Add 'data-enable-active' media: rcar-vin: Handle data-enable polarity dt-bindings: media: rcar-vin: Clarify optional props Documentation/devicetree/bindings/media/rcar_vin.txt | 18 ++++++++++++++++++ .../devicetree/bindings/media/video-interfaces.txt | 2 ++ drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++ include/media/v4l2-mediabus.h | 2 ++ 5 files changed, 31 insertions(+) -- 2.7.4 ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 14:26 ` Jacopo Mondi -1 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Describe the optional endpoint properties for endpoint nodes of port@0 and port@1 of the R-Car VIN driver device tree bindings documentation. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index a19517e1..87f93ec 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -53,6 +53,14 @@ from local SoC CSI-2 receivers (port1) depending on SoC. from external SoC pins described in video-interfaces.txt[1]. Describing more then one endpoint in port 0 is invalid. Only VIN instances that are connected to external pins should have port 0. + + - Optional properties for endpoint nodes of port@0: + - hsync-active: see [1] for description. Default is active high. + - vsync-active: see [1] for description. Default is active high. + + If both HSYNC and VSYNC polarities are not specified, embedded + synchronization is selected. + - port 1 - sub-nodes describing one or more endpoints connected to the VIN from local SoC CSI-2 receivers. The endpoint numbers must use the following schema. @@ -62,6 +70,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC. - Endpoint 2 - sub-node describing the endpoint connected to CSI40 - Endpoint 3 - sub-node describing the endpoint connected to CSI41 + Endpoint nodes of port@1 do not support any optional endpoint property. + Device node example for Gen2 platforms -------------------------------------- -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Describe the optional endpoint properties for endpoint nodes of port at 0 and port at 1 of the R-Car VIN driver device tree bindings documentation. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index a19517e1..87f93ec 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -53,6 +53,14 @@ from local SoC CSI-2 receivers (port1) depending on SoC. from external SoC pins described in video-interfaces.txt[1]. Describing more then one endpoint in port 0 is invalid. Only VIN instances that are connected to external pins should have port 0. + + - Optional properties for endpoint nodes of port at 0: + - hsync-active: see [1] for description. Default is active high. + - vsync-active: see [1] for description. Default is active high. + + If both HSYNC and VSYNC polarities are not specified, embedded + synchronization is selected. + - port 1 - sub-nodes describing one or more endpoints connected to the VIN from local SoC CSI-2 receivers. The endpoint numbers must use the following schema. @@ -62,6 +70,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC. - Endpoint 2 - sub-node describing the endpoint connected to CSI40 - Endpoint 3 - sub-node describing the endpoint connected to CSI41 + Endpoint nodes of port at 1 do not support any optional endpoint property. + Device node example for Gen2 platforms -------------------------------------- -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Describe the optional endpoint properties for endpoint nodes of port@0 and port@1 of the R-Car VIN driver device tree bindings documentation. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/media/rcar_vin.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index a19517e1..87f93ec 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -53,6 +53,14 @@ from local SoC CSI-2 receivers (port1) depending on SoC. from external SoC pins described in video-interfaces.txt[1]. Describing more then one endpoint in port 0 is invalid. Only VIN instances that are connected to external pins should have port 0. + + - Optional properties for endpoint nodes of port@0: + - hsync-active: see [1] for description. Default is active high. + - vsync-active: see [1] for description. Default is active high. + + If both HSYNC and VSYNC polarities are not specified, embedded + synchronization is selected. + - port 1 - sub-nodes describing one or more endpoints connected to the VIN from local SoC CSI-2 receivers. The endpoint numbers must use the following schema. @@ -62,6 +70,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC. - Endpoint 2 - sub-node describing the endpoint connected to CSI40 - Endpoint 3 - sub-node describing the endpoint connected to CSI41 + Endpoint nodes of port@1 do not support any optional endpoint property. + Device node example for Gen2 platforms -------------------------------------- -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 2/6] dt-bindings: media: Document data-enable-active property 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 14:26 ` Jacopo Mondi -1 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Add 'data-enable-active' property to endpoint node properties list. The property allows to specify the polarity of the data-enable signal, which when in active state determinates when data lines have to sampled for valid pixel data. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index 258b8df..9839d26 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -109,6 +109,8 @@ Optional endpoint properties Note, that if HSYNC and VSYNC polarities are not specified, embedded synchronization may be required, where supported. - data-active: similar to HSYNC and VSYNC, specifies data line polarity. +- data-enable-active: similar to HSYNC and VSYNC, specifies the data enable + signal polarity. - field-even-active: field signal level during the even field data transmission. - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock signal. -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 2/6] dt-bindings: media: Document data-enable-active property @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Add 'data-enable-active' property to endpoint node properties list. The property allows to specify the polarity of the data-enable signal, which when in active state determinates when data lines have to sampled for valid pixel data. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index 258b8df..9839d26 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -109,6 +109,8 @@ Optional endpoint properties Note, that if HSYNC and VSYNC polarities are not specified, embedded synchronization may be required, where supported. - data-active: similar to HSYNC and VSYNC, specifies data line polarity. +- data-enable-active: similar to HSYNC and VSYNC, specifies the data enable + signal polarity. - field-even-active: field signal level during the even field data transmission. - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock signal. -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 2/6] dt-bindings: media: Document data-enable-active property @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Add 'data-enable-active' property to endpoint node properties list. The property allows to specify the polarity of the data-enable signal, which when in active state determinates when data lines have to sampled for valid pixel data. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index 258b8df..9839d26 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -109,6 +109,8 @@ Optional endpoint properties Note, that if HSYNC and VSYNC polarities are not specified, embedded synchronization may be required, where supported. - data-active: similar to HSYNC and VSYNC, specifies data line polarity. +- data-enable-active: similar to HSYNC and VSYNC, specifies the data enable + signal polarity. - field-even-active: field signal level during the even field data transmission. - pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock signal. -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 14:26 ` Jacopo Mondi -1 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Parse the newly defined 'data-enable-active' property in parallel endpoint parsing function. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++ include/media/v4l2-mediabus.h | 2 ++ 2 files changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index 3f77aa3..6105191 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus( flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH : V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW; + if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v)) + flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH : + V4L2_MBUS_DATA_ENABLE_LOW; + bus->flags = flags; } diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 4d8626c..4bbb5f3 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -45,6 +45,8 @@ /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */ #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH BIT(12) #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW BIT(13) +#define V4L2_MBUS_DATA_ENABLE_HIGH BIT(14) +#define V4L2_MBUS_DATA_ENABLE_LOW BIT(15) /* Serial flags */ /* How many lanes the client can use */ -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Parse the newly defined 'data-enable-active' property in parallel endpoint parsing function. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++ include/media/v4l2-mediabus.h | 2 ++ 2 files changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index 3f77aa3..6105191 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus( flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH : V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW; + if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v)) + flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH : + V4L2_MBUS_DATA_ENABLE_LOW; + bus->flags = flags; } diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 4d8626c..4bbb5f3 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -45,6 +45,8 @@ /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */ #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH BIT(12) #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW BIT(13) +#define V4L2_MBUS_DATA_ENABLE_HIGH BIT(14) +#define V4L2_MBUS_DATA_ENABLE_LOW BIT(15) /* Serial flags */ /* How many lanes the client can use */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Parse the newly defined 'data-enable-active' property in parallel endpoint parsing function. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++ include/media/v4l2-mediabus.h | 2 ++ 2 files changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index 3f77aa3..6105191 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus( flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH : V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW; + if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v)) + flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH : + V4L2_MBUS_DATA_ENABLE_LOW; + bus->flags = flags; } diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 4d8626c..4bbb5f3 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -45,6 +45,8 @@ /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */ #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH BIT(12) #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW BIT(13) +#define V4L2_MBUS_DATA_ENABLE_HIGH BIT(14) +#define V4L2_MBUS_DATA_ENABLE_LOW BIT(15) /* Serial flags */ /* How many lanes the client can use */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active' 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 14:26 ` Jacopo Mondi -1 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Describe optional endpoint property 'data-enable-active' for R-Car VIN. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Rob Herring <robh@kernel.org> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se --- Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index 87f93ec..8130849 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -57,6 +57,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC. - Optional properties for endpoint nodes of port@0: - hsync-active: see [1] for description. Default is active high. - vsync-active: see [1] for description. Default is active high. + - data-enable-active: polarity of CLKENB signal, see [1] for + description. Default is active high. If both HSYNC and VSYNC polarities are not specified, embedded synchronization is selected. -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active' @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Describe optional endpoint property 'data-enable-active' for R-Car VIN. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Rob Herring <robh@kernel.org> Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas at ragnatech.se --- Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index 87f93ec..8130849 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -57,6 +57,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC. - Optional properties for endpoint nodes of port@0: - hsync-active: see [1] for description. Default is active high. - vsync-active: see [1] for description. Default is active high. + - data-enable-active: polarity of CLKENB signal, see [1] for + description. Default is active high. If both HSYNC and VSYNC polarities are not specified, embedded synchronization is selected. -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active' @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Describe optional endpoint property 'data-enable-active' for R-Car VIN. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Reviewed-by: Rob Herring <robh@kernel.org> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se --- Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index 87f93ec..8130849 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -57,6 +57,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC. - Optional properties for endpoint nodes of port@0: - hsync-active: see [1] for description. Default is active high. - vsync-active: see [1] for description. Default is active high. + - data-enable-active: polarity of CLKENB signal, see [1] for + description. Default is active high. If both HSYNC and VSYNC polarities are not specified, embedded synchronization is selected. -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 14:26 ` Jacopo Mondi -1 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Handle data-enable signal polarity. If the polarity is not specifically requested to be active low, use the active high default. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index d2b7002..9145b56 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -123,6 +123,7 @@ /* Video n Data Mode Register 2 bits */ #define VNDMR2_VPS (1 << 30) #define VNDMR2_HPS (1 << 29) +#define VNDMR2_CES (1 << 28) #define VNDMR2_FTEV (1 << 17) #define VNDMR2_VLV(n) ((n & 0xf) << 12) @@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin) /* Vsync Signal Polarity Select */ if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) dmr2 |= VNDMR2_VPS; + + /* Data Enable Polarity Select */ + if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW) + dmr2 |= VNDMR2_CES; } /* -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Handle data-enable signal polarity. If the polarity is not specifically requested to be active low, use the active high default. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index d2b7002..9145b56 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -123,6 +123,7 @@ /* Video n Data Mode Register 2 bits */ #define VNDMR2_VPS (1 << 30) #define VNDMR2_HPS (1 << 29) +#define VNDMR2_CES (1 << 28) #define VNDMR2_FTEV (1 << 17) #define VNDMR2_VLV(n) ((n & 0xf) << 12) @@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin) /* Vsync Signal Polarity Select */ if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) dmr2 |= VNDMR2_VPS; + + /* Data Enable Polarity Select */ + if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW) + dmr2 |= VNDMR2_CES; } /* -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Handle data-enable signal polarity. If the polarity is not specifically requested to be active low, use the active high default. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index d2b7002..9145b56 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -123,6 +123,7 @@ /* Video n Data Mode Register 2 bits */ #define VNDMR2_VPS (1 << 30) #define VNDMR2_HPS (1 << 29) +#define VNDMR2_CES (1 << 28) #define VNDMR2_FTEV (1 << 17) #define VNDMR2_VLV(n) ((n & 0xf) << 12) @@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin) /* Vsync Signal Polarity Select */ if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) dmr2 |= VNDMR2_VPS; + + /* Data Enable Polarity Select */ + if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW) + dmr2 |= VNDMR2_CES; } /* -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 14:26 ` Jacopo Mondi -1 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: devicetree, linux-renesas-soc, robh+dt, hans.verkuil, sakari.ailus, Jacopo Mondi, mchehab, linux-arm-kernel, linux-media Add a note to the R-Car VIN interface bindings to clarify that all properties listed as generic properties in video-interfaces.txt can be included in port@0 endpoint, but if not explicitly listed in the interface bindings documentation, they do not modify it behaviour. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index 8130849..03544c7 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. instances that are connected to external pins should have port 0. - Optional properties for endpoint nodes of port@0: + + All properties described in [1] and which apply to the selected + media bus type could be optionally listed here to better describe + the current hardware configuration, but only the following ones do + actually modify the VIN interface behaviour: + - hsync-active: see [1] for description. Default is active high. - vsync-active: see [1] for description. Default is active high. - data-enable-active: polarity of CLKENB signal, see [1] for -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: linux-arm-kernel Add a note to the R-Car VIN interface bindings to clarify that all properties listed as generic properties in video-interfaces.txt can be included in port at 0 endpoint, but if not explicitly listed in the interface bindings documentation, they do not modify it behaviour. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index 8130849..03544c7 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. instances that are connected to external pins should have port 0. - Optional properties for endpoint nodes of port at 0: + + All properties described in [1] and which apply to the selected + media bus type could be optionally listed here to better describe + the current hardware configuration, but only the following ones do + actually modify the VIN interface behaviour: + - hsync-active: see [1] for description. Default is active high. - vsync-active: see [1] for description. Default is active high. - data-enable-active: polarity of CLKENB signal, see [1] for -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-12 14:26 ` Jacopo Mondi 0 siblings, 0 replies; 44+ messages in thread From: Jacopo Mondi @ 2018-06-12 14:26 UTC (permalink / raw) To: niklas.soderlund, laurent.pinchart, horms, geert Cc: Jacopo Mondi, mchehab, sakari.ailus, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Add a note to the R-Car VIN interface bindings to clarify that all properties listed as generic properties in video-interfaces.txt can be included in port@0 endpoint, but if not explicitly listed in the interface bindings documentation, they do not modify it behaviour. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index 8130849..03544c7 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. instances that are connected to external pins should have port 0. - Optional properties for endpoint nodes of port@0: + + All properties described in [1] and which apply to the selected + media bus type could be optionally listed here to better describe + the current hardware configuration, but only the following ones do + actually modify the VIN interface behaviour: + - hsync-active: see [1] for description. Default is active high. - vsync-active: see [1] for description. Default is active high. - data-enable-active: polarity of CLKENB signal, see [1] for -- 2.7.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-06-12 14:26 ` Jacopo Mondi (?) @ 2018-06-12 15:45 ` Sakari Ailus -1 siblings, 0 replies; 44+ messages in thread From: Sakari Ailus @ 2018-06-12 15:45 UTC (permalink / raw) To: Jacopo Mondi Cc: devicetree, robh+dt, linux-renesas-soc, horms, hans.verkuil, laurent.pinchart, niklas.soderlund, geert, mchehab, linux-arm-kernel, linux-media Hi Jacopo, On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > Add a note to the R-Car VIN interface bindings to clarify that all > properties listed as generic properties in video-interfaces.txt can > be included in port@0 endpoint, but if not explicitly listed in the > interface bindings documentation, they do not modify it behaviour. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > index 8130849..03544c7 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > instances that are connected to external pins should have port 0. > > - Optional properties for endpoint nodes of port@0: > + > + All properties described in [1] and which apply to the selected > + media bus type could be optionally listed here to better describe > + the current hardware configuration, but only the following ones do > + actually modify the VIN interface behaviour: > + I don't think this should be needed. You should only have properties that describe the hardware configuration in a given system. > - hsync-active: see [1] for description. Default is active high. > - vsync-active: see [1] for description. Default is active high. > - data-enable-active: polarity of CLKENB signal, see [1] for > -- > 2.7.4 > -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-12 15:45 ` Sakari Ailus 0 siblings, 0 replies; 44+ messages in thread From: Sakari Ailus @ 2018-06-12 15:45 UTC (permalink / raw) To: linux-arm-kernel Hi Jacopo, On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > Add a note to the R-Car VIN interface bindings to clarify that all > properties listed as generic properties in video-interfaces.txt can > be included in port at 0 endpoint, but if not explicitly listed in the > interface bindings documentation, they do not modify it behaviour. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > index 8130849..03544c7 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > instances that are connected to external pins should have port 0. > > - Optional properties for endpoint nodes of port at 0: > + > + All properties described in [1] and which apply to the selected > + media bus type could be optionally listed here to better describe > + the current hardware configuration, but only the following ones do > + actually modify the VIN interface behaviour: > + I don't think this should be needed. You should only have properties that describe the hardware configuration in a given system. > - hsync-active: see [1] for description. Default is active high. > - vsync-active: see [1] for description. Default is active high. > - data-enable-active: polarity of CLKENB signal, see [1] for > -- > 2.7.4 > -- Sakari Ailus sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-12 15:45 ` Sakari Ailus 0 siblings, 0 replies; 44+ messages in thread From: Sakari Ailus @ 2018-06-12 15:45 UTC (permalink / raw) To: Jacopo Mondi Cc: niklas.soderlund, laurent.pinchart, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hi Jacopo, On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > Add a note to the R-Car VIN interface bindings to clarify that all > properties listed as generic properties in video-interfaces.txt can > be included in port@0 endpoint, but if not explicitly listed in the > interface bindings documentation, they do not modify it behaviour. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > index 8130849..03544c7 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > instances that are connected to external pins should have port 0. > > - Optional properties for endpoint nodes of port@0: > + > + All properties described in [1] and which apply to the selected > + media bus type could be optionally listed here to better describe > + the current hardware configuration, but only the following ones do > + actually modify the VIN interface behaviour: > + I don't think this should be needed. You should only have properties that describe the hardware configuration in a given system. > - hsync-active: see [1] for description. Default is active high. > - vsync-active: see [1] for description. Default is active high. > - data-enable-active: polarity of CLKENB signal, see [1] for > -- > 2.7.4 > -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-06-12 15:45 ` Sakari Ailus (?) @ 2018-06-12 16:15 ` Laurent Pinchart -1 siblings, 0 replies; 44+ messages in thread From: Laurent Pinchart @ 2018-06-12 16:15 UTC (permalink / raw) To: Sakari Ailus Cc: devicetree, robh+dt, linux-renesas-soc, horms, geert, niklas.soderlund, Jacopo Mondi, mchehab, hans.verkuil, linux-arm-kernel, linux-media Hello, On Tuesday, 12 June 2018 18:45:53 EEST Sakari Ailus wrote: > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > Add a note to the R-Car VIN interface bindings to clarify that all > > properties listed as generic properties in video-interfaces.txt can > > be included in port@0 endpoint, but if not explicitly listed in the > > interface bindings documentation, they do not modify it behaviour. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > 8130849..03544c7 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > SoC. > > > > instances that are connected to external pins should have port 0. > > > > - Optional properties for endpoint nodes of port@0: > > + > > + All properties described in [1] and which apply to the selected > > + media bus type could be optionally listed here to better describe > > + the current hardware configuration, but only the following ones > > do > > + actually modify the VIN interface behaviour: > > + > > I don't think this should be needed. You should only have properties that > describe the hardware configuration in a given system. I agree. Please list explicitly all properties that are applicable for this device, and don't tell which one(s) are used by the driver. > > - hsync-active: see [1] for description. Default is active high. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-12 16:15 ` Laurent Pinchart 0 siblings, 0 replies; 44+ messages in thread From: Laurent Pinchart @ 2018-06-12 16:15 UTC (permalink / raw) To: linux-arm-kernel Hello, On Tuesday, 12 June 2018 18:45:53 EEST Sakari Ailus wrote: > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > Add a note to the R-Car VIN interface bindings to clarify that all > > properties listed as generic properties in video-interfaces.txt can > > be included in port at 0 endpoint, but if not explicitly listed in the > > interface bindings documentation, they do not modify it behaviour. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > 8130849..03544c7 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > SoC. > > > > instances that are connected to external pins should have port 0. > > > > - Optional properties for endpoint nodes of port at 0: > > + > > + All properties described in [1] and which apply to the selected > > + media bus type could be optionally listed here to better describe > > + the current hardware configuration, but only the following ones > > do > > + actually modify the VIN interface behaviour: > > + > > I don't think this should be needed. You should only have properties that > describe the hardware configuration in a given system. I agree. Please list explicitly all properties that are applicable for this device, and don't tell which one(s) are used by the driver. > > - hsync-active: see [1] for description. Default is active high. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-12 16:15 ` Laurent Pinchart 0 siblings, 0 replies; 44+ messages in thread From: Laurent Pinchart @ 2018-06-12 16:15 UTC (permalink / raw) To: Sakari Ailus Cc: Jacopo Mondi, niklas.soderlund, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hello, On Tuesday, 12 June 2018 18:45:53 EEST Sakari Ailus wrote: > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > Add a note to the R-Car VIN interface bindings to clarify that all > > properties listed as generic properties in video-interfaces.txt can > > be included in port@0 endpoint, but if not explicitly listed in the > > interface bindings documentation, they do not modify it behaviour. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > 8130849..03544c7 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > SoC. > > > > instances that are connected to external pins should have port 0. > > > > - Optional properties for endpoint nodes of port@0: > > + > > + All properties described in [1] and which apply to the selected > > + media bus type could be optionally listed here to better describe > > + the current hardware configuration, but only the following ones > > do > > + actually modify the VIN interface behaviour: > > + > > I don't think this should be needed. You should only have properties that > describe the hardware configuration in a given system. I agree. Please list explicitly all properties that are applicable for this device, and don't tell which one(s) are used by the driver. > > - hsync-active: see [1] for description. Default is active high. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-06-12 15:45 ` Sakari Ailus (?) @ 2018-06-13 8:54 ` jacopo mondi -1 siblings, 0 replies; 44+ messages in thread From: jacopo mondi @ 2018-06-13 8:54 UTC (permalink / raw) To: Sakari Ailus Cc: devicetree, robh+dt, linux-renesas-soc, horms, geert, laurent.pinchart, niklas.soderlund, Jacopo Mondi, mchehab, hans.verkuil, linux-arm-kernel, linux-media [-- Attachment #1.1: Type: text/plain, Size: 2961 bytes --] Hi Sakari, On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > Add a note to the R-Car VIN interface bindings to clarify that all > > properties listed as generic properties in video-interfaces.txt can > > be included in port@0 endpoint, but if not explicitly listed in the > > interface bindings documentation, they do not modify it behaviour. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > index 8130849..03544c7 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > instances that are connected to external pins should have port 0. > > > > - Optional properties for endpoint nodes of port@0: > > + > > + All properties described in [1] and which apply to the selected > > + media bus type could be optionally listed here to better describe > > + the current hardware configuration, but only the following ones do > > + actually modify the VIN interface behaviour: > > + > > I don't think this should be needed. You should only have properties that > describe the hardware configuration in a given system. > There has been quite some debate on this, and please bear with me here for re-proposing it: I started by removing properties in some DT files for older Renesas board which listed endpoint properties not documented in the VIN's bindings and not parsed by the VIN driver [1] Niklas (but Simon and Geert seems to agree here) opposed to that patch, as those properties where described in 'video-interfaces.txt' and even if not parsed by the current driver implementation, they actually describe hardware. I rebated that only properties listed in the device bindings documentation should actually be used, and having properties not parsed by the driver confuses users, which may expect changing them modifies the interface configuration, which does not happens at the moment. This came out as a middle ground from a discussion with Niklas. As stated in the cover letter if this patch makes someone uncomfortable, feel free to drop it not to hold back the rest of the series which has been well received instead. Thanks j [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > - hsync-active: see [1] for description. Default is active high. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for > > -- > > 2.7.4 > > > > -- > Sakari Ailus > sakari.ailus@linux.intel.com [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-13 8:54 ` jacopo mondi 0 siblings, 0 replies; 44+ messages in thread From: jacopo mondi @ 2018-06-13 8:54 UTC (permalink / raw) To: linux-arm-kernel Hi Sakari, On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > Add a note to the R-Car VIN interface bindings to clarify that all > > properties listed as generic properties in video-interfaces.txt can > > be included in port at 0 endpoint, but if not explicitly listed in the > > interface bindings documentation, they do not modify it behaviour. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > index 8130849..03544c7 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > instances that are connected to external pins should have port 0. > > > > - Optional properties for endpoint nodes of port at 0: > > + > > + All properties described in [1] and which apply to the selected > > + media bus type could be optionally listed here to better describe > > + the current hardware configuration, but only the following ones do > > + actually modify the VIN interface behaviour: > > + > > I don't think this should be needed. You should only have properties that > describe the hardware configuration in a given system. > There has been quite some debate on this, and please bear with me here for re-proposing it: I started by removing properties in some DT files for older Renesas board which listed endpoint properties not documented in the VIN's bindings and not parsed by the VIN driver [1] Niklas (but Simon and Geert seems to agree here) opposed to that patch, as those properties where described in 'video-interfaces.txt' and even if not parsed by the current driver implementation, they actually describe hardware. I rebated that only properties listed in the device bindings documentation should actually be used, and having properties not parsed by the driver confuses users, which may expect changing them modifies the interface configuration, which does not happens at the moment. This came out as a middle ground from a discussion with Niklas. As stated in the cover letter if this patch makes someone uncomfortable, feel free to drop it not to hold back the rest of the series which has been well received instead. Thanks j [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > - hsync-active: see [1] for description. Default is active high. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for > > -- > > 2.7.4 > > > > -- > Sakari Ailus > sakari.ailus at linux.intel.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180613/e8557938/attachment.sig> ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-13 8:54 ` jacopo mondi 0 siblings, 0 replies; 44+ messages in thread From: jacopo mondi @ 2018-06-13 8:54 UTC (permalink / raw) To: Sakari Ailus Cc: Jacopo Mondi, niklas.soderlund, laurent.pinchart, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc [-- Attachment #1: Type: text/plain, Size: 2961 bytes --] Hi Sakari, On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > Add a note to the R-Car VIN interface bindings to clarify that all > > properties listed as generic properties in video-interfaces.txt can > > be included in port@0 endpoint, but if not explicitly listed in the > > interface bindings documentation, they do not modify it behaviour. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > index 8130849..03544c7 100644 > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > instances that are connected to external pins should have port 0. > > > > - Optional properties for endpoint nodes of port@0: > > + > > + All properties described in [1] and which apply to the selected > > + media bus type could be optionally listed here to better describe > > + the current hardware configuration, but only the following ones do > > + actually modify the VIN interface behaviour: > > + > > I don't think this should be needed. You should only have properties that > describe the hardware configuration in a given system. > There has been quite some debate on this, and please bear with me here for re-proposing it: I started by removing properties in some DT files for older Renesas board which listed endpoint properties not documented in the VIN's bindings and not parsed by the VIN driver [1] Niklas (but Simon and Geert seems to agree here) opposed to that patch, as those properties where described in 'video-interfaces.txt' and even if not parsed by the current driver implementation, they actually describe hardware. I rebated that only properties listed in the device bindings documentation should actually be used, and having properties not parsed by the driver confuses users, which may expect changing them modifies the interface configuration, which does not happens at the moment. This came out as a middle ground from a discussion with Niklas. As stated in the cover letter if this patch makes someone uncomfortable, feel free to drop it not to hold back the rest of the series which has been well received instead. Thanks j [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > - hsync-active: see [1] for description. Default is active high. > > - vsync-active: see [1] for description. Default is active high. > > - data-enable-active: polarity of CLKENB signal, see [1] for > > -- > > 2.7.4 > > > > -- > Sakari Ailus > sakari.ailus@linux.intel.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-06-13 8:54 ` jacopo mondi (?) (?) @ 2018-06-27 5:24 ` Niklas Söderlund -1 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-06-27 5:24 UTC (permalink / raw) To: jacopo mondi Cc: devicetree, robh+dt, linux-renesas-soc, horms, geert, laurent.pinchart, Sakari Ailus, Jacopo Mondi, mchehab, hans.verkuil, linux-arm-kernel, linux-media Hi Jacopo, Sakari and Laurent, Jacopo, thanks for your patch and continued work on resolving this. On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > Hi Sakari, > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > Add a note to the R-Car VIN interface bindings to clarify that all > > > properties listed as generic properties in video-interfaces.txt can > > > be included in port@0 endpoint, but if not explicitly listed in the > > > interface bindings documentation, they do not modify it behaviour. > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > --- > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > index 8130849..03544c7 100644 > > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > > instances that are connected to external pins should have port 0. > > > > > > - Optional properties for endpoint nodes of port@0: > > > + > > > + All properties described in [1] and which apply to the selected > > > + media bus type could be optionally listed here to better describe > > > + the current hardware configuration, but only the following ones do > > > + actually modify the VIN interface behaviour: > > > + I'm not sure the description have to be as explicit to that the properties in 'video-interfaces.txt' are not currently used by the driver. I find it not relevant which ones are used or not, what is important for me is that all properties in 'video-interfaces.txt' which can be used to describe the specific bus are valid for the DT description. On a side note, in rcar_vin.txt we have this section describing the Gen2 bindings: The per-board settings Gen2 platforms: - port sub-node describing a single endpoint connected to the vin as described in video-interfaces.txt[1]. Only the first one will be considered as each vin interface has one input port. This whole series only deals with documenting the Gen3 optional properties and not the Gen2. Maybe with parallel input support for Gen3 patches on there way to making it upstream this series should be extended to in a good way merge the port@0 optional properties for both generations of hardware? > > > > I don't think this should be needed. You should only have properties that > > describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > here for re-proposing it: I started by removing properties in some DT > files for older Renesas board which listed endpoint properties not > documented in the VIN's bindings and not parsed by the VIN driver [1] > Niklas (but Simon and Geert seems to agree here) opposed to that > patch, as those properties where described in 'video-interfaces.txt' and > even if not parsed by the current driver implementation, they actually > describe hardware. I rebated that only properties listed in the device > bindings documentation should actually be used, and having properties > not parsed by the driver confuses users, which may expect changing > them modifies the interface configuration, which does not happens at > the moment. > > This came out as a middle ground from a discussion with Niklas. As > stated in the cover letter if this patch makes someone uncomfortable, feel > free to drop it not to hold back the rest of the series which has been > well received instead. What I don't agree with and sparked this debate from my side was the deletion of properties in dts files which correctly does describe the bus but which are not currently parsed by the driver. To me that is decreasing the value of the dts. If on the other hand the goal is to deprecate a property from the video-interfaces.txt by slowly removing them from dts where the driver don't use them I'm all for it. But I don't think this is the case here right? > > Thanks > j > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > - hsync-active: see [1] for description. Default is active high. > > > - vsync-active: see [1] for description. Default is active high. > > > - data-enable-active: polarity of CLKENB signal, see [1] for > > > -- > > > 2.7.4 > > > > > > > -- > > Sakari Ailus > > sakari.ailus@linux.intel.com -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-27 5:24 ` Niklas Söderlund 0 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-06-27 5:24 UTC (permalink / raw) To: linux-arm-kernel Hi Jacopo, Sakari and Laurent, Jacopo, thanks for your patch and continued work on resolving this. On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > Hi Sakari, > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > Add a note to the R-Car VIN interface bindings to clarify that all > > > properties listed as generic properties in video-interfaces.txt can > > > be included in port at 0 endpoint, but if not explicitly listed in the > > > interface bindings documentation, they do not modify it behaviour. > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > --- > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > index 8130849..03544c7 100644 > > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > > instances that are connected to external pins should have port 0. > > > > > > - Optional properties for endpoint nodes of port at 0: > > > + > > > + All properties described in [1] and which apply to the selected > > > + media bus type could be optionally listed here to better describe > > > + the current hardware configuration, but only the following ones do > > > + actually modify the VIN interface behaviour: > > > + I'm not sure the description have to be as explicit to that the properties in 'video-interfaces.txt' are not currently used by the driver. I find it not relevant which ones are used or not, what is important for me is that all properties in 'video-interfaces.txt' which can be used to describe the specific bus are valid for the DT description. On a side note, in rcar_vin.txt we have this section describing the Gen2 bindings: The per-board settings Gen2 platforms: - port sub-node describing a single endpoint connected to the vin as described in video-interfaces.txt[1]. Only the first one will be considered as each vin interface has one input port. This whole series only deals with documenting the Gen3 optional properties and not the Gen2. Maybe with parallel input support for Gen3 patches on there way to making it upstream this series should be extended to in a good way merge the port at 0 optional properties for both generations of hardware? > > > > I don't think this should be needed. You should only have properties that > > describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > here for re-proposing it: I started by removing properties in some DT > files for older Renesas board which listed endpoint properties not > documented in the VIN's bindings and not parsed by the VIN driver [1] > Niklas (but Simon and Geert seems to agree here) opposed to that > patch, as those properties where described in 'video-interfaces.txt' and > even if not parsed by the current driver implementation, they actually > describe hardware. I rebated that only properties listed in the device > bindings documentation should actually be used, and having properties > not parsed by the driver confuses users, which may expect changing > them modifies the interface configuration, which does not happens at > the moment. > > This came out as a middle ground from a discussion with Niklas. As > stated in the cover letter if this patch makes someone uncomfortable, feel > free to drop it not to hold back the rest of the series which has been > well received instead. What I don't agree with and sparked this debate from my side was the deletion of properties in dts files which correctly does describe the bus but which are not currently parsed by the driver. To me that is decreasing the value of the dts. If on the other hand the goal is to deprecate a property from the video-interfaces.txt by slowly removing them from dts where the driver don't use them I'm all for it. But I don't think this is the case here right? > > Thanks > j > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > - hsync-active: see [1] for description. Default is active high. > > > - vsync-active: see [1] for description. Default is active high. > > > - data-enable-active: polarity of CLKENB signal, see [1] for > > > -- > > > 2.7.4 > > > > > > > -- > > Sakari Ailus > > sakari.ailus at linux.intel.com -- Regards, Niklas S?derlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-27 5:24 ` Niklas Söderlund 0 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-06-27 5:24 UTC (permalink / raw) To: jacopo mondi Cc: Sakari Ailus, Jacopo Mondi, laurent.pinchart, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hi Jacopo, Sakari and Laurent, Jacopo, thanks for your patch and continued work on resolving this. On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > Hi Sakari, > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > Add a note to the R-Car VIN interface bindings to clarify that all > > > properties listed as generic properties in video-interfaces.txt can > > > be included in port@0 endpoint, but if not explicitly listed in the > > > interface bindings documentation, they do not modify it behaviour. > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > --- > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > index 8130849..03544c7 100644 > > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > > instances that are connected to external pins should have port 0. > > > > > > - Optional properties for endpoint nodes of port@0: > > > + > > > + All properties described in [1] and which apply to the selected > > > + media bus type could be optionally listed here to better describe > > > + the current hardware configuration, but only the following ones do > > > + actually modify the VIN interface behaviour: > > > + I'm not sure the description have to be as explicit to that the properties in 'video-interfaces.txt' are not currently used by the driver. I find it not relevant which ones are used or not, what is important for me is that all properties in 'video-interfaces.txt' which can be used to describe the specific bus are valid for the DT description. On a side note, in rcar_vin.txt we have this section describing the Gen2 bindings: The per-board settings Gen2 platforms: - port sub-node describing a single endpoint connected to the vin as described in video-interfaces.txt[1]. Only the first one will be considered as each vin interface has one input port. This whole series only deals with documenting the Gen3 optional properties and not the Gen2. Maybe with parallel input support for Gen3 patches on there way to making it upstream this series should be extended to in a good way merge the port@0 optional properties for both generations of hardware? > > > > I don't think this should be needed. You should only have properties that > > describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > here for re-proposing it: I started by removing properties in some DT > files for older Renesas board which listed endpoint properties not > documented in the VIN's bindings and not parsed by the VIN driver [1] > Niklas (but Simon and Geert seems to agree here) opposed to that > patch, as those properties where described in 'video-interfaces.txt' and > even if not parsed by the current driver implementation, they actually > describe hardware. I rebated that only properties listed in the device > bindings documentation should actually be used, and having properties > not parsed by the driver confuses users, which may expect changing > them modifies the interface configuration, which does not happens at > the moment. > > This came out as a middle ground from a discussion with Niklas. As > stated in the cover letter if this patch makes someone uncomfortable, feel > free to drop it not to hold back the rest of the series which has been > well received instead. What I don't agree with and sparked this debate from my side was the deletion of properties in dts files which correctly does describe the bus but which are not currently parsed by the driver. To me that is decreasing the value of the dts. If on the other hand the goal is to deprecate a property from the video-interfaces.txt by slowly removing them from dts where the driver don't use them I'm all for it. But I don't think this is the case here right? > > Thanks > j > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > - hsync-active: see [1] for description. Default is active high. > > > - vsync-active: see [1] for description. Default is active high. > > > - data-enable-active: polarity of CLKENB signal, see [1] for > > > -- > > > 2.7.4 > > > > > > > -- > > Sakari Ailus > > sakari.ailus@linux.intel.com -- Regards, Niklas S�derlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-06-27 5:24 ` Niklas Söderlund 0 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-06-27 5:24 UTC (permalink / raw) To: jacopo mondi Cc: Sakari Ailus, Jacopo Mondi, laurent.pinchart, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hi Jacopo, Sakari and Laurent, Jacopo, thanks for your patch and continued work on resolving this. On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > Hi Sakari, > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > Add a note to the R-Car VIN interface bindings to clarify that all > > > properties listed as generic properties in video-interfaces.txt can > > > be included in port@0 endpoint, but if not explicitly listed in the > > > interface bindings documentation, they do not modify it behaviour. > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > --- > > > Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > index 8130849..03544c7 100644 > > > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on SoC. > > > instances that are connected to external pins should have port 0. > > > > > > - Optional properties for endpoint nodes of port@0: > > > + > > > + All properties described in [1] and which apply to the selected > > > + media bus type could be optionally listed here to better describe > > > + the current hardware configuration, but only the following ones do > > > + actually modify the VIN interface behaviour: > > > + I'm not sure the description have to be as explicit to that the properties in 'video-interfaces.txt' are not currently used by the driver. I find it not relevant which ones are used or not, what is important for me is that all properties in 'video-interfaces.txt' which can be used to describe the specific bus are valid for the DT description. On a side note, in rcar_vin.txt we have this section describing the Gen2 bindings: The per-board settings Gen2 platforms: - port sub-node describing a single endpoint connected to the vin as described in video-interfaces.txt[1]. Only the first one will be considered as each vin interface has one input port. This whole series only deals with documenting the Gen3 optional properties and not the Gen2. Maybe with parallel input support for Gen3 patches on there way to making it upstream this series should be extended to in a good way merge the port@0 optional properties for both generations of hardware? > > > > I don't think this should be needed. You should only have properties that > > describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > here for re-proposing it: I started by removing properties in some DT > files for older Renesas board which listed endpoint properties not > documented in the VIN's bindings and not parsed by the VIN driver [1] > Niklas (but Simon and Geert seems to agree here) opposed to that > patch, as those properties where described in 'video-interfaces.txt' and > even if not parsed by the current driver implementation, they actually > describe hardware. I rebated that only properties listed in the device > bindings documentation should actually be used, and having properties > not parsed by the driver confuses users, which may expect changing > them modifies the interface configuration, which does not happens at > the moment. > > This came out as a middle ground from a discussion with Niklas. As > stated in the cover letter if this patch makes someone uncomfortable, feel > free to drop it not to hold back the rest of the series which has been > well received instead. What I don't agree with and sparked this debate from my side was the deletion of properties in dts files which correctly does describe the bus but which are not currently parsed by the driver. To me that is decreasing the value of the dts. If on the other hand the goal is to deprecate a property from the video-interfaces.txt by slowly removing them from dts where the driver don't use them I'm all for it. But I don't think this is the case here right? > > Thanks > j > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > - hsync-active: see [1] for description. Default is active high. > > > - vsync-active: see [1] for description. Default is active high. > > > - data-enable-active: polarity of CLKENB signal, see [1] for > > > -- > > > 2.7.4 > > > > > > > -- > > Sakari Ailus > > sakari.ailus@linux.intel.com -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-06-27 5:24 ` Niklas Söderlund (?) @ 2018-07-02 7:19 ` Laurent Pinchart -1 siblings, 0 replies; 44+ messages in thread From: Laurent Pinchart @ 2018-07-02 7:19 UTC (permalink / raw) To: Niklas Söderlund Cc: devicetree, jacopo mondi, robh+dt, linux-renesas-soc, horms, geert, Sakari Ailus, Jacopo Mondi, mchehab, hans.verkuil, linux-arm-kernel, linux-media Hi Niklas, On Wednesday, 27 June 2018 08:24:31 EEST Niklas Söderlund wrote: > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > >>> Add a note to the R-Car VIN interface bindings to clarify that all > >>> properties listed as generic properties in video-interfaces.txt can > >>> be included in port@0 endpoint, but if not explicitly listed in the > >>> interface bindings documentation, they do not modify it behaviour. > >>> > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >>> --- > >>> > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > >>> 8130849..03544c7 100644 > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > >>> SoC. > >>> > >>> instances that are connected to external pins should have port > >>> 0. > >>> > >>> - Optional properties for endpoint nodes of port@0: > >>> + > >>> + All properties described in [1] and which apply to the > >>> selected > >>> + media bus type could be optionally listed here to better > >>> describe > >>> + the current hardware configuration, but only the following > >>> ones do > >>> + actually modify the VIN interface behaviour: > >>> + > > I'm not sure the description have to be as explicit to that the > properties in 'video-interfaces.txt' are not currently used by the > driver. I find it not relevant which ones are used or not, what is > important for me is that all properties in 'video-interfaces.txt' which > can be used to describe the specific bus are valid for the DT > description. I agree with you. The driver is irrelevant in this context. What matters is which properties are applicable to the bus. For instance, if the VIN parallel input supports configurable polarities for the h/v sync signals, hsync-active and vsync-active should be listed in the bindings. On the other hand, if the polarities are fixed, then the properties are not needed. > On a side note, in rcar_vin.txt we have this section describing the Gen2 > bindings: > > The per-board settings Gen2 platforms: > - port sub-node describing a single endpoint connected to the vin > as described in video-interfaces.txt[1]. Only the first one will > be considered as each vin interface has one input port. > > This whole series only deals with documenting the Gen3 optional > properties and not the Gen2. Maybe with parallel input support for Gen3 > patches on there way to making it upstream this series should be > extended to in a good way merge the port@0 optional properties for both > generations of hardware? That would be nice too :-) > >> I don't think this should be needed. You should only have properties > >> that describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > > here for re-proposing it: I started by removing properties in some DT > > files for older Renesas board which listed endpoint properties not > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > Niklas (but Simon and Geert seems to agree here) opposed to that > > patch, as those properties where described in 'video-interfaces.txt' and > > even if not parsed by the current driver implementation, they actually > > describe hardware. I rebated that only properties listed in the device > > bindings documentation should actually be used, and having properties > > not parsed by the driver confuses users, which may expect changing > > them modifies the interface configuration, which does not happens at > > the moment. > > > > This came out as a middle ground from a discussion with Niklas. As > > stated in the cover letter if this patch makes someone uncomfortable, feel > > free to drop it not to hold back the rest of the series which has been > > well received instead. > > What I don't agree with and sparked this debate from my side was the > deletion of properties in dts files which correctly does describe the > bus but which are not currently parsed by the driver. To me that is > decreasing the value of the dts. If on the other hand the goal is to > deprecate a property from the video-interfaces.txt by slowly removing > them from dts where the driver don't use them I'm all for it. But I > don't think this is the case here right? I think you're right, I don't think that's the case. We should not remove properties from the dts files when they correctly describe the hardware and have an added-value. On the other hand, if a property correctly describes the hardware, but is constrained to a single value due to hardware limitations, then we can omit it. > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > >>> - hsync-active: see [1] for description. Default is active high. > >>> - vsync-active: see [1] for description. Default is active high. > >>> - data-enable-active: polarity of CLKENB signal, see [1] for -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-02 7:19 ` Laurent Pinchart 0 siblings, 0 replies; 44+ messages in thread From: Laurent Pinchart @ 2018-07-02 7:19 UTC (permalink / raw) To: linux-arm-kernel Hi Niklas, On Wednesday, 27 June 2018 08:24:31 EEST Niklas S?derlund wrote: > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > >>> Add a note to the R-Car VIN interface bindings to clarify that all > >>> properties listed as generic properties in video-interfaces.txt can > >>> be included in port at 0 endpoint, but if not explicitly listed in the > >>> interface bindings documentation, they do not modify it behaviour. > >>> > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >>> --- > >>> > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > >>> 8130849..03544c7 100644 > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > >>> SoC. > >>> > >>> instances that are connected to external pins should have port > >>> 0. > >>> > >>> - Optional properties for endpoint nodes of port at 0: > >>> + > >>> + All properties described in [1] and which apply to the > >>> selected > >>> + media bus type could be optionally listed here to better > >>> describe > >>> + the current hardware configuration, but only the following > >>> ones do > >>> + actually modify the VIN interface behaviour: > >>> + > > I'm not sure the description have to be as explicit to that the > properties in 'video-interfaces.txt' are not currently used by the > driver. I find it not relevant which ones are used or not, what is > important for me is that all properties in 'video-interfaces.txt' which > can be used to describe the specific bus are valid for the DT > description. I agree with you. The driver is irrelevant in this context. What matters is which properties are applicable to the bus. For instance, if the VIN parallel input supports configurable polarities for the h/v sync signals, hsync-active and vsync-active should be listed in the bindings. On the other hand, if the polarities are fixed, then the properties are not needed. > On a side note, in rcar_vin.txt we have this section describing the Gen2 > bindings: > > The per-board settings Gen2 platforms: > - port sub-node describing a single endpoint connected to the vin > as described in video-interfaces.txt[1]. Only the first one will > be considered as each vin interface has one input port. > > This whole series only deals with documenting the Gen3 optional > properties and not the Gen2. Maybe with parallel input support for Gen3 > patches on there way to making it upstream this series should be > extended to in a good way merge the port at 0 optional properties for both > generations of hardware? That would be nice too :-) > >> I don't think this should be needed. You should only have properties > >> that describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > > here for re-proposing it: I started by removing properties in some DT > > files for older Renesas board which listed endpoint properties not > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > Niklas (but Simon and Geert seems to agree here) opposed to that > > patch, as those properties where described in 'video-interfaces.txt' and > > even if not parsed by the current driver implementation, they actually > > describe hardware. I rebated that only properties listed in the device > > bindings documentation should actually be used, and having properties > > not parsed by the driver confuses users, which may expect changing > > them modifies the interface configuration, which does not happens at > > the moment. > > > > This came out as a middle ground from a discussion with Niklas. As > > stated in the cover letter if this patch makes someone uncomfortable, feel > > free to drop it not to hold back the rest of the series which has been > > well received instead. > > What I don't agree with and sparked this debate from my side was the > deletion of properties in dts files which correctly does describe the > bus but which are not currently parsed by the driver. To me that is > decreasing the value of the dts. If on the other hand the goal is to > deprecate a property from the video-interfaces.txt by slowly removing > them from dts where the driver don't use them I'm all for it. But I > don't think this is the case here right? I think you're right, I don't think that's the case. We should not remove properties from the dts files when they correctly describe the hardware and have an added-value. On the other hand, if a property correctly describes the hardware, but is constrained to a single value due to hardware limitations, then we can omit it. > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > >>> - hsync-active: see [1] for description. Default is active high. > >>> - vsync-active: see [1] for description. Default is active high. > >>> - data-enable-active: polarity of CLKENB signal, see [1] for -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-02 7:19 ` Laurent Pinchart 0 siblings, 0 replies; 44+ messages in thread From: Laurent Pinchart @ 2018-07-02 7:19 UTC (permalink / raw) To: Niklas Söderlund Cc: jacopo mondi, Sakari Ailus, Jacopo Mondi, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hi Niklas, On Wednesday, 27 June 2018 08:24:31 EEST Niklas Söderlund wrote: > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > >>> Add a note to the R-Car VIN interface bindings to clarify that all > >>> properties listed as generic properties in video-interfaces.txt can > >>> be included in port@0 endpoint, but if not explicitly listed in the > >>> interface bindings documentation, they do not modify it behaviour. > >>> > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >>> --- > >>> > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > >>> 8130849..03544c7 100644 > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > >>> SoC. > >>> > >>> instances that are connected to external pins should have port > >>> 0. > >>> > >>> - Optional properties for endpoint nodes of port@0: > >>> + > >>> + All properties described in [1] and which apply to the > >>> selected > >>> + media bus type could be optionally listed here to better > >>> describe > >>> + the current hardware configuration, but only the following > >>> ones do > >>> + actually modify the VIN interface behaviour: > >>> + > > I'm not sure the description have to be as explicit to that the > properties in 'video-interfaces.txt' are not currently used by the > driver. I find it not relevant which ones are used or not, what is > important for me is that all properties in 'video-interfaces.txt' which > can be used to describe the specific bus are valid for the DT > description. I agree with you. The driver is irrelevant in this context. What matters is which properties are applicable to the bus. For instance, if the VIN parallel input supports configurable polarities for the h/v sync signals, hsync-active and vsync-active should be listed in the bindings. On the other hand, if the polarities are fixed, then the properties are not needed. > On a side note, in rcar_vin.txt we have this section describing the Gen2 > bindings: > > The per-board settings Gen2 platforms: > - port sub-node describing a single endpoint connected to the vin > as described in video-interfaces.txt[1]. Only the first one will > be considered as each vin interface has one input port. > > This whole series only deals with documenting the Gen3 optional > properties and not the Gen2. Maybe with parallel input support for Gen3 > patches on there way to making it upstream this series should be > extended to in a good way merge the port@0 optional properties for both > generations of hardware? That would be nice too :-) > >> I don't think this should be needed. You should only have properties > >> that describe the hardware configuration in a given system. > > > > There has been quite some debate on this, and please bear with me > > here for re-proposing it: I started by removing properties in some DT > > files for older Renesas board which listed endpoint properties not > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > Niklas (but Simon and Geert seems to agree here) opposed to that > > patch, as those properties where described in 'video-interfaces.txt' and > > even if not parsed by the current driver implementation, they actually > > describe hardware. I rebated that only properties listed in the device > > bindings documentation should actually be used, and having properties > > not parsed by the driver confuses users, which may expect changing > > them modifies the interface configuration, which does not happens at > > the moment. > > > > This came out as a middle ground from a discussion with Niklas. As > > stated in the cover letter if this patch makes someone uncomfortable, feel > > free to drop it not to hold back the rest of the series which has been > > well received instead. > > What I don't agree with and sparked this debate from my side was the > deletion of properties in dts files which correctly does describe the > bus but which are not currently parsed by the driver. To me that is > decreasing the value of the dts. If on the other hand the goal is to > deprecate a property from the video-interfaces.txt by slowly removing > them from dts where the driver don't use them I'm all for it. But I > don't think this is the case here right? I think you're right, I don't think that's the case. We should not remove properties from the dts files when they correctly describe the hardware and have an added-value. On the other hand, if a property correctly describes the hardware, but is constrained to a single value due to hardware limitations, then we can omit it. > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > >>> - hsync-active: see [1] for description. Default is active high. > >>> - vsync-active: see [1] for description. Default is active high. > >>> - data-enable-active: polarity of CLKENB signal, see [1] for -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-07-02 7:19 ` Laurent Pinchart (?) @ 2018-07-03 19:52 ` jacopo mondi -1 siblings, 0 replies; 44+ messages in thread From: jacopo mondi @ 2018-07-03 19:52 UTC (permalink / raw) To: Laurent Pinchart Cc: devicetree, robh+dt, linux-renesas-soc, horms, geert, Sakari Ailus, Niklas Söderlund, Jacopo Mondi, mchehab, hans.verkuil, linux-arm-kernel, linux-media [-- Attachment #1.1: Type: text/plain, Size: 7006 bytes --] Hi Laurent, Niklas, On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas Söderlund wrote: > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > >>> properties listed as generic properties in video-interfaces.txt can > > >>> be included in port@0 endpoint, but if not explicitly listed in the > > >>> interface bindings documentation, they do not modify it behaviour. > > >>> > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > >>> --- > > >>> > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > >>> 1 file changed, 6 insertions(+) > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > >>> 8130849..03544c7 100644 > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > >>> SoC. > > >>> > > >>> instances that are connected to external pins should have port > > >>> 0. > > >>> > > >>> - Optional properties for endpoint nodes of port@0: > > >>> + > > >>> + All properties described in [1] and which apply to the > > >>> selected > > >>> + media bus type could be optionally listed here to better > > >>> describe > > >>> + the current hardware configuration, but only the following > > >>> ones do > > >>> + actually modify the VIN interface behaviour: > > >>> + > > > > I'm not sure the description have to be as explicit to that the > > properties in 'video-interfaces.txt' are not currently used by the > > driver. I find it not relevant which ones are used or not, what is > > important for me is that all properties in 'video-interfaces.txt' which > > can be used to describe the specific bus are valid for the DT > > description. > > I agree with you. The driver is irrelevant in this context. What matters is > which properties are applicable to the bus. For instance, if the VIN parallel > input supports configurable polarities for the h/v sync signals, hsync-active > and vsync-active should be listed in the bindings. On the other hand, if the > polarities are fixed, then the properties are not needed. > Fine, I'll surrender then, you're too many to fight against :p Joking aside, I see your points, and I'll resend dropping this last bit and documenting which properties applies to the interface. I can list: - vsync-active (added by this series) - hsync-active (added by this series) - data-enable-active (added by this series) - field-active-even (to be added) I haven't find a way to configure the plck-sample value in the VINs registers. Should we skip it? I also think 'bus-width' and 'data-shift' are not directly configurable in registers but depends on SoC, the selected input interface, and the input image format (see tables at 26.3.1 datasheet revision 1.00), so it makes sense list them as acceptable properties. Do I have your ack on this so I can re-spin? > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > bindings: > > > > The per-board settings Gen2 platforms: > > - port sub-node describing a single endpoint connected to the vin > > as described in video-interfaces.txt[1]. Only the first one will > > be considered as each vin interface has one input port. > > > > This whole series only deals with documenting the Gen3 optional > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > patches on there way to making it upstream this series should be > > extended to in a good way merge the port@0 optional properties for both > > generations of hardware? > > That would be nice too :-) I see.. Well, I would keep the Gen2/Gen3 sections separate, as they have a different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could have several 'port' nodes enclosed in a 'ports' one). But I could indeed reference the optional endpoint properties of 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do you have any preference? Thanks j > > > >> I don't think this should be needed. You should only have properties > > >> that describe the hardware configuration in a given system. > > > > > > There has been quite some debate on this, and please bear with me > > > here for re-proposing it: I started by removing properties in some DT > > > files for older Renesas board which listed endpoint properties not > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > patch, as those properties where described in 'video-interfaces.txt' and > > > even if not parsed by the current driver implementation, they actually > > > describe hardware. I rebated that only properties listed in the device > > > bindings documentation should actually be used, and having properties > > > not parsed by the driver confuses users, which may expect changing > > > them modifies the interface configuration, which does not happens at > > > the moment. > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > free to drop it not to hold back the rest of the series which has been > > > well received instead. > > > > What I don't agree with and sparked this debate from my side was the > > deletion of properties in dts files which correctly does describe the > > bus but which are not currently parsed by the driver. To me that is > > decreasing the value of the dts. If on the other hand the goal is to > > deprecate a property from the video-interfaces.txt by slowly removing > > them from dts where the driver don't use them I'm all for it. But I > > don't think this is the case here right? > > I think you're right, I don't think that's the case. > > We should not remove properties from the dts files when they correctly > describe the hardware and have an added-value. On the other hand, if a > property correctly describes the hardware, but is constrained to a single > value due to hardware limitations, then we can omit it. > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > >>> - vsync-active: see [1] for description. Default is active high. > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > -- > Regards, > > Laurent Pinchart > > > [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-03 19:52 ` jacopo mondi 0 siblings, 0 replies; 44+ messages in thread From: jacopo mondi @ 2018-07-03 19:52 UTC (permalink / raw) To: linux-arm-kernel Hi Laurent, Niklas, On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas S?derlund wrote: > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > >>> properties listed as generic properties in video-interfaces.txt can > > >>> be included in port at 0 endpoint, but if not explicitly listed in the > > >>> interface bindings documentation, they do not modify it behaviour. > > >>> > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > >>> --- > > >>> > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > >>> 1 file changed, 6 insertions(+) > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > >>> 8130849..03544c7 100644 > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > >>> SoC. > > >>> > > >>> instances that are connected to external pins should have port > > >>> 0. > > >>> > > >>> - Optional properties for endpoint nodes of port at 0: > > >>> + > > >>> + All properties described in [1] and which apply to the > > >>> selected > > >>> + media bus type could be optionally listed here to better > > >>> describe > > >>> + the current hardware configuration, but only the following > > >>> ones do > > >>> + actually modify the VIN interface behaviour: > > >>> + > > > > I'm not sure the description have to be as explicit to that the > > properties in 'video-interfaces.txt' are not currently used by the > > driver. I find it not relevant which ones are used or not, what is > > important for me is that all properties in 'video-interfaces.txt' which > > can be used to describe the specific bus are valid for the DT > > description. > > I agree with you. The driver is irrelevant in this context. What matters is > which properties are applicable to the bus. For instance, if the VIN parallel > input supports configurable polarities for the h/v sync signals, hsync-active > and vsync-active should be listed in the bindings. On the other hand, if the > polarities are fixed, then the properties are not needed. > Fine, I'll surrender then, you're too many to fight against :p Joking aside, I see your points, and I'll resend dropping this last bit and documenting which properties applies to the interface. I can list: - vsync-active (added by this series) - hsync-active (added by this series) - data-enable-active (added by this series) - field-active-even (to be added) I haven't find a way to configure the plck-sample value in the VINs registers. Should we skip it? I also think 'bus-width' and 'data-shift' are not directly configurable in registers but depends on SoC, the selected input interface, and the input image format (see tables at 26.3.1 datasheet revision 1.00), so it makes sense list them as acceptable properties. Do I have your ack on this so I can re-spin? > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > bindings: > > > > The per-board settings Gen2 platforms: > > - port sub-node describing a single endpoint connected to the vin > > as described in video-interfaces.txt[1]. Only the first one will > > be considered as each vin interface has one input port. > > > > This whole series only deals with documenting the Gen3 optional > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > patches on there way to making it upstream this series should be > > extended to in a good way merge the port at 0 optional properties for both > > generations of hardware? > > That would be nice too :-) I see.. Well, I would keep the Gen2/Gen3 sections separate, as they have a different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could have several 'port' nodes enclosed in a 'ports' one). But I could indeed reference the optional endpoint properties of 'port at 0' of Gen3 bindings in Gen2 ones. Or the other way around. Do you have any preference? Thanks j > > > >> I don't think this should be needed. You should only have properties > > >> that describe the hardware configuration in a given system. > > > > > > There has been quite some debate on this, and please bear with me > > > here for re-proposing it: I started by removing properties in some DT > > > files for older Renesas board which listed endpoint properties not > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > patch, as those properties where described in 'video-interfaces.txt' and > > > even if not parsed by the current driver implementation, they actually > > > describe hardware. I rebated that only properties listed in the device > > > bindings documentation should actually be used, and having properties > > > not parsed by the driver confuses users, which may expect changing > > > them modifies the interface configuration, which does not happens at > > > the moment. > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > free to drop it not to hold back the rest of the series which has been > > > well received instead. > > > > What I don't agree with and sparked this debate from my side was the > > deletion of properties in dts files which correctly does describe the > > bus but which are not currently parsed by the driver. To me that is > > decreasing the value of the dts. If on the other hand the goal is to > > deprecate a property from the video-interfaces.txt by slowly removing > > them from dts where the driver don't use them I'm all for it. But I > > don't think this is the case here right? > > I think you're right, I don't think that's the case. > > We should not remove properties from the dts files when they correctly > describe the hardware and have an added-value. On the other hand, if a > property correctly describes the hardware, but is constrained to a single > value due to hardware limitations, then we can omit it. > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > >>> - vsync-active: see [1] for description. Default is active high. > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > -- > Regards, > > Laurent Pinchart > > > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180703/77435bfe/attachment.sig> ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-03 19:52 ` jacopo mondi 0 siblings, 0 replies; 44+ messages in thread From: jacopo mondi @ 2018-07-03 19:52 UTC (permalink / raw) To: Laurent Pinchart Cc: Niklas Söderlund, Sakari Ailus, Jacopo Mondi, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc [-- Attachment #1: Type: text/plain, Size: 7006 bytes --] Hi Laurent, Niklas, On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas Söderlund wrote: > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > >>> properties listed as generic properties in video-interfaces.txt can > > >>> be included in port@0 endpoint, but if not explicitly listed in the > > >>> interface bindings documentation, they do not modify it behaviour. > > >>> > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > >>> --- > > >>> > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > >>> 1 file changed, 6 insertions(+) > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > >>> 8130849..03544c7 100644 > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > >>> SoC. > > >>> > > >>> instances that are connected to external pins should have port > > >>> 0. > > >>> > > >>> - Optional properties for endpoint nodes of port@0: > > >>> + > > >>> + All properties described in [1] and which apply to the > > >>> selected > > >>> + media bus type could be optionally listed here to better > > >>> describe > > >>> + the current hardware configuration, but only the following > > >>> ones do > > >>> + actually modify the VIN interface behaviour: > > >>> + > > > > I'm not sure the description have to be as explicit to that the > > properties in 'video-interfaces.txt' are not currently used by the > > driver. I find it not relevant which ones are used or not, what is > > important for me is that all properties in 'video-interfaces.txt' which > > can be used to describe the specific bus are valid for the DT > > description. > > I agree with you. The driver is irrelevant in this context. What matters is > which properties are applicable to the bus. For instance, if the VIN parallel > input supports configurable polarities for the h/v sync signals, hsync-active > and vsync-active should be listed in the bindings. On the other hand, if the > polarities are fixed, then the properties are not needed. > Fine, I'll surrender then, you're too many to fight against :p Joking aside, I see your points, and I'll resend dropping this last bit and documenting which properties applies to the interface. I can list: - vsync-active (added by this series) - hsync-active (added by this series) - data-enable-active (added by this series) - field-active-even (to be added) I haven't find a way to configure the plck-sample value in the VINs registers. Should we skip it? I also think 'bus-width' and 'data-shift' are not directly configurable in registers but depends on SoC, the selected input interface, and the input image format (see tables at 26.3.1 datasheet revision 1.00), so it makes sense list them as acceptable properties. Do I have your ack on this so I can re-spin? > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > bindings: > > > > The per-board settings Gen2 platforms: > > - port sub-node describing a single endpoint connected to the vin > > as described in video-interfaces.txt[1]. Only the first one will > > be considered as each vin interface has one input port. > > > > This whole series only deals with documenting the Gen3 optional > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > patches on there way to making it upstream this series should be > > extended to in a good way merge the port@0 optional properties for both > > generations of hardware? > > That would be nice too :-) I see.. Well, I would keep the Gen2/Gen3 sections separate, as they have a different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could have several 'port' nodes enclosed in a 'ports' one). But I could indeed reference the optional endpoint properties of 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do you have any preference? Thanks j > > > >> I don't think this should be needed. You should only have properties > > >> that describe the hardware configuration in a given system. > > > > > > There has been quite some debate on this, and please bear with me > > > here for re-proposing it: I started by removing properties in some DT > > > files for older Renesas board which listed endpoint properties not > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > patch, as those properties where described in 'video-interfaces.txt' and > > > even if not parsed by the current driver implementation, they actually > > > describe hardware. I rebated that only properties listed in the device > > > bindings documentation should actually be used, and having properties > > > not parsed by the driver confuses users, which may expect changing > > > them modifies the interface configuration, which does not happens at > > > the moment. > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > free to drop it not to hold back the rest of the series which has been > > > well received instead. > > > > What I don't agree with and sparked this debate from my side was the > > deletion of properties in dts files which correctly does describe the > > bus but which are not currently parsed by the driver. To me that is > > decreasing the value of the dts. If on the other hand the goal is to > > deprecate a property from the video-interfaces.txt by slowly removing > > them from dts where the driver don't use them I'm all for it. But I > > don't think this is the case here right? > > I think you're right, I don't think that's the case. > > We should not remove properties from the dts files when they correctly > describe the hardware and have an added-value. On the other hand, if a > property correctly describes the hardware, but is constrained to a single > value due to hardware limitations, then we can omit it. > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > >>> - vsync-active: see [1] for description. Default is active high. > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > -- > Regards, > > Laurent Pinchart > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props 2018-07-03 19:52 ` jacopo mondi (?) (?) @ 2018-07-06 15:54 ` Niklas Söderlund -1 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-07-06 15:54 UTC (permalink / raw) To: jacopo mondi Cc: devicetree, robh+dt, linux-renesas-soc, horms, geert, Laurent Pinchart, Sakari Ailus, Jacopo Mondi, mchehab, hans.verkuil, linux-arm-kernel, linux-media Hi Jacopo, Thanks for not giving up on this work :-) On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote: > Hi Laurent, Niklas, > > On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > > Hi Niklas, > > > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas Söderlund wrote: > > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > > >>> properties listed as generic properties in video-interfaces.txt can > > > >>> be included in port@0 endpoint, but if not explicitly listed in the > > > >>> interface bindings documentation, they do not modify it behaviour. > > > >>> > > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > >>> --- > > > >>> > > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > >>> 1 file changed, 6 insertions(+) > > > >>> > > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > > >>> 8130849..03544c7 100644 > > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > > >>> SoC. > > > >>> > > > >>> instances that are connected to external pins should have port > > > >>> 0. > > > >>> > > > >>> - Optional properties for endpoint nodes of port@0: > > > >>> + > > > >>> + All properties described in [1] and which apply to the > > > >>> selected > > > >>> + media bus type could be optionally listed here to better > > > >>> describe > > > >>> + the current hardware configuration, but only the following > > > >>> ones do > > > >>> + actually modify the VIN interface behaviour: > > > >>> + > > > > > > I'm not sure the description have to be as explicit to that the > > > properties in 'video-interfaces.txt' are not currently used by the > > > driver. I find it not relevant which ones are used or not, what is > > > important for me is that all properties in 'video-interfaces.txt' which > > > can be used to describe the specific bus are valid for the DT > > > description. > > > > I agree with you. The driver is irrelevant in this context. What matters is > > which properties are applicable to the bus. For instance, if the VIN parallel > > input supports configurable polarities for the h/v sync signals, hsync-active > > and vsync-active should be listed in the bindings. On the other hand, if the > > polarities are fixed, then the properties are not needed. > > > > Fine, I'll surrender then, you're too many to fight against :p > > Joking aside, I see your points, and I'll resend dropping this last > bit and documenting which properties applies to the interface. > > I can list: > - vsync-active (added by this series) > - hsync-active (added by this series) > - data-enable-active (added by this series) > - field-active-even (to be added) > > I haven't find a way to configure the plck-sample value in the VINs > registers. Should we skip it? IIRC my findings agree with yours, no register to control plck-sample in the VIN register set. > > I also think 'bus-width' and 'data-shift' are not directly configurable in > registers but depends on SoC, the selected input interface, and the > input image format (see tables at 26.3.1 datasheet revision 1.00), so > it makes sense list them as acceptable properties. > > Do I have your ack on this so I can re-spin? I think I understand your plan and I think it sounds ok, please try it :-) > > > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > > bindings: > > > > > > The per-board settings Gen2 platforms: > > > - port sub-node describing a single endpoint connected to the vin > > > as described in video-interfaces.txt[1]. Only the first one will > > > be considered as each vin interface has one input port. > > > > > > This whole series only deals with documenting the Gen3 optional > > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > > patches on there way to making it upstream this series should be > > > extended to in a good way merge the port@0 optional properties for both > > > generations of hardware? > > > > That would be nice too :-) > > I see.. > > Well, I would keep the Gen2/Gen3 sections separate, as they have a > different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could > have several 'port' nodes enclosed in a 'ports' one). > > But I could indeed reference the optional endpoint properties of > 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do > you have any preference? My preference would be to list the properties in the Gen2 section and in the Gen3 port@0 section refer to the Gen2 optional properties. > > Thanks > j > > > > > > > >> I don't think this should be needed. You should only have properties > > > >> that describe the hardware configuration in a given system. > > > > > > > > There has been quite some debate on this, and please bear with me > > > > here for re-proposing it: I started by removing properties in some DT > > > > files for older Renesas board which listed endpoint properties not > > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > > patch, as those properties where described in 'video-interfaces.txt' and > > > > even if not parsed by the current driver implementation, they actually > > > > describe hardware. I rebated that only properties listed in the device > > > > bindings documentation should actually be used, and having properties > > > > not parsed by the driver confuses users, which may expect changing > > > > them modifies the interface configuration, which does not happens at > > > > the moment. > > > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > > free to drop it not to hold back the rest of the series which has been > > > > well received instead. > > > > > > What I don't agree with and sparked this debate from my side was the > > > deletion of properties in dts files which correctly does describe the > > > bus but which are not currently parsed by the driver. To me that is > > > decreasing the value of the dts. If on the other hand the goal is to > > > deprecate a property from the video-interfaces.txt by slowly removing > > > them from dts where the driver don't use them I'm all for it. But I > > > don't think this is the case here right? > > > > I think you're right, I don't think that's the case. > > > > We should not remove properties from the dts files when they correctly > > describe the hardware and have an added-value. On the other hand, if a > > property correctly describes the hardware, but is constrained to a single > > value due to hardware limitations, then we can omit it. > > > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > > >>> - vsync-active: see [1] for description. Default is active high. > > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > > > -- > > Regards, > > > > Laurent Pinchart > > > > > > -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-06 15:54 ` Niklas Söderlund 0 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-07-06 15:54 UTC (permalink / raw) To: linux-arm-kernel Hi Jacopo, Thanks for not giving up on this work :-) On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote: > Hi Laurent, Niklas, > > On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > > Hi Niklas, > > > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas S?derlund wrote: > > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > > >>> properties listed as generic properties in video-interfaces.txt can > > > >>> be included in port at 0 endpoint, but if not explicitly listed in the > > > >>> interface bindings documentation, they do not modify it behaviour. > > > >>> > > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > >>> --- > > > >>> > > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > >>> 1 file changed, 6 insertions(+) > > > >>> > > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > > >>> 8130849..03544c7 100644 > > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > > >>> SoC. > > > >>> > > > >>> instances that are connected to external pins should have port > > > >>> 0. > > > >>> > > > >>> - Optional properties for endpoint nodes of port at 0: > > > >>> + > > > >>> + All properties described in [1] and which apply to the > > > >>> selected > > > >>> + media bus type could be optionally listed here to better > > > >>> describe > > > >>> + the current hardware configuration, but only the following > > > >>> ones do > > > >>> + actually modify the VIN interface behaviour: > > > >>> + > > > > > > I'm not sure the description have to be as explicit to that the > > > properties in 'video-interfaces.txt' are not currently used by the > > > driver. I find it not relevant which ones are used or not, what is > > > important for me is that all properties in 'video-interfaces.txt' which > > > can be used to describe the specific bus are valid for the DT > > > description. > > > > I agree with you. The driver is irrelevant in this context. What matters is > > which properties are applicable to the bus. For instance, if the VIN parallel > > input supports configurable polarities for the h/v sync signals, hsync-active > > and vsync-active should be listed in the bindings. On the other hand, if the > > polarities are fixed, then the properties are not needed. > > > > Fine, I'll surrender then, you're too many to fight against :p > > Joking aside, I see your points, and I'll resend dropping this last > bit and documenting which properties applies to the interface. > > I can list: > - vsync-active (added by this series) > - hsync-active (added by this series) > - data-enable-active (added by this series) > - field-active-even (to be added) > > I haven't find a way to configure the plck-sample value in the VINs > registers. Should we skip it? IIRC my findings agree with yours, no register to control plck-sample in the VIN register set. > > I also think 'bus-width' and 'data-shift' are not directly configurable in > registers but depends on SoC, the selected input interface, and the > input image format (see tables at 26.3.1 datasheet revision 1.00), so > it makes sense list them as acceptable properties. > > Do I have your ack on this so I can re-spin? I think I understand your plan and I think it sounds ok, please try it :-) > > > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > > bindings: > > > > > > The per-board settings Gen2 platforms: > > > - port sub-node describing a single endpoint connected to the vin > > > as described in video-interfaces.txt[1]. Only the first one will > > > be considered as each vin interface has one input port. > > > > > > This whole series only deals with documenting the Gen3 optional > > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > > patches on there way to making it upstream this series should be > > > extended to in a good way merge the port at 0 optional properties for both > > > generations of hardware? > > > > That would be nice too :-) > > I see.. > > Well, I would keep the Gen2/Gen3 sections separate, as they have a > different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could > have several 'port' nodes enclosed in a 'ports' one). > > But I could indeed reference the optional endpoint properties of > 'port at 0' of Gen3 bindings in Gen2 ones. Or the other way around. Do > you have any preference? My preference would be to list the properties in the Gen2 section and in the Gen3 port at 0 section refer to the Gen2 optional properties. > > Thanks > j > > > > > > > >> I don't think this should be needed. You should only have properties > > > >> that describe the hardware configuration in a given system. > > > > > > > > There has been quite some debate on this, and please bear with me > > > > here for re-proposing it: I started by removing properties in some DT > > > > files for older Renesas board which listed endpoint properties not > > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > > patch, as those properties where described in 'video-interfaces.txt' and > > > > even if not parsed by the current driver implementation, they actually > > > > describe hardware. I rebated that only properties listed in the device > > > > bindings documentation should actually be used, and having properties > > > > not parsed by the driver confuses users, which may expect changing > > > > them modifies the interface configuration, which does not happens at > > > > the moment. > > > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > > free to drop it not to hold back the rest of the series which has been > > > > well received instead. > > > > > > What I don't agree with and sparked this debate from my side was the > > > deletion of properties in dts files which correctly does describe the > > > bus but which are not currently parsed by the driver. To me that is > > > decreasing the value of the dts. If on the other hand the goal is to > > > deprecate a property from the video-interfaces.txt by slowly removing > > > them from dts where the driver don't use them I'm all for it. But I > > > don't think this is the case here right? > > > > I think you're right, I don't think that's the case. > > > > We should not remove properties from the dts files when they correctly > > describe the hardware and have an added-value. On the other hand, if a > > property correctly describes the hardware, but is constrained to a single > > value due to hardware limitations, then we can omit it. > > > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > > >>> - vsync-active: see [1] for description. Default is active high. > > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > > > -- > > Regards, > > > > Laurent Pinchart > > > > > > -- Regards, Niklas S?derlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-06 15:54 ` Niklas Söderlund 0 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-07-06 15:54 UTC (permalink / raw) To: jacopo mondi Cc: Laurent Pinchart, Sakari Ailus, Jacopo Mondi, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hi Jacopo, Thanks for not giving up on this work :-) On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote: > Hi Laurent, Niklas, > > On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > > Hi Niklas, > > > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas S�derlund wrote: > > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > > >>> properties listed as generic properties in video-interfaces.txt can > > > >>> be included in port@0 endpoint, but if not explicitly listed in the > > > >>> interface bindings documentation, they do not modify it behaviour. > > > >>> > > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > >>> --- > > > >>> > > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > >>> 1 file changed, 6 insertions(+) > > > >>> > > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > > >>> 8130849..03544c7 100644 > > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > > >>> SoC. > > > >>> > > > >>> instances that are connected to external pins should have port > > > >>> 0. > > > >>> > > > >>> - Optional properties for endpoint nodes of port@0: > > > >>> + > > > >>> + All properties described in [1] and which apply to the > > > >>> selected > > > >>> + media bus type could be optionally listed here to better > > > >>> describe > > > >>> + the current hardware configuration, but only the following > > > >>> ones do > > > >>> + actually modify the VIN interface behaviour: > > > >>> + > > > > > > I'm not sure the description have to be as explicit to that the > > > properties in 'video-interfaces.txt' are not currently used by the > > > driver. I find it not relevant which ones are used or not, what is > > > important for me is that all properties in 'video-interfaces.txt' which > > > can be used to describe the specific bus are valid for the DT > > > description. > > > > I agree with you. The driver is irrelevant in this context. What matters is > > which properties are applicable to the bus. For instance, if the VIN parallel > > input supports configurable polarities for the h/v sync signals, hsync-active > > and vsync-active should be listed in the bindings. On the other hand, if the > > polarities are fixed, then the properties are not needed. > > > > Fine, I'll surrender then, you're too many to fight against :p > > Joking aside, I see your points, and I'll resend dropping this last > bit and documenting which properties applies to the interface. > > I can list: > - vsync-active (added by this series) > - hsync-active (added by this series) > - data-enable-active (added by this series) > - field-active-even (to be added) > > I haven't find a way to configure the plck-sample value in the VINs > registers. Should we skip it? IIRC my findings agree with yours, no register to control plck-sample in the VIN register set. > > I also think 'bus-width' and 'data-shift' are not directly configurable in > registers but depends on SoC, the selected input interface, and the > input image format (see tables at 26.3.1 datasheet revision 1.00), so > it makes sense list them as acceptable properties. > > Do I have your ack on this so I can re-spin? I think I understand your plan and I think it sounds ok, please try it :-) > > > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > > bindings: > > > > > > The per-board settings Gen2 platforms: > > > - port sub-node describing a single endpoint connected to the vin > > > as described in video-interfaces.txt[1]. Only the first one will > > > be considered as each vin interface has one input port. > > > > > > This whole series only deals with documenting the Gen3 optional > > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > > patches on there way to making it upstream this series should be > > > extended to in a good way merge the port@0 optional properties for both > > > generations of hardware? > > > > That would be nice too :-) > > I see.. > > Well, I would keep the Gen2/Gen3 sections separate, as they have a > different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could > have several 'port' nodes enclosed in a 'ports' one). > > But I could indeed reference the optional endpoint properties of > 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do > you have any preference? My preference would be to list the properties in the Gen2 section and in the Gen3 port@0 section refer to the Gen2 optional properties. > > Thanks > j > > > > > > > >> I don't think this should be needed. You should only have properties > > > >> that describe the hardware configuration in a given system. > > > > > > > > There has been quite some debate on this, and please bear with me > > > > here for re-proposing it: I started by removing properties in some DT > > > > files for older Renesas board which listed endpoint properties not > > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > > patch, as those properties where described in 'video-interfaces.txt' and > > > > even if not parsed by the current driver implementation, they actually > > > > describe hardware. I rebated that only properties listed in the device > > > > bindings documentation should actually be used, and having properties > > > > not parsed by the driver confuses users, which may expect changing > > > > them modifies the interface configuration, which does not happens at > > > > the moment. > > > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > > free to drop it not to hold back the rest of the series which has been > > > > well received instead. > > > > > > What I don't agree with and sparked this debate from my side was the > > > deletion of properties in dts files which correctly does describe the > > > bus but which are not currently parsed by the driver. To me that is > > > decreasing the value of the dts. If on the other hand the goal is to > > > deprecate a property from the video-interfaces.txt by slowly removing > > > them from dts where the driver don't use them I'm all for it. But I > > > don't think this is the case here right? > > > > I think you're right, I don't think that's the case. > > > > We should not remove properties from the dts files when they correctly > > describe the hardware and have an added-value. On the other hand, if a > > property correctly describes the hardware, but is constrained to a single > > value due to hardware limitations, then we can omit it. > > > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > > >>> - vsync-active: see [1] for description. Default is active high. > > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > > > -- > > Regards, > > > > Laurent Pinchart > > > > > > -- Regards, Niklas S�derlund ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props @ 2018-07-06 15:54 ` Niklas Söderlund 0 siblings, 0 replies; 44+ messages in thread From: Niklas Söderlund @ 2018-07-06 15:54 UTC (permalink / raw) To: jacopo mondi Cc: Laurent Pinchart, Sakari Ailus, Jacopo Mondi, horms, geert, mchehab, hans.verkuil, robh+dt, devicetree, linux-arm-kernel, linux-media, linux-renesas-soc Hi Jacopo, Thanks for not giving up on this work :-) On 2018-07-03 21:52:38 +0200, Jacopo Mondi wrote: > Hi Laurent, Niklas, > > On Mon, Jul 02, 2018 at 10:19:25AM +0300, Laurent Pinchart wrote: > > Hi Niklas, > > > > On Wednesday, 27 June 2018 08:24:31 EEST Niklas Söderlund wrote: > > > On 2018-06-13 10:54:55 +0200, Jacopo Mondi wrote: > > > > On Tue, Jun 12, 2018 at 06:45:53PM +0300, Sakari Ailus wrote: > > > >> On Tue, Jun 12, 2018 at 04:26:06PM +0200, Jacopo Mondi wrote: > > > >>> Add a note to the R-Car VIN interface bindings to clarify that all > > > >>> properties listed as generic properties in video-interfaces.txt can > > > >>> be included in port@0 endpoint, but if not explicitly listed in the > > > >>> interface bindings documentation, they do not modify it behaviour. > > > >>> > > > >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > >>> --- > > > >>> > > > >>> Documentation/devicetree/bindings/media/rcar_vin.txt | 6 ++++++ > > > >>> 1 file changed, 6 insertions(+) > > > >>> > > > >>> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> b/Documentation/devicetree/bindings/media/rcar_vin.txt index > > > >>> 8130849..03544c7 100644 > > > >>> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > > > >>> @@ -55,6 +55,12 @@ from local SoC CSI-2 receivers (port1) depending on > > > >>> SoC. > > > >>> > > > >>> instances that are connected to external pins should have port > > > >>> 0. > > > >>> > > > >>> - Optional properties for endpoint nodes of port@0: > > > >>> + > > > >>> + All properties described in [1] and which apply to the > > > >>> selected > > > >>> + media bus type could be optionally listed here to better > > > >>> describe > > > >>> + the current hardware configuration, but only the following > > > >>> ones do > > > >>> + actually modify the VIN interface behaviour: > > > >>> + > > > > > > I'm not sure the description have to be as explicit to that the > > > properties in 'video-interfaces.txt' are not currently used by the > > > driver. I find it not relevant which ones are used or not, what is > > > important for me is that all properties in 'video-interfaces.txt' which > > > can be used to describe the specific bus are valid for the DT > > > description. > > > > I agree with you. The driver is irrelevant in this context. What matters is > > which properties are applicable to the bus. For instance, if the VIN parallel > > input supports configurable polarities for the h/v sync signals, hsync-active > > and vsync-active should be listed in the bindings. On the other hand, if the > > polarities are fixed, then the properties are not needed. > > > > Fine, I'll surrender then, you're too many to fight against :p > > Joking aside, I see your points, and I'll resend dropping this last > bit and documenting which properties applies to the interface. > > I can list: > - vsync-active (added by this series) > - hsync-active (added by this series) > - data-enable-active (added by this series) > - field-active-even (to be added) > > I haven't find a way to configure the plck-sample value in the VINs > registers. Should we skip it? IIRC my findings agree with yours, no register to control plck-sample in the VIN register set. > > I also think 'bus-width' and 'data-shift' are not directly configurable in > registers but depends on SoC, the selected input interface, and the > input image format (see tables at 26.3.1 datasheet revision 1.00), so > it makes sense list them as acceptable properties. > > Do I have your ack on this so I can re-spin? I think I understand your plan and I think it sounds ok, please try it :-) > > > > On a side note, in rcar_vin.txt we have this section describing the Gen2 > > > bindings: > > > > > > The per-board settings Gen2 platforms: > > > - port sub-node describing a single endpoint connected to the vin > > > as described in video-interfaces.txt[1]. Only the first one will > > > be considered as each vin interface has one input port. > > > > > > This whole series only deals with documenting the Gen3 optional > > > properties and not the Gen2. Maybe with parallel input support for Gen3 > > > patches on there way to making it upstream this series should be > > > extended to in a good way merge the port@0 optional properties for both > > > generations of hardware? > > > > That would be nice too :-) > > I see.. > > Well, I would keep the Gen2/Gen3 sections separate, as they have a > different layout of port nodes (Gen2 has a single 'port' node, while Gen3 could > have several 'port' nodes enclosed in a 'ports' one). > > But I could indeed reference the optional endpoint properties of > 'port@0' of Gen3 bindings in Gen2 ones. Or the other way around. Do > you have any preference? My preference would be to list the properties in the Gen2 section and in the Gen3 port@0 section refer to the Gen2 optional properties. > > Thanks > j > > > > > > > >> I don't think this should be needed. You should only have properties > > > >> that describe the hardware configuration in a given system. > > > > > > > > There has been quite some debate on this, and please bear with me > > > > here for re-proposing it: I started by removing properties in some DT > > > > files for older Renesas board which listed endpoint properties not > > > > documented in the VIN's bindings and not parsed by the VIN driver [1] > > > > Niklas (but Simon and Geert seems to agree here) opposed to that > > > > patch, as those properties where described in 'video-interfaces.txt' and > > > > even if not parsed by the current driver implementation, they actually > > > > describe hardware. I rebated that only properties listed in the device > > > > bindings documentation should actually be used, and having properties > > > > not parsed by the driver confuses users, which may expect changing > > > > them modifies the interface configuration, which does not happens at > > > > the moment. > > > > > > > > This came out as a middle ground from a discussion with Niklas. As > > > > stated in the cover letter if this patch makes someone uncomfortable, feel > > > > free to drop it not to hold back the rest of the series which has been > > > > well received instead. > > > > > > What I don't agree with and sparked this debate from my side was the > > > deletion of properties in dts files which correctly does describe the > > > bus but which are not currently parsed by the driver. To me that is > > > decreasing the value of the dts. If on the other hand the goal is to > > > deprecate a property from the video-interfaces.txt by slowly removing > > > them from dts where the driver don't use them I'm all for it. But I > > > don't think this is the case here right? > > > > I think you're right, I don't think that's the case. > > > > We should not remove properties from the dts files when they correctly > > describe the hardware and have an added-value. On the other hand, if a > > property correctly describes the hardware, but is constrained to a single > > value due to hardware limitations, then we can omit it. > > > > > > [1] https://www.spinics.net/lists/arm-kernel/msg656302.html > > > > > > > >>> - hsync-active: see [1] for description. Default is active high. > > > >>> - vsync-active: see [1] for description. Default is active high. > > > >>> - data-enable-active: polarity of CLKENB signal, see [1] for > > > > -- > > Regards, > > > > Laurent Pinchart > > > > > > -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2018-07-06 15:54 UTC | newest] Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-12 14:26 [PATCH v4 0/6] media: rcar-vin: Brush endpoint properties Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` [PATCH v4 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` [PATCH v4 2/6] dt-bindings: media: Document data-enable-active property Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` [PATCH v4 3/6] media: v4l2-fwnode: parse 'data-enable-active' prop Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` [PATCH v4 4/6] dt-bindings: media: rcar-vin: Add 'data-enable-active' Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` [PATCH v4 5/6] media: rcar-vin: Handle data-enable polarity Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` [PATCH v4 6/6] dt-bindings: media: rcar-vin: Clarify optional props Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 14:26 ` Jacopo Mondi 2018-06-12 15:45 ` Sakari Ailus 2018-06-12 15:45 ` Sakari Ailus 2018-06-12 15:45 ` Sakari Ailus 2018-06-12 16:15 ` Laurent Pinchart 2018-06-12 16:15 ` Laurent Pinchart 2018-06-12 16:15 ` Laurent Pinchart 2018-06-13 8:54 ` jacopo mondi 2018-06-13 8:54 ` jacopo mondi 2018-06-13 8:54 ` jacopo mondi 2018-06-27 5:24 ` Niklas Söderlund 2018-06-27 5:24 ` Niklas Söderlund 2018-06-27 5:24 ` Niklas Söderlund 2018-06-27 5:24 ` Niklas Söderlund 2018-07-02 7:19 ` Laurent Pinchart 2018-07-02 7:19 ` Laurent Pinchart 2018-07-02 7:19 ` Laurent Pinchart 2018-07-03 19:52 ` jacopo mondi 2018-07-03 19:52 ` jacopo mondi 2018-07-03 19:52 ` jacopo mondi 2018-07-06 15:54 ` Niklas Söderlund 2018-07-06 15:54 ` Niklas Söderlund 2018-07-06 15:54 ` Niklas Söderlund 2018-07-06 15:54 ` Niklas Söderlund
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.