All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
@ 2016-07-02 23:55 frowand.list
  2016-07-02 23:55 ` [RFC PATCH 1/1] device tree connectors, using plugs and sockets frowand.list
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: frowand.list @ 2016-07-02 23:55 UTC (permalink / raw)
  To: robh+dt, david, pantelis.antoniou, stephen.boyd, broonie,
	grant.likely, mark.rutland
  Cc: mporter, koen, linux, marex, wsa, devicetree, linux-kernel,
	linux-i2c, panto

From: Frank Rowand <frank.rowand@am.sony.com>

Hi All,

This is version 2 of this email.

Changes from version 1:

  - some rewording of the text
  - removed new (theoretical) dtc directive "/connector/"
  - added compatibility between mother board and daughter board
  - added info on applying a single .dtbo to different connectors
  - attached an RFC patch showing the required kernel changes
  - changes to mother board .dts connector node:
     - removed target_path property
     - added connector-socket property
  - changes to daughter board .dts connector node:
     - added connector-plug property


I've been trying to wrap my head around what Pantelis and Rob have written
on the subject of a device tree representation of a connector for a
daughter board to connect to (eg a cape or a shield) and the representation
of the daughter board.  (Or any other physically pluggable object.)

After trying to make sense of what had been written (or presented via slides
at a conference - thanks Pantelis!), I decided to go back to first principals
of what we are trying to accomplish.  I came up with some really simple bogus
examples to try to explain what my thought process is.

This is an extremely simple example to illustrate the concepts.  It is not
meant to represent the complexity of a real board.

To start with, assume that the device that will eventually be on a daughter
board is first soldered onto the mother board.  The mother board contains
two devices connected via bus spi_1.  One device is described in the .dts
file, the other is described in an included .dtsi file.
Then the device tree files will look like:

$ cat board.dts
/dts-v1/;

/ {
        #address-cells = < 1 >;
        #size-cells = < 1 >;

        tree_1: soc@0 {
                reg = <0x0 0x0>;

                spi_1: spi1 {
                };
        };

};

&spi_1 {
        ethernet-switch@0 {
                compatible = "micrel,ks8995m";
        };
};

#include "spi_codec.dtsi"


$ cat spi_codec.dtsi
&spi_1 {
	codec@1 {
		compatible = "ti,tlv320aic26";
	};
};


#----- codec chip on cape

Then suppose I move the codec chip to a cape.  Then I will have the same
exact .dts and .dtsi and everything still works.


@----- codec chip on cape, overlay

If I want to use overlays, I only have to add the version and "/plugin/",
then use the '-@' flag for dtc (both for the previous board.dts and
this spi_codec_overlay.dts):

$ cat spi_codec_overlay.dts
/dts-v1/;

/plugin/;

&spi_1 {
	codec@1 {
		compatible = "ti,tlv320aic26";
	};
};


Pantelis pointed out that the syntax has changed to be:
   /dts-v1/ /plugin/;


#----- codec chip on cape, overlay, connector

Now we move into the realm of connectors.  My mental model of what the
hardware and driver look like has not changed.  The only thing that has
changed is that I want to be able to specify that the connector that
the cape is plugged into has some pins that are the spi bus /soc/spi1.

The following _almost_ but not quite gets me what I want.  Note that
the only thing the connector node does is provide some kind of
pointer or reference to what node(s) are physically routed through
the connector.  The connector node does not need to describe the pins;
it only has to point to the node that describes the pins.

This example will turn out to be not sufficient.  It is a stepping
stone in building my mental model.

$ cat board_with_connector.dts
/dts-v1/;

/ {
	#address-cells = < 1 >;
	#size-cells = < 1 >;

	tree_1: soc@0 {
		reg = <0x0 0x0>;

		spi_1: spi1 {
		};
	};

	connector_1: connector_1 {
		spi1 {
			target_phandle = <&spi_1>;
		};
	};

};

&spi_1 {
	ethernet-switch@0 {
		compatible = "micrel,ks8995m";
	};
};


$ cat spi_codec_overlay_with_connector.dts
/dts-v1/;

/plugin/;

&connector_1 {
	spi1 {
		codec@1 {
			compatible = "ti,tlv320aic26";
		};
	};
};


The result is that the overlay fixup for spi1 on the cape will
relocate the spi1 node to /connector_1 in the host tree, so
this does not solve the connector linkage yet:

-- chunk from the decompiled board_with_connector.dtb:

	__symbols__ {
		connector_1 = "/connector_1";
	};

-- chunk from the decompiled spi_codec_overlay_with_connector.dtb:

	fragment@0 {
		target = <0xffffffff>;
		__overlay__ {
			spi1 {
				codec@1 {
					compatible = "ti,tlv320aic26";
				};
			};
		};
	};
	__fixups__ {
		connector_1 = "/fragment@0:target:0";
	};


After applying the overlay, the codec@1 node will be at
/connector_1/spi1/codec@1.  What I want is for that node
to be at /spi1/codec@1.



#----- magic new syntax

What I really want is some way to tell dtc that I want to do one
level of dereferencing when resolving the path of device nodes
contained by the connector node in the overlay dts.

Version 1 of this email suggested using dtc magic to do this extra
level of dereferencing.  This version of the email has changed to
have the kernel code that applies the overlay do the extra level
of dereferencing.

The property "connector-socket" tells the kernel overlay code
that this is a socket.  The overlay code does not actually
do anything special as a result of this property; it is simply
used as a sanity check that this node really is a socket.  The
person writing the mother board .dts must provide the
target_phandle property, which points to a node responsible for
some of the pins on the connector.

The property "connector-plug" tells the kernel overlay code
that each child node in the overlay corresponds to a node in the
socket, and the socket will contain one property that is
a phandle pointing to the node that is the target of that child
node in the overlay node.


$ cat board_with_connector_v2.dts

/dts-v1/;

/ {
	#address-cells = < 1 >;
	#size-cells = < 1 >;

	tree_1: soc@0 {
		reg = <0x0 0x0>;

		spi_1: spi1 {
		};
	};

	connector_1: connector_1 {
		compatible = "11-pin-accessory";
		connector-socket;
		spi1 {
			target_phandle = <&spi_1>;
		};
	};

};

&spi_1 {
	ethernet-switch@0 {
		compatible = "micrel,ks8995m";
	};
};


$ cat spi_codec_overlay_with_connector_v2.dts

/dts-v1/;

/plugin/;

&connector_1 {
	connector-plug;
	compatible = "11-pin-accessory";

	spi1 {
		codec@1 {
			compatible = "ti,tlv320aic26";
		};
	};
};


The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
is unchanged from the previous example, but the kernel overlay
code will do the correct extra level of dereferencing when it
detects the connector-plug property in the overlay.

The one remaining piece that this patch does not provide is how
the overlay manager (which does not yet exist in the mainline
tree) can apply an overlay to two different targets.  That
final step should be a trivial change to of_overlay_create(),
adding a parameter that is a mapping of the target (or maybe
even targets) in the overlay to different targets in the
active device tree.

This seems like a more straight forward way to handle connectors.

First, ignoring pinctrl and pinmux, what does everyone think?

Then, the next step is whether pinctrl and pinmux work with this method.
Pantelis, can you point me to a good example for

  1) an in-tree board dts file
  2) an overlay file (I am assuming out of tree) that applies to the board
  3) an in-tree .dtsi file that would provide the same features as
     the overlay file if it was included by the board dts file

It should be easier to discuss pinctrl and pinmux with an example.

-Frank

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/1] device tree connectors, using plugs and sockets.
  2016-07-02 23:55 [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual frowand.list
@ 2016-07-02 23:55 ` frowand.list
  2016-07-03  6:06 ` [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual Frank Rowand
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: frowand.list @ 2016-07-02 23:55 UTC (permalink / raw)
  To: robh+dt, david, pantelis.antoniou, stephen.boyd, broonie,
	grant.likely, mark.rutland
  Cc: mporter, koen, linux, marex, wsa, devicetree, linux-kernel,
	linux-i2c, panto

From: Frank Rowand <frank.rowand@am.sony.com>

This patch has been compiled but has not been booted.  It is likely
to contain bugs.

Problem: mother boards may contain multiple connectors that daughter
boards may be attached to.  If two of the daughter boards can be
described by the same .dtsi file, then it should be possible to
apply the same .dtbo overlay file for each of the boards, specifying
a different target connector for each board.

This patch provides the foundation to allow that to occur, by
creating the two halves of a connector, the socket on the mother
board and the plug on the daughter board.

The one remaining piece that this patch does not provide is how
the overlay manager (which does not yet exist in the mainline
tree) can apply an overlay to two different targets.  That
final step should be a trivial change to of_overlay_create(),
adding a parameter that is a mapping of the target (or maybe
even targets) in the overlay to different targets in the
active device tree.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/overlay.c | 141 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 118 insertions(+), 23 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 82250815e9a5..df9b0097a1e1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -25,8 +25,9 @@
 
 /**
  * struct of_overlay_info - Holds a single overlay info
- * @target:	target of the overlay operation
- * @overlay:	pointer to the overlay contents node
+ * @target:		target of the overlay operation
+ * @overlay:		pointer to the overlay contents node
+ * @plug_targets:	pointer to array of plug target pointers
  *
  * Holds a single overlay state, including all the overlay logs &
  * records.
@@ -34,6 +35,7 @@
 struct of_overlay_info {
 	struct device_node *target;
 	struct device_node *overlay;
+	struct device_node **plug_targets;
 };
 
 /**
@@ -54,7 +56,8 @@ struct of_overlay {
 };
 
 static int of_overlay_apply_one(struct of_overlay *ov,
-		struct device_node *target, const struct device_node *overlay);
+		struct device_node *target, const struct device_node *overlay,
+		bool plug_node, struct of_overlay_info *ovinfo);
 
 static int of_overlay_apply_single_property(struct of_overlay *ov,
 		struct device_node *target, struct property *prop)
@@ -97,7 +100,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 	tchild = of_get_child_by_name(target, cname);
 	if (tchild != NULL) {
 		/* apply overlay recursively */
-		ret = of_overlay_apply_one(ov, tchild, child);
+		ret = of_overlay_apply_one(ov, tchild, child, false, NULL);
 		of_node_put(tchild);
 	} else {
 		/* create empty tree as a target */
@@ -112,7 +115,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 		if (ret)
 			return ret;
 
-		ret = of_overlay_apply_one(ov, tchild, child);
+		ret = of_overlay_apply_one(ov, tchild, child, false, NULL);
 		if (ret)
 			return ret;
 	}
@@ -128,33 +131,108 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
  * by using the changeset.
  */
 static int of_overlay_apply_one(struct of_overlay *ov,
-		struct device_node *target, const struct device_node *overlay)
+		struct device_node *target, const struct device_node *overlay,
+		bool plug_node, struct of_overlay_info *ovinfo)
 {
-	struct device_node *child;
+	struct device_node *overlay_child;
+	struct device_node *plug_target;
 	struct property *prop;
 	int ret;
+	u32 val;
 
-	for_each_property_of_node(overlay, prop) {
-		ret = of_overlay_apply_single_property(ov, target, prop);
-		if (ret) {
-			pr_err("%s: Failed to apply prop @%s/%s\n",
-				__func__, target->full_name, prop->name);
-			return ret;
+	if (!plug_node) {
+		for_each_property_of_node(overlay, prop) {
+			ret = of_overlay_apply_single_property(ov, target, prop);
+			if (ret) {
+				pr_err("%s: Failed to apply prop @%s/%s\n",
+					__func__, target->full_name, prop->name);
+				return ret;
+			}
 		}
 	}
 
-	for_each_child_of_node(overlay, child) {
-		ret = of_overlay_apply_single_device_node(ov, target, child);
-		if (ret != 0) {
-			pr_err("%s: Failed to apply single node @%s/%s\n",
-					__func__, target->full_name,
-					child->name);
-			of_node_put(child);
-			return ret;
+	if (plug_node) {
+		struct device_node *socket_child;
+		int child_cnt = 0;
+
+		if (WARN_ON(!ovinfo))
+			return -EINVAL;
+
+		for_each_child_of_node(overlay->child, overlay_child) {
+			child_cnt++;
+		}
+		/* plug_targets[] is NULL terminated */
+		child_cnt++;
+		ovinfo->plug_targets = kcalloc(child_cnt,
+					       sizeof(*ovinfo->plug_targets),
+					       GFP_KERNEL);
+
+		child_cnt = 0;
+		for_each_child_of_node(overlay->child, overlay_child) {
+
+			socket_child = of_get_child_by_name(target,
+							    overlay_child->name);
+			ret = of_property_read_u32(socket_child,
+						   "target_phandle", &val);
+			of_node_put(socket_child);
+			if (ret != 0)
+				goto overlay_child;
+			plug_target = of_find_node_by_phandle(val);
+			if (!plug_target)
+				goto overlay_child;
+			/* save for of_node_put() in of_free_overlay_info() */
+			ovinfo->plug_targets[child_cnt++] = plug_target;
+
+			ret = of_overlay_apply_single_device_node(ov, plug_target,
+								  overlay_child);
+			if (ret != 0)
+				goto plug_target;
+		}
+
+	} else {
+		for_each_child_of_node(overlay, overlay_child) {
+			ret = of_overlay_apply_single_device_node(ov, target,
+								  overlay_child);
+			if (ret != 0)
+				goto overlay_child;
 		}
 	}
 
 	return 0;
+
+plug_target:
+	of_node_put(plug_target);
+overlay_child:
+	of_node_put(overlay_child);
+	return ret;
+}
+
+static bool plug_compatible(struct device_node *overlay,
+			    struct device_node *target)
+{
+	struct property *prop_plug;
+	struct property *prop_socket;
+	const char *c_plug;
+	const char *c_socket;
+	bool socket_node;
+
+	socket_node = of_property_read_bool(target, "connector-socket");
+	if (!socket_node)
+		return false;
+
+	prop_plug = of_find_property(overlay, "compatible", NULL);
+	prop_socket = of_find_property(target, "compatible", NULL);
+
+	for (c_socket = of_prop_next_string(prop_plug, NULL); c_socket;
+	     c_socket = of_prop_next_string(prop_plug, c_socket)) {
+		for (c_plug = of_prop_next_string(prop_plug, NULL); c_plug;
+		     c_plug = of_prop_next_string(prop_plug, c_plug)) {
+			if (!of_compat_cmp(c_plug, c_socket, strlen(c_plug)))
+				return true;
+		}
+	}
+
+	return false;
 }
 
 /**
@@ -168,13 +246,24 @@ static int of_overlay_apply_one(struct of_overlay *ov,
  */
 static int of_overlay_apply(struct of_overlay *ov)
 {
-	int i, err;
+	int i, err = 0;
+	bool plug_node = false;
 
 	/* first we apply the overlays atomically */
 	for (i = 0; i < ov->count; i++) {
 		struct of_overlay_info *ovinfo = &ov->ovinfo_tab[i];
 
-		err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay);
+		plug_node = of_property_read_bool(ovinfo->overlay,
+						  "connector-plug");
+		if (plug_node) {
+			if (!plug_compatible(ovinfo->target, ovinfo->overlay))
+				err = -ENODEV;
+		}
+
+		if (err == 0)
+			err = of_overlay_apply_one(ov, ovinfo->target,
+						   ovinfo->overlay, plug_node,
+						   ovinfo);
 		if (err != 0) {
 			pr_err("%s: overlay failed '%s'\n",
 				__func__, ovinfo->target->full_name);
@@ -309,6 +398,7 @@ static int of_build_overlay_info(struct of_overlay *ov,
 static int of_free_overlay_info(struct of_overlay *ov)
 {
 	struct of_overlay_info *ovinfo;
+	struct device_node **plug_targets;
 	int i;
 
 	/* do it in reverse */
@@ -317,6 +407,11 @@ static int of_free_overlay_info(struct of_overlay *ov)
 
 		of_node_put(ovinfo->target);
 		of_node_put(ovinfo->overlay);
+		plug_targets = ovinfo->plug_targets;
+		while (*plug_targets != NULL) {
+			of_node_put(*plug_targets);
+			plug_targets++;
+		}
 	}
 	kfree(ov->ovinfo_tab);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-02 23:55 [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual frowand.list
  2016-07-02 23:55 ` [RFC PATCH 1/1] device tree connectors, using plugs and sockets frowand.list
