[v2] fpga: zynq-fpga: Ensure proper xCAP interface switch

Message ID 20230531095624.1802757-1-nava.kishore.manne@amd.com
State New
Headers
Series [v2] fpga: zynq-fpga: Ensure proper xCAP interface switch |

Commit Message

Manne, Nava kishore May 31, 2023, 9:56 a.m. UTC
  From: Alfonso Rodriguez <alfonso.rodriguezm@upm.es>

The Zynq platform has PCAP, ICAP and JTAG interfaces for configuring
programmable logic (PL). The existing driver implementation uses the
PCAP interface to configure the PL. Before switching the PL configuration
interface from PCAP to ICAP make sure that all outstanding Transactions
relevant to the PL configuration should be completed by the PCAP interface
otherwise it may lead to PL configuration issues.

This patch provides a required fix to ensure that all existing PL
transactions are completed before switching from PCAP to ICAP.

For detailed information relevant to PL configuration interfaces refer
Zynq 7000 TRM (section 6.5.1).
Link: https://docs.xilinx.com/v/u/en-US/ug585-Zynq-7000-TRM

Signed-off-by: Alfonso Rodriguez <alfonso.rodriguezm@upm.es>
Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
---
Changes for v2:
              - Updated commit message and added Doc link as suggested by Yilun.

 drivers/fpga/zynq-fpga.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
  

Comments

Xu Yilun June 3, 2023, 5:27 p.m. UTC | #1
On 2023-05-31 at 15:26:24 +0530, Nava kishore Manne wrote:
> From: Alfonso Rodriguez <alfonso.rodriguezm@upm.es>
> 
> The Zynq platform has PCAP, ICAP and JTAG interfaces for configuring
> programmable logic (PL). The existing driver implementation uses the
> PCAP interface to configure the PL. Before switching the PL configuration
> interface from PCAP to ICAP make sure that all outstanding Transactions
> relevant to the PL configuration should be completed by the PCAP interface
> otherwise it may lead to PL configuration issues.
> 
> This patch provides a required fix to ensure that all existing PL
> transactions are completed before switching from PCAP to ICAP.
> 
> For detailed information relevant to PL configuration interfaces refer
> Zynq 7000 TRM (section 6.5.1).
> Link: https://docs.xilinx.com/v/u/en-US/ug585-Zynq-7000-TRM
> 
> Signed-off-by: Alfonso Rodriguez <alfonso.rodriguezm@upm.es>
> Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
> ---
> Changes for v2:
>               - Updated commit message and added Doc link as suggested by Yilun.
> 
>  drivers/fpga/zynq-fpga.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index ae0da361e6c6..88db9ac36887 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -493,15 +493,16 @@ static int zynq_fpga_ops_write_complete(struct fpga_manager *mgr,
>  	if (err)
>  		return err;
>  
> -	/* Release 'PR' control back to the ICAP */
> -	zynq_fpga_write(priv, CTRL_OFFSET,
> -		zynq_fpga_read(priv, CTRL_OFFSET) & ~CTRL_PCAP_PR_MASK);
> -
>  	err = zynq_fpga_poll_timeout(priv, INT_STS_OFFSET, intr_status,
>  				     intr_status & IXR_PCFG_DONE_MASK,
>  				     INIT_POLL_DELAY,
>  				     INIT_POLL_TIMEOUT);
>  
> +	/* Release 'PR' control back to the ICAP */
> +	zynq_fpga_write(priv, CTRL_OFFSET,
> +			zynq_fpga_read(priv, CTRL_OFFSET)
> +			& ~CTRL_PCAP_PR_MASK);

Don't put the & at the beginning of the line:

	zynq_fpga_read(priv, CTRL_OFFSET) &
	~CTRL_PCAP_PR_MASK);

or just:
	zynq_fpga_read(priv, CTRL_OFFSET) & ~CTRL_PCAP_PR_MASK);

80 lines is not a must now.

Others look good to me.

Thanks,
Yilun

> +
>  	clk_disable(priv->clk);
>  
>  	if (err)
> -- 
> 2.25.1
>
  

Patch

diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index ae0da361e6c6..88db9ac36887 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -493,15 +493,16 @@  static int zynq_fpga_ops_write_complete(struct fpga_manager *mgr,
 	if (err)
 		return err;
 
-	/* Release 'PR' control back to the ICAP */
-	zynq_fpga_write(priv, CTRL_OFFSET,
-		zynq_fpga_read(priv, CTRL_OFFSET) & ~CTRL_PCAP_PR_MASK);
-
 	err = zynq_fpga_poll_timeout(priv, INT_STS_OFFSET, intr_status,
 				     intr_status & IXR_PCFG_DONE_MASK,
 				     INIT_POLL_DELAY,
 				     INIT_POLL_TIMEOUT);
 
+	/* Release 'PR' control back to the ICAP */
+	zynq_fpga_write(priv, CTRL_OFFSET,
+			zynq_fpga_read(priv, CTRL_OFFSET)
+			& ~CTRL_PCAP_PR_MASK);
+
 	clk_disable(priv->clk);
 
 	if (err)