Hi Tom,

	Thanks for the review.
Please find my response inline.

-----Original Message-----
From: Tom Rix <trix@redhat.com>
Sent: Friday, January 15, 2021 11:37 PM
To: Nava kishore Manne <redacted>; mdf@kernel.org;
robh+dt@kernel.org; Michal Simek [off-list ref]; linux-
fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Cc: git <redacted>; chinnikishore369@gmail.com
Subject: Re: [PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx
DFX AXI shutdown manger


On 1/14/21 5:34 PM, Nava kishore Manne wrote:

quoted

This patch Adds compatible value for Xilinx Dynamic Function
eXchnage(DFX) AXI Shutdown manager IP.

A multi patch set should have a cover letter.

Use git format-patch --cover-letter

Will  fix in v2.

quoted

Signed-off-by: Nava kishore Manne <redacted>
---
 .../bindings/fpga/xilinx-pr-decoupler.txt     | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git
a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
index 4284d293fa61..42fca058a5f4 100644

--- a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
+++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt

@@ -7,13 +7,26 @@ changes from passing through the bridge.  The

controller can also  couple / enable the bridges which allows traffic
to pass through the  bridge normally.

Below is some wordsmithing to make the compatibility explicit and remove
unneeded words

Will fix in v2

quoted

+Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
+Softcore

is compatible with the Xilinx LogiCORE pr-decoupler.

Yes, Both IP's are compatible with respect to the IP registers spec.

quoted

+
+The Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown
+manager manages one or more shutdown managers / fpga bridges.
+The controller can shutdown/passthrough the bridges which prevents

Delete 4 lines above.

Will fix in v2.

The DFX AXI shutdown manager prevents

Will fix in v2.

quoted

+AXI traffic from passing through the bridge. The controller safely
+handles AXI4MM and AXI4-Lite interfaces on a Reconfigurable Partition
+when it is undergoing dynamic reconfiguration (DFX), preventing
+system

I don't know about the use of '(DFX)' here, above it is Dynamic Function
eXchange'

Here it is dynamic reconfiguration, maybe just delete the '(DFX)'

Will fix in v2.

quoted

+deadlock that can occur if AXI transactions are interrupted by DFX
+
 The Driver supports only MMIO handling. A PR region can have multiple
-PR Decouplers which can be handled independently or chained via
decouple/ -decouple_status signals.
+PR Decouplers/AXI shutdown manager which can be handled

independently

quoted

+or chained via decouple/ decouple_status signals.

Being compatible, i don't think it is necessary to add to / change this section.

quoted

 Required properties:
 - compatible		: Should contain "xlnx,pr-decoupler-1.00" followed

by

quoted

-                          "xlnx,pr-decoupler"
+                          "xlnx,pr-decoupler" or
+			  "xlnx,dfx-axi-shutdown-manager-1.00" followed by
+			  "xlnx,dfx-axi-shutdown-manager"
 - regs			: base address and size for decoupler module
 - clocks		: input clock to IP
 - clock-names		: should contain "aclk"

Adding an example similar to the existing would be helpful.

Will fix in v2.

Regards,
Navakishore.