@ 2016-07-03  6:06 ` Frank Rowand
  2016-07-04 15:22 ` Mark Brown
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Frank Rowand @ 2016-07-03  6:06 UTC (permalink / raw)
  To: robh+dt, david, pantelis.antoniou, stephen.boyd, broonie,
	grant.likely, mark.rutland
  Cc: mporter, koen, linux, marex, wsa, devicetree, linux-kernel,
	linux-i2c, panto

On 07/02/16 16:55, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> Hi All,
> 
> This is version 2 of this email.

< snip >

> The one remaining piece that this patch does not provide is how
> the overlay manager (which does not yet exist in the mainline
> tree) can apply an overlay to two different targets.  That
> final step should be a trivial change to of_overlay_create(),
> adding a parameter that is a mapping of the target (or maybe
> even targets) in the overlay to different targets in the
> active device tree.
> 
> This seems like a more straight forward way to handle connectors.
> 
> First, ignoring pinctrl and pinmux, what does everyone think?
> 
> Then, the next step is whether pinctrl and pinmux work with this method.
> Pantelis, can you point me to a good example for
> 
>   1) an in-tree board dts file
>   2) an overlay file (I am assuming out of tree) that applies to the board
>   3) an in-tree .dtsi file that would provide the same features as
>      the overlay file if it was included by the board dts file
> 
> It should be easier to discuss pinctrl and pinmux with an example.

And I should have added that there are other complexities beyond
pinctrl and pinmux that are not addressed.  Baby steps first,
before the whole enchilada.

