On Fri, Jul 15, 2022 at 11:33:54AM +0200, Krzysztof Kozlowski wrote:

On 14/07/2022 14:26, Ivan Bornyakov wrote:

quoted

Add Device Tree Binding doc for Lattice ECP5 FPGA manager using slave
SPI to load .bit formatted uncompressed bitstream image.

Signed-off-by: Ivan Bornyakov <redacted>
---
 .../fpga/lattice,ecp5-spi-fpga-mgr.yaml       | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml

diff --git a/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml
new file mode 100644
index 000000000000..79868f9c84e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/lattice,ecp5-spi-fpga-mgr.yaml

@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fpga/lattice,ecp5-spi-fpga-mgr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lattice ECP5 FPGA manager.
+
+maintainers:
+  - Ivan Bornyakov <i.bornyakov@metrotek.ru>
+
+description:
+  Device Tree Bindings for Lattice ECP5 FPGA Manager using slave SPI to
+  load the uncompressed bitstream in .bit format.

s/Device Tree Bindings for//

Instead describe the hardware you are adding bindings for. What is a
"Manager"? It is so broad and unspecific... It is some dedicated
hardware to communicate with FPGA or you just called this regular FPGA
interface exposed to the CPU/SoC?

"FPGA Manager" is a kernel subsystem that exports a set of functions for
programming an FPGA with a bitstream image.
See Documentation/driver-api/fpga/fpga-mgr.rst

quoted

+
+properties:
+  compatible:
+    enum:
+      - lattice,ecp5-spi-fpga-mgr

Do not encode interface name in compatible so no "spi".

Recently when I submitted FPGA manager for Microchip PolarFire, I was
asked the opposite, to add "spi" in compatible. The reason was that FPGA
can be programmed through other interfaces as well.

quoted

+
+  reg:
+    description: SPI chip select
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 60000000

Reference spi/spi-peripheral-props.yaml in allOf

quoted

+
+  program-gpios:
+    description:
+      A GPIO line connected to PROGRAMN (active low) pin of the device.
+      Initiates configuration sequence.
+    maxItems: 1
+
+  init-gpios:
+    description:
+      A GPIO line connected to INITN (active low) pin of the device.
+      Indicates the FPGA is ready to be configured.
+    maxItems: 1
+
+  done-gpios:
+    description:
+      A GPIO line connected to DONE (active high) pin of the device.
+      Indicates that the configuration sequence is complete.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - program-gpios
+  - init-gpios
+  - done-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+            #address-cells = <1>;

Wrong indentation. 4-spaces for DTS example.

quoted

+            #size-cells = <0>;
+
+            fpga_mgr@0 {

No underscores in node names.

quoted

+                    compatible = "lattice,ecp5-spi-fpga-mgr";
+                    spi-max-frequency = <20000000>;
+                    reg = <0>;
+                    program-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
+                    init-gpios = <&gpio3 3 GPIO_ACTIVE_LOW>;
+                    done-gpios = <&gpio3 2 GPIO_ACTIVE_HIGH>;
+            };
+    };

Best regards,
Krzysztof