devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] media: renesas-ceu: Introduce default mbus configuration
@ 2018-09-13 13:59 Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 1/3] dt-bindings: media: renesas-ceu: Refer to video-interfaces.txt Jacopo Mondi
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jacopo Mondi @ 2018-09-13 13:59 UTC (permalink / raw)
  To: sakari.ailus, mchehab, robh+dt, mark.rutland
  Cc: Jacopo Mondi, slongerbeam, laurent.pinchart, linux-media,
	devicetree, linux-renesas-soc

Hi Sakari,
   this is an attempt to use the newly introduce ability to specify default
configurations for media device drivers.

This patch depends on your and Steve's work at:
<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-fwnode-next>

I also updated the DT bindings as you suggested, documenting all other
properties the CEU interface allows to configure.

Thanks
  j

Jacopo Mondi (3):
  dt-bindings: media: renesas-ceu: Refer to video-interfaces.txt
  dt-bindings: media: renesas-ceu: Add more endpoint properties
  media: renesas-ceu: Use default mbus settings

 .../devicetree/bindings/media/renesas,ceu.txt        | 14 +++++++++-----
 arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi    |  4 ----
 drivers/media/platform/renesas-ceu.c                 | 20 +++++++++++---------
 3 files changed, 20 insertions(+), 18 deletions(-)

--
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] dt-bindings: media: renesas-ceu: Refer to video-interfaces.txt
  2018-09-13 13:59 [PATCH 0/3] media: renesas-ceu: Introduce default mbus configuration Jacopo Mondi
@ 2018-09-13 13:59 ` Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 2/3] dt-bindings: media: renesas-ceu: Add more endpoint properties Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 3/3] media: renesas-ceu: Use default mbus settings Jacopo Mondi
  2 siblings, 0 replies; 6+ messages in thread
From: Jacopo Mondi @ 2018-09-13 13:59 UTC (permalink / raw)
  To: sakari.ailus, mchehab, robh+dt, mark.rutland
  Cc: Jacopo Mondi, slongerbeam, laurent.pinchart, linux-media,
	devicetree, linux-renesas-soc

Refer to video-interfaces.txt when describing standard properties.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/renesas,ceu.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt b/Documentation/devicetree/bindings/media/renesas,ceu.txt
index 8a7a616..23e91106 100644
--- a/Documentation/devicetree/bindings/media/renesas,ceu.txt
+++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
@@ -17,15 +17,15 @@ Required properties:
 The CEU supports a single parallel input and should contain a single 'port'
 subnode with a single 'endpoint'. Connection to input devices are modeled
 according to the video interfaces OF bindings specified in:
-Documentation/devicetree/bindings/media/video-interfaces.txt
+[1] Documentation/devicetree/bindings/media/video-interfaces.txt
 
 Optional endpoint properties applicable to parallel input bus described in
 the above mentioned "video-interfaces.txt" file are supported.
 
-- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH respectively.
-  If property is not present, default is active high.
-- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH respectively.
-  If property is not present, default is active high.
+- hsync-active: See [1] for description. If property is not present,
+  default is active high.
+- vsync-active: See [1] for description. If property is not present,
+  default is active high.
 
 Example:
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/3] dt-bindings: media: renesas-ceu: Add more endpoint properties
  2018-09-13 13:59 [PATCH 0/3] media: renesas-ceu: Introduce default mbus configuration Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 1/3] dt-bindings: media: renesas-ceu: Refer to video-interfaces.txt Jacopo Mondi
@ 2018-09-13 13:59 ` Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 3/3] media: renesas-ceu: Use default mbus settings Jacopo Mondi
  2 siblings, 0 replies; 6+ messages in thread
From: Jacopo Mondi @ 2018-09-13 13:59 UTC (permalink / raw)
  To: sakari.ailus, mchehab, robh+dt, mark.rutland
  Cc: Jacopo Mondi, slongerbeam, laurent.pinchart, linux-media,
	devicetree, linux-renesas-soc

As the v4l2-fwnode framework now allows specifying defaults configurations,
expand the description of the optional endpoint properties for the CEU
interface to better explain which are their defaults values.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/renesas,ceu.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt b/Documentation/devicetree/bindings/media/renesas,ceu.txt
index 23e91106..3e2a265 100644
--- a/Documentation/devicetree/bindings/media/renesas,ceu.txt
+++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
@@ -26,6 +26,10 @@ the above mentioned "video-interfaces.txt" file are supported.
   default is active high.
 - vsync-active: See [1] for description. If property is not present,
   default is active high.
+- bus-width: See [1] for description. Accepted values are '8' and '16'.
+  If property is not present, default is '8'.
+- field-even-active: See [1] for description. If property is not present,
+  an even field is identified by a logic 0 (active-low signal).

 Example:

