Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx
 DFX AXI shutdown manger
To:     Nava kishore Manne <nava.manne@xilinx.com>, mdf@kernel.org,
        robh+dt@kernel.org, michal.simek@xilinx.com,
        linux-fpga@vger.kernel.org, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Cc:     git@xilinx.com, chinnikishore369@gmail.com
References: <20210115013431.27667-1-nava.manne@xilinx.com>
From:   Tom Rix <trix@redhat.com>
Message-ID: <2dd1a504-9bb7-3bdb-028e-7edd0ed99668@redhat.com>
Date:   Fri, 15 Jan 2021 10:07:25 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: <20210115013431.27667-1-nava.manne@xilinx.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


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

> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
>  .../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
>  
> +Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
> +Softcore
is compatible with the Xilinx LogiCORE pr-decoupler.
> +
> +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.

The DFX AXI shutdown manager prevents

> +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)'

> +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.

Tom