[PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Jeremy Kerr]

This change introduces a proposed layout for describing FSI busses in
the device tree. While the bus is probe-able, we'd still like a method
of describing subordinate (eg i2c) busses that are behind FSI devices.

The FSI core will be responsible for matching probed slaves & engines to
their device tree nodes, so the FSI device drivers' probe() functions
will be passed a struct device with the appropriate of_node populated
where a matching DT node is found.

RFC at this stage, so I'd welcome any feedback.

Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt

diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
new file mode 100644
index 0000000..b5e85c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/fsi.txt
@@ -0,0 +1,144 @@
+FSI bus & engine generic device tree bindings
+=============================================
+
+The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
+engines within those slaves. However, we have a facility to match devicetree
+nodes to probed engines. This allows for fsi engines to expose non-probeable
+busses, which are then exposed by the device tree. For example, a FSI engine
+that is an I2C master - the I2C bus can be described by the device tree under
+the engine's device tree node.
+
+FSI masters may require their own DT nodes (to describe the master HW itself);
+that requirement is defined by the master's implementation, and is described by
+the fsi-master-* binding specifications.
+
+Under the masters' nodes, we can describe the bus topology using nodes to
+represent the FSI slaves and their slave engines. As a basic outline:
+
+  fsi-master {
+      /* top-level of FSI bus topology, bound to a FSI master driver and
+       * exposes a FSI bus */
+
+      fsi-slave@<link,id> {
+          /* this node defines the FSI slave device, and is handled
+           * entirely with FSI core code */
+
+          fsi-slave-engine@<addr> {
+              /* this node defines the engine endpoint & address range, which
+               * is bound to the relevant fsi device driver */
+               ...
+          };
+
+          fsi-slave-engine@<addr> {
+              ...
+          };
+
+      };
+  };
+
+Note that since the bus is probe-able, some (or all) of the topology may
+not be described; this binding only provides an optional facility for
+adding subordinate device tree nodes as children of FSI engines.
+
+FSI masters
+-----------
+
+FSI master nodes declare themselves as such with the "fsi-master" compatible
+value. It's likely that an implementation-specific compatible value will
+be needed as well, for example:
+
+    compatible = "fsi-master-gpio", "fsi-master";
+
+Since the master nodes describe the top-level of the FSI topology, they also
+need to declare the FSI-standard addressing scheme. This requires two cells for
+addresses (link index and slave ID), and no size:
+
+    #address-cells = <2>;
+    #size-cells = <0>;
+
+FSI slaves
+----------
+
+Slaves are identified by a (link-index, slave-id) pair, so require two cells
+for an address identifier. Since these are not a range, no size cells are
+required. For an example, a slave on link 1, with ID 2, could be represented
+as:
+
+    cfam@1,2 {
+        reg = <1 2>;
+	[...];
+    }
+
+Each slave provides an address-space, under which the engines are accessible.
+That address space has a maximum of 23 bits, so we use one cell to represent
+addresses and sizes in the slave address space:
+
+    #address-cells = <1>;
+    #size-cells = <1>;
+
+
+FSI engines (devices)
+---------------------
+
+Engines are identified by their address under the slaves' address spaces. We
+use a single cell for address and size. Engine nodes represent the endpoint
+FSI device, and are passed to those FSI device drivers' ->probe() functions.
+
+For example, for a slave using a single 0x400-byte page starting at address
+0xc00:
+
+    engine@c00 {
+        reg = <0xc00 0x400>;
+    };
+
+
+Full example
+------------
+
+Here's an example that illustrates:
+ - a FSI master
+   - connected to a FSI slave
+     - that contains an engine that is an I2C master
+       - connected to an I2C EEPROM
+
+The FSI master may be connected to additional slaves, and slaves may have
+additional engines, but they don't necessarily need to be describe in the
+device tree if no extra platform information is required.
+
+    /* The GPIO-based FSI master node, describing the top level of the
+     * FSI bus
+     */
+    gpio-fsi {
+        compatible = "fsi-master-gpio", "fsi-master";
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        /* A FSI slave (aka. CFAM) at link 0, ID 0. */
+        cfam@0,0 {
+            reg = <0 0>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+
+            /* FSI engine at 0xc00, using a single page. In this example,
+             * it's an I2C master controller, so subnodes describe the
+             * I2C bus.
+             */
+            i2c-controller@c00 {
+                reg = <0xc00 0x400>;
+
+                /* Engine-specific data. In this case, we're describing an
+                 * I2C bus, so we're conforming to the generic I2C binding
+                 */
+                compatible = "ibm,fsi-i2c";
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                /* I2C endpoint device: an Atmel EEPROM */
+                eeprom@50 {
+                    compatible = "atmel,24c256";
+                    reg = <0x50>;
+                    pagesize = <64>;
+                };
+            };
+        };
+    };
-- 
2.7.4