-Frank

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-02 23:55 [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual frowand.list
  2016-07-02 23:55 ` [RFC PATCH 1/1] device tree connectors, using plugs and sockets frowand.list
  2016-07-03  6:06 ` [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual Frank Rowand
@ 2016-07-04 15:22 ` Mark Brown
  2016-07-04 18:15   ` Frank Rowand
  2016-07-05 18:01   ` Pantelis Antoniou
  2016-07-07  7:15 ` David Gibson
  4 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2016-07-04 15:22 UTC (permalink / raw)
  To: frowand.list
  Cc: robh+dt, david, pantelis.antoniou, stephen.boyd, grant.likely,
	mark.rutland, mporter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c, panto

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

On Sat, Jul 02, 2016 at 04:55:49PM -0700, frowand.list@gmail.com wrote:

> This is an extremely simple example to illustrate the concepts.  It is not
> meant to represent the complexity of a real board.
> 
> To start with, assume that the device that will eventually be on a daughter
> board is first soldered onto the mother board.  The mother board contains
> two devices connected via bus spi_1.  One device is described in the .dts
> file, the other is described in an included .dtsi file.
> Then the device tree files will look like:

Can I suggest not using SPI as an example here?  It's particularly
messy since addresses are essentially just a random signal that can be
totally separate to the controller hardware which might be adding more
complexity early on in building up your model than is really desirable.
It will need to be dealt with but perhaps not right now.  I2C might be
easier.

The initial issue with SPI is that you really need to do something like
bring out individual slots on the bus rather than the bus as a whole
since you're going to need a remapping layer to map chip selects on the
module to chip selects on the host board.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-04 15:22 ` Mark Brown
@ 2016-07-04 18:15   ` Frank Rowand
  0 siblings, 0 replies; 14+ messages in thread
From: Frank Rowand @ 2016-07-04 18:15 UTC (permalink / raw)
  To: Mark Brown
  Cc: robh+dt, david, pantelis.antoniou, stephen.boyd, grant.likely,
	mark.rutland, mporter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c, panto

On 07/04/16 08:22, Mark Brown wrote:
> On Sat, Jul 02, 2016 at 04:55:49PM -0700, frowand.list@gmail.com wrote:
> 
>> This is an extremely simple example to illustrate the concepts.  It is not
>> meant to represent the complexity of a real board.
>>
>> To start with, assume that the device that will eventually be on a daughter
>> board is first soldered onto the mother board.  The mother board contains
>> two devices connected via bus spi_1.  One device is described in the .dts
>> file, the other is described in an included .dtsi file.
>> Then the device tree files will look like:
> 
> Can I suggest not using SPI as an example here?  It's particularly
> messy since addresses are essentially just a random signal that can be
> totally separate to the controller hardware which might be adding more
> complexity early on in building up your model than is really desirable.
> It will need to be dealt with but perhaps not right now.  I2C might be
> easier.
> 
> The initial issue with SPI is that you really need to do something like
> bring out individual slots on the bus rather than the bus as a whole
> since you're going to need a remapping layer to map chip selects on the
> module to chip selects on the host board.
> 

Yes, thank you for pointing that out.

For the purposes of the mental model, when thinking about what I wrote,
just change SPI to I2C everywhere.

-Frank

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
@ 2016-07-05 18:01   ` Pantelis Antoniou
  0 siblings, 0 replies; 14+ messages in thread
From: Pantelis Antoniou @ 2016-07-05 18:01 UTC (permalink / raw)
  To: frowand.list
  Cc: robh+dt, David Gibson, stephen.boyd, broonie, Grant Likely,
	mark.rutland, Matt Porter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c

Hi Frank,

Sorry for taking a bit to reply, had to grok it well first.

> On Jul 3, 2016, at 02:55 , frowand.list@gmail.com wrote:
> 
> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> Hi All,
> 
> This is version 2 of this email.
> 
> Changes from version 1:
> 
>  - some rewording of the text
>  - removed new (theoretical) dtc directive "/connector/"
>  - added compatibility between mother board and daughter board
>  - added info on applying a single .dtbo to different connectors
>  - attached an RFC patch showing the required kernel changes
>  - changes to mother board .dts connector node:
>     - removed target_path property
>     - added connector-socket property
>  - changes to daughter board .dts connector node:
>     - added connector-plug property
> 
> 
> I've been trying to wrap my head around what Pantelis and Rob have written
> on the subject of a device tree representation of a connector for a
> daughter board to connect to (eg a cape or a shield) and the representation
> of the daughter board.  (Or any other physically pluggable object.)
> 
> After trying to make sense of what had been written (or presented via slides
> at a conference - thanks Pantelis!), I decided to go back to first principals
> of what we are trying to accomplish.  I came up with some really simple bogus
> examples to try to explain what my thought process is.
> 
> This is an extremely simple example to illustrate the concepts.  It is not
> meant to represent the complexity of a real board.
> 
> To start with, assume that the device that will eventually be on a daughter
> board is first soldered onto the mother board.  The mother board contains
> two devices connected via bus spi_1.  One device is described in the .dts
> file, the other is described in an included .dtsi file.
> Then the device tree files will look like:
> 
> $ cat board.dts
> /dts-v1/;
> 
> / {
>        #address-cells = < 1 >;
>        #size-cells = < 1 >;
> 
>        tree_1: soc@0 {
>                reg = <0x0 0x0>;
> 
>                spi_1: spi1 {
>                };
>        };
> 
> };
> 
> &spi_1 {
>        ethernet-switch@0 {
>                compatible = "micrel,ks8995m";
>        };
> };
> 
> #include "spi_codec.dtsi"
> 
> 
> $ cat spi_codec.dtsi
> &spi_1 {
> 	codec@1 {
> 		compatible = "ti,tlv320aic26";
> 	};
> };
> 
> 
> #----- codec chip on cape
> 
> Then suppose I move the codec chip to a cape.  Then I will have the same
> exact .dts and .dtsi and everything still works.
> 
> 
> @----- codec chip on cape, overlay
> 
> If I want to use overlays, I only have to add the version and "/plugin/",
> then use the '-@' flag for dtc (both for the previous board.dts and
> this spi_codec_overlay.dts):
> 
> $ cat spi_codec_overlay.dts
> /dts-v1/;
> 
> /plugin/;
> 
> &spi_1 {
> 	codec@1 {
> 		compatible = "ti,tlv320aic26";
> 	};
> };
> 
> 
> Pantelis pointed out that the syntax has changed to be:
>   /dts-v1/ /plugin/;
> 
> 
> #----- codec chip on cape, overlay, connector
> 
> Now we move into the realm of connectors.  My mental model of what the
> hardware and driver look like has not changed.  The only thing that has
> changed is that I want to be able to specify that the connector that
> the cape is plugged into has some pins that are the spi bus /soc/spi1.
> 
> The following _almost_ but not quite gets me what I want.  Note that
> the only thing the connector node does is provide some kind of
> pointer or reference to what node(s) are physically routed through
> the connector.  The connector node does not need to describe the pins;
> it only has to point to the node that describes the pins.
> 
> This example will turn out to be not sufficient.  It is a stepping
> stone in building my mental model.
> 
> $ cat board_with_connector.dts
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1: connector_1 {
> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> 
> $ cat spi_codec_overlay_with_connector.dts
> /dts-v1/;
> 
> /plugin/;
> 
> &connector_1 {
> 	spi1 {
> 		codec@1 {
> 			compatible = "ti,tlv320aic26";
> 		};
> 	};
> };
> 

You target connector_1. In theory multiples of the same connector
may be available.

There are complications about how they are applied. A method that’s not
referring to a single phandle/path is going to be needed.

Thinking about it a bit more maybe we can sugar it with DTC with something like
this:

$ cat arduino_connector.dts

/dts-v1/ /plugin/ /portable/;

&arduino_connector {
	spi1 {
		codec@1 {
			compatible = “ti,tlv320aic26”;
		};
	};
};

$ cat board_with_arduino_connectors.dts
/dts-v1/;

/ {
	#address-cells = < 1 >;
	#size-cells = < 1 >;

	tree_1: soc@0 {
		reg = <0x0 0x0>;

		spi_1: spi1 {
		};
	};

	connector_1 {
		connector-socket;
		compatible = “arduino_connector”;
		status = “okay”;

		spi1 {
			target_phandle = <&spi_1>;
		};
	};

	connector_2 {
		connector-socket;
		compatible = “arduino_connector”;
		
		spi2 {
			target_phandle = <&spi_2>;
		};
	};

};

&spi_1 {
	ethernet-switch@0 {
		compatible = "micrel,ks8995m";
	};
};

&spi_2 {
	ethernet-switch@0 {
		compatible = "micrel,ks8995m";
	};
};

The &arduino_connector construct at a portable overlay can be resolved
as follows:

fragment0 {
	target-compatible = “arduino_connector“;
	….
};

The new thing here is the ‘target-compatible’ option which the
loader will use to find the target node.

> 
> The result is that the overlay fixup for spi1 on the cape will
> relocate the spi1 node to /connector_1 in the host tree, so
> this does not solve the connector linkage yet:
> 
> -- chunk from the decompiled board_with_connector.dtb:
> 
> 	__symbols__ {
> 		connector_1 = "/connector_1";
> 	};
> 

^ See above. Not going to work cause we need to support multiple
identical connectors on the same board.
 
> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
> 
> 	fragment@0 {
> 		target = <0xffffffff>;
> 		__overlay__ {
> 			spi1 {
> 				codec@1 {
> 					compatible = "ti,tlv320aic26";
> 				};
> 			};
> 		};
> 	};
> 	__fixups__ {
> 		connector_1 = "/fragment@0:target:0";
> 	};
> 
> 
> After applying the overlay, the codec@1 node will be at
> /connector_1/spi1/codec@1.  What I want is for that node
> to be at /spi1/codec@1.
> 
> 
> 
> #----- magic new syntax
> 
> What I really want is some way to tell dtc that I want to do one
> level of dereferencing when resolving the path of device nodes
> contained by the connector node in the overlay dts.
> 
> Version 1 of this email suggested using dtc magic to do this extra
> level of dereferencing.  This version of the email has changed to
> have the kernel code that applies the overlay do the extra level
> of dereferencing.
> 
> The property "connector-socket" tells the kernel overlay code
> that this is a socket.  The overlay code does not actually
> do anything special as a result of this property; it is simply
> used as a sanity check that this node really is a socket.  The
> person writing the mother board .dts must provide the
> target_phandle property, which points to a node responsible for
> some of the pins on the connector.
> 
> The property "connector-plug" tells the kernel overlay code
> that each child node in the overlay corresponds to a node in the
> socket, and the socket will contain one property that is
> a phandle pointing to the node that is the target of that child
> node in the overlay node.
> 
> 
> $ cat board_with_connector_v2.dts
> 
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1: connector_1 {
> 		compatible = "11-pin-accessory";
> 		connector-socket;
> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> 
> $ cat spi_codec_overlay_with_connector_v2.dts
> 
> /dts-v1/;
> 
> /plugin/;
> 
> &connector_1 {
> 	connector-plug;

^ we won’t need this, nor the compatible string with the 
version I mentioned earlier

> 	compatible = "11-pin-accessory";
> 
> 	spi1 {
> 		codec@1 {
> 			compatible = "ti,tlv320aic26";
> 		};
> 	};
> };
> 
> 
> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
> is unchanged from the previous example, but the kernel overlay
> code will do the correct extra level of dereferencing when it
> detects the connector-plug property in the overlay.
> 
> The one remaining piece that this patch does not provide is how
> the overlay manager (which does not yet exist in the mainline
> tree) can apply an overlay to two different targets.  That
> final step should be a trivial change to of_overlay_create(),
> adding a parameter that is a mapping of the target (or maybe
> even targets) in the overlay to different targets in the
> active device tree.
> 
> This seems like a more straight forward way to handle connectors.
> 
> First, ignoring pinctrl and pinmux, what does everyone think?
> 
> Then, the next step is whether pinctrl and pinmux work with this method.
> Pantelis, can you point me to a good example for
> 
>  1) an in-tree board dts file
>  2) an overlay file (I am assuming out of tree) that applies to the board
>  3) an in-tree .dtsi file that would provide the same features as
>     the overlay file if it was included by the board dts file
> 

Looks good for a starting point. We need to figure out pinmux and
gpio/irq references for starter.

It is imperative that references do not leak out of the connector
node.

> It should be easier to discuss pinctrl and pinmux with an example.
> 
> -Frank
> —

Regards

— Pantelis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
@ 2016-07-05 18:01   ` Pantelis Antoniou
  0 siblings, 0 replies; 14+ messages in thread
From: Pantelis Antoniou @ 2016-07-05 18:01 UTC (permalink / raw)
  To: frowand.list-Re5JQEeQqe8AvxtiuMwx3w
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, David Gibson,
	stephen.boyd-QSEj5FYQhm4dnm+yROfE0A,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, Grant Likely,
	mark.rutland-5wv7dgnIgG8, Matt Porter,
	koen-QLwJDigV5abLmq1fohREcCpxlwaOVQ5f,
	linux-0h96xk9xTtrk1uMJSBkQmQ, marex-ynQEQJNshbs,
	wsa-z923LK4zBo2bacvFa/9K2g, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA

Hi Frank,

Sorry for taking a bit to reply, had to grok it well first.

> On Jul 3, 2016, at 02:55 , frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> 
> From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
> 
> Hi All,
> 
> This is version 2 of this email.
> 
> Changes from version 1:
> 
>  - some rewording of the text
>  - removed new (theoretical) dtc directive "/connector/"
>  - added compatibility between mother board and daughter board
>  - added info on applying a single .dtbo to different connectors
>  - attached an RFC patch showing the required kernel changes
>  - changes to mother board .dts connector node:
>     - removed target_path property
>     - added connector-socket property
>  - changes to daughter board .dts connector node:
>     - added connector-plug property
> 
> 
> I've been trying to wrap my head around what Pantelis and Rob have written
> on the subject of a device tree representation of a connector for a
> daughter board to connect to (eg a cape or a shield) and the representation
> of the daughter board.  (Or any other physically pluggable object.)
> 
> After trying to make sense of what had been written (or presented via slides
> at a conference - thanks Pantelis!), I decided to go back to first principals
> of what we are trying to accomplish.  I came up with some really simple bogus
> examples to try to explain what my thought process is.
> 
> This is an extremely simple example to illustrate the concepts.  It is not
> meant to represent the complexity of a real board.
> 
> To start with, assume that the device that will eventually be on a daughter
> board is first soldered onto the mother board.  The mother board contains
> two devices connected via bus spi_1.  One device is described in the .dts
> file, the other is described in an included .dtsi file.
> Then the device tree files will look like:
> 
> $ cat board.dts
> /dts-v1/;
> 
> / {
>        #address-cells = < 1 >;
>        #size-cells = < 1 >;
> 
>        tree_1: soc@0 {
>                reg = <0x0 0x0>;
> 
>                spi_1: spi1 {
>                };
>        };
> 
> };
> 
> &spi_1 {
>        ethernet-switch@0 {
>                compatible = "micrel,ks8995m";
>        };
> };
> 
> #include "spi_codec.dtsi"
> 
> 
> $ cat spi_codec.dtsi
> &spi_1 {
> 	codec@1 {
> 		compatible = "ti,tlv320aic26";
> 	};
> };
> 
> 
> #----- codec chip on cape
> 
> Then suppose I move the codec chip to a cape.  Then I will have the same
> exact .dts and .dtsi and everything still works.
> 
> 
> @----- codec chip on cape, overlay
> 
> If I want to use overlays, I only have to add the version and "/plugin/",
> then use the '-@' flag for dtc (both for the previous board.dts and
> this spi_codec_overlay.dts):
> 
> $ cat spi_codec_overlay.dts
> /dts-v1/;
> 
> /plugin/;
> 
> &spi_1 {
> 	codec@1 {
> 		compatible = "ti,tlv320aic26";
> 	};
> };
> 
> 
> Pantelis pointed out that the syntax has changed to be:
>   /dts-v1/ /plugin/;
> 
> 
> #----- codec chip on cape, overlay, connector
> 
> Now we move into the realm of connectors.  My mental model of what the
> hardware and driver look like has not changed.  The only thing that has
> changed is that I want to be able to specify that the connector that
> the cape is plugged into has some pins that are the spi bus /soc/spi1.
> 
> The following _almost_ but not quite gets me what I want.  Note that
> the only thing the connector node does is provide some kind of
> pointer or reference to what node(s) are physically routed through
> the connector.  The connector node does not need to describe the pins;
> it only has to point to the node that describes the pins.
> 
> This example will turn out to be not sufficient.  It is a stepping
> stone in building my mental model.
> 
> $ cat board_with_connector.dts
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1: connector_1 {
> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> 
> $ cat spi_codec_overlay_with_connector.dts
> /dts-v1/;
> 
> /plugin/;
> 
> &connector_1 {
> 	spi1 {
> 		codec@1 {
> 			compatible = "ti,tlv320aic26";
> 		};
> 	};
> };
> 

You target connector_1. In theory multiples of the same connector
may be available.

There are complications about how they are applied. A method that’s not
referring to a single phandle/path is going to be needed.

Thinking about it a bit more maybe we can sugar it with DTC with something like
this:

$ cat arduino_connector.dts

/dts-v1/ /plugin/ /portable/;

&arduino_connector {
	spi1 {
		codec@1 {
			compatible = “ti,tlv320aic26”;
		};
	};
};

$ cat board_with_arduino_connectors.dts
/dts-v1/;

/ {
	#address-cells = < 1 >;
	#size-cells = < 1 >;

	tree_1: soc@0 {
		reg = <0x0 0x0>;

		spi_1: spi1 {
		};
	};

	connector_1 {
		connector-socket;
		compatible = “arduino_connector”;
		status = “okay”;

		spi1 {
			target_phandle = <&spi_1>;
		};
	};

	connector_2 {
		connector-socket;
		compatible = “arduino_connector”;
		
		spi2 {
			target_phandle = <&spi_2>;
		};
	};

};

&spi_1 {
	ethernet-switch@0 {
		compatible = "micrel,ks8995m";
	};
};

&spi_2 {
	ethernet-switch@0 {
		compatible = "micrel,ks8995m";
	};
};

The &arduino_connector construct at a portable overlay can be resolved
as follows:

fragment0 {
	target-compatible = “arduino_connector“;
	….
};

The new thing here is the ‘target-compatible’ option which the
loader will use to find the target node.

> 
> The result is that the overlay fixup for spi1 on the cape will
> relocate the spi1 node to /connector_1 in the host tree, so
> this does not solve the connector linkage yet:
> 
> -- chunk from the decompiled board_with_connector.dtb:
> 
> 	__symbols__ {
> 		connector_1 = "/connector_1";
> 	};
> 

^ See above. Not going to work cause we need to support multiple
identical connectors on the same board.
 
> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
> 
> 	fragment@0 {
> 		target = <0xffffffff>;
> 		__overlay__ {
> 			spi1 {
> 				codec@1 {
> 					compatible = "ti,tlv320aic26";
> 				};
> 			};
> 		};
> 	};
> 	__fixups__ {
> 		connector_1 = "/fragment@0:target:0";
> 	};
> 
> 
> After applying the overlay, the codec@1 node will be at
> /connector_1/spi1/codec@1.  What I want is for that node
> to be at /spi1/codec@1.
> 
> 
> 
> #----- magic new syntax
> 
> What I really want is some way to tell dtc that I want to do one
> level of dereferencing when resolving the path of device nodes
> contained by the connector node in the overlay dts.
> 
> Version 1 of this email suggested using dtc magic to do this extra
> level of dereferencing.  This version of the email has changed to
> have the kernel code that applies the overlay do the extra level
> of dereferencing.
> 
> The property "connector-socket" tells the kernel overlay code
> that this is a socket.  The overlay code does not actually
> do anything special as a result of this property; it is simply
> used as a sanity check that this node really is a socket.  The
> person writing the mother board .dts must provide the
> target_phandle property, which points to a node responsible for
> some of the pins on the connector.
> 
> The property "connector-plug" tells the kernel overlay code
> that each child node in the overlay corresponds to a node in the
> socket, and the socket will contain one property that is
> a phandle pointing to the node that is the target of that child
> node in the overlay node.
> 
> 
> $ cat board_with_connector_v2.dts
> 
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1: connector_1 {
> 		compatible = "11-pin-accessory";
> 		connector-socket;
> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> 
> $ cat spi_codec_overlay_with_connector_v2.dts
> 
> /dts-v1/;
> 
> /plugin/;
> 
> &connector_1 {
> 	connector-plug;

^ we won’t need this, nor the compatible string with the 
version I mentioned earlier

> 	compatible = "11-pin-accessory";
> 
> 	spi1 {
> 		codec@1 {
> 			compatible = "ti,tlv320aic26";
> 		};
> 	};
> };
> 
> 
> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
> is unchanged from the previous example, but the kernel overlay
> code will do the correct extra level of dereferencing when it
> detects the connector-plug property in the overlay.
> 
> The one remaining piece that this patch does not provide is how
> the overlay manager (which does not yet exist in the mainline
> tree) can apply an overlay to two different targets.  That
> final step should be a trivial change to of_overlay_create(),
> adding a parameter that is a mapping of the target (or maybe
> even targets) in the overlay to different targets in the
> active device tree.
> 
> This seems like a more straight forward way to handle connectors.
> 
> First, ignoring pinctrl and pinmux, what does everyone think?
> 
> Then, the next step is whether pinctrl and pinmux work with this method.
> Pantelis, can you point me to a good example for
> 
>  1) an in-tree board dts file
>  2) an overlay file (I am assuming out of tree) that applies to the board
>  3) an in-tree .dtsi file that would provide the same features as
>     the overlay file if it was included by the board dts file
> 

Looks good for a starting point. We need to figure out pinmux and
gpio/irq references for starter.

It is imperative that references do not leak out of the connector
node.

> It should be easier to discuss pinctrl and pinmux with an example.
> 
> -Frank
> —

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-05 18:01   ` Pantelis Antoniou
  (?)
@ 2016-07-06 17:40   ` Frank Rowand
  -1 siblings, 0 replies; 14+ messages in thread
From: Frank Rowand @ 2016-07-06 17:40 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: robh+dt, David Gibson, stephen.boyd, broonie, Grant Likely,
	mark.rutland, Matt Porter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c

On 07/05/16 11:01, Pantelis Antoniou wrote:
> Hi Frank,
> 
> Sorry for taking a bit to reply, had to grok it well first.
> 
>> On Jul 3, 2016, at 02:55 , frowand.list@gmail.com wrote:
>>
>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> Hi All,
>>
>> This is version 2 of this email.
>>
>> Changes from version 1:
>>
>>  - some rewording of the text
>>  - removed new (theoretical) dtc directive "/connector/"
>>  - added compatibility between mother board and daughter board
>>  - added info on applying a single .dtbo to different connectors
>>  - attached an RFC patch showing the required kernel changes
>>  - changes to mother board .dts connector node:
>>     - removed target_path property
>>     - added connector-socket property
>>  - changes to daughter board .dts connector node:
>>     - added connector-plug property
>>
>>
>> I've been trying to wrap my head around what Pantelis and Rob have written
>> on the subject of a device tree representation of a connector for a
>> daughter board to connect to (eg a cape or a shield) and the representation
>> of the daughter board.  (Or any other physically pluggable object.)
>>
>> After trying to make sense of what had been written (or presented via slides
>> at a conference - thanks Pantelis!), I decided to go back to first principals
>> of what we are trying to accomplish.  I came up with some really simple bogus
>> examples to try to explain what my thought process is.
>>
>> This is an extremely simple example to illustrate the concepts.  It is not
>> meant to represent the complexity of a real board.
>>
>> To start with, assume that the device that will eventually be on a daughter
>> board is first soldered onto the mother board.  The mother board contains
>> two devices connected via bus spi_1.  One device is described in the .dts
>> file, the other is described in an included .dtsi file.
>> Then the device tree files will look like:
>>
>> $ cat board.dts
>> /dts-v1/;
>>
>> / {
>>        #address-cells = < 1 >;
>>        #size-cells = < 1 >;
>>
>>        tree_1: soc@0 {
>>                reg = <0x0 0x0>;
>>
>>                spi_1: spi1 {
>>                };
>>        };
>>
>> };
>>
>> &spi_1 {
>>        ethernet-switch@0 {
>>                compatible = "micrel,ks8995m";
>>        };
>> };
>>
>> #include "spi_codec.dtsi"
>>
>>
>> $ cat spi_codec.dtsi
>> &spi_1 {
>> 	codec@1 {
>> 		compatible = "ti,tlv320aic26";
>> 	};
>> };
>>
>>
>> #----- codec chip on cape
>>
>> Then suppose I move the codec chip to a cape.  Then I will have the same
>> exact .dts and .dtsi and everything still works.
>>
>>
>> @----- codec chip on cape, overlay
>>
>> If I want to use overlays, I only have to add the version and "/plugin/",
>> then use the '-@' flag for dtc (both for the previous board.dts and
>> this spi_codec_overlay.dts):
>>
>> $ cat spi_codec_overlay.dts
>> /dts-v1/;
>>
>> /plugin/;
>>
>> &spi_1 {
>> 	codec@1 {
>> 		compatible = "ti,tlv320aic26";
>> 	};
>> };
>>
>>
>> Pantelis pointed out that the syntax has changed to be:
>>   /dts-v1/ /plugin/;
>>
>>
>> #----- codec chip on cape, overlay, connector
>>
>> Now we move into the realm of connectors.  My mental model of what the
>> hardware and driver look like has not changed.  The only thing that has
>> changed is that I want to be able to specify that the connector that
>> the cape is plugged into has some pins that are the spi bus /soc/spi1.
>>
>> The following _almost_ but not quite gets me what I want.  Note that
>> the only thing the connector node does is provide some kind of
>> pointer or reference to what node(s) are physically routed through
>> the connector.  The connector node does not need to describe the pins;
>> it only has to point to the node that describes the pins.
>>


>> This example will turn out to be not sufficient.  It is a stepping
>> stone in building my mental model.

  ^^^^^^^^^^^^^^^^^^^^^^

>>
>> $ cat board_with_connector.dts
>> /dts-v1/;
>>
>> / {
>> 	#address-cells = < 1 >;
>> 	#size-cells = < 1 >;
>>
>> 	tree_1: soc@0 {
>> 		reg = <0x0 0x0>;
>>
>> 		spi_1: spi1 {
>> 		};
>> 	};
>>
>> 	connector_1: connector_1 {
>> 		spi1 {
>> 			target_phandle = <&spi_1>;
>> 		};
>> 	};
>>
>> };
>>
>> &spi_1 {
>> 	ethernet-switch@0 {
>> 		compatible = "micrel,ks8995m";
>> 	};
>> };
>>
>>
>> $ cat spi_codec_overlay_with_connector.dts
>> /dts-v1/;
>>
>> /plugin/;
>>
>> &connector_1 {
>> 	spi1 {
>> 		codec@1 {
>> 			compatible = "ti,tlv320aic26";
>> 		};
>> 	};
>> };
>>

There is an "off by one" here.  The above method was a mental stepping
stone.  You are correct that it does not work.

The stuff below "#----- magic new syntax" is what you need to look
at.  That was where I resolved the problem that remains at this step.


> 
> You target connector_1. In theory multiples of the same connector
> may be available.
> 
> There are complications about how they are applied. A method that’s not
> referring to a single phandle/path is going to be needed.

Yes.

I addressed that near the end of the original email.  I'll copy that
paragraph up here:

   The one remaining piece that this patch does not provide is how
   the overlay manager (which does not yet exist in the mainline
   tree) can apply an overlay to two different targets.  That
   final step should be a trivial change to of_overlay_create(),
   adding a parameter that is a mapping of the target (or maybe
   even targets) in the overlay to different targets in the
   active device tree.


> Thinking about it a bit more maybe we can sugar it with DTC with something like
> this:
> 
> $ cat arduino_connector.dts
> 
> /dts-v1/ /plugin/ /portable/;
> 
> &arduino_connector {
> 	spi1 {
> 		codec@1 {
> 			compatible = “ti,tlv320aic26”;
> 		};
> 	};
> };
> 
> $ cat board_with_arduino_connectors.dts
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1 {
> 		connector-socket;
> 		compatible = “arduino_connector”;
> 		status = “okay”;
> 
> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> 	connector_2 {
> 		connector-socket;
> 		compatible = “arduino_connector”;
> 		
> 		spi2 {
> 			target_phandle = <&spi_2>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> &spi_2 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> The &arduino_connector construct at a portable overlay can be resolved
> as follows:
> 
> fragment0 {
> 	target-compatible = “arduino_connector“;
> 	….
> };
> 
> The new thing here is the ‘target-compatible’ option which the
> loader will use to find the target node.

I'm with Rob.  I would prefer to not add more ways a target property
can be specified.

I also do not like putting the target in the overlay .dtbo because
it should be possible to use the same exact .dtbo for two different
identical daughter boards plugged into two different connectors.

The .dtbo should be describing the daughter board.  It should not
be describing the mother board.

The method of specifying which connector a .dtbo applies to should
not be contained in the .dtbo.  It should be given to the overlay
manager as a separate piece of information.

But I do accept that the .dtbo needs to specify that a node on the
daughter board has a target on the motherboard, and I am willing
to specify a single default target on the motherboard.  In the
case of a single connector, the default target should just work.
That seems like a pragmatic useful result despite my philosophical
dislike of specifying a specific target in the .dtbo.


>>
>> The result is that the overlay fixup for spi1 on the cape will
>> relocate the spi1 node to /connector_1 in the host tree, so
>> this does not solve the connector linkage yet:
>>
>> -- chunk from the decompiled board_with_connector.dtb:
>>
>> 	__symbols__ {
>> 		connector_1 = "/connector_1";
>> 	};
>>
> 
> ^ See above. Not going to work cause we need to support multiple
> identical connectors on the same board.
>  
>> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
>>
>> 	fragment@0 {
>> 		target = <0xffffffff>;
>> 		__overlay__ {
>> 			spi1 {
>> 				codec@1 {
>> 					compatible = "ti,tlv320aic26";
>> 				};
>> 			};
>> 		};
>> 	};
>> 	__fixups__ {
>> 		connector_1 = "/fragment@0:target:0";
>> 	};
>>
>>
>> After applying the overlay, the codec@1 node will be at
>> /connector_1/spi1/codec@1.  What I want is for that node
>> to be at /spi1/codec@1.
>>
>>
>>
>> #----- magic new syntax
>>
>> What I really want is some way to tell dtc that I want to do one
>> level of dereferencing when resolving the path of device nodes
>> contained by the connector node in the overlay dts.
>>
>> Version 1 of this email suggested using dtc magic to do this extra
>> level of dereferencing.  This version of the email has changed to
>> have the kernel code that applies the overlay do the extra level
>> of dereferencing.
>>
>> The property "connector-socket" tells the kernel overlay code
>> that this is a socket.  The overlay code does not actually
>> do anything special as a result of this property; it is simply
>> used as a sanity check that this node really is a socket.  The
>> person writing the mother board .dts must provide the
>> target_phandle property, which points to a node responsible for
>> some of the pins on the connector.
>>
>> The property "connector-plug" tells the kernel overlay code
>> that each child node in the overlay corresponds to a node in the
>> socket, and the socket will contain one property that is
>> a phandle pointing to the node that is the target of that child
>> node in the overlay node.
>>
>>
>> $ cat board_with_connector_v2.dts
>>
>> /dts-v1/;
>>
>> / {
>> 	#address-cells = < 1 >;
>> 	#size-cells = < 1 >;
>>
>> 	tree_1: soc@0 {
>> 		reg = <0x0 0x0>;
>>
>> 		spi_1: spi1 {
>> 		};
>> 	};
>>
>> 	connector_1: connector_1 {
>> 		compatible = "11-pin-accessory";
>> 		connector-socket;
>> 		spi1 {
>> 			target_phandle = <&spi_1>;
>> 		};
>> 	};
>>
>> };
>>
>> &spi_1 {
>> 	ethernet-switch@0 {
>> 		compatible = "micrel,ks8995m";
>> 	};
>> };
>>
>>
>> $ cat spi_codec_overlay_with_connector_v2.dts
>>
>> /dts-v1/;
>>
>> /plugin/;
>>
>> &connector_1 {
>> 	connector-plug;
> 
> ^ we won’t need this, nor the compatible string with the 
> version I mentioned earlier
> 
>> 	compatible = "11-pin-accessory";
>>
>> 	spi1 {
>> 		codec@1 {
>> 			compatible = "ti,tlv320aic26";
>> 		};
>> 	};
>> };
>>
>>
>> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
>> is unchanged from the previous example, but the kernel overlay
>> code will do the correct extra level of dereferencing when it
>> detects the connector-plug property in the overlay.
>>
>> The one remaining piece that this patch does not provide is how
>> the overlay manager (which does not yet exist in the mainline
>> tree) can apply an overlay to two different targets.  That
>> final step should be a trivial change to of_overlay_create(),
>> adding a parameter that is a mapping of the target (or maybe
>> even targets) in the overlay to different targets in the
>> active device tree.
>>
>> This seems like a more straight forward way to handle connectors.
>>
>> First, ignoring pinctrl and pinmux, what does everyone think?
>>
>> Then, the next step is whether pinctrl and pinmux work with this method.
>> Pantelis, can you point me to a good example for
>>
>>  1) an in-tree board dts file
>>  2) an overlay file (I am assuming out of tree) that applies to the board
>>  3) an in-tree .dtsi file that would provide the same features as
>>     the overlay file if it was included by the board dts file
>>
> 
> Looks good for a starting point. We need to figure out pinmux and
> gpio/irq references for starter.

Yes.  But I would like to understand the problem space before
continuing on looking at solutions to the problem.

> 
> It is imperative that references do not leak out of the connector
> node.

Yes.  I would include that in a list of design constraints.


> 
>> It should be easier to discuss pinctrl and pinmux with an example.
>>
>> -Frank
>> —
> 
> Regards
> 
> — Pantelis
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-02 23:55 [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual frowand.list
                   ` (3 preceding siblings ...)
  2016-07-05 18:01   ` Pantelis Antoniou
@ 2016-07-07  7:15 ` David Gibson
  2016-07-08  7:26   ` Pantelis Antoniou
  2016-07-08 19:22   ` Frank Rowand
  4 siblings, 2 replies; 14+ messages in thread
From: David Gibson @ 2016-07-07  7:15 UTC (permalink / raw)
  To: frowand.list
  Cc: robh+dt, pantelis.antoniou, stephen.boyd, broonie, grant.likely,
	mark.rutland, mporter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c, panto

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

On Sat, Jul 02, 2016 at 04:55:49PM -0700, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> Hi All,
> 
> This is version 2 of this email.
> 
> Changes from version 1:
> 
>   - some rewording of the text
>   - removed new (theoretical) dtc directive "/connector/"
>   - added compatibility between mother board and daughter board
>   - added info on applying a single .dtbo to different connectors
>   - attached an RFC patch showing the required kernel changes
>   - changes to mother board .dts connector node:
>      - removed target_path property
>      - added connector-socket property
>   - changes to daughter board .dts connector node:
>      - added connector-plug property
> 
> 
> I've been trying to wrap my head around what Pantelis and Rob have written
> on the subject of a device tree representation of a connector for a
> daughter board to connect to (eg a cape or a shield) and the representation
> of the daughter board.  (Or any other physically pluggable object.)
> 
> After trying to make sense of what had been written (or presented via slides
> at a conference - thanks Pantelis!), I decided to go back to first principals
> of what we are trying to accomplish.  I came up with some really simple bogus
> examples to try to explain what my thought process is.
> 
> This is an extremely simple example to illustrate the concepts.  It is not
> meant to represent the complexity of a real board.
> 
> To start with, assume that the device that will eventually be on a daughter
> board is first soldered onto the mother board.  The mother board contains
> two devices connected via bus spi_1.  One device is described in the .dts
> file, the other is described in an included .dtsi file.
> Then the device tree files will look like:
> 
> $ cat board.dts
> /dts-v1/;
> 
> / {
>         #address-cells = < 1 >;
>         #size-cells = < 1 >;
> 
>         tree_1: soc@0 {
>                 reg = <0x0 0x0>;
> 
>                 spi_1: spi1 {
>                 };
>         };
> 
> };
> 
> &spi_1 {
>         ethernet-switch@0 {
>                 compatible = "micrel,ks8995m";
>         };
> };
> 
> #include "spi_codec.dtsi"
> 
> 
> $ cat spi_codec.dtsi
> &spi_1 {
> 	codec@1 {
> 		compatible = "ti,tlv320aic26";
> 	};
> };
> 
> 
> #----- codec chip on cape
> 
> Then suppose I move the codec chip to a cape.  Then I will have the same
> exact .dts and .dtsi and everything still works.
> 
> 
> @----- codec chip on cape, overlay
> 
> If I want to use overlays, I only have to add the version and "/plugin/",
> then use the '-@' flag for dtc (both for the previous board.dts and
> this spi_codec_overlay.dts):
> 
> $ cat spi_codec_overlay.dts
> /dts-v1/;
> 
> /plugin/;
> 
> &spi_1 {
> 	codec@1 {
> 		compatible = "ti,tlv320aic26";
> 	};
> };
> 
> 
> Pantelis pointed out that the syntax has changed to be:
>    /dts-v1/ /plugin/;
> 
> 
> #----- codec chip on cape, overlay, connector
> 
> Now we move into the realm of connectors.  My mental model of what the
> hardware and driver look like has not changed.  The only thing that has
> changed is that I want to be able to specify that the connector that
> the cape is plugged into has some pins that are the spi bus /soc/spi1.
> 
> The following _almost_ but not quite gets me what I want.  Note that
> the only thing the connector node does is provide some kind of
> pointer or reference to what node(s) are physically routed through
> the connector.  The connector node does not need to describe the pins;
> it only has to point to the node that describes the pins.
> 
> This example will turn out to be not sufficient.  It is a stepping
> stone in building my mental model.
> 
> $ cat board_with_connector.dts
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1: connector_1 {
> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> 
> $ cat spi_codec_overlay_with_connector.dts
> /dts-v1/;
> 
> /plugin/;
> 
> &connector_1 {
> 	spi1 {
> 		codec@1 {
> 			compatible = "ti,tlv320aic26";
> 		};
> 	};
> };
> 
> 
> The result is that the overlay fixup for spi1 on the cape will
> relocate the spi1 node to /connector_1 in the host tree, so
> this does not solve the connector linkage yet:
> 
> -- chunk from the decompiled board_with_connector.dtb:
> 
> 	__symbols__ {
> 		connector_1 = "/connector_1";
> 	};
> 
> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
> 
> 	fragment@0 {
> 		target = <0xffffffff>;
> 		__overlay__ {
> 			spi1 {
> 				codec@1 {
> 					compatible = "ti,tlv320aic26";
> 				};
> 			};
> 		};
> 	};
> 	__fixups__ {
> 		connector_1 = "/fragment@0:target:0";
> 	};
> 
> 
> After applying the overlay, the codec@1 node will be at
> /connector_1/spi1/codec@1.  What I want is for that node
> to be at /spi1/codec@1.
> 
> 
> 
> #----- magic new syntax
> 
> What I really want is some way to tell dtc that I want to do one
> level of dereferencing when resolving the path of device nodes
> contained by the connector node in the overlay dts.
> 
> Version 1 of this email suggested using dtc magic to do this extra
> level of dereferencing.  This version of the email has changed to
> have the kernel code that applies the overlay do the extra level
> of dereferencing.
> 
> The property "connector-socket" tells the kernel overlay code
> that this is a socket.  The overlay code does not actually
> do anything special as a result of this property; it is simply
> used as a sanity check that this node really is a socket.  The
> person writing the mother board .dts must provide the
> target_phandle property, which points to a node responsible for
> some of the pins on the connector.
> 
> The property "connector-plug" tells the kernel overlay code
> that each child node in the overlay corresponds to a node in the
> socket, and the socket will contain one property that is
> a phandle pointing to the node that is the target of that child
> node in the overlay node.
> 
> 
> $ cat board_with_connector_v2.dts
> 
> /dts-v1/;
> 
> / {
> 	#address-cells = < 1 >;
> 	#size-cells = < 1 >;
> 
> 	tree_1: soc@0 {
> 		reg = <0x0 0x0>;
> 
> 		spi_1: spi1 {
> 		};
> 	};
> 
> 	connector_1: connector_1 {
> 		compatible = "11-pin-accessory";
> 		connector-socket;

I don't see any advantage to allowing connectors anywhere in the tree:
pretty much by definition a connector is a "whole board" concept.  So
I think instead they should all go in a new special node under the
root, say /connectors.  With that done, you don't need the
connector-socket tag any more.

> 		spi1 {
> 			target_phandle = <&spi_1>;
> 		};
> 	};
> 
> };
> 
> &spi_1 {
> 	ethernet-switch@0 {
> 		compatible = "micrel,ks8995m";
> 	};
> };
> 
> 
> $ cat spi_codec_overlay_with_connector_v2.dts
> 
> /dts-v1/;
> 
> /plugin/;
> 
> &connector_1 {
> 	connector-plug;
> 	compatible = "11-pin-accessory";
> 
> 	spi1 {
> 		codec@1 {
> 			compatible = "ti,tlv320aic26";
> 		};
> 	};
> };
> 
> 
> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
> is unchanged from the previous example, but the kernel overlay
> code will do the correct extra level of dereferencing when it
> detects the connector-plug property in the overlay.
> 
> The one remaining piece that this patch does not provide is how
> the overlay manager (which does not yet exist in the mainline
> tree) can apply an overlay to two different targets.  That
> final step should be a trivial change to of_overlay_create(),
> adding a parameter that is a mapping of the target (or maybe
> even targets) in the overlay to different targets in the
> active device tree.
> 
> This seems like a more straight forward way to handle connectors.
> 
> First, ignoring pinctrl and pinmux, what does everyone think?
> 
> Then, the next step is whether pinctrl and pinmux work with this method.
> Pantelis, can you point me to a good example for
> 
>   1) an in-tree board dts file
>   2) an overlay file (I am assuming out of tree) that applies to the board
>   3) an in-tree .dtsi file that would provide the same features as
>      the overlay file if it was included by the board dts file
> 
> It should be easier to discuss pinctrl and pinmux with an example.

Hrm.. so I think you're trying to stick too close to the existing
overlay model.  Something I've always disliked about that model is
that the plugin can overlay *anywhere* in the master tree, meaning it
must have intimate knowledge of that tree.  Instead of using the
global __symbols__, there should be a set of "symbols" local to the
specific connector (socket), which are the *only* points which the
plugin is allowed to overlay or reference.

Given that we're going to need new code to support this new connector
model, I think we should also fix some of the uglies in the current
overlay format while we're at it.

I have to run now, but I'll try to send out a counter-proposal
shortly.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-07  7:15 ` David Gibson
@ 2016-07-08  7:26   ` Pantelis Antoniou
  2016-07-08  7:43     ` David Gibson
  2016-07-08 19:20     ` Frank Rowand
  2016-07-08 19:22   ` Frank Rowand
  1 sibling, 2 replies; 14+ messages in thread
From: Pantelis Antoniou @ 2016-07-08  7:26 UTC (permalink / raw)
  To: David Gibson
  Cc: frowand.list, robh+dt, stephen.boyd, broonie, Grant Likely,
	mark.rutland, Matt Porter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c

Hi David,

> On Jul 7, 2016, at 10:15 , David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> On Sat, Jul 02, 2016 at 04:55:49PM -0700, frowand.list@gmail.com wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
>> 
>> Hi All,
>> 
>> This is version 2 of this email.
>> 
>> Changes from version 1:
>> 
>>  - some rewording of the text
>>  - removed new (theoretical) dtc directive "/connector/"
>>  - added compatibility between mother board and daughter board
>>  - added info on applying a single .dtbo to different connectors
>>  - attached an RFC patch showing the required kernel changes
>>  - changes to mother board .dts connector node:
>>     - removed target_path property
>>     - added connector-socket property
>>  - changes to daughter board .dts connector node:
>>     - added connector-plug property
>> 
>> 
>> I've been trying to wrap my head around what Pantelis and Rob have written
>> on the subject of a device tree representation of a connector for a
>> daughter board to connect to (eg a cape or a shield) and the representation
>> of the daughter board.  (Or any other physically pluggable object.)
>> 
>> After trying to make sense of what had been written (or presented via slides
>> at a conference - thanks Pantelis!), I decided to go back to first principals
>> of what we are trying to accomplish.  I came up with some really simple bogus
>> examples to try to explain what my thought process is.
>> 
>> This is an extremely simple example to illustrate the concepts.  It is not
>> meant to represent the complexity of a real board.
>> 
>> To start with, assume that the device that will eventually be on a daughter
>> board is first soldered onto the mother board.  The mother board contains
>> two devices connected via bus spi_1.  One device is described in the .dts
>> file, the other is described in an included .dtsi file.
>> Then the device tree files will look like:
>> 
>> $ cat board.dts
>> /dts-v1/;
>> 
>> / {
>>        #address-cells = < 1 >;
>>        #size-cells = < 1 >;
>> 
>>        tree_1: soc@0 {
>>                reg = <0x0 0x0>;
>> 
>>                spi_1: spi1 {
>>                };
>>        };
>> 
>> };
>> 
>> &spi_1 {
>>        ethernet-switch@0 {
>>                compatible = "micrel,ks8995m";
>>        };
>> };
>> 
>> #include "spi_codec.dtsi"
>> 
>> 
>> $ cat spi_codec.dtsi
>> &spi_1 {
>> 	codec@1 {
>> 		compatible = "ti,tlv320aic26";
>> 	};
>> };
>> 
>> 
>> #----- codec chip on cape
>> 
>> Then suppose I move the codec chip to a cape.  Then I will have the same
>> exact .dts and .dtsi and everything still works.
>> 
>> 
>> @----- codec chip on cape, overlay
>> 
>> If I want to use overlays, I only have to add the version and "/plugin/",
>> then use the '-@' flag for dtc (both for the previous board.dts and
>> this spi_codec_overlay.dts):
>> 
>> $ cat spi_codec_overlay.dts
>> /dts-v1/;
>> 
>> /plugin/;
>> 
>> &spi_1 {
>> 	codec@1 {
>> 		compatible = "ti,tlv320aic26";
>> 	};
>> };
>> 
>> 
>> Pantelis pointed out that the syntax has changed to be:
>>   /dts-v1/ /plugin/;
>> 
>> 
>> #----- codec chip on cape, overlay, connector
>> 
>> Now we move into the realm of connectors.  My mental model of what the
>> hardware and driver look like has not changed.  The only thing that has
>> changed is that I want to be able to specify that the connector that
>> the cape is plugged into has some pins that are the spi bus /soc/spi1.
>> 
>> The following _almost_ but not quite gets me what I want.  Note that
>> the only thing the connector node does is provide some kind of
>> pointer or reference to what node(s) are physically routed through
>> the connector.  The connector node does not need to describe the pins;
>> it only has to point to the node that describes the pins.
>> 
>> This example will turn out to be not sufficient.  It is a stepping
>> stone in building my mental model.
>> 
>> $ cat board_with_connector.dts
>> /dts-v1/;
>> 
>> / {
>> 	#address-cells = < 1 >;
>> 	#size-cells = < 1 >;
>> 
>> 	tree_1: soc@0 {
>> 		reg = <0x0 0x0>;
>> 
>> 		spi_1: spi1 {
>> 		};
>> 	};
>> 
>> 	connector_1: connector_1 {
>> 		spi1 {
>> 			target_phandle = <&spi_1>;
>> 		};
>> 	};
>> 
>> };
>> 
>> &spi_1 {
>> 	ethernet-switch@0 {
>> 		compatible = "micrel,ks8995m";
>> 	};
>> };
>> 
>> 
>> $ cat spi_codec_overlay_with_connector.dts
>> /dts-v1/;
>> 
>> /plugin/;
>> 
>> &connector_1 {
>> 	spi1 {
>> 		codec@1 {
>> 			compatible = "ti,tlv320aic26";
>> 		};
>> 	};
>> };
>> 
>> 
>> The result is that the overlay fixup for spi1 on the cape will
>> relocate the spi1 node to /connector_1 in the host tree, so
>> this does not solve the connector linkage yet:
>> 
>> -- chunk from the decompiled board_with_connector.dtb:
>> 
>> 	__symbols__ {
>> 		connector_1 = "/connector_1";
>> 	};
>> 
>> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
>> 
>> 	fragment@0 {
>> 		target = <0xffffffff>;
>> 		__overlay__ {
>> 			spi1 {
>> 				codec@1 {
>> 					compatible = "ti,tlv320aic26";
>> 				};
>> 			};
>> 		};
>> 	};
>> 	__fixups__ {
>> 		connector_1 = "/fragment@0:target:0";
>> 	};
>> 
>> 
>> After applying the overlay, the codec@1 node will be at
>> /connector_1/spi1/codec@1.  What I want is for that node
>> to be at /spi1/codec@1.
>> 
>> 
>> 
>> #----- magic new syntax
>> 
>> What I really want is some way to tell dtc that I want to do one
>> level of dereferencing when resolving the path of device nodes
>> contained by the connector node in the overlay dts.
>> 
>> Version 1 of this email suggested using dtc magic to do this extra
>> level of dereferencing.  This version of the email has changed to
>> have the kernel code that applies the overlay do the extra level
>> of dereferencing.
>> 
>> The property "connector-socket" tells the kernel overlay code
>> that this is a socket.  The overlay code does not actually
>> do anything special as a result of this property; it is simply
>> used as a sanity check that this node really is a socket.  The
>> person writing the mother board .dts must provide the
>> target_phandle property, which points to a node responsible for
>> some of the pins on the connector.
>> 
>> The property "connector-plug" tells the kernel overlay code
>> that each child node in the overlay corresponds to a node in the
>> socket, and the socket will contain one property that is
>> a phandle pointing to the node that is the target of that child
>> node in the overlay node.
>> 
>> 
>> $ cat board_with_connector_v2.dts
>> 
>> /dts-v1/;
>> 
>> / {
>> 	#address-cells = < 1 >;
>> 	#size-cells = < 1 >;
>> 
>> 	tree_1: soc@0 {
>> 		reg = <0x0 0x0>;
>> 
>> 		spi_1: spi1 {
>> 		};
>> 	};
>> 
>> 	connector_1: connector_1 {
>> 		compatible = "11-pin-accessory";
>> 		connector-socket;
> 
> I don't see any advantage to allowing connectors anywhere in the tree:
> pretty much by definition a connector is a "whole board" concept.  So
> I think instead they should all go in a new special node under the
> root, say /connectors.  With that done, you don't need the
> connector-socket tag any more.
> 

I’m fine with that. But only for the portable connector case.

>> 		spi1 {
>> 			target_phandle = <&spi_1>;
>> 		};
>> 	};
>> 
>> };
>> 
>> &spi_1 {
>> 	ethernet-switch@0 {
>> 		compatible = "micrel,ks8995m";
>> 	};
>> };
>> 
>> 
>> $ cat spi_codec_overlay_with_connector_v2.dts
>> 
>> /dts-v1/;
>> 
>> /plugin/;
>> 
>> &connector_1 {
>> 	connector-plug;
>> 	compatible = "11-pin-accessory";
>> 
>> 	spi1 {
>> 		codec@1 {
>> 			compatible = "ti,tlv320aic26";
>> 		};
>> 	};
>> };
>> 
>> 
>> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
>> is unchanged from the previous example, but the kernel overlay
>> code will do the correct extra level of dereferencing when it
>> detects the connector-plug property in the overlay.
>> 
>> The one remaining piece that this patch does not provide is how
>> the overlay manager (which does not yet exist in the mainline
>> tree) can apply an overlay to two different targets.  That
>> final step should be a trivial change to of_overlay_create(),
>> adding a parameter that is a mapping of the target (or maybe
>> even targets) in the overlay to different targets in the
>> active device tree.
>> 
>> This seems like a more straight forward way to handle connectors.
>> 
>> First, ignoring pinctrl and pinmux, what does everyone think?
>> 
>> Then, the next step is whether pinctrl and pinmux work with this method.
>> Pantelis, can you point me to a good example for
>> 
>>  1) an in-tree board dts file
>>  2) an overlay file (I am assuming out of tree) that applies to the board
>>  3) an in-tree .dtsi file that would provide the same features as
>>     the overlay file if it was included by the board dts file
>> 
>> It should be easier to discuss pinctrl and pinmux with an example.
> 
> Hrm.. so I think you're trying to stick too close to the existing
> overlay model.  Something I've always disliked about that model is
> that the plugin can overlay *anywhere* in the master tree, meaning it
> must have intimate knowledge of that tree.  Instead of using the
> global __symbols__, there should be a set of "symbols" local to the
> specific connector (socket), which are the *only* points which the
> plugin is allowed to overlay or reference.
> 

