Hi Rob,

 Thanks for providing the review comments..
Please find my response inline.

-----Original Message-----
From: Rob Herring [mailto:robh@kernel.org]
Sent: Saturday, February 23, 2019 2:01 AM
To: Nava kishore Manne <redacted>
Cc: mark.rutland@arm.com; Michal Simek <redacted>; Rajan Vaja
[off-list ref]; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; devicetree@vger.kernel.org; Jolly Shah
[off-list ref]; chinnikishore369@gmail.com
Subject: Re: [PATCH v3 6/6] dt-bindings: fpga: Add bindings for ZynqMP fpga
driver

On Wed, Jan 23, 2019 at 2:46 PM Nava kishore Manne [off-list ref]
wrote:

quoted

Hi Rob,



Thanks for providing the comments...

Please fix your mailer to send plain text emails to mail lists.

Thanks for pointing it..

quoted

quoted

-----Original Message-----

quoted

From: Rob Herring [mailto:robh@kernel.org]

quoted

Sent: Monday, January 21, 2019 9:19 PM

quoted

To: Nava kishore Manne <redacted>

quoted

Cc: mark.rutland@arm.com; Michal Simek <redacted>; Rajan
Vaja

quoted

[off-list ref]; linux-arm-kernel@lists.infradead.org; linux-

quoted

kernel@vger.kernel.org; devicetree@vger.kernel.org; Jolly Shah

quoted

[off-list ref]; chinnikishore369@gmail.com

quoted

Subject: Re: [PATCH v3 6/6] dt-bindings: fpga: Add bindings for
ZynqMP fpga

quoted

driver

quoted

quoted

On Mon, Jan 21, 2019 at 11:08:35PM +0530, Nava kishore Manne wrote:

quoted

quoted

Add documentation to describe Xilinx ZynqMP fpga driver bindings.

quoted

quoted

quoted

quoted

Signed-off-by: Nava kishore Manne <redacted>

quoted

quoted

---

quoted

quoted

Changes for v3:

quoted

quoted

                        -Removed PCAP as a child node to the FW
and Created

quoted

quoted

                        an independent node since PCAP driver is a
consumer

quoted

quoted

                        not a provider.

quoted

quoted

quoted

quoted

 .../bindings/fpga/xlnx,zynqmp-pcap-fpga.txt         | 13 +++++++++++++

quoted

quoted

 1 file changed, 13 insertions(+)

quoted

quoted

 create mode 100644

quoted

quoted

Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt

quoted

quoted

quoted

quoted

diff --git

quoted

quoted

a/Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt

quoted

quoted

b/Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt

quoted

quoted

new file mode 100644

quoted

quoted

index 000000000000..1f6f58872311

quoted

quoted

--- /dev/null

quoted

quoted

+++ b/Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga
+++ .txt

quoted

quoted

@@ -0,0 +1,13 @@

quoted

quoted

+Device Tree zynqmp-fpga bindings for the Zynq Ultrascale+ MPSoC

quoted

quoted

+controlled using ZynqMP SoC firmware interface For Bitstream

quoted

quoted

+configuration on ZynqMp Soc uses processor configuration

quoted

quoted

+port(PCAP) to configure the programmable logic(PL) through PS by

quoted

quoted

+using FW interface.

quoted

quoted

+

quoted

quoted

+Required properties:

quoted

quoted

+- compatible: should contain "xlnx,zynqmp-pcap-fpga"

quoted

quoted

+

quoted

quoted

+Example:

quoted

quoted

+      zynqmp_pcap: pcap {

quoted

quoted

+                     compatible = "xlnx,zynqmp-pcap-fpga";

quoted

quoted

+      };

quoted

quoted

There's no need for a DT node. Just make the firware driver create a
platform

quoted

device for pcap.

quoted

This driver being used by the FPGA manager framework(fpga-region) to apply

overlay's so this node is needed AFAIK.

The binding says nothing about child nodes and what they are. Please define
that.

Do you mean I need to add the example usage of the pcap node like as below in the binding doc?

Device Tree Example for Full Reconfiguration
============================================

Live Device Tree contains:

        zynqmp_pcap: pcap {
                compatible = "xlnx,zynqmp-pcap-fpga";
        };

        fpga_region0: fpga-region0 {
                compatible = "fpga-region";
                fpga-mgr = <&zynqmp_pcap>;
                #address-cells = <0x1>;
                #size-cells = <0x1>;
                ranges;
        };

DT Overlay contains:
/dts-v1/ /plugin/;
/ {
fragment@0 {
        target = <&fpga_region0>;
        #address-cells = <1>;
        #size-cells = <1>;
        __overlay__ {
                #address-cells = <1>;
                #size-cells = <1>;

                firmware-name = "zynqmp-gpio.bin";

                gpio1: gpio@40000000 {
                        compatible = "xlnx,xps-gpio-1.00.a";
                        reg = <0x40000000 0x10000>;
                        gpio-controller;
                        #gpio-cells = <0x2>;
                        xlnx,gpio-width= <0x6>;
                };
        };
};
 
Please correct me if my understanding is wrong.

Regards,
Navakishore.