[PATCH v2] fpga: altera-cvp: Retry teardown and reset CVP state on failure

muhammad.nazim.amirul.nazle.asmade@altera.com posted 1 patch 1 week, 3 days ago
drivers/fpga/altera-cvp.c | 38 +++++++++++++++++++++++++++++++++++---
1 file changed, 35 insertions(+), 3 deletions(-)
[PATCH v2] fpga: altera-cvp: Retry teardown and reset CVP state on failure
Posted by muhammad.nazim.amirul.nazle.asmade@altera.com 1 week, 3 days ago
From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

The driver polls CFG_RDY for 0 as notification that the endpoint has
finished teardown and a new session can start safely. After a bad
bitstream that poll can time out: CFG_RDY may stay stuck when
HIP_CLK_SEL=0 if the device has already left usermode before the poll.

Retrying teardown alone is not enough. Exit CvP mode and switch the HIP
clock that feeds CFG_RDY so CVP_STATUS becomes responsive again. The
second teardown is more than a plain retry: it runs after that CVP_MODE
clear and clock switch. One such retry is sufficient.

Introduce altera_cvp_recovery() to wrap this cleanup and single retry.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
v1: https://lore.kernel.org/linux-fpga/20260618112410.303-1-muhammad.nazim.amirul.nazle.asmade@altera.com/
v2:
- Rewrite commit message to explain CFG_RDY timeout, HIP clock switch,
  and why one retry is enough
- Move CvP mode/HIP clock cleanup into recovery via
  altera_cvp_disable_cvp_mode() helper
- Reduce retries to 1 (first teardown + one retry after cleanup)
- Use dev_warn for per-teardown CFG_RDY timeout; one final
  success/failure message from recovery

 drivers/fpga/altera-cvp.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index 44badfd11e1b..ce9d2ed68b55 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -63,6 +63,7 @@
 #define ALTERA_CVP_V1_SIZE	4
 #define ALTERA_CVP_V2_SIZE	4096
 
+#define CVP_TEARDOWN_MAX_RETRY 1
 /* Optional CvP config error status check for debugging */
 static bool altera_cvp_chkcfg;
 
@@ -306,7 +307,38 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
 	ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
 				     conf->priv->poll_time_us);
 	if (ret)
-		dev_err(&mgr->dev, "CFG_RDY == 0 timeout\n");
+		dev_warn(&mgr->dev, "CFG_RDY == 0 timeout\n");
+
+	return ret;
+}
+
+static void altera_cvp_disable_cvp_mode(struct altera_cvp_conf *conf)
+{
+	u32 val;
+
+	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+	val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
+	val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
+	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+}
+
+static int altera_cvp_recovery(struct fpga_manager *mgr,
+			       struct fpga_image_info *info)
+{
+	struct altera_cvp_conf *conf = mgr->priv;
+	int ret, retry;
+
+	ret = altera_cvp_teardown(mgr, info);
+	for (retry = 0; ret && retry < CVP_TEARDOWN_MAX_RETRY; retry++) {
+		altera_cvp_disable_cvp_mode(conf);
+		ret = altera_cvp_teardown(mgr, info);
+	}
+
+	if (ret)
+		dev_err(&mgr->dev, "Tear-down failed after %d retries\n",
+			CVP_TEARDOWN_MAX_RETRY);
+	else
+		dev_info(&mgr->dev, "Tear-down successful\n");
 
 	return ret;
 }
@@ -343,7 +375,7 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
 
 	if (val & VSE_CVP_STATUS_CFG_RDY) {
 		dev_warn(&mgr->dev, "CvP already started, tear down first\n");
-		ret = altera_cvp_teardown(mgr, info);
+		ret = altera_cvp_recovery(mgr, info);
 		if (ret)
 			return ret;
 	}
@@ -484,7 +516,7 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
 	u32 mask, val;
 	int ret;
 
-	ret = altera_cvp_teardown(mgr, info);
+	ret = altera_cvp_recovery(mgr, info);
 	if (ret)
 		return ret;
 
-- 
2.43.7
Re: [PATCH v2] fpga: altera-cvp: Retry teardown and reset CVP state on failure
Posted by Xu Yilun 6 days, 15 hours ago
> +static void altera_cvp_disable_cvp_mode(struct altera_cvp_conf *conf)
> +{
> +	u32 val;
> +
> +	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
> +	val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
> +	val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
> +	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
> +}

Use the helper for existing code, or there are still duplications.
Split a preparatory patch for this.
Re: [PATCH v2] fpga: altera-cvp: Retry teardown and reset CVP state on failure
Posted by Nazle Asmade, Muhammad Nazim Amirul 2 days, 2 hours ago
On 19/9/2026 12:53 am, Xu Yilun wrote:
>> +static void altera_cvp_disable_cvp_mode(struct altera_cvp_conf *conf)
>> +{
>> +	u32 val;
>> +
>> +	altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
>> +	val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
>> +	val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
>> +	altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
>> +}
> Use the helper for existing code, or there are still duplications.
> Split a preparatory patch for this.


Hi Yilun,

I send out a series patch which cover the main teardown and helper.

https://lore.kernel.org/all/20260923053233.10890-1-muhammad.nazim.amirul.nazle.asmade@altera.com/

BR,

Nazim
Re: [PATCH v2] fpga: altera-cvp: Retry teardown and reset CVP state on failure
Posted by Xu Yilun 6 days, 15 hours ago
> +static int altera_cvp_recovery(struct fpga_manager *mgr,
> +			       struct fpga_image_info *info)
> +{
> +	struct altera_cvp_conf *conf = mgr->priv;
> +	int ret, retry;
> +
> +	ret = altera_cvp_teardown(mgr, info);
> +	for (retry = 0; ret && retry < CVP_TEARDOWN_MAX_RETRY; retry++) {
> +		altera_cvp_disable_cvp_mode(conf);
> +		ret = altera_cvp_teardown(mgr, info);
> +	}

If one retry is sufficient, no need to define CVP_TEARDOWN_MAX_RETRY, no
need to loop.

	if (ret) {
		/*
		 * IIUC, say something like CVP_STATUS may stuck and you
		 * re-activate the CVP_STATUS then try teardown again.
		 * Please re-org the word.
		 */
		altera_cvp_disable_cvp_mode(conf);
		ret = altera_cvp_teardown(mgr, info);
	}

> +
> +	if (ret)
> +		dev_err(&mgr->dev, "Tear-down failed after %d retries\n",
> +			CVP_TEARDOWN_MAX_RETRY);

You always retry once when failure, no need to tell I've retried.

> +	else
> +		dev_info(&mgr->dev, "Tear-down successful\n");

Only speak up when something goes wrong or suspicious.

>  
>  	return ret;
>  }