This is about the portable connector case.

We can figure out some way for local symbols for the portable
case to work, but the global symbols for board specific overlays must
be able to work.

There are different use cases that call for both non-portable and portable
overlays.

> Given that we're going to need new code to support this new connector
> model, I think we should also fix some of the uglies in the current
> overlay format while we're at it.
> 

We need to keep compatibility with the old format. There are 5 million
RPIs sold, half a million beaglebones and C.H.I.P. is coming out too.
They all use overlays in one form or another.

That’s not counting all the custom boards that actively use them.

We have a user base now.

> I have to run now, but I'll try to send out a counter-proposal
> shortly.
> 
> -- 
> David Gibson			| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> 				| _way_ _around_!
> http://www.ozlabs.org/~dgibson

Regards

— Pantelis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-08  7:26   ` Pantelis Antoniou
@ 2016-07-08  7:43     ` David Gibson
  2016-07-08 19:20     ` Frank Rowand
  1 sibling, 0 replies; 14+ messages in thread
From: David Gibson @ 2016-07-08  7:43 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: frowand.list, robh+dt, stephen.boyd, broonie, Grant Likely,
	mark.rutland, Matt Porter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c

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

On Fri, Jul 08, 2016 at 10:26:15AM +0300, Pantelis Antoniou wrote:
> Hi David,
> 
> > On Jul 7, 2016, at 10:15 , David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > On Sat, Jul 02, 2016 at 04:55:49PM -0700, frowand.list@gmail.com wrote:
> >> From: Frank Rowand <frank.rowand@am.sony.com>
> >> 
> >> Hi All,
> >> 
> >> This is version 2 of this email.
> >> 
> >> Changes from version 1:
> >> 
> >>  - some rewording of the text
> >>  - removed new (theoretical) dtc directive "/connector/"
> >>  - added compatibility between mother board and daughter board
> >>  - added info on applying a single .dtbo to different connectors
> >>  - attached an RFC patch showing the required kernel changes
> >>  - changes to mother board .dts connector node:
> >>     - removed target_path property
> >>     - added connector-socket property
> >>  - changes to daughter board .dts connector node:
> >>     - added connector-plug property
> >> 
> >> 
> >> I've been trying to wrap my head around what Pantelis and Rob have written
> >> on the subject of a device tree representation of a connector for a
> >> daughter board to connect to (eg a cape or a shield) and the representation
> >> of the daughter board.  (Or any other physically pluggable object.)
> >> 
> >> After trying to make sense of what had been written (or presented via slides
> >> at a conference - thanks Pantelis!), I decided to go back to first principals
> >> of what we are trying to accomplish.  I came up with some really simple bogus
> >> examples to try to explain what my thought process is.
> >> 
> >> This is an extremely simple example to illustrate the concepts.  It is not
> >> meant to represent the complexity of a real board.
> >> 
> >> To start with, assume that the device that will eventually be on a daughter
> >> board is first soldered onto the mother board.  The mother board contains
> >> two devices connected via bus spi_1.  One device is described in the .dts
> >> file, the other is described in an included .dtsi file.
> >> Then the device tree files will look like:
> >> 
> >> $ cat board.dts
> >> /dts-v1/;
> >> 
> >> / {
> >>        #address-cells = < 1 >;
> >>        #size-cells = < 1 >;
> >> 
> >>        tree_1: soc@0 {
> >>                reg = <0x0 0x0>;
> >> 
> >>                spi_1: spi1 {
> >>                };
> >>        };
> >> 
> >> };
> >> 
> >> &spi_1 {
> >>        ethernet-switch@0 {
> >>                compatible = "micrel,ks8995m";
> >>        };
> >> };
> >> 
> >> #include "spi_codec.dtsi"
> >> 
> >> 
> >> $ cat spi_codec.dtsi
> >> &spi_1 {
> >> 	codec@1 {
> >> 		compatible = "ti,tlv320aic26";
> >> 	};
> >> };
> >> 
> >> 
> >> #----- codec chip on cape
> >> 
> >> Then suppose I move the codec chip to a cape.  Then I will have the same
> >> exact .dts and .dtsi and everything still works.
> >> 
> >> 
> >> @----- codec chip on cape, overlay
> >> 
> >> If I want to use overlays, I only have to add the version and "/plugin/",
> >> then use the '-@' flag for dtc (both for the previous board.dts and
> >> this spi_codec_overlay.dts):
> >> 
> >> $ cat spi_codec_overlay.dts
> >> /dts-v1/;
> >> 
> >> /plugin/;
> >> 
> >> &spi_1 {
> >> 	codec@1 {
> >> 		compatible = "ti,tlv320aic26";
> >> 	};
> >> };
> >> 
> >> 
> >> Pantelis pointed out that the syntax has changed to be:
> >>   /dts-v1/ /plugin/;
> >> 
> >> 
> >> #----- codec chip on cape, overlay, connector
> >> 
> >> Now we move into the realm of connectors.  My mental model of what the
> >> hardware and driver look like has not changed.  The only thing that has
> >> changed is that I want to be able to specify that the connector that
> >> the cape is plugged into has some pins that are the spi bus /soc/spi1.
> >> 
> >> The following _almost_ but not quite gets me what I want.  Note that
> >> the only thing the connector node does is provide some kind of
> >> pointer or reference to what node(s) are physically routed through
> >> the connector.  The connector node does not need to describe the pins;
> >> it only has to point to the node that describes the pins.
> >> 
> >> This example will turn out to be not sufficient.  It is a stepping
> >> stone in building my mental model.
> >> 
> >> $ cat board_with_connector.dts
> >> /dts-v1/;
> >> 
> >> / {
> >> 	#address-cells = < 1 >;
> >> 	#size-cells = < 1 >;
> >> 
> >> 	tree_1: soc@0 {
> >> 		reg = <0x0 0x0>;
> >> 
> >> 		spi_1: spi1 {
> >> 		};
> >> 	};
> >> 
> >> 	connector_1: connector_1 {
> >> 		spi1 {
> >> 			target_phandle = <&spi_1>;
> >> 		};
> >> 	};
> >> 
> >> };
> >> 
> >> &spi_1 {
> >> 	ethernet-switch@0 {
> >> 		compatible = "micrel,ks8995m";
> >> 	};
> >> };
> >> 
> >> 
> >> $ cat spi_codec_overlay_with_connector.dts
> >> /dts-v1/;
> >> 
> >> /plugin/;
> >> 
> >> &connector_1 {
> >> 	spi1 {
> >> 		codec@1 {
> >> 			compatible = "ti,tlv320aic26";
> >> 		};
> >> 	};
> >> };
> >> 
> >> 
> >> The result is that the overlay fixup for spi1 on the cape will
> >> relocate the spi1 node to /connector_1 in the host tree, so
> >> this does not solve the connector linkage yet:
> >> 
> >> -- chunk from the decompiled board_with_connector.dtb:
> >> 
> >> 	__symbols__ {
> >> 		connector_1 = "/connector_1";
> >> 	};
> >> 
> >> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
> >> 
> >> 	fragment@0 {
> >> 		target = <0xffffffff>;
> >> 		__overlay__ {
> >> 			spi1 {
> >> 				codec@1 {
> >> 					compatible = "ti,tlv320aic26";
> >> 				};
> >> 			};
> >> 		};
> >> 	};
> >> 	__fixups__ {
> >> 		connector_1 = "/fragment@0:target:0";
> >> 	};
> >> 
> >> 
> >> After applying the overlay, the codec@1 node will be at
> >> /connector_1/spi1/codec@1.  What I want is for that node
> >> to be at /spi1/codec@1.
> >> 
> >> 
> >> 
> >> #----- magic new syntax
> >> 
> >> What I really want is some way to tell dtc that I want to do one
> >> level of dereferencing when resolving the path of device nodes
> >> contained by the connector node in the overlay dts.
> >> 
> >> Version 1 of this email suggested using dtc magic to do this extra
> >> level of dereferencing.  This version of the email has changed to
> >> have the kernel code that applies the overlay do the extra level
> >> of dereferencing.
> >> 
> >> The property "connector-socket" tells the kernel overlay code
> >> that this is a socket.  The overlay code does not actually
> >> do anything special as a result of this property; it is simply
> >> used as a sanity check that this node really is a socket.  The
> >> person writing the mother board .dts must provide the
> >> target_phandle property, which points to a node responsible for
> >> some of the pins on the connector.
> >> 
> >> The property "connector-plug" tells the kernel overlay code
> >> that each child node in the overlay corresponds to a node in the
> >> socket, and the socket will contain one property that is
> >> a phandle pointing to the node that is the target of that child
> >> node in the overlay node.
> >> 
> >> 
> >> $ cat board_with_connector_v2.dts
> >> 
> >> /dts-v1/;
> >> 
> >> / {
> >> 	#address-cells = < 1 >;
> >> 	#size-cells = < 1 >;
> >> 
> >> 	tree_1: soc@0 {
> >> 		reg = <0x0 0x0>;
> >> 
> >> 		spi_1: spi1 {
> >> 		};
> >> 	};
> >> 
> >> 	connector_1: connector_1 {
> >> 		compatible = "11-pin-accessory";
> >> 		connector-socket;
> > 
> > I don't see any advantage to allowing connectors anywhere in the tree:
> > pretty much by definition a connector is a "whole board" concept.  So
> > I think instead they should all go in a new special node under the
> > root, say /connectors.  With that done, you don't need the
> > connector-socket tag any more.
> > 
> 
> I’m fine with that. But only for the portable connector case.

Ok.

> >> 		spi1 {
> >> 			target_phandle = <&spi_1>;
> >> 		};
> >> 	};
> >> 
> >> };
> >> 
> >> &spi_1 {
> >> 	ethernet-switch@0 {
> >> 		compatible = "micrel,ks8995m";
> >> 	};
> >> };
> >> 
> >> 
> >> $ cat spi_codec_overlay_with_connector_v2.dts
> >> 
> >> /dts-v1/;
> >> 
> >> /plugin/;
> >> 
> >> &connector_1 {
> >> 	connector-plug;
> >> 	compatible = "11-pin-accessory";
> >> 
> >> 	spi1 {
> >> 		codec@1 {
> >> 			compatible = "ti,tlv320aic26";
> >> 		};
> >> 	};
> >> };
> >> 
> >> 
> >> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
> >> is unchanged from the previous example, but the kernel overlay
> >> code will do the correct extra level of dereferencing when it
> >> detects the connector-plug property in the overlay.
> >> 
> >> The one remaining piece that this patch does not provide is how
> >> the overlay manager (which does not yet exist in the mainline
> >> tree) can apply an overlay to two different targets.  That
> >> final step should be a trivial change to of_overlay_create(),
> >> adding a parameter that is a mapping of the target (or maybe
> >> even targets) in the overlay to different targets in the
> >> active device tree.
> >> 
> >> This seems like a more straight forward way to handle connectors.
> >> 
> >> First, ignoring pinctrl and pinmux, what does everyone think?
> >> 
> >> Then, the next step is whether pinctrl and pinmux work with this method.
> >> Pantelis, can you point me to a good example for
> >> 
> >>  1) an in-tree board dts file
> >>  2) an overlay file (I am assuming out of tree) that applies to the board
> >>  3) an in-tree .dtsi file that would provide the same features as
> >>     the overlay file if it was included by the board dts file
> >> 
> >> It should be easier to discuss pinctrl and pinmux with an example.
> > 
> > Hrm.. so I think you're trying to stick too close to the existing
> > overlay model.  Something I've always disliked about that model is
> > that the plugin can overlay *anywhere* in the master tree, meaning it
> > must have intimate knowledge of that tree.  Instead of using the
> > global __symbols__, there should be a set of "symbols" local to the
> > specific connector (socket), which are the *only* points which the
> > plugin is allowed to overlay or reference.
> > 
> 
> This is about the portable connector case.

Right, if I understand you correctly, that's the only case I'm
discussing here.

> We can figure out some way for local symbols for the portable
> case to work, but the global symbols for board specific overlays must
> be able to work.
> 
> There are different use cases that call for both non-portable and portable
> overlays.

Right, I assumed that.  I'm suggesting this connector format in
addition to the existing overlay format (although I think the
connector format should be preferred when possible).

> > Given that we're going to need new code to support this new connector
> > model, I think we should also fix some of the uglies in the current
> > overlay format while we're at it.
> > 
> 
> We need to keep compatibility with the old format. There are 5 million
> RPIs sold, half a million beaglebones and C.H.I.P. is coming out too.
> They all use overlays in one form or another.
> 
> That’s not counting all the custom boards that actively use them.
> 
> We have a user base now.

Sorry, I wasn't clear.  I'm not suggesting we abolish the existing
overlay format, or alter it.  Despite its flaws it has a userbase, as
you say.

But I'm saying if we're creating a new format for portable connectors,
we shouldn't inherit those flaws when we don't have to.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-08  7:26   ` Pantelis Antoniou
  2016-07-08  7:43     ` David Gibson
@ 2016-07-08 19:20     ` Frank Rowand
  2016-07-08 19:25       ` Frank Rowand
  1 sibling, 1 reply; 14+ messages in thread
From: Frank Rowand @ 2016-07-08 19:20 UTC (permalink / raw)
  To: Pantelis Antoniou, David Gibson
  Cc: robh+dt, stephen.boyd, broonie, Grant Likely, mark.rutland,
	Matt Porter, koen, linux, marex, wsa, devicetree, linux-kernel,
	linux-i2c, Frank Rowand

On 07/08/16 00:26, Pantelis Antoniou wrote:
> Hi David,
> 
>> On Jul 7, 2016, at 10:15 , David Gibson <david@gibson.dropbear.id.au> wrote:
>>

< snip >

>> Given that we're going to need new code to support this new connector
>> model, I think we should also fix some of the uglies in the current
>> overlay format while we're at it.
>>
> 
> We need to keep compatibility with the old format. There are 5 million
> RPIs sold, half a million beaglebones and C.H.I.P. is coming out too.
> They all use overlays in one form or another.
> 
> That’s not counting all the custom boards that actively use them.
> 
> We have a user base now.

Please not that I AM NOT suggesting the we remove compatibility with
the old format!!!

But I need to push back on the idea that we have a user base that we
need to keep compatibility with.  If I understand correctly, that
user base is based on using much code that is not in mainline, including
an altered dtc and a cape manager.

People using out of tree code can not use the fact that code exists and
is being widely used to force us to mainline that out of tree code.
That is the risk of using out of tree code.

I do not want to start a big discussion about this now since there is
no plan to remove the compatibility at this point.

-Frank

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-07  7:15 ` David Gibson
  2016-07-08  7:26   ` Pantelis Antoniou
@ 2016-07-08 19:22   ` Frank Rowand
  1 sibling, 0 replies; 14+ messages in thread
