Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1632318 > unrolled thread

[PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver Kieran Bingham <kbingham@kernel.org> - 2017-04-27 20:30 +0200
    [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482 Kieran Bingham <kbingham@kernel.org> - 2017-04-27 20:30 +0200
      Re: [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and  ADV7482 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-28 11:00 +0200
        Re: [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482 Geert Uytterhoeven <geert@linux-m68k.org> - 2017-04-28 12:10 +0200
          Re: [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and  ADV7482 Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> - 2017-04-28 12:10 +0200
    [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous subdevs Kieran Bingham <kbingham@kernel.org> - 2017-04-27 20:30 +0200
      Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous  subdevs Sakari Ailus <sakari.ailus@iki.fi> - 2017-04-27 23:50 +0200
        Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous  subdevs Kieran Bingham <kieran.bingham@ideasonboard.com> - 2017-04-28 00:20 +0200
          Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous  subdevs Sakari Ailus <sakari.ailus@iki.fi> - 2017-04-28 01:00 +0200
            Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous  subdevs Kieran Bingham <kieran.bingham@ideasonboard.com> - 2017-04-28 02:10 +0200
    Re: [PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver Simon Horman <horms@verge.net.au> - 2017-04-28 09:20 +0200
      Re: [PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> - 2017-04-28 10:50 +0200
        Re: [PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver Simon Horman <horms@verge.net.au> - 2017-05-01 10:40 +0200

#1632318 — [PATCH 0/5] RFC: ADV748x HDMI/Analog video receiver

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

This is an RFC for the Analog Devices ADV748x driver, and follows on from a
previous posting by Niklas Söderlund [0] of an earlier incarnation of this
driver.

This is an early posting of the driver following the release early, release
often method after quite a bit of refactoring in an attempt to bring it
closer to mainline.

Aside from a few bug fixes, and considerable refactoring this driver:
 - is refactored to multiple object files
 - defines multiple sub devices for the output paths.
 - has independant controls for both HDMI and Analog video paths
 - extends V4L2 async matching to support 'ports' on V4L2_ASYNC_MATCH_OF

The ADV7481 and ADV7482 support two video pipelines which can run independently
of each other, with each pipeline terminating in a CSI-2 output: TXA (4-Lane)
and TXB (1-Lane)

The ADV7480 (Not yet included here), ADV7481, and ADV7482 are all derivatives,
with the following features

            Analog   HDMI  MHL  4-Lane  1-Lane
              In      In         CSI     CSI
 ADV7480               X    X     X
 ADV7481      X        X    X     X       X
 ADV7482      X        X          X       X

This RFC statically routes the HDMI in through the TXA and the CVBS through the
TXB for early development, though I anticipate splitting the TXA/HDMI and 
TXB/AFE (Analog Front End) into distinct sub devices, allowing configurable
routing. This split is dependant upon on-going 'incremental binding' work being
done and thus is not yet included in this RFC.

Further potential development areas include:
 - ADV7480 Support (No AFE)
 - MHL support (Not present on ADV7482)
 - EDID support
 - CEC Support
 - Configurable I2C addressing
 - Interrupt handling for format changes and hotplug detect.

However, this driver and series is functional without the above, though if
there are mandatory areas which block mainline integration please let me know
and I will prioritise that in development.

Particular topics for discussion and review requested here include:

 - Device tree bindings specification and port listings review

 - Async Subdev port matching on V4L2_ASYNC_MATCH_OF

There are two implementations possible for the subdevice matching:
   A) Matching on a root dev-node, with a port specification
   B) Creating a new V4L2_ASYNC_MATCH_OF_ENDPOINT

This series posts an initial version utilising Method A.

Here we extend the subdevice and allow the drivers to assign defined 'port'
numbers to subdevices so that they can be distinguised against a common root
of_node.

This method involves the least change overall, and doesn't require the ADV7482
to parse it's device tree in advance.


Method B) is an option I have also considered, but requires as stated that the
driver must parse it's device tree, and instead of passing a root of_node, would
pass the endpoint for matching.

The difficulty here will be in communicating to the consumer / async-notifier
that instead of the V4L2_ASYNC_MATCH_OF, a V4L2_ASYNC_MATCH_OF_ENDPOINT should
be used. Thoughts on this would be appreciated.


This series presents the following patches:

 [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous
 [PATCH 2/5] rcar-vin: Match sources against ports if specified.
 [PATCH 3/5] media: i2c: adv748x: add adv748x driver
 [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482
 [PATCH 5/5] arm64: dts: r8a7796: salvator-x: enable VIN, CSI and ADV7482

Patch 1 provides V4L2 support for the 'Method A' mentioned above.
Patch 2 is dependant upon Niklas' rcar-vin series [1], and adds support to the
 binding multiple subdevices from a single DT node.
Patch 3 is the new driver
Patch 4 and 5 add support to the Salvator-X platforms where this code has been
 tested

I appreciate that there are still some 'rough edges' in the driver, which is
still under development, but all comments are welcome.


[0] http://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg05196.html
[1] https://git.ragnatech.se/linux rcar-vin-elinux-v7 


Kieran Bingham (5):
  v4l2-subdev: Provide a port mapping for asynchronous subdevs
  rcar-vin: Match sources against ports if specified.
  media: i2c: adv748x: add adv748x driver
  arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482
  arm64: dts: r8a7796: salvator-x: enable VIN, CSI and ADV7482

 .../devicetree/bindings/media/i2c/adv748x.txt      |  63 ++
 MAINTAINERS                                        |   6 +
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 129 ++++
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 129 ++++
 drivers/media/i2c/Kconfig                          |  10 +
 drivers/media/i2c/Makefile                         |   1 +
 drivers/media/i2c/adv748x/Makefile                 |   6 +
 drivers/media/i2c/adv748x/adv748x-afe.c            | 614 ++++++++++++++++++
 drivers/media/i2c/adv748x/adv748x-core.c           | 573 +++++++++++++++++
 drivers/media/i2c/adv748x/adv748x-hdmi.c           | 690 +++++++++++++++++++++
 drivers/media/i2c/adv748x/adv748x.h                | 157 +++++
 drivers/media/platform/rcar-vin/rcar-core.c        |  15 +-
 drivers/media/v4l2-core/v4l2-async.c               |   7 +
 drivers/media/v4l2-core/v4l2-subdev.c              |   1 +
 include/media/v4l2-async.h                         |   1 +
 include/media/v4l2-subdev.h                        |   2 +
 16 files changed, 2397 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/adv748x.txt
 create mode 100644 drivers/media/i2c/adv748x/Makefile
 create mode 100644 drivers/media/i2c/adv748x/adv748x-afe.c
 create mode 100644 drivers/media/i2c/adv748x/adv748x-core.c
 create mode 100644 drivers/media/i2c/adv748x/adv748x-hdmi.c
 create mode 100644 drivers/media/i2c/adv748x/adv748x.h

-- 
2.7.4

[toc] | [next] | [standalone]

#1632321 — [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Provide bindings between the VIN, CSI and the ADV7482 on the r8a7795.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 129 +++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index 27b9bae60dc0..a20623faa9d2 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -196,6 +196,22 @@
 			};
 		};
 	};
+
+	hdmi {
+		port {
+			hdmi_in: endpoint {
+				remote-endpoint = <&adv7482_hdmi>;
+			};
+		};
+	};
+
+	cvbs {
+		port {
+			cvbs_in: endpoint {
+				remote-endpoint = <&adv7482_ain8>;
+			};
+		};
+	};
 };
 
 &du {
@@ -387,6 +403,50 @@
 	};
 };
 
+&i2c4 {
+	status = "okay";
+
+	clock-frequency = <100000>;
+
+	video_receiver@70 {
+		compatible = "adi,adv7482";
+		reg = <0x70>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@8 {
+			adv7482_ain8: endpoint@1 {
+				remote-endpoint = <&cvbs_in>;
+			};
+		};
+
+		port@9 {
+			adv7482_hdmi: endpoint@1 {
+				remote-endpoint = <&hdmi_in>;
+			};
+		};
+
+		port@11 {
+			reg = <11>;
+			adv7482_txa: endpoint@1 {
+				clock-lanes = <0>;
+				data-lanes = <1 2 3 4>;
+				remote-endpoint = <&csi40_in>;
+			};
+		};
+
+		port@12 {
+			reg = <12>;
+			adv7482_txb: endpoint@1 {
+				clock-lanes = <0>;
+				data-lanes = <1>;
+				remote-endpoint = <&csi20_in>;
+			};
+		};
+	};
+};
+
 &rcar_sound {
 	pinctrl-0 = <&sound_pins &sound_clk_pins>;
 	pinctrl-names = "default";
@@ -577,3 +637,72 @@
 &pciec1 {
 	status = "okay";
 };
+
+&vin0 {
+	status = "okay";
+};
+
+&vin1 {
+	status = "okay";
+};
+
+&vin2 {
+	status = "okay";
+};
+
+&vin3 {
+	status = "okay";
+};
+
+&vin4 {
+	status = "okay";
+};
+
+&vin5 {
+	status = "okay";
+};
+
+&vin6 {
+	status = "okay";
+};
+
+&vin7 {
+	status = "okay";
+};
+
+&csi20 {
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+			csi20_in: endpoint@0 {
+				clock-lanes = <0>;
+				data-lanes = <1>;
+				remote-endpoint = <&adv7482_txb>;
+			};
+		};
+	};
+};
+
+&csi40 {
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			csi40_in: endpoint@0 {
+				clock-lanes = <0>;
+				data-lanes = <1 2 3 4>;
+				remote-endpoint = <&adv7482_txa>;
+			};
+		};
+	};
+};
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1632689 — Re: [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482

Hello!

On 4/27/2017 9:26 PM, Kieran Bingham wrote:

> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> Provide bindings between the VIN, CSI and the ADV7482 on the r8a7795.
>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 129 +++++++++++++++++++++
>  1 file changed, 129 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> index 27b9bae60dc0..a20623faa9d2 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
[...]
> @@ -387,6 +403,50 @@
>  	};
>  };
>
> +&i2c4 {
> +	status = "okay";
> +
> +	clock-frequency = <100000>;
> +
> +	video_receiver@70 {

    Hyphens are preferred in the node names.

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1632760 — Re: [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482

On Fri, Apr 28, 2017 at 10:52 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 4/27/2017 9:26 PM, Kieran Bingham wrote:
>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>
> [...]
>>
>> @@ -387,6 +403,50 @@
>>         };
>>  };
>>
>> +&i2c4 {
>> +       status = "okay";
>> +
>> +       clock-frequency = <100000>;
>> +
>> +       video_receiver@70 {
>
>
>    Hyphens are preferred in the node names.

Definitely: make W=1 dtbs

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1632770 — Re: [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482

On 28/04/17 11:04, Geert Uytterhoeven wrote:
> On Fri, Apr 28, 2017 at 10:52 AM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>> On 4/27/2017 9:26 PM, Kieran Bingham wrote:
>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
>>
>> [...]
>>>
>>> @@ -387,6 +403,50 @@
>>>         };
>>>  };
>>>
>>> +&i2c4 {
>>> +       status = "okay";
>>> +
>>> +       clock-frequency = <100000>;
>>> +
>>> +       video_receiver@70 {
>>
>>
>>    Hyphens are preferred in the node names.
> 
> Definitely: make W=1 dtbs


Thanks guys, - I didn't know about that one.
I'll update my build script so it's always in place :)

--
Regards

Kieran


> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

[toc] | [prev] | [next] | [standalone]

#1632322 — [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous subdevs

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Devices such as the the ADV748x support multiple parallel stream routes
through a single chip. This leads towards needing to provide multiple
distinct entities and subdevs from a single device-tree node.

To distinguish these separate outputs, the device-tree binding must
specify each endpoint link with a unique (to the device) non-zero port
number.

This number allows async subdev registrations to identify the correct
subdevice to bind and link.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-async.c  | 7 +++++++
 drivers/media/v4l2-core/v4l2-subdev.c | 1 +
 include/media/v4l2-async.h            | 1 +
 include/media/v4l2-subdev.h           | 2 ++
 4 files changed, 11 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 1815e54e8a38..875e6ce646ec 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -42,6 +42,13 @@ static bool match_devname(struct v4l2_subdev *sd,
 
 static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
 {
+	/*
+	 * If set, we must match the device tree port, with the subdev port.
+	 * This is a fast match, so do this first
+	 */
+	if (sd->port && sd->port != asd->match.of.port)
+		return -1;
+
 	return !of_node_cmp(of_node_full_name(sd->of_node),
 			    of_node_full_name(asd->match.of.node));
 }
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index da78497ae5ed..67f816f90ac3 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -607,6 +607,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
 	sd->flags = 0;
 	sd->name[0] = '\0';
 	sd->grp_id = 0;
+	sd->port = 0;
 	sd->dev_priv = NULL;
 	sd->host_priv = NULL;
 #if defined(CONFIG_MEDIA_CONTROLLER)
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 5b501309b6a7..2988960613ec 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -56,6 +56,7 @@ struct v4l2_async_subdev {
 	union {
 		struct {
 			const struct device_node *node;
+			u32 port;
 		} of;
 		struct {
 			const char *name;
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 0ab1c5df6fac..1c1731b491e5 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -782,6 +782,7 @@ struct v4l2_subdev_platform_data {
  * @ctrl_handler: The control handler of this subdev. May be NULL.
  * @name: Name of the sub-device. Please notice that the name must be unique.
  * @grp_id: can be used to group similar subdevs. Value is driver-specific
+ * @port: driver-specific value to bind multiple subdevs with a single DT node.
  * @dev_priv: pointer to private data
  * @host_priv: pointer to private data used by the device where the subdev
  *	is attached.
@@ -814,6 +815,7 @@ struct v4l2_subdev {
 	struct v4l2_ctrl_handler *ctrl_handler;
 	char name[V4L2_SUBDEV_NAME_SIZE];
 	u32 grp_id;
+	u32 port;
 	void *dev_priv;
 	void *host_priv;
 	struct video_device *devnode;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1632389 — Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous subdevs

Hi Kieran,

Could I ask you to rebase your patches on top of my V4L2 fwnode patches
here?

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi>

It depends on the fwnode graph patches, merged here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-merge>

I expect the fwnode graph patches in v4.12 so we'll have them in media-tree
master soon.

(I'm pushing these branches right now, it may take a while until it's really
there.)

On Thu, Apr 27, 2017 at 07:26:00PM +0100, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> Devices such as the the ADV748x support multiple parallel stream routes
> through a single chip. This leads towards needing to provide multiple
> distinct entities and subdevs from a single device-tree node.
> 
> To distinguish these separate outputs, the device-tree binding must
> specify each endpoint link with a unique (to the device) non-zero port
> number.
> 
> This number allows async subdev registrations to identify the correct
> subdevice to bind and link.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-async.c  | 7 +++++++
>  drivers/media/v4l2-core/v4l2-subdev.c | 1 +
>  include/media/v4l2-async.h            | 1 +
>  include/media/v4l2-subdev.h           | 2 ++
>  4 files changed, 11 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 1815e54e8a38..875e6ce646ec 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -42,6 +42,13 @@ static bool match_devname(struct v4l2_subdev *sd,
>  
>  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>  {
> +	/*
> +	 * If set, we must match the device tree port, with the subdev port.
> +	 * This is a fast match, so do this first
> +	 */
> +	if (sd->port && sd->port != asd->match.of.port)

Zero is an entirely valid value for a port. I think it'd be good not to
depend on non-zero port values for port matching.

> +		return -1;

Any particular reason to return -1 from a function with bool return type?

> +
>  	return !of_node_cmp(of_node_full_name(sd->of_node),
>  			    of_node_full_name(asd->match.of.node));
>  }
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index da78497ae5ed..67f816f90ac3 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -607,6 +607,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
>  	sd->flags = 0;
>  	sd->name[0] = '\0';
>  	sd->grp_id = 0;
> +	sd->port = 0;
>  	sd->dev_priv = NULL;
>  	sd->host_priv = NULL;
>  #if defined(CONFIG_MEDIA_CONTROLLER)
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 5b501309b6a7..2988960613ec 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -56,6 +56,7 @@ struct v4l2_async_subdev {
>  	union {
>  		struct {
>  			const struct device_node *node;
> +			u32 port;

What if instead of storing the device's OF node, you'd store the port node
and used that for matching?

Would that also solve the problem or do I miss something?

>  		} of;
>  		struct {
>  			const char *name;
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 0ab1c5df6fac..1c1731b491e5 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -782,6 +782,7 @@ struct v4l2_subdev_platform_data {
>   * @ctrl_handler: The control handler of this subdev. May be NULL.
>   * @name: Name of the sub-device. Please notice that the name must be unique.
>   * @grp_id: can be used to group similar subdevs. Value is driver-specific
> + * @port: driver-specific value to bind multiple subdevs with a single DT node.
>   * @dev_priv: pointer to private data
>   * @host_priv: pointer to private data used by the device where the subdev
>   *	is attached.
> @@ -814,6 +815,7 @@ struct v4l2_subdev {
>  	struct v4l2_ctrl_handler *ctrl_handler;
>  	char name[V4L2_SUBDEV_NAME_SIZE];
>  	u32 grp_id;
> +	u32 port;
>  	void *dev_priv;
>  	void *host_priv;
>  	struct video_device *devnode;

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1632410 — Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous subdevs

Hi Sakari,

Thanks for taking a look

On 27/04/17 22:43, Sakari Ailus wrote:
> Hi Kieran,
> 
> Could I ask you to rebase your patches on top of my V4L2 fwnode patches
> here?
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi>
> 
> It depends on the fwnode graph patches, merged here:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-merge>
> 
> I expect the fwnode graph patches in v4.12 so we'll have them in media-tree
> master soon.
> 
> (I'm pushing these branches right now, it may take a while until it's really
> there.)

Sure, I'll merge those into my base.

> On Thu, Apr 27, 2017 at 07:26:00PM +0100, Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> Devices such as the the ADV748x support multiple parallel stream routes
>> through a single chip. This leads towards needing to provide multiple
>> distinct entities and subdevs from a single device-tree node.
>>
>> To distinguish these separate outputs, the device-tree binding must
>> specify each endpoint link with a unique (to the device) non-zero port
>> number.
>>
>> This number allows async subdev registrations to identify the correct
>> subdevice to bind and link.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  drivers/media/v4l2-core/v4l2-async.c  | 7 +++++++
>>  drivers/media/v4l2-core/v4l2-subdev.c | 1 +
>>  include/media/v4l2-async.h            | 1 +
>>  include/media/v4l2-subdev.h           | 2 ++
>>  4 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>> index 1815e54e8a38..875e6ce646ec 100644
>> --- a/drivers/media/v4l2-core/v4l2-async.c
>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>> @@ -42,6 +42,13 @@ static bool match_devname(struct v4l2_subdev *sd,
>>  
>>  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>>  {
>> +	/*
>> +	 * If set, we must match the device tree port, with the subdev port.
>> +	 * This is a fast match, so do this first
>> +	 */
>> +	if (sd->port && sd->port != asd->match.of.port)
> 
> Zero is an entirely valid value for a port. I think it'd be good not to
> depend on non-zero port values for port matching.

Well then that pretty much dashes my chances on not parsing the DT in the ADV
driver.



>> +		return -1;
> 
> Any particular reason to return -1 from a function with bool return type?

Ahem, I clearly can't read ;-)
I think my mindset was thinking strcmp or something...


>> +
>>  	return !of_node_cmp(of_node_full_name(sd->of_node),
>>  			    of_node_full_name(asd->match.of.node));
>>  }
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index da78497ae5ed..67f816f90ac3 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -607,6 +607,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
>>  	sd->flags = 0;
>>  	sd->name[0] = '\0';
>>  	sd->grp_id = 0;
>> +	sd->port = 0;
>>  	sd->dev_priv = NULL;
>>  	sd->host_priv = NULL;
>>  #if defined(CONFIG_MEDIA_CONTROLLER)
>> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
>> index 5b501309b6a7..2988960613ec 100644
>> --- a/include/media/v4l2-async.h
>> +++ b/include/media/v4l2-async.h
>> @@ -56,6 +56,7 @@ struct v4l2_async_subdev {
>>  	union {
>>  		struct {
>>  			const struct device_node *node;
>> +			u32 port;
> 
> What if instead of storing the device's OF node, you'd store the port node
> and used that for matching?
> 
> Would that also solve the problem or do I miss something?

Actually - I was 'trying' to prevent having to parse the DT in the adv748x
driver if I didn't need to.

Once I have to parse the DT, then yes, I think storing the endpoint node is
probably the best thing to compare against.

And actually - you might have just solved my open question in the cover letter ...

I had got stuck in my mindset that if I were to use the endpoint 'leaf' node as
a comparator - that it would be 'instead' of the root node.

But actually - it could just be root-node + leaf-node to compare, which then
allows us the fallback of comparing just the root nodes if the leaf isn't set.

I'll respin with this either tomorrow or early next week.

> 
>>  		} of;
>>  		struct {
>>  			const char *name;
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index 0ab1c5df6fac..1c1731b491e5 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -782,6 +782,7 @@ struct v4l2_subdev_platform_data {
>>   * @ctrl_handler: The control handler of this subdev. May be NULL.
>>   * @name: Name of the sub-device. Please notice that the name must be unique.
>>   * @grp_id: can be used to group similar subdevs. Value is driver-specific
>> + * @port: driver-specific value to bind multiple subdevs with a single DT node.
>>   * @dev_priv: pointer to private data
>>   * @host_priv: pointer to private data used by the device where the subdev
>>   *	is attached.
>> @@ -814,6 +815,7 @@ struct v4l2_subdev {
>>  	struct v4l2_ctrl_handler *ctrl_handler;
>>  	char name[V4L2_SUBDEV_NAME_SIZE];
>>  	u32 grp_id;
>> +	u32 port;
>>  	void *dev_priv;
>>  	void *host_priv;
>>  	struct video_device *devnode;
> 

Regards

Kieran

[toc] | [prev] | [next] | [standalone]

#1632428 — Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous subdevs

Hi Kieran,

On Thu, Apr 27, 2017 at 11:13:50PM +0100, Kieran Bingham wrote:
> Hi Sakari,
> 
> Thanks for taking a look

Sure! :-)

> 
> On 27/04/17 22:43, Sakari Ailus wrote:
> > Hi Kieran,
> > 
> > Could I ask you to rebase your patches on top of my V4L2 fwnode patches
> > here?
> > 
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi>
> > 
> > It depends on the fwnode graph patches, merged here:
> > 
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-merge>
> > 
> > I expect the fwnode graph patches in v4.12 so we'll have them in media-tree
> > master soon.
> > 
> > (I'm pushing these branches right now, it may take a while until it's really
> > there.)
> 
> Sure, I'll merge those into my base.
> 
> > On Thu, Apr 27, 2017 at 07:26:00PM +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>
> >> Devices such as the the ADV748x support multiple parallel stream routes
> >> through a single chip. This leads towards needing to provide multiple
> >> distinct entities and subdevs from a single device-tree node.
> >>
> >> To distinguish these separate outputs, the device-tree binding must
> >> specify each endpoint link with a unique (to the device) non-zero port
> >> number.
> >>
> >> This number allows async subdev registrations to identify the correct
> >> subdevice to bind and link.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> ---
> >>  drivers/media/v4l2-core/v4l2-async.c  | 7 +++++++
> >>  drivers/media/v4l2-core/v4l2-subdev.c | 1 +
> >>  include/media/v4l2-async.h            | 1 +
> >>  include/media/v4l2-subdev.h           | 2 ++
> >>  4 files changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> >> index 1815e54e8a38..875e6ce646ec 100644
> >> --- a/drivers/media/v4l2-core/v4l2-async.c
> >> +++ b/drivers/media/v4l2-core/v4l2-async.c
> >> @@ -42,6 +42,13 @@ static bool match_devname(struct v4l2_subdev *sd,
> >>  
> >>  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> >>  {
> >> +	/*
> >> +	 * If set, we must match the device tree port, with the subdev port.
> >> +	 * This is a fast match, so do this first
> >> +	 */
> >> +	if (sd->port && sd->port != asd->match.of.port)
> > 
> > Zero is an entirely valid value for a port. I think it'd be good not to
> > depend on non-zero port values for port matching.
> 
> Well then that pretty much dashes my chances on not parsing the DT in the ADV
> driver.

Hmm. I guess there's no really a way to avoid it. But we could make it
easier 

> 
> 
> 
> >> +		return -1;
> > 
> > Any particular reason to return -1 from a function with bool return type?
> 
> Ahem, I clearly can't read ;-)
> I think my mindset was thinking strcmp or something...

But -1 is perfectly valid. If you wanted to make it look really interesting,
you could return -!false and still have exactly the same functionality. ;-)

> 
> 
> >> +
> >>  	return !of_node_cmp(of_node_full_name(sd->of_node),
> >>  			    of_node_full_name(asd->match.of.node));
> >>  }
> >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> >> index da78497ae5ed..67f816f90ac3 100644
> >> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> >> @@ -607,6 +607,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
> >>  	sd->flags = 0;
> >>  	sd->name[0] = '\0';
> >>  	sd->grp_id = 0;
> >> +	sd->port = 0;
> >>  	sd->dev_priv = NULL;
> >>  	sd->host_priv = NULL;
> >>  #if defined(CONFIG_MEDIA_CONTROLLER)
> >> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> >> index 5b501309b6a7..2988960613ec 100644
> >> --- a/include/media/v4l2-async.h
> >> +++ b/include/media/v4l2-async.h
> >> @@ -56,6 +56,7 @@ struct v4l2_async_subdev {
> >>  	union {
> >>  		struct {
> >>  			const struct device_node *node;
> >> +			u32 port;
> > 
> > What if instead of storing the device's OF node, you'd store the port node
> > and used that for matching?
> > 
> > Would that also solve the problem or do I miss something?
> 
> Actually - I was 'trying' to prevent having to parse the DT in the adv748x
> driver if I didn't need to.
> 
> Once I have to parse the DT, then yes, I think storing the endpoint node is
> probably the best thing to compare against.
> 
> And actually - you might have just solved my open question in the cover letter ...
> 
> I had got stuck in my mindset that if I were to use the endpoint 'leaf' node as
> a comparator - that it would be 'instead' of the root node.
> 
> But actually - it could just be root-node + leaf-node to compare, which then
> allows us the fallback of comparing just the root nodes if the leaf isn't set.
> 
> I'll respin with this either tomorrow or early next week.

Endpoints are indeed another option.

Is there something that would prevent switching from device node matching to
port / endpoint matching altogether? I don't think the driver changes should
be difficult to make.

Supporting different options there will be painful as it will likely require
help from the driver to implement both --- separately.

> 
> > 
> >>  		} of;
> >>  		struct {
> >>  			const char *name;
> >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> >> index 0ab1c5df6fac..1c1731b491e5 100644
> >> --- a/include/media/v4l2-subdev.h
> >> +++ b/include/media/v4l2-subdev.h
> >> @@ -782,6 +782,7 @@ struct v4l2_subdev_platform_data {
> >>   * @ctrl_handler: The control handler of this subdev. May be NULL.
> >>   * @name: Name of the sub-device. Please notice that the name must be unique.
> >>   * @grp_id: can be used to group similar subdevs. Value is driver-specific
> >> + * @port: driver-specific value to bind multiple subdevs with a single DT node.
> >>   * @dev_priv: pointer to private data
> >>   * @host_priv: pointer to private data used by the device where the subdev
> >>   *	is attached.
> >> @@ -814,6 +815,7 @@ struct v4l2_subdev {
> >>  	struct v4l2_ctrl_handler *ctrl_handler;
> >>  	char name[V4L2_SUBDEV_NAME_SIZE];
> >>  	u32 grp_id;
> >> +	u32 port;
> >>  	void *dev_priv;
> >>  	void *host_priv;
> >>  	struct video_device *devnode;
> > 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1632448 — Re: [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous subdevs

On 27/04/17 23:49, Sakari Ailus wrote:
> Hi Kieran,
> 
> On Thu, Apr 27, 2017 at 11:13:50PM +0100, Kieran Bingham wrote:
>> Hi Sakari,
>>
>> Thanks for taking a look
> 
> Sure! :-)
> 
>>
>> On 27/04/17 22:43, Sakari Ailus wrote:
>>> Hi Kieran,
>>>
>>> Could I ask you to rebase your patches on top of my V4L2 fwnode patches
>>> here?
>>>
>>> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi>
>>>
>>> It depends on the fwnode graph patches, merged here:
>>>
>>> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-merge>
>>>
>>> I expect the fwnode graph patches in v4.12 so we'll have them in media-tree
>>> master soon.
>>>
>>> (I'm pushing these branches right now, it may take a while until it's really
>>> there.)
>>
>> Sure, I'll merge those into my base.
>>
>>> On Thu, Apr 27, 2017 at 07:26:00PM +0100, Kieran Bingham wrote:
>>>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>>
>>>> Devices such as the the ADV748x support multiple parallel stream routes
>>>> through a single chip. This leads towards needing to provide multiple
>>>> distinct entities and subdevs from a single device-tree node.
>>>>
>>>> To distinguish these separate outputs, the device-tree binding must
>>>> specify each endpoint link with a unique (to the device) non-zero port
>>>> number.
>>>>
>>>> This number allows async subdev registrations to identify the correct
>>>> subdevice to bind and link.
>>>>
>>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>> ---
>>>>  drivers/media/v4l2-core/v4l2-async.c  | 7 +++++++
>>>>  drivers/media/v4l2-core/v4l2-subdev.c | 1 +
>>>>  include/media/v4l2-async.h            | 1 +
>>>>  include/media/v4l2-subdev.h           | 2 ++
>>>>  4 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>>>> index 1815e54e8a38..875e6ce646ec 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-async.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>>>> @@ -42,6 +42,13 @@ static bool match_devname(struct v4l2_subdev *sd,
>>>>  
>>>>  static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>>>>  {
>>>> +	/*
>>>> +	 * If set, we must match the device tree port, with the subdev port.
>>>> +	 * This is a fast match, so do this first
>>>> +	 */
>>>> +	if (sd->port && sd->port != asd->match.of.port)
>>>
>>> Zero is an entirely valid value for a port. I think it'd be good not to
>>> depend on non-zero port values for port matching.
>>
>> Well then that pretty much dashes my chances on not parsing the DT in the ADV
>> driver.
> 
> Hmm. I guess there's no really a way to avoid it. But we could make it
> easier 
> 
>>
>>
>>
>>>> +		return -1;
>>>
>>> Any particular reason to return -1 from a function with bool return type?
>>
>> Ahem, I clearly can't read ;-)
>> I think my mindset was thinking strcmp or something...
> 
> But -1 is perfectly valid. If you wanted to make it look really interesting,
> you could return -!false and still have exactly the same functionality. ;-)

I'll consider -!false a recommendation for my next patch :D



>>>> +
>>>>  	return !of_node_cmp(of_node_full_name(sd->of_node),
>>>>  			    of_node_full_name(asd->match.of.node));
>>>>  }
>>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>>>> index da78497ae5ed..67f816f90ac3 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>>>> @@ -607,6 +607,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
>>>>  	sd->flags = 0;
>>>>  	sd->name[0] = '\0';
>>>>  	sd->grp_id = 0;
>>>> +	sd->port = 0;
>>>>  	sd->dev_priv = NULL;
>>>>  	sd->host_priv = NULL;
>>>>  #if defined(CONFIG_MEDIA_CONTROLLER)
>>>> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
>>>> index 5b501309b6a7..2988960613ec 100644
>>>> --- a/include/media/v4l2-async.h
>>>> +++ b/include/media/v4l2-async.h
>>>> @@ -56,6 +56,7 @@ struct v4l2_async_subdev {
>>>>  	union {
>>>>  		struct {
>>>>  			const struct device_node *node;
>>>> +			u32 port;
>>>
>>> What if instead of storing the device's OF node, you'd store the port node
>>> and used that for matching?
>>>
>>> Would that also solve the problem or do I miss something?
>>
>> Actually - I was 'trying' to prevent having to parse the DT in the adv748x
>> driver if I didn't need to.
>>
>> Once I have to parse the DT, then yes, I think storing the endpoint node is
>> probably the best thing to compare against.
>>
>> And actually - you might have just solved my open question in the cover letter ...
>>
>> I had got stuck in my mindset that if I were to use the endpoint 'leaf' node as
>> a comparator - that it would be 'instead' of the root node.
>>
>> But actually - it could just be root-node + leaf-node to compare, which then
>> allows us the fallback of comparing just the root nodes if the leaf isn't set.
>>
>> I'll respin with this either tomorrow or early next week.
> 
> Endpoints are indeed another option.
> 
> Is there something that would prevent switching from device node matching to
> port / endpoint matching altogether? I don't think the driver changes should
> be difficult to make.
> 
> Supporting different options there will be painful as it will likely require
> help from the driver to implement both --- separately.

Ok - so a bit of pain either way...

IMO - 'endpoint' matching is 'more correct' as then we are tying the subdev with
precisely the object that it is linking to.

It looks like there are 11 users of v4l2_async_notifier_register(), so yes,
hopefully it might not actually be so much effort to go through and adapt each
to match to the endpoint of_node instead!

I'll have a go at this and see how far I get down the rabbit hole.

>>>>  		} of;
>>>>  		struct {
>>>>  			const char *name;
>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>>>> index 0ab1c5df6fac..1c1731b491e5 100644
>>>> --- a/include/media/v4l2-subdev.h
>>>> +++ b/include/media/v4l2-subdev.h
>>>> @@ -782,6 +782,7 @@ struct v4l2_subdev_platform_data {
>>>>   * @ctrl_handler: The control handler of this subdev. May be NULL.
>>>>   * @name: Name of the sub-device. Please notice that the name must be unique.
>>>>   * @grp_id: can be used to group similar subdevs. Value is driver-specific
>>>> + * @port: driver-specific value to bind multiple subdevs with a single DT node.
>>>>   * @dev_priv: pointer to private data
>>>>   * @host_priv: pointer to private data used by the device where the subdev
>>>>   *	is attached.
>>>> @@ -814,6 +815,7 @@ struct v4l2_subdev {
>>>>  	struct v4l2_ctrl_handler *ctrl_handler;
>>>>  	char name[V4L2_SUBDEV_NAME_SIZE];
>>>>  	u32 grp_id;
>>>> +	u32 port;
>>>>  	void *dev_priv;
>>>>  	void *host_priv;
>>>>  	struct video_device *devnode;
>>>
> 

[toc] | [prev] | [next] | [standalone]

#1632572

On Thu, Apr 27, 2017 at 07:25:59PM +0100, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> This is an RFC for the Analog Devices ADV748x driver, and follows on from a
> previous posting by Niklas Söderlund [0] of an earlier incarnation of this
> driver.

...

> This series presents the following patches:
> 
>  [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous
>  [PATCH 2/5] rcar-vin: Match sources against ports if specified.
>  [PATCH 3/5] media: i2c: adv748x: add adv748x driver
>  [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482
>  [PATCH 5/5] arm64: dts: r8a7796: salvator-x: enable VIN, CSI and ADV7482

I am marking the above dts patches as "RFC" and do not plan to apply them
unless you ping me or repost them. Assuming they don't cause any
regressions I would be happy to consider applying them as soon as their
dependencies are accepted.

[toc] | [prev] | [next] | [standalone]

#1632662

Hi Simon,

On 28/04/17 08:09, Simon Horman wrote:
> On Thu, Apr 27, 2017 at 07:25:59PM +0100, Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> This is an RFC for the Analog Devices ADV748x driver, and follows on from a
>> previous posting by Niklas Söderlund [0] of an earlier incarnation of this
>> driver.
> 
> ...
> 
>> This series presents the following patches:
>>
>>  [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous
>>  [PATCH 2/5] rcar-vin: Match sources against ports if specified.
>>  [PATCH 3/5] media: i2c: adv748x: add adv748x driver
>>  [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482
>>  [PATCH 5/5] arm64: dts: r8a7796: salvator-x: enable VIN, CSI and ADV7482
> 
> I am marking the above dts patches as "RFC" and do not plan to apply them
> unless you ping me or repost them.

Yes, sorry - the whole series was supposed to be marked as RFC, but I didn't
think about it - and apparently only applied the tag to the cover letter.

Apologies for any confusion.

> Assuming they don't cause any
> regressions I would be happy to consider applying them as soon as their
> dependencies are accepted.

Does that mean you've done a cursory glance over the content ? :-)

In this instance, the port numbers need to revert back to a zero-base, but I
would appreciate an eye on how and where I've put the representation of the
physical hdmi/cvbs connectors. Having modified plenty of DT, but not actually
submitted much - I still feel 'new' at it - so I'm sure I may not have followed
the standards quite right yet.

The dts patches are based heavily on the previous posting by Niklas, but I have
extended to put the extra hdmi and cvbs links in.

Regards
--
Kieran

[toc] | [prev] | [next] | [standalone]

#1633574

On Fri, Apr 28, 2017 at 09:47:05AM +0100, Kieran Bingham wrote:
> Hi Simon,
> 
> On 28/04/17 08:09, Simon Horman wrote:
> > On Thu, Apr 27, 2017 at 07:25:59PM +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>
> >> This is an RFC for the Analog Devices ADV748x driver, and follows on from a
> >> previous posting by Niklas Söderlund [0] of an earlier incarnation of this
> >> driver.
> > 
> > ...
> > 
> >> This series presents the following patches:
> >>
> >>  [PATCH 1/5] v4l2-subdev: Provide a port mapping for asynchronous
> >>  [PATCH 2/5] rcar-vin: Match sources against ports if specified.
> >>  [PATCH 3/5] media: i2c: adv748x: add adv748x driver
> >>  [PATCH 4/5] arm64: dts: r8a7795: salvator-x: enable VIN, CSI and ADV7482
> >>  [PATCH 5/5] arm64: dts: r8a7796: salvator-x: enable VIN, CSI and ADV7482
> > 
> > I am marking the above dts patches as "RFC" and do not plan to apply them
> > unless you ping me or repost them.
> 
> Yes, sorry - the whole series was supposed to be marked as RFC, but I didn't
> think about it - and apparently only applied the tag to the cover letter.
> 
> Apologies for any confusion.

It was clear enough, though an tag RFC in every patch would be better.
In any case I was referring to how I have handled these patches in
patchwork.

Apologies for any confusion.

> > Assuming they don't cause any
> > regressions I would be happy to consider applying them as soon as their
> > dependencies are accepted.
> 
> Does that mean you've done a cursory glance over the content ? :-)

Yes, I did take a quick glance.

> In this instance, the port numbers need to revert back to a zero-base,
> but I would appreciate an eye on how and where I've put the
> representation of the physical hdmi/cvbs connectors. Having modified
> plenty of DT, but not actually submitted much - I still feel 'new' at it
> - so I'm sure I may not have followed the standards quite right yet.

Assuming you are talking about where in the DT file the hdmi and cvbs nodes
should go, I think this is somewhat arbitrary so long as they are within
the top-level node - what you have looks good to me.

> The dts patches are based heavily on the previous posting by Niklas, but I have
> extended to put the extra hdmi and cvbs links in.
> 
> Regards
> --
> Kieran
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web