Hi Tom, Thanks for the review. Please find my response inline. > -----Original Message----- > From: Tom Rix <trix@xxxxxxxxxx> > Sent: Friday, January 15, 2021 11:37 PM > To: Nava kishore Manne <navam@xxxxxxxxxx>; mdf@xxxxxxxxxx; > robh+dt@xxxxxxxxxx; Michal Simek <michals@xxxxxxxxxx>; linux- > fpga@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Cc: git <git@xxxxxxxxxx>; chinnikishore369@xxxxxxxxx > 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: > > 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. > > Signed-off-by: Nava kishore Manne <nava.manne@xxxxxxxxxx> > > --- > > .../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 > > > > +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. > > + > > +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. > > +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. > > +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 > > +or chained via decouple/ decouple_status signals. > Being compatible, i don't think it is necessary to add to / change this section. > > > > Required properties: > > - compatible : Should contain "xlnx,pr-decoupler-1.00" followed > by > > - "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.