From: Frank Rowand @ 2016-07-08 19:22 UTC (permalink / raw)
  To: David Gibson
  Cc: robh+dt, pantelis.antoniou, stephen.boyd, broonie, grant.likely,
	mark.rutland, mporter, koen, linux, marex, wsa, devicetree,
	linux-kernel, linux-i2c, panto

On 07/07/16 00:15, David Gibson wrote:
> On Sat, Jul 02, 2016 at 04:55:49PM -0700, frowand.list@gmail.com wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> Hi All,
>>
>> This is version 2 of this email.
>>
>> Changes from version 1:
>>
>>   - some rewording of the text
>>   - removed new (theoretical) dtc directive "/connector/"
>>   - added compatibility between mother board and daughter board
>>   - added info on applying a single .dtbo to different connectors
>>   - attached an RFC patch showing the required kernel changes
>>   - changes to mother board .dts connector node:
>>      - removed target_path property
>>      - added connector-socket property
>>   - changes to daughter board .dts connector node:
>>      - added connector-plug property
>>
>>
>> I've been trying to wrap my head around what Pantelis and Rob have written
>> on the subject of a device tree representation of a connector for a
>> daughter board to connect to (eg a cape or a shield) and the representation
>> of the daughter board.  (Or any other physically pluggable object.)
>>
>> After trying to make sense of what had been written (or presented via slides
>> at a conference - thanks Pantelis!), I decided to go back to first principals
>> of what we are trying to accomplish.  I came up with some really simple bogus
>> examples to try to explain what my thought process is.
>>
>> This is an extremely simple example to illustrate the concepts.  It is not
>> meant to represent the complexity of a real board.
>>
>> To start with, assume that the device that will eventually be on a daughter
>> board is first soldered onto the mother board.  The mother board contains
>> two devices connected via bus spi_1.  One device is described in the .dts
>> file, the other is described in an included .dtsi file.
>> Then the device tree files will look like:
>>
>> $ cat board.dts
>> /dts-v1/;
>>
>> / {
>>         #address-cells = < 1 >;
>>         #size-cells = < 1 >;
>>
>>         tree_1: soc@0 {
>>                 reg = <0x0 0x0>;
>>
>>                 spi_1: spi1 {
>>                 };
>>         };
>>
>> };
>>
>> &spi_1 {
>>         ethernet-switch@0 {
>>                 compatible = "micrel,ks8995m";
>>         };
>> };
>>
>> #include "spi_codec.dtsi"
>>
>>
>> $ cat spi_codec.dtsi
>> &spi_1 {
>> 	codec@1 {
>> 		compatible = "ti,tlv320aic26";
>> 	};
>> };
>>
>>
>> #----- codec chip on cape
>>
>> Then suppose I move the codec chip to a cape.  Then I will have the same
>> exact .dts and .dtsi and everything still works.
>>
>>
>> @----- codec chip on cape, overlay
>>
>> If I want to use overlays, I only have to add the version and "/plugin/",
>> then use the '-@' flag for dtc (both for the previous board.dts and
>> this spi_codec_overlay.dts):
>>
>> $ cat spi_codec_overlay.dts
>> /dts-v1/;
>>
>> /plugin/;
>>
>> &spi_1 {
>> 	codec@1 {
>> 		compatible = "ti,tlv320aic26";
>> 	};
>> };
>>
>>
>> Pantelis pointed out that the syntax has changed to be:
>>    /dts-v1/ /plugin/;
>>
>>
>> #----- codec chip on cape, overlay, connector
>>
>> Now we move into the realm of connectors.  My mental model of what the
>> hardware and driver look like has not changed.  The only thing that has
>> changed is that I want to be able to specify that the connector that
>> the cape is plugged into has some pins that are the spi bus /soc/spi1.
>>
>> The following _almost_ but not quite gets me what I want.  Note that
>> the only thing the connector node does is provide some kind of
>> pointer or reference to what node(s) are physically routed through
>> the connector.  The connector node does not need to describe the pins;
>> it only has to point to the node that describes the pins.
>>
>> This example will turn out to be not sufficient.  It is a stepping
>> stone in building my mental model.
>>
>> $ cat board_with_connector.dts
>> /dts-v1/;
>>
>> / {
>> 	#address-cells = < 1 >;
>> 	#size-cells = < 1 >;
>>
>> 	tree_1: soc@0 {
>> 		reg = <0x0 0x0>;
>>
>> 		spi_1: spi1 {
>> 		};
>> 	};
>>
>> 	connector_1: connector_1 {
>> 		spi1 {
>> 			target_phandle = <&spi_1>;
>> 		};
>> 	};
>>
>> };
>>
>> &spi_1 {
>> 	ethernet-switch@0 {
>> 		compatible = "micrel,ks8995m";
>> 	};
>> };
>>
>>
>> $ cat spi_codec_overlay_with_connector.dts
>> /dts-v1/;
>>
>> /plugin/;
>>
>> &connector_1 {
>> 	spi1 {
>> 		codec@1 {
>> 			compatible = "ti,tlv320aic26";
>> 		};
>> 	};
>> };
>>
>>
>> The result is that the overlay fixup for spi1 on the cape will
>> relocate the spi1 node to /connector_1 in the host tree, so
>> this does not solve the connector linkage yet:
>>
>> -- chunk from the decompiled board_with_connector.dtb:
>>
>> 	__symbols__ {
>> 		connector_1 = "/connector_1";
>> 	};
>>
>> -- chunk from the decompiled spi_codec_overlay_with_connector.dtb:
>>
>> 	fragment@0 {
>> 		target = <0xffffffff>;
>> 		__overlay__ {
>> 			spi1 {
>> 				codec@1 {
>> 					compatible = "ti,tlv320aic26";
>> 				};
>> 			};
>> 		};
>> 	};
>> 	__fixups__ {
>> 		connector_1 = "/fragment@0:target:0";
>> 	};
>>
>>
>> After applying the overlay, the codec@1 node will be at
>> /connector_1/spi1/codec@1.  What I want is for that node
>> to be at /spi1/codec@1.
>>
>>
>>
>> #----- magic new syntax
>>
>> What I really want is some way to tell dtc that I want to do one
>> level of dereferencing when resolving the path of device nodes
>> contained by the connector node in the overlay dts.
>>
>> Version 1 of this email suggested using dtc magic to do this extra
>> level of dereferencing.  This version of the email has changed to
>> have the kernel code that applies the overlay do the extra level
>> of dereferencing.
>>
>> The property "connector-socket" tells the kernel overlay code
>> that this is a socket.  The overlay code does not actually
>> do anything special as a result of this property; it is simply
>> used as a sanity check that this node really is a socket.  The
>> person writing the mother board .dts must provide the
>> target_phandle property, which points to a node responsible for
>> some of the pins on the connector.
>>
>> The property "connector-plug" tells the kernel overlay code
>> that each child node in the overlay corresponds to a node in the
>> socket, and the socket will contain one property that is
>> a phandle pointing to the node that is the target of that child
>> node in the overlay node.
>>
>>
>> $ cat board_with_connector_v2.dts
>>
>> /dts-v1/;
>>
>> / {
>> 	#address-cells = < 1 >;
>> 	#size-cells = < 1 >;
>>
>> 	tree_1: soc@0 {
>> 		reg = <0x0 0x0>;
>>
>> 		spi_1: spi1 {
>> 		};
>> 	};
>>
>> 	connector_1: connector_1 {
>> 		compatible = "11-pin-accessory";
>> 		connector-socket;
> 
> I don't see any advantage to allowing connectors anywhere in the tree:
> pretty much by definition a connector is a "whole board" concept.  So
> I think instead they should all go in a new special node under the
> root, say /connectors.  With that done, you don't need the
> connector-socket tag any more.

That seems like a good idea.


> 
>> 		spi1 {
>> 			target_phandle = <&spi_1>;
>> 		};
>> 	};
>>
>> };
>>
>> &spi_1 {
>> 	ethernet-switch@0 {
>> 		compatible = "micrel,ks8995m";
>> 	};
>> };
>>
>>
>> $ cat spi_codec_overlay_with_connector_v2.dts
>>
>> /dts-v1/;
>>
>> /plugin/;
>>
>> &connector_1 {
>> 	connector-plug;
>> 	compatible = "11-pin-accessory";
>>
>> 	spi1 {
>> 		codec@1 {
>> 			compatible = "ti,tlv320aic26";
>> 		};
>> 	};
>> };
>>
>>
>> The spi_codec_overlay_with_connector_v2.dtb __fixups__ information
>> is unchanged from the previous example, but the kernel overlay
>> code will do the correct extra level of dereferencing when it
>> detects the connector-plug property in the overlay.
>>
>> The one remaining piece that this patch does not provide is how
>> the overlay manager (which does not yet exist in the mainline
>> tree) can apply an overlay to two different targets.  That
>> final step should be a trivial change to of_overlay_create(),
>> adding a parameter that is a mapping of the target (or maybe
>> even targets) in the overlay to different targets in the
>> active device tree.
>>
>> This seems like a more straight forward way to handle connectors.
>>
>> First, ignoring pinctrl and pinmux, what does everyone think?
>>
>> Then, the next step is whether pinctrl and pinmux work with this method.
>> Pantelis, can you point me to a good example for
>>
>>   1) an in-tree board dts file
>>   2) an overlay file (I am assuming out of tree) that applies to the board
>>   3) an in-tree .dtsi file that would provide the same features as
>>      the overlay file if it was included by the board dts file
>>
>> It should be easier to discuss pinctrl and pinmux with an example.