--
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/3] media: renesas-ceu: Use default mbus settings
  2018-09-13 13:59 [PATCH 0/3] media: renesas-ceu: Introduce default mbus configuration Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 1/3] dt-bindings: media: renesas-ceu: Refer to video-interfaces.txt Jacopo Mondi
  2018-09-13 13:59 ` [PATCH 2/3] dt-bindings: media: renesas-ceu: Add more endpoint properties Jacopo Mondi
@ 2018-09-13 13:59 ` Jacopo Mondi
  2018-09-13 14:22   ` Sakari Ailus
  2 siblings, 1 reply; 6+ messages in thread
From: Jacopo Mondi @ 2018-09-13 13:59 UTC (permalink / raw)
  To: sakari.ailus, mchehab, robh+dt, mark.rutland
  Cc: Jacopo Mondi, slongerbeam, laurent.pinchart, linux-media,
	devicetree, linux-renesas-soc

As the v4l2-fwnode now allows drivers to set defaults, and eventually
override them by specifying properties in DTS, use defaults for the CEU
driver.

Also remove endpoint properties from the gr-peach-audiocamerashield as
they match the defaults now specified in the driver code.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi |  4 ----
 drivers/media/platform/renesas-ceu.c              | 20 +++++++++++---------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
index e31a9e3..8d77579 100644
--- a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
+++ b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
@@ -69,10 +69,6 @@

 	port {
 		ceu_in: endpoint {
-			hsync-active = <1>;
-			vsync-active = <1>;
-			bus-width = <8>;
-			pclk-sample = <1>;
 			remote-endpoint = <&mt9v111_out>;
 		};
 	};
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
index 035f1d3..150196f 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -1551,7 +1551,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
 		return ret;

 	for (i = 0; i < num_ep; i++) {
-		struct v4l2_fwnode_endpoint fw_ep = { .bus_type = 0 };
+		struct v4l2_fwnode_endpoint fw_ep = {
+			.bus_type = V4L2_MBUS_PARALLEL,
+			.bus = {
+				.parallel = {
+					.flags = V4L2_MBUS_HSYNC_ACTIVE_HIGH |
+						 V4L2_MBUS_VSYNC_ACTIVE_HIGH,
+					.bus_width = 8,
+				},
+			},
+		};

 		ep = of_graph_get_endpoint_by_regs(of, 0, i);
 		if (!ep) {
@@ -1564,14 +1573,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
 		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep);
 		if (ret) {
 			dev_err(ceudev->dev,
-				"Unable to parse endpoint #%u.\n", i);
-			goto error_cleanup;
-		}
-
-		if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) {
-			dev_err(ceudev->dev,
-				"Only parallel input supported.\n");
-			ret = -EINVAL;
+				"Unable to parse endpoint #%u: %d.\n", i, ret);
 			goto error_cleanup;
 		}

--
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] media: renesas-ceu: Use default mbus settings
  2018-09-13 13:59 ` [PATCH 3/3] media: renesas-ceu: Use default mbus settings Jacopo Mondi
@ 2018-09-13 14:22   ` Sakari Ailus
  2018-09-13 14:28     ` jacopo mondi
  0 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2018-09-13 14:22 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: mchehab, robh+dt, mark.rutland, slongerbeam, laurent.pinchart,
	linux-media, devicetree, linux-renesas-soc

Hi Jacopo,

On Thu, Sep 13, 2018 at 03:59:51PM +0200, Jacopo Mondi wrote:
> As the v4l2-fwnode now allows drivers to set defaults, and eventually
> override them by specifying properties in DTS, use defaults for the CEU
> driver.
> 
> Also remove endpoint properties from the gr-peach-audiocamerashield as
> they match the defaults now specified in the driver code.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi |  4 ----
>  drivers/media/platform/renesas-ceu.c              | 20 +++++++++++---------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
> index e31a9e3..8d77579 100644
> --- a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
> +++ b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
> @@ -69,10 +69,6 @@
> 
>  	port {
>  		ceu_in: endpoint {
> -			hsync-active = <1>;
> -			vsync-active = <1>;
> -			bus-width = <8>;
> -			pclk-sample = <1>;

Do I understand correctly that pclk-sample was never relevant for the
hardware, and is removed because of that? You could mention that in the
commit message. That's perhaps a minor detail.

The set seems good to me.

>  			remote-endpoint = <&mt9v111_out>;
>  		};
>  	};
> diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
> index 035f1d3..150196f 100644
> --- a/drivers/media/platform/renesas-ceu.c
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -1551,7 +1551,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
>  		return ret;
> 
>  	for (i = 0; i < num_ep; i++) {
> -		struct v4l2_fwnode_endpoint fw_ep = { .bus_type = 0 };
> +		struct v4l2_fwnode_endpoint fw_ep = {
> +			.bus_type = V4L2_MBUS_PARALLEL,
> +			.bus = {
> +				.parallel = {
> +					.flags = V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> +						 V4L2_MBUS_VSYNC_ACTIVE_HIGH,
> +					.bus_width = 8,
> +				},
> +			},
> +		};
> 
>  		ep = of_graph_get_endpoint_by_regs(of, 0, i);
>  		if (!ep) {
> @@ -1564,14 +1573,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
>  		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep);
>  		if (ret) {
>  			dev_err(ceudev->dev,
> -				"Unable to parse endpoint #%u.\n", i);
> -			goto error_cleanup;
> -		}
> -
> -		if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) {
> -			dev_err(ceudev->dev,
> -				"Only parallel input supported.\n");
> -			ret = -EINVAL;
> +				"Unable to parse endpoint #%u: %d.\n", i, ret);
>  			goto error_cleanup;
>  		}
> 

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] media: renesas-ceu: Use default mbus settings
  2018-09-13 14:22   ` Sakari Ailus
@ 2018-09-13 14:28     ` jacopo mondi
  0 siblings, 0 replies; 6+ messages in thread
