Message-ID: <20160125165302.GA14212@rob-hp-laptop>
Date:	Mon, 25 Jan 2016 10:53:02 -0600
From:	Rob Herring <robh@...nel.org>
To:	atull@...nsource.altera.com
Cc:	Moritz Fischer <moritz.fischer@...us.com>,
	Josh Cartwright <joshc@...com>, gregkh@...uxfoundation.org,
	monstr@...str.eu, michal.simek@...inx.com,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Jonathan Corbet <corbet@....net>, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
	pantelis.antoniou@...sulko.com, delicious.quinoa@...il.com,
	dinguyen@...nsource.altera.com
Subject: Re: [PATCH v15 1/6] fpga: add bindings document for fpga area and
 fpga bus

On Wed, Jan 20, 2016 at 01:24:22PM -0600, atull@...nsource.altera.com wrote:
> From: Alan Tull <atull@...nsource.altera.com>
> 
> New bindings document for FPGA Area for reprogramming
> FPGA's under Device Tree control
> 
> Signed-off-by: Alan Tull <atull@...nsource.altera.com>
> ---
> v9:  initial version added to this patchset
> v10: s/fpga/FPGA/g
>      replace DT overlay example with slightly more complicated example
>      move to staging/simple-fpga-bus
> v11: No change in this patch for v11 of the patch set
> v12: Moved out of staging.
>      Changed to use FPGA bridges framework instead of resets
>      for bridges.
> v13: bridge@...f20000 -> bridge@...00000, etc
>      Leave out directly talking about overlays
>      Remove regs and clocks directly under simple-fpga-bus in example
>      Use common "firmware-name" binding instead of "fpga-firmware"
> v14: Use firmware-name in bindings description
>      Call it FPGA Area
>      Remove bindings that specify FPGA Manager and FPGA Bridges
> v15: Cleanup as per Rob's comments
>      Combine usage doc with bindings document
>      Document as being Altera specific
>      Additions and changes to add FPGA Bus
> ---
>  .../bindings/fpga/altera-fpga-bus-fpga-area.txt    |  452 ++++++++++++++++++++
>  1 file changed, 452 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/altera-fpga-bus-fpga-area.txt
> 
> diff --git a/Documentation/devicetree/bindings/fpga/altera-fpga-bus-fpga-area.txt b/Documentation/devicetree/bindings/fpga/altera-fpga-bus-fpga-area.txt
> new file mode 100644
> index 0000000..8ea38ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/altera-fpga-bus-fpga-area.txt
> @@ -0,0 +1,452 @@
> +Altera FPGA Area and FPGA Bus Device Tree Binding

Getting closer I think...

