Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Bard Liao <yung-chuan.liao@linux.intel.com>
To:     alsa-devel@alsa-project.org, vkoul@kernel.org
Cc:     vinod.koul@linaro.org, linux-kernel@vger.kernel.org,
        pierre-louis.bossart@linux.intel.com, bard.liao@intel.com
Subject: [PATCH 1/3] soundwire: intel_bus_common: enable interrupts last
Date:   Thu, 18 May 2023 10:41:17 +0800
Message-Id: <20230518024119.164160-2-yung-chuan.liao@linux.intel.com>
In-Reply-To: <20230518024119.164160-1-yung-chuan.liao@linux.intel.com>
References: <20230518024119.164160-1-yung-chuan.liao@linux.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

It's not clear why we enabled interrupts in the Cadence IP first. The
logical programming sequence should be to first start the bus, and
only second to enable the interrupts.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_bus_common.c | 61 +++++++++++++---------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/drivers/soundwire/intel_bus_common.c b/drivers/soundwire/intel_bus_common.c
index f180e3bea989..901d4f5736c1 100644
--- a/drivers/soundwire/intel_bus_common.c
+++ b/drivers/soundwire/intel_bus_common.c
@@ -16,12 +16,6 @@ int intel_start_bus(struct sdw_intel *sdw)
 	struct sdw_bus *bus = &cdns->bus;
 	int ret;
 
-	ret = sdw_cdns_enable_interrupt(cdns, true);
-	if (ret < 0) {
-		dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret);
-		return ret;
-	}
-
 	/*
 	 * follow recommended programming flows to avoid timeouts when
 	 * gsync is enabled
@@ -32,30 +26,33 @@ int intel_start_bus(struct sdw_intel *sdw)
 	ret = sdw_cdns_init(cdns);
 	if (ret < 0) {
 		dev_err(dev, "%s: unable to initialize Cadence IP: %d\n", __func__, ret);
-		goto err_interrupt;
+		return ret;
 	}
 
 	ret = sdw_cdns_exit_reset(cdns);
 	if (ret < 0) {
 		dev_err(dev, "%s: unable to exit bus reset sequence: %d\n", __func__, ret);
-		goto err_interrupt;
+		return ret;
 	}
 
 	if (bus->multi_link) {
 		ret = sdw_intel_sync_go(sdw);
 		if (ret < 0) {
 			dev_err(dev, "%s: sync go failed: %d\n", __func__, ret);
-			goto err_interrupt;
+			return ret;
 		}
 	}
+
+	ret = sdw_cdns_enable_interrupt(cdns, true);
+	if (ret < 0) {
+		dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret);
+		return ret;
+	}
+
 	sdw_cdns_check_self_clearing_bits(cdns, __func__,
 					  true, INTEL_MASTER_RESET_ITERATIONS);
 
 	return 0;
-
-err_interrupt:
-	sdw_cdns_enable_interrupt(cdns, false);
-	return ret;
 }
 
 int intel_start_bus_after_reset(struct sdw_intel *sdw)
@@ -86,12 +83,6 @@ int intel_start_bus_after_reset(struct sdw_intel *sdw)
 		status = SDW_UNATTACH_REQUEST_MASTER_RESET;
 		sdw_clear_slave_status(bus, status);
 
-		ret = sdw_cdns_enable_interrupt(cdns, true);
-		if (ret < 0) {
-			dev_err(dev, "cannot enable interrupts during resume\n");
-			return ret;
-		}
-
 		/*
 		 * follow recommended programming flows to avoid
 		 * timeouts when gsync is enabled
@@ -115,31 +106,36 @@ int intel_start_bus_after_reset(struct sdw_intel *sdw)
 	ret = sdw_cdns_clock_restart(cdns, !clock_stop0);
 	if (ret < 0) {
 		dev_err(dev, "unable to restart clock during resume\n");
-		goto err_interrupt;
+		if (!clock_stop0)
+			sdw_cdns_enable_interrupt(cdns, false);
+		return ret;
 	}
 
 	if (!clock_stop0) {
 		ret = sdw_cdns_exit_reset(cdns);
 		if (ret < 0) {
 			dev_err(dev, "unable to exit bus reset sequence during resume\n");
-			goto err_interrupt;
+			return ret;
 		}
 
 		if (bus->multi_link) {
 			ret = sdw_intel_sync_go(sdw);
 			if (ret < 0) {
 				dev_err(sdw->cdns.dev, "sync go failed during resume\n");
-				goto err_interrupt;
+				return ret;
 			}
 		}
+
+		ret = sdw_cdns_enable_interrupt(cdns, true);
+		if (ret < 0) {
+			dev_err(dev, "cannot enable interrupts during resume\n");
+			return ret;
+		}
+
 	}
 	sdw_cdns_check_self_clearing_bits(cdns, __func__, true, INTEL_MASTER_RESET_ITERATIONS);
 
 	return 0;
-
-err_interrupt:
-	sdw_cdns_enable_interrupt(cdns, false);
-	return ret;
 }
 
 void intel_check_clock_stop(struct sdw_intel *sdw)
@@ -158,16 +154,15 @@ int intel_start_bus_after_clock_stop(struct sdw_intel *sdw)
 	struct sdw_cdns *cdns = &sdw->cdns;
 	int ret;
 
-	ret = sdw_cdns_enable_interrupt(cdns, true);
-	if (ret < 0) {
-		dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret);
-		return ret;
-	}
-
 	ret = sdw_cdns_clock_restart(cdns, false);
 	if (ret < 0) {
 		dev_err(dev, "%s: unable to restart clock: %d\n", __func__, ret);
-		sdw_cdns_enable_interrupt(cdns, false);
+		return ret;
+	}
+
+	ret = sdw_cdns_enable_interrupt(cdns, true);
+	if (ret < 0) {
+		dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret);
 		return ret;
 	}
 
-- 
2.25.1