From: jacopo mondi @ 2018-09-13 14:28 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Jacopo Mondi, mchehab, robh+dt, mark.rutland, slongerbeam,
	laurent.pinchart, linux-media, devicetree, linux-renesas-soc

[-- Attachment #1: Type: text/plain, Size: 3296 bytes --]

Hi Sakari,

On Thu, Sep 13, 2018 at 05:22:36PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Thu, Sep 13, 2018 at 03:59:51PM +0200, Jacopo Mondi wrote:
> > As the v4l2-fwnode now allows drivers to set defaults, and eventually
> > override them by specifying properties in DTS, use defaults for the CEU
> > driver.
> >
> > Also remove endpoint properties from the gr-peach-audiocamerashield as
> > they match the defaults now specified in the driver code.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi |  4 ----
> >  drivers/media/platform/renesas-ceu.c              | 20 +++++++++++---------
> >  2 files changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
> > index e31a9e3..8d77579 100644
> > --- a/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
> > +++ b/arch/arm/boot/dts/gr-peach-audiocamerashield.dtsi
> > @@ -69,10 +69,6 @@
> >
> >  	port {
> >  		ceu_in: endpoint {
> > -			hsync-active = <1>;
> > -			vsync-active = <1>;
> > -			bus-width = <8>;
> > -			pclk-sample = <1>;
>
> Do I understand correctly that pclk-sample was never relevant for the
> hardware, and is removed because of that? You could mention that in the
> commit message. That's perhaps a minor detail.

Correct, it's not relevant for the CEU, as it cannot be configured.

>
> The set seems good to me.

I can add that to commit message and re-send, or if it's easier for
you, you can change the last paragraph when applying to:

Also remove endpoint properties from the gr-peach-audiocamerashield as
they match the defaults now specified in the driver code
(h/vsync-active and bus-width) or are not relevant to the interface
as they cannot be configured (pclk-sample).

Thanks
  j

>
> >  			remote-endpoint = <&mt9v111_out>;
> >  		};
> >  	};
> > diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
> > index 035f1d3..150196f 100644
> > --- a/drivers/media/platform/renesas-ceu.c
> > +++ b/drivers/media/platform/renesas-ceu.c
> > @@ -1551,7 +1551,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
> >  		return ret;
> >
> >  	for (i = 0; i < num_ep; i++) {
> > -		struct v4l2_fwnode_endpoint fw_ep = { .bus_type = 0 };
> > +		struct v4l2_fwnode_endpoint fw_ep = {
> > +			.bus_type = V4L2_MBUS_PARALLEL,
> > +			.bus = {
> > +				.parallel = {
> > +					.flags = V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > +						 V4L2_MBUS_VSYNC_ACTIVE_HIGH,
> > +					.bus_width = 8,
> > +				},
> > +			},
> > +		};
> >
> >  		ep = of_graph_get_endpoint_by_regs(of, 0, i);
> >  		if (!ep) {
> > @@ -1564,14 +1573,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
> >  		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &fw_ep);
> >  		if (ret) {
> >  			dev_err(ceudev->dev,
> > -				"Unable to parse endpoint #%u.\n", i);
> > -			goto error_cleanup;
> > -		}
> > -
> > -		if (fw_ep.bus_type != V4L2_MBUS_PARALLEL) {
> > -			dev_err(ceudev->dev,
> > -				"Only parallel input supported.\n");
> > -			ret = -EINVAL;
> > +				"Unable to parse endpoint #%u: %d.\n", i, ret);
> >  			goto error_cleanup;
> >  		}
> >
>
> --
> Kind regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-13 14:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-13 13:59 [PATCH 0/3] media: renesas-ceu: Introduce default mbus configuration Jacopo Mondi
2018-09-13 13:59 ` [PATCH 1/3] dt-bindings: media: renesas-ceu: Refer to video-interfaces.txt Jacopo Mondi
2018-09-13 13:59 ` [PATCH 2/3] dt-bindings: media: renesas-ceu: Add more endpoint properties Jacopo Mondi
2018-09-13 13:59 ` [PATCH 3/3] media: renesas-ceu: Use default mbus settings Jacopo Mondi
2018-09-13 14:22   ` Sakari Ailus
2018-09-13 14:28     ` jacopo mondi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).