--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Joel Stanley]

On Tue, May 2, 2017 at 3:25 PM, Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org> wrote:
> This change introduces a proposed layout for describing FSI busses in
> the device tree. While the bus is probe-able, we'd still like a method
> of describing subordinate (eg i2c) busses that are behind FSI devices.
>
> The FSI core will be responsible for matching probed slaves & engines to
> their device tree nodes, so the FSI device drivers' probe() functions
> will be passed a struct device with the appropriate of_node populated
> where a matching DT node is found.
>
> RFC at this stage, so I'd welcome any feedback.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

Looks good to me Jeremy. One small question on the description, but
please add Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> to future
revisions.

> ---
>  Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> new file mode 100644
> index 0000000..b5e85c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> @@ -0,0 +1,144 @@
> +FSI bus & engine generic device tree bindings
> +=============================================
> +
> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
> +engines within those slaves. However, we have a facility to match devicetree
> +nodes to probed engines. This allows for fsi engines to expose non-probeable
> +busses, which are then exposed by the device tree. For example, a FSI engine

A nit: Can we can expose any device as part of the engine, not just a bus?

> +that is an I2C master - the I2C bus can be described by the device tree under
> +the engine's device tree node.

Cheers,

Joel
--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Jeremy Kerr]

Hi Joel,

> Looks good to me Jeremy. One small question on the description, but
> please add Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> to future
> revisions.

Thanks for the review!

>> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
>> +engines within those slaves. However, we have a facility to match devicetree
>> +nodes to probed engines. This allows for fsi engines to expose non-probeable
>> +busses, which are then exposed by the device tree. For example, a FSI engine
> 
> A nit: Can we can expose any device as part of the engine, not just a bus?

Absolutely. I'll clarify in a future version. Something like:

  This allows for fsi engines to expose non-probeable busses, or for
  drivers to access extra device properties, by data described in the
  device tree.

Cheers,


Jeremy
--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Brad Bishop]

> On May 2, 2017, at 1:55 AM, Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org> wrote:
> 
> This change introduces a proposed layout for describing FSI busses in
> the device tree. While the bus is probe-able, we'd still like a method
> of describing subordinate (eg i2c) busses that are behind FSI devices.
> 
> The FSI core will be responsible for matching probed slaves & engines to
> their device tree nodes, so the FSI device drivers' probe() functions
> will be passed a struct device with the appropriate of_node populated
> where a matching DT node is found.
> 
> RFC at this stage, so I'd welcome any feedback.
> 
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

Acked-by: Brad Bishop <bradleyb-r5pk2Da7Bxt8sGd51Jp2sdBPR1lH4CV8@public.gmane.org>

with some very inconsequential nits...

