diff mbox series

[v2,1/3] dmaengine: dmatest: Prevent to run on misconfigured channel

Message ID 20200922115847.30100-1-andriy.shevchenko@linux.intel.com (mailing list archive)
State Accepted
Headers show
Series [v2,1/3] dmaengine: dmatest: Prevent to run on misconfigured channel | expand

Commit Message

Andy Shevchenko Sept. 22, 2020, 11:58 a.m. UTC 
From: Vladimir Murzin <vladimir.murzin@arm.com>

Andy reported that commit 6b41030fdc79 ("dmaengine: dmatest:
Restore default for channel") broke his scripts for the case
where "busy" channel is used for configuration with expectation
that run command would do nothing. Instead, behavior was
(unintentionally) changed to treat such case as under-configuration
and progress with defaults, i.e. run command would start a test
with default setting for channel (which would use all channels).

Restore original behavior with tracking status of channel setter
so we can distinguish between misconfigured and under-configured
cases in run command and act accordingly.

Fixes: 6b41030fdc79 ("dmaengine: dmatest: Restore default for channel")
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: improve title and the commit message (Vinod), added tag (Peter)
 drivers/dma/dmatest.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

Comments

Vinod Koul Sept. 22, 2020, 2:50 p.m. UTC | #1 
On 22-09-20, 14:58, Andy Shevchenko wrote:
> From: Vladimir Murzin <vladimir.murzin@arm.com>
> 
> Andy reported that commit 6b41030fdc79 ("dmaengine: dmatest:
> Restore default for channel") broke his scripts for the case
> where "busy" channel is used for configuration with expectation
> that run command would do nothing. Instead, behavior was
> (unintentionally) changed to treat such case as under-configuration
> and progress with defaults, i.e. run command would start a test
> with default setting for channel (which would use all channels).
> 
> Restore original behavior with tracking status of channel setter
> so we can distinguish between misconfigured and under-configured
> cases in run command and act accordingly.

Applied all, thanks
diff mbox series

Patch

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index b2790641370a..ac28d5eea913 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -129,6 +129,7 @@  struct dmatest_params {
  * @nr_channels:	number of channels under test
  * @lock:		access protection to the fields of this structure
  * @did_init:		module has been initialized completely
+ * @last_error:		test has faced configuration issues
  */
 static struct dmatest_info {
 	/* Test parameters */
@@ -137,6 +138,7 @@  static struct dmatest_info {
 	/* Internal state */
 	struct list_head	channels;
 	unsigned int		nr_channels;
+	int			last_error;
 	struct mutex		lock;
 	bool			did_init;
 } test_info = {
@@ -1202,10 +1204,22 @@  static int dmatest_run_set(const char *val, const struct kernel_param *kp)
 		return ret;
 	} else if (dmatest_run) {
 		if (!is_threaded_test_pending(info)) {
-			pr_info("No channels configured, continue with any\n");
-			if (!is_threaded_test_run(info))
-				stop_threaded_test(info);
-			add_threaded_test(info);
+			/*
+			 * We have nothing to run. This can be due to:
+			 */
+			ret = info->last_error;
+			if (ret) {
+				/* 1) Misconfiguration */
+				pr_err("Channel misconfigured, can't continue\n");
+				mutex_unlock(&info->lock);
+				return ret;
+			} else {
+				/* 2) We rely on defaults */
+				pr_info("No channels configured, continue with any\n");
+				if (!is_threaded_test_run(info))
+					stop_threaded_test(info);
+				add_threaded_test(info);
+			}
 		}
 		start_threaded_tests(info);
 	} else {
@@ -1222,7 +1236,7 @@  static int dmatest_chan_set(const char *val, const struct kernel_param *kp)
 	struct dmatest_info *info = &test_info;
 	struct dmatest_chan *dtc;
 	char chan_reset_val[20];
-	int ret = 0;
+	int ret;
 
 	mutex_lock(&info->lock);
 	ret = param_set_copystring(val, kp);
@@ -1277,12 +1291,14 @@  static int dmatest_chan_set(const char *val, const struct kernel_param *kp)
 		goto add_chan_err;
 	}
 
+	info->last_error = ret;
 	mutex_unlock(&info->lock);
 
 	return ret;
 
 add_chan_err:
 	param_set_copystring(chan_reset_val, kp);
+	info->last_error = ret;
 	mutex_unlock(&info->lock);
 
 	return ret;