> 
> Hrm.. so I think you're trying to stick too close to the existing
> overlay model.  Something I've always disliked about that model is
> that the plugin can overlay *anywhere* in the master tree, meaning it
> must have intimate knowledge of that tree.  Instead of using the
> global __symbols__, there should be a set of "symbols" local to the
> specific connector (socket), which are the *only* points which the
> plugin is allowed to overlay or reference.

That sounds like a good idea.


> Given that we're going to need new code to support this new connector
> model, I think we should also fix some of the uglies in the current
> overlay format while we're at it.

I like that way of thinking.


> I have to run now, but I'll try to send out a counter-proposal
> shortly.

-Frank

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual
  2016-07-08 19:20     ` Frank Rowand
@ 2016-07-08 19:25       ` Frank Rowand
  0 siblings, 0 replies; 14+ messages in thread
From: Frank Rowand @ 2016-07-08 19:25 UTC (permalink / raw)
  To: Pantelis Antoniou, David Gibson
  Cc: robh+dt, stephen.boyd, broonie, Grant Likely, mark.rutland,
	Matt Porter, koen, linux, marex, wsa, devicetree, linux-kernel,
	linux-i2c, Frank Rowand

On 07/08/16 12:20, Frank Rowand wrote:
> On 07/08/16 00:26, Pantelis Antoniou wrote:
>> Hi David,
>>
>>> On Jul 7, 2016, at 10:15 , David Gibson <david@gibson.dropbear.id.au> wrote:
>>>
> 
> < snip >
> 
>>> Given that we're going to need new code to support this new connector
>>> model, I think we should also fix some of the uglies in the current
>>> overlay format while we're at it.
>>>
>>
>> We need to keep compatibility with the old format. There are 5 million
>> RPIs sold, half a million beaglebones and C.H.I.P. is coming out too.
>> They all use overlays in one form or another.
>>
>> That’s not counting all the custom boards that actively use them.
>>
>> We have a user base now.
> 
> Please not that I AM NOT suggesting the we remove compatibility with
         ^^^ note