> ---
> Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
> 1 file changed, 144 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt
> 
> diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> new file mode 100644
> index 0000000..b5e85c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> @@ -0,0 +1,144 @@
> +FSI bus & engine generic device tree bindings
> +=============================================
> +
> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
> +engines within those slaves. However, we have a facility to match devicetree
> +nodes to probed engines. This allows for fsi engines to expose non-probeable
> +busses, which are then exposed by the device tree. For example, a FSI engine
> +that is an I2C master - the I2C bus can be described by the device tree under
> +the engine's device tree node.
> +
> +FSI masters may require their own DT nodes (to describe the master HW itself);
> +that requirement is defined by the master's implementation, and is described by
> +the fsi-master-* binding specifications.
> +
> +Under the masters' nodes, we can describe the bus topology using nodes to
> +represent the FSI slaves and their slave engines. As a basic outline:
> +
> +  fsi-master {

an FSI master

> +      /* top-level of FSI bus topology, bound to a FSI master driver and
> +       * exposes a FSI bus */

an FSI bus

> +
> +      fsi-slave@<link,id> {
> +          /* this node defines the FSI slave device, and is handled
> +           * entirely with FSI core code */
> +
> +          fsi-slave-engine@<addr> {
> +              /* this node defines the engine endpoint & address range, which
> +               * is bound to the relevant fsi device driver */
> +               ...
> +          };
> +
> +          fsi-slave-engine@<addr> {
> +              ...
> +          };
> +
> +      };
> +  };
> +
> +Note that since the bus is probe-able, some (or all) of the topology may
> +not be described; this binding only provides an optional facility for
> +adding subordinate device tree nodes as children of FSI engines.
> +
> +FSI masters
> +-----------
> +
> +FSI master nodes declare themselves as such with the "fsi-master" compatible
> +value. It's likely that an implementation-specific compatible value will
> +be needed as well, for example:
> +
> +    compatible = "fsi-master-gpio", "fsi-master";
> +
> +Since the master nodes describe the top-level of the FSI topology, they also
> +need to declare the FSI-standard addressing scheme. This requires two cells for
> +addresses (link index and slave ID), and no size:
> +
> +    #address-cells = <2>;
> +    #size-cells = <0>;
> +
> +FSI slaves
> +----------
> +
> +Slaves are identified by a (link-index, slave-id) pair, so require two cells
> +for an address identifier. Since these are not a range, no size cells are
> +required. For an example, a slave on link 1, with ID 2, could be represented
> +as:
> +
> +    cfam@1,2 {
> +        reg = <1 2>;
> +	[...];
> +    }
> +
> +Each slave provides an address-space, under which the engines are accessible.
> +That address space has a maximum of 23 bits, so we use one cell to represent
> +addresses and sizes in the slave address space:
> +
> +    #address-cells = <1>;
> +    #size-cells = <1>;
> +
> +
> +FSI engines (devices)
> +---------------------
> +
> +Engines are identified by their address under the slaves' address spaces. We
> +use a single cell for address and size. Engine nodes represent the endpoint
> +FSI device, and are passed to those FSI device drivers' ->probe() functions.
> +
> +For example, for a slave using a single 0x400-byte page starting at address
> +0xc00:
> +
> +    engine@c00 {
> +        reg = <0xc00 0x400>;
> +    };
> +
> +
> +Full example
> +------------
> +
> +Here's an example that illustrates:
> + - a FSI master

an FSI master

> +   - connected to a FSI slave

an FSI slave