> +
> +Alan Tull 2016
> +
> + CONTENTS
> + - Introduction
> + - Terminology
> + - Overview
> + - Constraints
> + - FPGA Bus
> + - FPGA Area
> + - Supported Use Models
> + - Sequence
> + - Device Tree Examples
> +
> +
> +Introduction
> +============
> +
> +FPGA Buses and FPGA Areas are introduced as a way to solve the problem of how to
> +reprogram an Altera FPGA under an operating system and have the new hardware
> +show up in the device tree.  By adding these bindings to the Device Tree, a
> +system can have the information needed to do the FPGA programming to add the
> +desired hardware, and also the information about the devices to be added to the
> +Device Tree once the programming has succeeded.
> +
> +
> +Terminology
> +===========
> +
> +Full Reconfiguration
> + * The entire FPGA is programmed.
> +
> +Partial Reconfiguration (PR)
> + * The FPGA is broken up into regions.  One of these regions is reprogrammed
> +   while the rest of the FPGA is not affected.  Not all FPGA's support this.
> +
> +FPGA Manager & FPGA Manager Framework
> + * An FPGA Manager is a hardware block that programs an FPGA under the control
> +   of a host processor.
> + * The FPGA Manager Framework provides drivers and functions to program an
> +   FPGA.
> +
> +FPGA Bridge & FPGA Bridge Framework
> + * Provides drivers and functions to control bridges that enable/disable
> +   data to the FPGA.
> + * FPGA Bridges should be disabled while the FPGA is being programmed to
> +   prevent spurious data on the bus.
> + * FPGA Bridges may not be needed in implementations where the FPGA Manager
> +   handles this.
> +
> +Freeze Blocks
> + * Freeze Blocks function as FPGA Bridges within the FPGA fabric.  In the case
> +   of PR, the buses from the processor are split within the FPGA.  Each PR
> +   region gets its own split of the buses, protected by an independently
> +   controlled Freeze Block.  Several busses may be connected to a single
> +   PR region; a Freeze Block controls the traffic of all these busses
> +   together.
> +
> +Controlling Bridge
> + * The processor and FPGA may be connected by multiple FPGA Bridges.  In a text
> +   based hierarchy, it is difficult to show this properly as a device would
> +   have several parents.
> + * The bridge that handles register access to the FPGA is designated the
> +   "controlling" bridge.
> + * The controlling bridge will be the target point under which the overlay is
> +   inserted.
> +
> +
> +Overview
> +========
> +
> +This binding introduces the FPGA Bus and FPGA Area.
> +
> +An FPGA Bus is a virtualized bus that contains the devices needed to be able to
> +program an FPGA device.  It contains a child FPGA Manager and may contain child
> +FPGA Bridges, if needed.  The FPGA Manager is the hardware block that handles
> +programming the FPGA.  FPGA Bridges function to prevent spurious data from the
> +FPGA going on the processor busses during FPGA programming.  In the case of
> +partial reconfiguration, additional bridges (Freeze Blocks) for each
> +reconfiguration region are required.
> +
> +An FPGA Area contains information about a section of an FPGA (in the case of
> +partial reconfiguration or the whole FPGA (for full reconfiguration).  The FPGA
> +Area contains the name of the FPGA image file to be programmed and the child
> +devices that will be contained in the FPGA after programming.
> +
> +The intended use is that device tree overlays can be used to add hardware to an
> +FPGA while an operating system is running.  In that case, the FPGA Bus and its
> +associated information will exist in the live device tree, while FPGA Areas are
> +added to the device tree by applying device tree overlays while the operating
> +system is running.  Loading such an overlay will cause the FPGA to be programmed
> +and the child devices to be populated.  Removing an overlay will cause the
> +devices to be removed from the device tree and free up those FPGA resources.
> +
> +
> +Constraints
> +===========
> +
> +It is beyond the scope of this document to fully describe all the FPGA design
> +constraints required to make partial reconfiguration work[1] [2], but a few
> +deserve quick mention.  A partial reconfiguration FPGA image must have
> +boundary connections that line up with those the current programming of the
> +FPGA.  Also, those during programming, those connections must be frozen.  This
> +can be achieved by "Freeze Blocks" which are FPGA Bridges that exist on the FPGA
> +fabric prior to the partial reconfiguration.
> +
> +
> +FPGA Bus
> +========
> +
> +An FPGA bus is a virtualized bus that specifies the devices needed to program an
> +FPGA.
> +
> +Required properties:
> +- compatible : should contain "altr,fpga-bus" and "simple-bus"
> +	       "simple-bus" is required to allow populating the child nodes.

I think I said this before, but you should not need simple-bus. Having 
it is wrong if the bus requires some configuration before enumerating 
the child nodes. The way you have it, the bridge driver could probe 
before the FPGA manager or vice-versa. I'm guessing one way will work 
and the other will not, and you are getting lucky. You should also test 
will the overlay applied before boot (i.e. single dtb with all areas 
populated). The simple rule is does the node (containing simple-bus) 
require configuration for child nodes to be accessed? If yes, you can't 
use simple-bus.

Part of your problem is you may need a custom match table. Using 
of_default_bus_match_table will only match the immediate child nodes of 
the starting node. Alternatively, you need to make the bridge node the 
starting node.

> +
> +A FPGA Bus should contain an FPGA Manager as a child node.
> +
> +A FPGA Bus may require FPGA Bridges as child nodes if the FPGA Manager does not
> +control the hardware bridges.
> +
> +The FPGA Bridge that allows memory mapped register access is designated the
> +"controlling" bridge.  This bridge serves as the insertion point of DT overlays.
> +Both the FPGA Area and the controlling bridge require the "simple-bus"
> +compatibility string to allow populating the child nodes contained in the
> +overlay.
> +
> +In the example below, fpgamgr@...06000 would be used to do any programming
> +operations on the FPGA and the two bridges specified would be disabled
> +during the programming and re-enabled afterwards if the programming
> +succeeds.
> +
> +Example:
> +	fpgabus@0 {

If you have a unit-address, then there should be a reg property or 
ranges parent bus addr.

> +		compatible = "altr,fpga-bus", "simple-bus";
> +		#address-cells = <0x1>;
> +		#size-cells = <0x1>;
> +		ranges;
> +
> +		fpgamgr@...06000 {

As mentioned in the other thread, this may be better done as a phandle, 
but I don't feel too strongly either way.

> +			compatible = "altr,socfpga-fpga-mgr";
> +			reg = <0xff706000 0x1000
> +			       0xffb90000 0x1000>;
> +			interrupts = <0 175 4>;
> +		};
> +
> +		bridge@0 {

Same here. Doesn't this bridge have some associated address range.

Essentially, the hierarchy from child devices up to the root should 
reflect the address decoding at each step.

> +			compatible = "altr,socfpga-lwhps2fpga-bridge",
> +				     "simple-bus";
> +			resets = <&rst LWHPS2FPGA_RESET>;
> +			reset-names = "lwhps2fpga";
> +			clocks = <&l4_main_clk>;
> +			#address-cells = <0x1>;
> +			#size-cells = <0x1>;
> +			ranges;
> +		};
> +
> +		bridge@1 {
> +			compatible = "altr,socfpga-hps2fpga-bridge";
> +			resets = <&rst HPS2FPGA_RESET>;
> +			reset-names = "hps2fpga";
> +			clocks = <&l4_main_clk>;
> +		};
> +	};
> +
> +FPGA Area
> +=========
> +
> +An FPGA Area details information about a section of an FPGA including the FPGA
> +image needed to program it and the hardware contained once it is programmed.
> +
> +An FPGA Area corresponds to the whole FPGA in the case of full reconfiguration
> +or a section of an FPGA in the case of partial reconfiguration.
> +
> +Required properties:
> +- compatible : should contain "altr,fpga-area"
> +- #address-cells, #size-cells, ranges: must be present to handle address space
> +  mapping for children.
> +
> +Optional properties:
> +- firmware-name : should contain the name of an FPGA image file located on the
> +  firmware search path.
> +- partial-reconfig : boolean property should be defined if partial
> +  reconfiguration of the FPGA is to be done, otherwise full reconfiguration
> +  is done.
> +
> +In the example below, the target path is the controlling bridge of the FPGA Bus
> +example.  The FPGA would be programmed with the image contained in the
> +"soc_system.rbf" specified.  Assuming programming succeeds, the child devices
> +would be populated afterwords.  In this particular example, ranges has two
> +chip selects as one memory region is tied to the host processor and the
> +other is a memory region on the FPGA.
> +
> +If there are no bridges in the FPGA Bus, the target path would point to
> +the FPGA Manager.

How do you not have a bridge? There has to be something preventing 
access. Do you really mean the bridge and fpga-mgr blocks are combined 
into 1?

> +
> +Example:
> +
> +/dts-v1/;
> +/plugin/;
> +/ {
> +	fragment@0 {
> +		target-path="/soc/fpgamgr@...06000/bridge@0";
> +		__overlay__ {
> +			#address-cells = <1>;
> +		        #size-cells = <1>;
> +
> +			bridge@...00000 {

Wouldn't this be area@...? area is a bit too generic. I'd use 
"fpga-area" for the node name too.

> +				compatible = "fpga-area";
> +
> +				#address-cells = <2>;
> +				#size-cells = <1>;
> +
> +				ranges = <0 0x00000000 0xc0000000 0x00010000>,
> +					 <1 0x00020000 0xff220000 0x00000008>,
> +					 <1 0x00010040 0xff210040 0x00000020>;
> +
> +				firmware-name = "soc_system.rbf";
> +
> +				onchip_memory2_0: memory@0 {
> +					device_type = "memory";
> +					compatible = "altr,onchipmem-15.1";
> +					reg = <0 0x00000000 0x00010000>;
> +				};
> +
> +				jtag_uart: serial@...020000 {
> +					compatible = "altr,juart-1.0";
> +					reg = <1 0x00020000 0x00000008>;
> +					interrupt-parent = <&intc>;
> +					interrupts = <0 42 4>;
> +				};
> +
> +				led_pio: gpio@...010040 {
> +					compatible = "altr,pio-1.0";
> +					reg = <1 0x00010040 0x00000020>;
> +					altr,gpio-bank-width = <4>;
> +					#gpio-cells = <2>;
> +					gpio-controller;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +Supported Use Models
> +====================
> +
> +Here's a list of supported use models.  We may need to add more.  Some uses are
> +specific to one FPGA device or another.
> +
> + * No FPGA Bridges
> +   In this case, the FPGA Manager which programs the FPGA also handles the
> +   bridges.  No FPGA Bridge devices are needed for full reconfiguration.
> +
> +   The DT overlay will specify the FPGA Manager as the overlay target.
> +
> + * Full reconfiguration with bridges
> +   In the case, there are several bridges between the processor and FPGA, that
> +   need to be disabled during full reconfiguration.  The live DT before the
> +   overlay is applied will have an FPGA Bus.
> +
> +   The DT overlay will specify the controlling bridge as the overlay target.
> +
> + * Partial reconfiguration with bridges in the FPGA
> +   In this case, the FPGA will have more than one section that will be
> +   programmed separately.  Other sections may be active on the bus while FPGA
> +   is being programmed.  To manage this, Freeze blocks need to exist on the FPGA
> +   that can freeze all the buses going to one FPGA area while the buses are
> +   enabled for other sections.
> +
> +Sequence
> +========
> +
> +When a DT overlay is loaded, the FPGA Area will be probed and will do the
> +following:
> + 1. Disable the FPGA Bridges.
> + 2. Use the the FPGA manager core to program the FPGA.
> + 3. Enable the FPGA Bridges.
> + 4. Call of_platform_populate resulting in device drivers getting probed.
> +
> +When the overlay is removed, the FPGA Area in it is removed.  This causes the
> +child nodes to be removed and then the bridges are disabled.
> +
> +Device Tree Examples
> +====================
> +
> +The intention of this section is to give some simple examples, focusing on
> +the placement of the elements detailed above, especially:
> + * FPGA Bus and associated FPGA Manager and FPGA Bridges
> + * FPGA Area and associated properties
> + * simple-bus
> + * ranges
> + * target-path
> +
> +For the purposes of this section, I'm dividing the Device Tree into two parts,
> +each with its own requirements.  The two parts are:
> + * The live DT prior to the overlay being added
> + * The DT overlay
> +
> +The live Device Tree must contain an FPGA Bus, which has a child FPGA Manager to
> +handle programming the FPGA.  If FPGA Bridges need to be involved, they show up
> +in the DT as direct children of the FPGA Bus.  During full reconfiguration, the
> +FPGA Area will disable any bridges that are direct children of the FPGA Bus and
> +will re-enable them after FPGA programming has succeeded.
> +
> +The Device Tree Overlay will contain:
> + * "target-path"
> +   The insertion point where the the contents of the overlay will go into the
> +   live tree.
> + * "ranges"
> + * "firmware-name"
> +   Specifies the name of the FPGA image file on the firmware search
> +   path.  The search path is described in the firmware class documentation.
> + * "partial-reconfig"
> +   This binding is a boolean and should be present if partial reconfiguration
> +   is to be done.
> + * child nodes corresponding to hardware that will be loaded in this region of
> +   the FPGA.
> +
> +
> +Device Tree Example: Partial Reconfiguration with no Bridges
> +============================================================
> +
> +Live Device Tree contains:
> +	fpgamgr@...03000 {
> +		compatible = "altr,socfpga-a10-fpga-mgr", "simple-bus";
> +		clocks = <&l4_mp_clk>;
> +		resets = <&rst FPGAMGR_RESET>;
> +		reset-names = "fpgamgr";
> +		reg = <0xffd03000 0x1000
> +		       0xffcfe400 0x1000>;
> +
> +		#address-cells = <0x1>;
> +		#size-cells = <0x1>;
> +		ranges;
> +	};
> +
> +DT Overlay contains:
> +/dts-v1/;
> +/plugin/;
> +/ {
> +	fragment@0 {
> +		target-path="/soc/fpgamgr@...03000";  /* targeted to the manager */
> +		__overlay__ {
> +			#address-cells = <1>;
> +		        #size-cells = <1>;
> +
> +			area@0 {

Are the number of areas software configurable? If not, then the areas 
should be part of the base DT rather than the overlay.

> +				compatible = "fpga-area";
> +
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				ranges = <0x10010 0xff210010 0x10>;
> +
> +				firmware-name = "fit_pr_v1.rbf";
> +				partial-reconfig;
> +
> +				gpio@...010010 {

gpio@...10

> +					compatible = "altr,pio-1.0";
> +					reg = <0x10010 0x10>;
> +					altr,ngpio = <4>;
> +					#gpio-cells = <0x2>;
> +					gpio-controller;
> +				};
> +			};
> +		};
> +	};
> +};

Powered by blists - more mailing lists