On 16/01/2024 15:04, Michal Simek wrote: > Convert the generic fpga region DT binding to json-schema. > There are some differences compare to txt version. > 1. DT overlay can't be described in example that's why directly include > information from overlay to node which was referenced. It is visible in > example with /* DT Overlay contains: &... */ > > 2. All example have been rewritten to be simpler and describe only full > reconfiguration and partial reconfiguration with one bridge. > Completely drop the case where fpga region can inside partial > reconfiguration region which is already described in description > > 3. Fixed some typos in descriptions compare to txt version but most of it > is just c&p from txt file. > > Signed-off-by: Michal Simek <michal.simek@xxxxxxx> > --- ... > + -- > + [1] www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/ug/ug_partrecon.pdf > + [2] tspace.library.utoronto.ca/bitstream/1807/67932/1/Byma_Stuart_A_201411_MAS_thesis.pdf > + [3] https://www.xilinx.com/support/documentation/sw_manuals/xilinx14_1/ug702.pdf > + > +properties: > + $nodename: > + pattern: "^fpga-region(-[0-9]|[1-9][0-9]+)?$" I think you miss here one set of (). Please look at my previous pattern and test yours. This should fail, e.g. on fpga-region-0 > + > + compatible: > + const: fpga-region > + > + "#address-cells": true > + "#size-cells": true > + > + config-complete-timeout-us: > + description: > + The maximum time in microseconds time for the FPGA to go to operating > + mode after the region has been programmed. > + > + encrypted-fpga-config: > + type: boolean > + description: > + Set if the bitstream is encrypted. > + > + external-fpga-config: > + type: boolean > + description: > + Set if the FPGA has already been configured prior to OS boot up. > + > + firmware-name: > + maxItems: 1 > + description: > + Should contain the name of an FPGA image file located on the firmware > + search path. If this property shows up in a live device tree it indicates > + that the FPGA has already been programmed with this image. > + If this property is in an overlay targeting an FPGA region, it is > + a request to program the FPGA with that image. > + > + fpga-bridges: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + Should contain a list of phandles to FPGA Bridges that must be > + controlled during FPGA programming along with the parent FPGA bridge. > + This property is optional if the FPGA Manager handles the bridges. > + If the fpga-region is the child of an fpga-bridge, the list should not > + contain the parent bridge. > + > + fpga-mgr: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Should contain a phandle to an FPGA Manager. Child FPGA Regions > + inherit this property from their ancestor regions. An fpga-mgr property > + in a region will override any inherited FPGA manager. > + > + partial-fpga-config: > + type: boolean > + description: > + Set if partial reconfiguration is to be done, otherwise full > + reconfiguration is done. > + > + region-freeze-timeout-us: > + description: > + The maximum time in microseconds to wait for bridges to successfully > + become disabled before the region has been programmed. > + > + region-unfreeze-timeout-us: > + description: > + The maximum time in microseconds to wait for bridges to successfully > + become enabled after the region has been programmed. > + > +patternProperties: > + "@(0|[1-9a-f][0-9a-f]*)$": > + type: object You put (0|[...) to disallow @0001? I personally would not care, dtc handles this, and the pattern is confusing. Just @[0-9a-z]+$ > + > + "^[^@]+$": true I dislike it. How is this binding supposed to be used? If in standalone way, then you allow any property so what's the point of this schema? If fpga-bridge.yaml is referenced by other device-specific binding, then all properties will be evaluated here, so the same: you allow any property. Depending on the usage, this might be just like other generic, common schemas, so end with "additionalPropeties: true". > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + - fpga-mgr > + > +additionalProperties: false > + > +examples: > + - | > + /* > + * Full Reconfiguration without Bridges with DT overlay > + */ > + fpga_mgr0: fpga-mgr@f8007000 { > + compatible = "xlnx,zynq-devcfg-1.0"; > + reg = <0xf8007000 0x100>; > + clocks = <&clkc 12>; > + clock-names = "ref_clk"; > + interrupt-parent = <&intc>; > + interrupts = <0 8 4>; > + syscon = <&slcr>; > + }; I suggest to drop this node. The contents does not really matter for the bridge. > + > + fpga_region0: fpga-region { > + compatible = "fpga-region"; > + #address-cells = <1>; > + #size-cells = <1>; > + fpga-mgr = <&fpga_mgr0>; > + > + /* DT Overlay contains: &fpga_region0 */ > + firmware-name = "zynq-gpio.bin"; > + gpio@40000000 { > + compatible = "xlnx,xps-gpio-1.00.a"; > + reg = <0x40000000 0x10000>; > + gpio-controller; > + #gpio-cells = <2>; > + }; > + }; > + > + - | > + /* > + * Partial reconfiguration with bridge > + */ > + fpga_mgr1: fpga-mgr@f8007000 { > + compatible = "xlnx,zynq-devcfg-1.0"; > + reg = <0xf8007000 0x100>; > + clocks = <&clkc 12>; > + clock-names = "ref_clk"; > + interrupt-parent = <&intc>; > + interrupts = <0 8 4>; > + syscon = <&slcr>; > + }; > + > + fpga_bridge1: fpga-bridge@100000450 { > + compatible = "xlnx,pr-decoupler-1.00", "xlnx,pr-decoupler"; > + reg = <0x10000045 0x10>; > + clocks = <&clkc 15>; > + clock-names = "aclk"; > + }; Also drop both nodes. > + > + fpga_region1: fpga-region { > + compatible = "fpga-region"; > + #address-cells = <1>; > + #size-cells = <1>; > + fpga-mgr = <&fpga_mgr1>; > + fpga-bridges = <&fpga_bridge1>; > + partial-fpga-config; > + > + /* DT Overlay contains: &fpga_region1 */ > + firmware-name = "zynq-gpio-partial.bin"; > + clk100: clk100 { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <100000000>; > + bootph-all; > + }; > + axi { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + gpio@40000000 { > + compatible = "xlnx,xps-gpio-1.00.a"; > + reg = <0x40000000 0x10000>; > + #gpio-cells = <2>; > + gpio-controller; > + clocks = <&clk100>; > + }; > + }; > + }; Best regards, Krzysztof