> +     - that contains an engine that is an I2C master
> +       - connected to an I2C EEPROM
> +
> +The FSI master may be connected to additional slaves, and slaves may have
> +additional engines, but they don't necessarily need to be describe in the
> +device tree if no extra platform information is required.
> +
> +    /* The GPIO-based FSI master node, describing the top level of the
> +     * FSI bus
> +     */
> +    gpio-fsi {
> +        compatible = "fsi-master-gpio", "fsi-master";
> +        #address-cells = <2>;
> +        #size-cells = <0>;
> +
> +        /* A FSI slave (aka. CFAM) at link 0, ID 0. */
> +        cfam@0,0 {
> +            reg = <0 0>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +
> +            /* FSI engine at 0xc00, using a single page. In this example,
> +             * it's an I2C master controller, so subnodes describe the
> +             * I2C bus.
> +             */
> +            i2c-controller@c00 {
> +                reg = <0xc00 0x400>;
> +
> +                /* Engine-specific data. In this case, we're describing an
> +                 * I2C bus, so we're conforming to the generic I2C binding
> +                 */
> +                compatible = "ibm,fsi-i2c";
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                /* I2C endpoint device: an Atmel EEPROM */
> +                eeprom@50 {
> +                    compatible = "atmel,24c256";
> +                    reg = <0x50>;
> +                    pagesize = <64>;
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.7.4
--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Eddie James]

On 05/02/2017 12:55 AM, Jeremy Kerr wrote:
> This change introduces a proposed layout for describing FSI busses in
> the device tree. While the bus is probe-able, we'd still like a method
> of describing subordinate (eg i2c) busses that are behind FSI devices.
>
> The FSI core will be responsible for matching probed slaves & engines to
> their device tree nodes, so the FSI device drivers' probe() functions
> will be passed a struct device with the appropriate of_node populated
> where a matching DT node is found.
>
> RFC at this stage, so I'd welcome any feedback.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> ---
> +
> +Full example
> +------------
> +
> +    /* The GPIO-based FSI master node, describing the top level of the
> +     * FSI bus
> +     */
> +    gpio-fsi {
> +        compatible = "fsi-master-gpio", "fsi-master";
> +        #address-cells = <2>;
> +        #size-cells = <0>;
> +
> +        /* A FSI slave (aka. CFAM) at link 0, ID 0. */
> +        cfam@0,0 {
> +            reg = <0 0>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +
> +            /* FSI engine at 0xc00, using a single page. In this example,
> +             * it's an I2C master controller, so subnodes describe the
> +             * I2C bus.
> +             */
> +            i2c-controller@c00 {
> +                reg = <0xc00 0x400>;
> +
> +                /* Engine-specific data. In this case, we're describing an
> +                 * I2C bus, so we're conforming to the generic I2C binding
> +                 */
> +                compatible = "ibm,fsi-i2c";
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                /* I2C endpoint device: an Atmel EEPROM */
> +                eeprom@50 {
> +                    compatible = "atmel,24c256";
> +                    reg = <0x50>;
> +                    pagesize = <64>;
> +                };

I have no problem with this as an example, but it isn't exactly how the 
tree will look under the p8/p9 fsi-attached i2c master. We will instead 
need a list of ports on the master. Then you could put actual devices 
(eeproms, etc) under the ports if desired.

Looks like good documentation to me though. This is working well for my 
I2C driver in combination with your latest FSI patches, Jeremy.

Acked-by: Eddie James <eajames-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

> +            };
> +        };
> +    };

--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Jeremy Kerr]

Hi Eddie,

> I have no problem with this as an example, but it isn't exactly how the
> tree will look under the p8/p9 fsi-attached i2c master. We will instead
> need a list of ports on the master. Then you could put actual devices
> (eeproms, etc) under the ports if desired.

Yes, this was intended as a fairly simple example. That's a good point
though, and I'll change the compatible string there so that I'm not
implying that it's representing the actual FSI I2c device that we're
working on.

Thanks for the review.

Cheers,


Jeremy
--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Rob Herring]

On Tue, May 02, 2017 at 01:55:08PM +0800, Jeremy Kerr wrote:
> This change introduces a proposed layout for describing FSI busses in
> the device tree. While the bus is probe-able, we'd still like a method
> of describing subordinate (eg i2c) busses that are behind FSI devices.
> 
> The FSI core will be responsible for matching probed slaves & engines to
> their device tree nodes, so the FSI device drivers' probe() functions
> will be passed a struct device with the appropriate of_node populated
> where a matching DT node is found.
> 
> RFC at this stage, so I'd welcome any feedback.
> 
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt
> 
> diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> new file mode 100644
> index 0000000..b5e85c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> @@ -0,0 +1,144 @@
> +FSI bus & engine generic device tree bindings
> +=============================================
> +
> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and

s/Linux/the OS/

Otherwise, looks good to me.

Rob
--
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

Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses

[Jeremy Kerr]

Hi Rob,

>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
>> @@ -0,0 +1,144 @@
>> +FSI bus & engine generic device tree bindings
>> +=============================================
>> +
>> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
> 
> s/Linux/the OS/

Ah, of course. Will change.

> Otherwise, looks good to me.

OK, I'll roll another revision with the changes mentioned. I also have
the patches that add the DT parsing code to implement this binding.
Those only affect the FSI core, but I'd be keen to include
devicetree@vger in those follow-ups. Any objections?

Cheers,


Jeremy
--
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