> the old format!!!
> 
> But I need to push back on the idea that we have a user base that we
> need to keep compatibility with.  If I understand correctly, that
> user base is based on using much code that is not in mainline, including
> an altered dtc and a cape manager.
> 
> People using out of tree code can not use the fact that code exists and
> is being widely used to force us to mainline that out of tree code.
> That is the risk of using out of tree code.
> 
> I do not want to start a big discussion about this now since there is
> no plan to remove the compatibility at this point.
> 
> -Frank
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-07-08 19:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-02 23:55 [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual frowand.list
2016-07-02 23:55 ` [RFC PATCH 1/1] device tree connectors, using plugs and sockets frowand.list
2016-07-03  6:06 ` [RFC PATCH 0/1] Portable Device Tree Connector -- conceptual Frank Rowand
2016-07-04 15:22 ` Mark Brown
2016-07-04 18:15   ` Frank Rowand
2016-07-05 18:01 ` Pantelis Antoniou
2016-07-05 18:01   ` Pantelis Antoniou
2016-07-06 17:40   ` Frank Rowand
2016-07-07  7:15 ` David Gibson
2016-07-08  7:26   ` Pantelis Antoniou
2016-07-08  7:43     ` David Gibson
2016-07-08 19:20     ` Frank Rowand
2016-07-08 19:25       ` Frank Rowand
2016-07-08 19:22   ` Frank Rowand

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.