[PATCH 00/11] soundwire: intel: cleanups/fixes before code reorganization

[PATCH 00/11] soundwire: intel: cleanups/fixes before code reorganization

[Bard Liao]

This is a really big cleanup to introduce well defined helpers, prior to
a larger code reorganization.

Pierre-Louis Bossart (11):
  soundwire: intel: fix error handling on dai registration issues
  soundwire: intel: simplify flow and use devm_ for DAI registration
  soundwire: intel: move DAI registration and debugfs init earlier
  soundwire: intel: move all PDI initialization under
    intel_register_dai()
  soundwire: intel: remove clock_stop parameter in intel_shim_init()
  soundwire: intel: move shim initialization before power up/down
  soundwire: intel: remove intel_init() wrapper
  soundwire: intel: simplify read ops assignment
  soundwire: intel: introduce intel_shim_check_wake() helper
  soundwire: intel: introduce helpers to start bus
  soundwire: intel: add helper to stop bus

 drivers/soundwire/intel.c | 775 +++++++++++++++++++-------------------
 1 file changed, 380 insertions(+), 395 deletions(-)

-- 
2.25.1

[PATCH 01/11] soundwire: intel: fix error handling on dai registration issues

[Bard Liao]

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

The call to intel_register_dai() may fail because of memory allocation
issues or problems reported by the ASoC core. In all cases, when a
error is thrown the component is not registered, it's invalid to
unregister it.

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.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 01be62fa6c83..3bb29bc00d5a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1432,7 +1432,6 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	ret = intel_register_dai(sdw);
 	if (ret) {
 		dev_err(dev, "DAI registration failed: %d\n", ret);
-		snd_soc_unregister_component(dev);
 		goto err_interrupt;
 	}
 
-- 
2.25.1

[PATCH 02/11] soundwire: intel: simplify flow and use devm_ for DAI registration

[Bard Liao]

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

We already use devm_ for memory allocation but not for component/DAI
registration. The resource management can be based on devm_ in all
cases.

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.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 3bb29bc00d5a..a6fe91f2d964 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1203,8 +1203,8 @@ static int intel_register_dai(struct sdw_intel *sdw)
 	if (ret)
 		return ret;
 
-	return snd_soc_register_component(cdns->dev, &dai_component,
-					  dais, num_dai);
+	return devm_snd_soc_register_component(cdns->dev, &dai_component,
+					       dais, num_dai);
 }
 
 static int sdw_master_read_intel_prop(struct sdw_bus *bus)
@@ -1489,7 +1489,6 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 
 static void intel_link_remove(struct auxiliary_device *auxdev)
 {
-	struct device *dev = &auxdev->dev;
 	struct sdw_cdns *cdns = auxiliary_get_drvdata(auxdev);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
 	struct sdw_bus *bus = &cdns->bus;
@@ -1502,7 +1501,6 @@ static void intel_link_remove(struct auxiliary_device *auxdev)
 	if (!bus->prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
 		sdw_cdns_enable_interrupt(cdns, false);
-		snd_soc_unregister_component(dev);
 	}
 	sdw_bus_master_delete(bus);
 }
-- 
2.25.1

[PATCH 03/11] soundwire: intel: move DAI registration and debugfs init earlier

[Bard Liao]

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

These two steps can and should be done before starting up the clock
and the bus operation. This is a first step before re-grouping
functionality in well-defined callbacks.

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.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index a6fe91f2d964..d7852cc7dd96 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1393,6 +1393,15 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 
 	intel_pdi_ch_update(sdw);
 
+	/* Register DAIs */
+	ret = intel_register_dai(sdw);
+	if (ret) {
+		dev_err(dev, "DAI registration failed: %d\n", ret);
+		goto err_init;
+	}
+
+	intel_debugfs_init(sdw);
+
 	ret = sdw_cdns_enable_interrupt(cdns, true);
 	if (ret < 0) {
 		dev_err(dev, "cannot enable interrupts\n");
@@ -1428,15 +1437,6 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	sdw_cdns_check_self_clearing_bits(cdns, __func__,
 					  true, INTEL_MASTER_RESET_ITERATIONS);
 
-	/* Register DAIs */
-	ret = intel_register_dai(sdw);
-	if (ret) {
-		dev_err(dev, "DAI registration failed: %d\n", ret);
-		goto err_interrupt;
-	}
-
-	intel_debugfs_init(sdw);
-
 	/* Enable runtime PM */
 	if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) {
 		pm_runtime_set_autosuspend_delay(dev,
-- 
2.25.1

[PATCH 04/11] soundwire: intel: move all PDI initialization under intel_register_dai()

[Bard Liao]

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

Start regrouping functionality in high-level functions.

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.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index d7852cc7dd96..676de04c5070 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1171,11 +1171,20 @@ static int intel_create_dai(struct sdw_cdns *cdns,
 
 static int intel_register_dai(struct sdw_intel *sdw)
 {
+	struct sdw_cdns_stream_config config;
 	struct sdw_cdns *cdns = &sdw->cdns;
 	struct sdw_cdns_streams *stream;
 	struct snd_soc_dai_driver *dais;
 	int num_dai, ret, off = 0;
 
+	/* Read the PDI config and initialize cadence PDI */
+	intel_pdi_init(sdw, &config);
+	ret = sdw_cdns_pdi_init(cdns, config);
+	if (ret)
+		return ret;
+
+	intel_pdi_ch_update(sdw);
+
 	/* DAIs are created based on total number of PDIs supported */
 	num_dai = cdns->pcm.num_pdi;
 
@@ -1347,7 +1356,6 @@ static int intel_link_probe(struct auxiliary_device *auxdev,
 
 int intel_link_startup(struct auxiliary_device *auxdev)
 {
-	struct sdw_cdns_stream_config config;
 	struct device *dev = &auxdev->dev;
 	struct sdw_cdns *cdns = auxiliary_get_drvdata(auxdev);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
@@ -1385,14 +1393,6 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	if (ret)
 		goto err_init;
 
-	/* Read the PDI config and initialize cadence PDI */
-	intel_pdi_init(sdw, &config);
-	ret = sdw_cdns_pdi_init(cdns, config);
-	if (ret)
-		goto err_init;
-
-	intel_pdi_ch_update(sdw);
-
 	/* Register DAIs */
 	ret = intel_register_dai(sdw);
 	if (ret) {
-- 
2.25.1

[PATCH 05/11] soundwire: intel: remove clock_stop parameter in intel_shim_init()

[Bard Liao]

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

Simplify signature before further code reshuffling.

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.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 676de04c5070..2d828d98e153 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -391,7 +391,7 @@ static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw)
 	/* at this point Integration Glue has full control of the I/Os */
 }
 
-static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop)
+static int intel_shim_init(struct sdw_intel *sdw)
 {
 	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
@@ -1277,14 +1277,10 @@ static struct sdw_master_ops sdw_intel_ops = {
 
 static int intel_init(struct sdw_intel *sdw)
 {
-	bool clock_stop;
-
 	/* Initialize shim and controller */
 	intel_link_power_up(sdw);
 
-	clock_stop = sdw_cdns_is_clock_stop(&sdw->cdns);
-
-	intel_shim_init(sdw, clock_stop);
+	intel_shim_init(sdw);
 
 	return 0;
 }
-- 
2.25.1

[PATCH 06/11] soundwire: intel: move shim initialization before power up/down

[Bard Liao]

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

Move code around before additional simplification. No functionality
change.

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.c | 231 +++++++++++++++++++-------------------
 1 file changed, 115 insertions(+), 116 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 2d828d98e153..140cf36eb407 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -260,6 +260,121 @@ static void intel_debugfs_exit(struct sdw_intel *sdw) {}
 /*
  * shim ops
  */
+/* this needs to be called with shim_lock */
+static void intel_shim_glue_to_master_ip(struct sdw_intel *sdw)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	unsigned int link_id = sdw->instance;
+	u16 ioctl;
+
+	/* Switch to MIP from Glue logic */
+	ioctl = intel_readw(shim,  SDW_SHIM_IOCTL(link_id));
+
+	ioctl &= ~(SDW_SHIM_IOCTL_DOE);
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl &= ~(SDW_SHIM_IOCTL_DO);
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= (SDW_SHIM_IOCTL_MIF);
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl &= ~(SDW_SHIM_IOCTL_BKE);
+	ioctl &= ~(SDW_SHIM_IOCTL_COE);
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	/* at this point Master IP has full control of the I/Os */
+}
+
+/* this needs to be called with shim_lock */
+static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw)
+{
+	unsigned int link_id = sdw->instance;
+	void __iomem *shim = sdw->link_res->shim;
+	u16 ioctl;
+
+	/* Glue logic */
+	ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id));
+	ioctl |= SDW_SHIM_IOCTL_BKE;
+	ioctl |= SDW_SHIM_IOCTL_COE;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl &= ~(SDW_SHIM_IOCTL_MIF);
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	/* at this point Integration Glue has full control of the I/Os */
+}
+
+static int intel_shim_init(struct sdw_intel *sdw)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	unsigned int link_id = sdw->instance;
+	int ret = 0;
+	u16 ioctl = 0, act = 0;
+
+	mutex_lock(sdw->link_res->shim_lock);
+
+	/* Initialize Shim */
+	ioctl |= SDW_SHIM_IOCTL_BKE;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= SDW_SHIM_IOCTL_WPDD;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= SDW_SHIM_IOCTL_DO;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	ioctl |= SDW_SHIM_IOCTL_DOE;
+	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
+	usleep_range(10, 15);
+
+	intel_shim_glue_to_master_ip(sdw);
+
+	u16p_replace_bits(&act, 0x1, SDW_SHIM_CTMCTL_DOAIS);
+	act |= SDW_SHIM_CTMCTL_DACTQE;
+	act |= SDW_SHIM_CTMCTL_DODS;
+	intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act);
+	usleep_range(10, 15);
+
+	mutex_unlock(sdw->link_res->shim_lock);
+
+	return ret;
+}
+
+static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
+{
+	void __iomem *shim = sdw->link_res->shim;
+	unsigned int link_id = sdw->instance;
+	u16 wake_en, wake_sts;
+
+	mutex_lock(sdw->link_res->shim_lock);
+	wake_en = intel_readw(shim, SDW_SHIM_WAKEEN);
+
+	if (wake_enable) {
+		/* Enable the wakeup */
+		wake_en |= (SDW_SHIM_WAKEEN_ENABLE << link_id);
+		intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
+	} else {
+		/* Disable the wake up interrupt */
+		wake_en &= ~(SDW_SHIM_WAKEEN_ENABLE << link_id);
+		intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
+
+		/* Clear wake status */
+		wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS);
+		wake_sts |= (SDW_SHIM_WAKESTS_STATUS << link_id);
+		intel_writew(shim, SDW_SHIM_WAKESTS, wake_sts);
+	}
+	mutex_unlock(sdw->link_res->shim_lock);
+}
 
 static int intel_link_power_up(struct sdw_intel *sdw)
 {
@@ -340,122 +455,6 @@ static int intel_link_power_up(struct sdw_intel *sdw)
 	return ret;
 }
 
-/* this needs to be called with shim_lock */
-static void intel_shim_glue_to_master_ip(struct sdw_intel *sdw)
-{
-	void __iomem *shim = sdw->link_res->shim;
-	unsigned int link_id = sdw->instance;
-	u16 ioctl;
-
-	/* Switch to MIP from Glue logic */
-	ioctl = intel_readw(shim,  SDW_SHIM_IOCTL(link_id));
-
-	ioctl &= ~(SDW_SHIM_IOCTL_DOE);
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl &= ~(SDW_SHIM_IOCTL_DO);
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl |= (SDW_SHIM_IOCTL_MIF);
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl &= ~(SDW_SHIM_IOCTL_BKE);
-	ioctl &= ~(SDW_SHIM_IOCTL_COE);
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	/* at this point Master IP has full control of the I/Os */
-}
-
-/* this needs to be called with shim_lock */
-static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw)
-{
-	unsigned int link_id = sdw->instance;
-	void __iomem *shim = sdw->link_res->shim;
-	u16 ioctl;
-
-	/* Glue logic */
-	ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id));
-	ioctl |= SDW_SHIM_IOCTL_BKE;
-	ioctl |= SDW_SHIM_IOCTL_COE;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl &= ~(SDW_SHIM_IOCTL_MIF);
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	/* at this point Integration Glue has full control of the I/Os */
-}
-
-static int intel_shim_init(struct sdw_intel *sdw)
-{
-	void __iomem *shim = sdw->link_res->shim;
-	unsigned int link_id = sdw->instance;
-	int ret = 0;
-	u16 ioctl = 0, act = 0;
-
-	mutex_lock(sdw->link_res->shim_lock);
-
-	/* Initialize Shim */
-	ioctl |= SDW_SHIM_IOCTL_BKE;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl |= SDW_SHIM_IOCTL_WPDD;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl |= SDW_SHIM_IOCTL_DO;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	ioctl |= SDW_SHIM_IOCTL_DOE;
-	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
-	usleep_range(10, 15);
-
-	intel_shim_glue_to_master_ip(sdw);
-
-	u16p_replace_bits(&act, 0x1, SDW_SHIM_CTMCTL_DOAIS);
-	act |= SDW_SHIM_CTMCTL_DACTQE;
-	act |= SDW_SHIM_CTMCTL_DODS;
-	intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act);
-	usleep_range(10, 15);
-
-	mutex_unlock(sdw->link_res->shim_lock);
-
-	return ret;
-}
-
-static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
-{
-	void __iomem *shim = sdw->link_res->shim;
-	unsigned int link_id = sdw->instance;
-	u16 wake_en, wake_sts;
-
-	mutex_lock(sdw->link_res->shim_lock);
-	wake_en = intel_readw(shim, SDW_SHIM_WAKEEN);
-
-	if (wake_enable) {
-		/* Enable the wakeup */
-		wake_en |= (SDW_SHIM_WAKEEN_ENABLE << link_id);
-		intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
-	} else {
-		/* Disable the wake up interrupt */
-		wake_en &= ~(SDW_SHIM_WAKEEN_ENABLE << link_id);
-		intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
-
-		/* Clear wake status */
-		wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS);
-		wake_sts |= (SDW_SHIM_WAKESTS_STATUS << link_id);
-		intel_writew(shim, SDW_SHIM_WAKESTS, wake_sts);
-	}
-	mutex_unlock(sdw->link_res->shim_lock);
-}
-
 static int intel_link_power_down(struct sdw_intel *sdw)
 {
 	u32 link_control, spa_mask, cpa_mask;
-- 
2.25.1

[PATCH 07/11] soundwire: intel: remove intel_init() wrapper

[Bard Liao]

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

We can directly call intel_link_power_up and do power_up+shim_init in
the same function. This simplifies the code with a better symmetry
between power_up and power_down operations.

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.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 140cf36eb407..4eeb2b5c1594 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -311,15 +311,13 @@ static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw)
 	/* at this point Integration Glue has full control of the I/Os */
 }
 
-static int intel_shim_init(struct sdw_intel *sdw)
+/* this needs to be called with shim_lock */
+static void intel_shim_init(struct sdw_intel *sdw)
 {
 	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
-	int ret = 0;
 	u16 ioctl = 0, act = 0;
 
-	mutex_lock(sdw->link_res->shim_lock);
-
 	/* Initialize Shim */
 	ioctl |= SDW_SHIM_IOCTL_BKE;
 	intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
@@ -344,10 +342,6 @@ static int intel_shim_init(struct sdw_intel *sdw)
 	act |= SDW_SHIM_CTMCTL_DODS;
 	intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act);
 	usleep_range(10, 15);
-
-	mutex_unlock(sdw->link_res->shim_lock);
-
-	return ret;
 }
 
 static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
@@ -449,6 +443,9 @@ static int intel_link_power_up(struct sdw_intel *sdw)
 	*shim_mask |= BIT(link_id);
 
 	sdw->cdns.link_up = true;
+
+	intel_shim_init(sdw);
+
 out:
 	mutex_unlock(sdw->link_res->shim_lock);
 
@@ -1274,16 +1271,6 @@ static struct sdw_master_ops sdw_intel_ops = {
 	.post_bank_switch = intel_post_bank_switch,
 };
 
-static int intel_init(struct sdw_intel *sdw)
-{
-	/* Initialize shim and controller */
-	intel_link_power_up(sdw);
-
-	intel_shim_init(sdw);
-
-	return 0;
-}
-
 /*
  * probe and init (aux_dev_id argument is required by function prototype but not used)
  */
@@ -1384,7 +1371,7 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	}
 
 	/* Initialize shim, controller */
-	ret = intel_init(sdw);
+	ret = intel_link_power_up(sdw);
 	if (ret)
 		goto err_init;
 
@@ -1773,7 +1760,7 @@ static int __maybe_unused intel_resume(struct device *dev)
 			pm_runtime_idle(dev);
 	}
 
-	ret = intel_init(sdw);
+	ret = intel_link_power_up(sdw);
 	if (ret) {
 		dev_err(dev, "%s failed: %d\n", __func__, ret);
 		return ret;
@@ -1862,7 +1849,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 	clock_stop_quirks = sdw->link_res->clock_stop_quirks;
 
 	if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
-		ret = intel_init(sdw);
+		ret = intel_link_power_up(sdw);
 		if (ret) {
 			dev_err(dev, "%s failed: %d\n", __func__, ret);
 			return ret;
@@ -1910,7 +1897,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 						  true, INTEL_MASTER_RESET_ITERATIONS);
 
 	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
-		ret = intel_init(sdw);
+		ret = intel_link_power_up(sdw);
 		if (ret) {
 			dev_err(dev, "%s failed: %d\n", __func__, ret);
 			return ret;
@@ -1991,7 +1978,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 		if (!clock_stop0)
 			dev_err(dev, "%s invalid configuration, clock was not stopped", __func__);
 
-		ret = intel_init(sdw);
+		ret = intel_link_power_up(sdw);
 		if (ret) {
 			dev_err(dev, "%s failed: %d\n", __func__, ret);
 			return ret;
-- 
2.25.1

[PATCH 08/11] soundwire: intel: simplify read ops assignment

[Bard Liao]

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

We can assign the right callback directly in the ops structure. No
functionality change.

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.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 4eeb2b5c1594..66aab02b9323 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1261,7 +1261,7 @@ static int intel_prop_read(struct sdw_bus *bus)
 }
 
 static struct sdw_master_ops sdw_intel_ops = {
-	.read_prop = sdw_master_read_prop,
+	.read_prop = intel_prop_read,
 	.override_adr = sdw_dmi_override_adr,
 	.xfer_msg = cdns_xfer_msg,
 	.xfer_msg_defer = cdns_xfer_msg_defer,
@@ -1304,8 +1304,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev,
 
 	sdw_cdns_probe(cdns);
 
-	/* Set property read ops */
-	sdw_intel_ops.read_prop = intel_prop_read;
+	/* Set ops */
 	bus->ops = &sdw_intel_ops;
 
 	/* set driver data, accessed by snd_soc_dai_get_drvdata() */
-- 
2.25.1

[PATCH 09/11] soundwire: intel: introduce intel_shim_check_wake() helper

[Bard Liao]

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

Add new helper before code partitioning in order to avoid direct read
from specific register. No functionality change.

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.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 66aab02b9323..2ca924622153 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -344,6 +344,17 @@ static void intel_shim_init(struct sdw_intel *sdw)
 	usleep_range(10, 15);
 }
 
+static int intel_shim_check_wake(struct sdw_intel *sdw)
+{
+	void __iomem *shim;
+	u16 wake_sts;
+
+	shim = sdw->link_res->shim;
+	wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS);
+
+	return wake_sts & BIT(sdw->instance);
+}
+
 static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
 {
 	void __iomem *shim = sdw->link_res->shim;
@@ -1491,8 +1502,6 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
 	struct device *dev = &auxdev->dev;
 	struct sdw_intel *sdw;
 	struct sdw_bus *bus;
-	void __iomem *shim;
-	u16 wake_sts;
 
 	sdw = auxiliary_get_drvdata(auxdev);
 	bus = &sdw->cdns.bus;
@@ -1503,10 +1512,7 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
 		return 0;
 	}
 
-	shim = sdw->link_res->shim;
-	wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS);
-
-	if (!(wake_sts & BIT(sdw->instance)))
+	if (!intel_shim_check_wake(sdw))
 		return 0;
 
 	/* disable WAKEEN interrupt ASAP to prevent interrupt flood */
-- 
2.25.1

[PATCH 10/11] soundwire: intel: introduce helpers to start bus

[Bard Liao]

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

There are 3 different sequences to start the bus, let's move the
functionality to helpers.

There should be no functionality change, except in error cases where
the flow is improved with more consistent disabling of interrupts and
powering down.

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.c | 388 +++++++++++++++++++-------------------
 1 file changed, 195 insertions(+), 193 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 2ca924622153..abe14436d874 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1223,6 +1223,174 @@ static int intel_register_dai(struct sdw_intel *sdw)
 					       dais, num_dai);
 }
 
+static int intel_start_bus(struct sdw_intel *sdw)
+{
+	struct device *dev = sdw->cdns.dev;
+	struct sdw_cdns *cdns = &sdw->cdns;
+	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
+	 */
+	if (bus->multi_link)
+		intel_shim_sync_arm(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;
+	}
+
+	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;
+	}
+
+	if (bus->multi_link) {
+		ret = intel_shim_sync_go(sdw);
+		if (ret < 0) {
+			dev_err(dev, "%s: sync go failed: %d\n", __func__, ret);
+			goto err_interrupt;
+		}
+	}
+	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;
+}
+
+static int intel_start_bus_after_reset(struct sdw_intel *sdw)
+{
+	struct device *dev = sdw->cdns.dev;
+	struct sdw_cdns *cdns = &sdw->cdns;
+	struct sdw_bus *bus = &cdns->bus;
+	bool clock_stop0;
+	int status;
+	int ret;
+
+	/*
+	 * An exception condition occurs for the CLK_STOP_BUS_RESET
+	 * case if one or more masters remain active. In this condition,
+	 * all the masters are powered on for they are in the same power
+	 * domain. Master can preserve its context for clock stop0, so
+	 * there is no need to clear slave status and reset bus.
+	 */
+	clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);
+
+	if (!clock_stop0) {
+
+		/*
+		 * make sure all Slaves are tagged as UNATTACHED and
+		 * provide reason for reinitialization
+		 */
+
+		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
+		 */
+		if (bus->multi_link)
+			intel_shim_sync_arm(sdw);
+
+		/*
+		 * Re-initialize the IP since it was powered-off
+		 */
+		sdw_cdns_init(&sdw->cdns);
+
+	} else {
+		ret = sdw_cdns_enable_interrupt(cdns, true);
+		if (ret < 0) {
+			dev_err(dev, "cannot enable interrupts during resume\n");
+			return ret;
+		}
+	}
+
+	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) {
+		ret = sdw_cdns_exit_reset(cdns);
+		if (ret < 0) {
+			dev_err(dev, "unable to exit bus reset sequence during resume\n");
+			goto err_interrupt;
+		}
+
+		if (bus->multi_link) {
+			ret = intel_shim_sync_go(sdw);
+			if (ret < 0) {
+				dev_err(sdw->cdns.dev, "sync go failed during resume\n");
+				goto err_interrupt;
+			}
+		}
+	}
+	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;
+}
+
+static void intel_check_clock_stop(struct sdw_intel *sdw)
+{
+	struct device *dev = sdw->cdns.dev;
+	bool clock_stop0;
+
+	clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);
+	if (!clock_stop0)
+		dev_err(dev, "%s: invalid configuration, clock was not stopped\n", __func__);
+}
+
+static int intel_start_bus_after_clock_stop(struct sdw_intel *sdw)
+{
+	struct device *dev = sdw->cdns.dev;
+	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;
+	}
+
+	sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime no_quirks",
+					  true, INTEL_MASTER_RESET_ITERATIONS);
+
+	return 0;
+}
+
 static int sdw_master_read_intel_prop(struct sdw_bus *bus)
 {
 	struct sdw_master_prop *prop = &bus->prop;
@@ -1368,7 +1536,6 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK);
 	if (!multi_link) {
 		dev_dbg(dev, "Multi-link is disabled\n");
-		bus->multi_link = false;
 	} else {
 		/*
 		 * hardware-based synchronization is required regardless
@@ -1376,9 +1543,9 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 		 * synchronization is gated by gsync when the multi-master
 		 * mode is set.
 		 */
-		bus->multi_link = true;
 		bus->hw_sync_min_links = 1;
 	}
+	bus->multi_link = multi_link;
 
 	/* Initialize shim, controller */
 	ret = intel_link_power_up(sdw);
@@ -1389,46 +1556,18 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	ret = intel_register_dai(sdw);
 	if (ret) {
 		dev_err(dev, "DAI registration failed: %d\n", ret);
-		goto err_init;
+		goto err_power_up;
 	}
 
 	intel_debugfs_init(sdw);
 
-	ret = sdw_cdns_enable_interrupt(cdns, true);
-	if (ret < 0) {
-		dev_err(dev, "cannot enable interrupts\n");
-		goto err_init;
+	/* start bus */
+	ret = intel_start_bus(sdw);
+	if (ret) {
+		dev_err(dev, "bus start failed: %d\n", ret);
+		goto err_power_up;
 	}
 
-	/*
-	 * follow recommended programming flows to avoid timeouts when
-	 * gsync is enabled
-	 */
-	if (multi_link)
-		intel_shim_sync_arm(sdw);
-
-	ret = sdw_cdns_init(cdns);
-	if (ret < 0) {
-		dev_err(dev, "unable to initialize Cadence IP\n");
-		goto err_interrupt;
-	}
-
-	ret = sdw_cdns_exit_reset(cdns);
-	if (ret < 0) {
-		dev_err(dev, "unable to exit bus reset sequence\n");
-		goto err_interrupt;
-	}
-
-	if (multi_link) {
-		ret = intel_shim_sync_go(sdw);
-		if (ret < 0) {
-			dev_err(dev, "sync go failed: %d\n", ret);
-			goto err_interrupt;
-		}
-	}
-	sdw_cdns_check_self_clearing_bits(cdns, __func__,
-					  true, INTEL_MASTER_RESET_ITERATIONS);
-
 	/* Enable runtime PM */
 	if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) {
 		pm_runtime_set_autosuspend_delay(dev,
@@ -1473,8 +1612,8 @@ int intel_link_startup(struct auxiliary_device *auxdev)
 	sdw->startup_done = true;
 	return 0;
 
-err_interrupt:
-	sdw_cdns_enable_interrupt(cdns, false);
+err_power_up:
+	intel_link_power_down(sdw);
 err_init:
 	return ret;
 }
@@ -1738,7 +1877,6 @@ static int __maybe_unused intel_resume(struct device *dev)
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
 	struct sdw_bus *bus = &cdns->bus;
 	int link_flags;
-	bool multi_link;
 	int ret;
 
 	if (bus->prop.hw_disabled || !sdw->startup_done) {
@@ -1748,7 +1886,6 @@ static int __maybe_unused intel_resume(struct device *dev)
 	}
 
 	link_flags = md_flags >> (bus->link_id * 8);
-	multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK);
 
 	if (pm_runtime_suspended(dev)) {
 		dev_dbg(dev, "pm_runtime status was suspended, forcing active\n");
@@ -1777,41 +1914,13 @@ static int __maybe_unused intel_resume(struct device *dev)
 	 */
 	sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
 
-	ret = sdw_cdns_enable_interrupt(cdns, true);
+	ret = intel_start_bus(sdw);
 	if (ret < 0) {
-		dev_err(dev, "cannot enable interrupts during resume\n");
+		dev_err(dev, "cannot start bus during resume\n");
+		intel_link_power_down(sdw);
 		return ret;
 	}
 
-	/*
-	 * follow recommended programming flows to avoid timeouts when
-	 * gsync is enabled
-	 */
-	if (multi_link)
-		intel_shim_sync_arm(sdw);
-
-	ret = sdw_cdns_init(&sdw->cdns);
-	if (ret < 0) {
-		dev_err(dev, "unable to initialize Cadence IP during resume\n");
-		return ret;
-	}
-
-	ret = sdw_cdns_exit_reset(cdns);
-	if (ret < 0) {
-		dev_err(dev, "unable to exit bus reset sequence during resume\n");
-		return ret;
-	}
-
-	if (multi_link) {
-		ret = intel_shim_sync_go(sdw);
-		if (ret < 0) {
-			dev_err(dev, "sync go failed during resume\n");
-			return ret;
-		}
-	}
-	sdw_cdns_check_self_clearing_bits(cdns, __func__,
-					  true, INTEL_MASTER_RESET_ITERATIONS);
-
 	/*
 	 * after system resume, the pm_runtime suspend() may kick in
 	 * during the enumeration, before any children device force the
@@ -1824,7 +1933,7 @@ static int __maybe_unused intel_resume(struct device *dev)
 	 */
 	pm_runtime_mark_last_busy(dev);
 
-	return ret;
+	return 0;
 }
 
 static int __maybe_unused intel_resume_runtime(struct device *dev)
@@ -1833,10 +1942,6 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
 	struct sdw_bus *bus = &cdns->bus;
 	u32 clock_stop_quirks;
-	bool clock_stop0;
-	int link_flags;
-	bool multi_link;
-	int status;
 	int ret;
 
 	if (bus->prop.hw_disabled || !sdw->startup_done) {
@@ -1848,15 +1953,12 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 	/* unconditionally disable WAKEEN interrupt */
 	intel_shim_wake(sdw, false);
 
-	link_flags = md_flags >> (bus->link_id * 8);
-	multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK);
-
 	clock_stop_quirks = sdw->link_res->clock_stop_quirks;
 
 	if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
 		ret = intel_link_power_up(sdw);
 		if (ret) {
-			dev_err(dev, "%s failed: %d\n", __func__, ret);
+			dev_err(dev, "%s: power_up failed after teardown: %d\n", __func__, ret);
 			return ret;
 		}
 
@@ -1866,145 +1968,45 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 		 */
 		sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
 
-		ret = sdw_cdns_enable_interrupt(cdns, true);
+		ret = intel_start_bus(sdw);
 		if (ret < 0) {
-			dev_err(dev, "cannot enable interrupts during resume\n");
+			dev_err(dev, "%s: cannot start bus after teardown: %d\n", __func__, ret);
+			intel_link_power_down(sdw);
 			return ret;
 		}
 
-		/*
-		 * follow recommended programming flows to avoid
-		 * timeouts when gsync is enabled
-		 */
-		if (multi_link)
-			intel_shim_sync_arm(sdw);
-
-		ret = sdw_cdns_init(&sdw->cdns);
-		if (ret < 0) {
-			dev_err(dev, "unable to initialize Cadence IP during resume\n");
-			return ret;
-		}
-
-		ret = sdw_cdns_exit_reset(cdns);
-		if (ret < 0) {
-			dev_err(dev, "unable to exit bus reset sequence during resume\n");
-			return ret;
-		}
-
-		if (multi_link) {
-			ret = intel_shim_sync_go(sdw);
-			if (ret < 0) {
-				dev_err(dev, "sync go failed during resume\n");
-				return ret;
-			}
-		}
-		sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime TEARDOWN",
-						  true, INTEL_MASTER_RESET_ITERATIONS);
 
 	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
 		ret = intel_link_power_up(sdw);
 		if (ret) {
-			dev_err(dev, "%s failed: %d\n", __func__, ret);
+			dev_err(dev, "%s: power_up failed after bus reset: %d\n", __func__, ret);
 			return ret;
 		}
 
-		/*
-		 * An exception condition occurs for the CLK_STOP_BUS_RESET
-		 * case if one or more masters remain active. In this condition,
-		 * all the masters are powered on for they are in the same power
-		 * domain. Master can preserve its context for clock stop0, so
-		 * there is no need to clear slave status and reset bus.
-		 */
-		clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);
-
-		if (!clock_stop0) {
-
-			/*
-			 * make sure all Slaves are tagged as UNATTACHED and
-			 * provide reason for reinitialization
-			 */
-
-			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
-			 */
-			if (multi_link)
-				intel_shim_sync_arm(sdw);
-
-			/*
-			 * Re-initialize the IP since it was powered-off
-			 */
-			sdw_cdns_init(&sdw->cdns);
-
-		} else {
-			ret = sdw_cdns_enable_interrupt(cdns, true);
-			if (ret < 0) {
-				dev_err(dev, "cannot enable interrupts during resume\n");
-				return ret;
-			}
-		}
-
-		ret = sdw_cdns_clock_restart(cdns, !clock_stop0);
+		ret = intel_start_bus_after_reset(sdw);
 		if (ret < 0) {
-			dev_err(dev, "unable to restart clock during resume\n");
+			dev_err(dev, "%s: cannot start bus after reset: %d\n", __func__, ret);
+			intel_link_power_down(sdw);
 			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");
-				return ret;
-			}
-
-			if (multi_link) {
-				ret = intel_shim_sync_go(sdw);
-				if (ret < 0) {
-					dev_err(sdw->cdns.dev, "sync go failed during resume\n");
-					return ret;
-				}
-			}
-		}
-		sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime BUS_RESET",
-						  true, INTEL_MASTER_RESET_ITERATIONS);
-
 	} else if (!clock_stop_quirks) {
 
-		clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);
-		if (!clock_stop0)
-			dev_err(dev, "%s invalid configuration, clock was not stopped", __func__);
+		intel_check_clock_stop(sdw);
 
 		ret = intel_link_power_up(sdw);
 		if (ret) {
-			dev_err(dev, "%s failed: %d\n", __func__, ret);
+			dev_err(dev, "%s: power_up failed: %d\n", __func__, ret);
 			return ret;
 		}
 
-		ret = sdw_cdns_enable_interrupt(cdns, true);
+		ret = intel_start_bus_after_clock_stop(sdw);
 		if (ret < 0) {
-			dev_err(dev, "cannot enable interrupts during resume\n");
+			dev_err(dev, "%s: cannot start bus after clock stop: %d\n", __func__, ret);
+			intel_link_power_down(sdw);
 			return ret;
 		}
-
-		ret = sdw_cdns_clock_restart(cdns, false);
-		if (ret < 0) {
-			dev_err(dev, "unable to resume master during resume\n");
-			return ret;
-		}
-
-		sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime no_quirks",
-						  true, INTEL_MASTER_RESET_ITERATIONS);
 	} else {
-		dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
+		dev_err(dev, "%s: clock_stop_quirks %x unsupported\n",
 			__func__, clock_stop_quirks);
 		ret = -EINVAL;
 	}
-- 
2.25.1

[PATCH 11/11] soundwire: intel: add helper to stop bus

[Bard Liao]

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

We have three nearly identical sequences to stop the clock, let's
introduce a helper to reuse the same code.

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.c | 83 +++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 42 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index abe14436d874..15fe083e0402 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1391,6 +1391,38 @@ static int intel_start_bus_after_clock_stop(struct sdw_intel *sdw)
 	return 0;
 }
 
+static int intel_stop_bus(struct sdw_intel *sdw, bool clock_stop)
+{
+	struct device *dev = sdw->cdns.dev;
+	struct sdw_cdns *cdns = &sdw->cdns;
+	bool wake_enable = false;
+	int ret;
+
+	if (clock_stop) {
+		ret = sdw_cdns_clock_stop(cdns, true);
+		if (ret < 0)
+			dev_err(dev, "%s: cannot stop clock: %d\n", __func__, ret);
+		else
+			wake_enable = true;
+	}
+
+	ret = sdw_cdns_enable_interrupt(cdns, false);
+	if (ret < 0) {
+		dev_err(dev, "%s: cannot disable interrupts: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = intel_link_power_down(sdw);
+	if (ret) {
+		dev_err(dev, "%s: Link power down failed: %d\n", __func__, ret);
+		return ret;
+	}
+
+	intel_shim_wake(sdw, wake_enable);
+
+	return 0;
+}
+
 static int sdw_master_read_intel_prop(struct sdw_bus *bus)
 {
 	struct sdw_master_prop *prop = &bus->prop;
@@ -1790,20 +1822,12 @@ static int __maybe_unused intel_suspend(struct device *dev)
 		return 0;
 	}
 
-	ret = sdw_cdns_enable_interrupt(cdns, false);
+	ret = intel_stop_bus(sdw, false);
 	if (ret < 0) {
-		dev_err(dev, "cannot disable interrupts on suspend\n");
+		dev_err(dev, "%s: cannot stop bus: %d\n", __func__, ret);
 		return ret;
 	}
 
-	ret = intel_link_power_down(sdw);
-	if (ret) {
-		dev_err(dev, "Link power down failed: %d\n", ret);
-		return ret;
-	}
-
-	intel_shim_wake(sdw, false);
-
 	return 0;
 }
 
@@ -1824,44 +1848,19 @@ static int __maybe_unused intel_suspend_runtime(struct device *dev)
 	clock_stop_quirks = sdw->link_res->clock_stop_quirks;
 
 	if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
-
-		ret = sdw_cdns_enable_interrupt(cdns, false);
+		ret = intel_stop_bus(sdw, false);
 		if (ret < 0) {
-			dev_err(dev, "cannot disable interrupts on suspend\n");
+			dev_err(dev, "%s: cannot stop bus during teardown: %d\n",
+				__func__, ret);
 			return ret;
 		}
-
-		ret = intel_link_power_down(sdw);
-		if (ret) {
-			dev_err(dev, "Link power down failed: %d\n", ret);
-			return ret;
-		}
-
-		intel_shim_wake(sdw, false);
-
-	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET ||
-		   !clock_stop_quirks) {
-		bool wake_enable = true;
-
-		ret = sdw_cdns_clock_stop(cdns, true);
-		if (ret < 0) {
-			dev_err(dev, "cannot enable clock stop on suspend\n");
-			wake_enable = false;
-		}
-
-		ret = sdw_cdns_enable_interrupt(cdns, false);
+	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET || !clock_stop_quirks) {
+		ret = intel_stop_bus(sdw, true);
 		if (ret < 0) {
-			dev_err(dev, "cannot disable interrupts on suspend\n");
-			return ret;
-		}
-
-		ret = intel_link_power_down(sdw);
-		if (ret) {
-			dev_err(dev, "Link power down failed: %d\n", ret);
+			dev_err(dev, "%s: cannot stop bus during clock_stop: %d\n",
+				__func__, ret);
 			return ret;
 		}
-
-		intel_shim_wake(sdw, wake_enable);
 	} else {
 		dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
 			__func__, clock_stop_quirks);
-- 
2.25.1

Re: [PATCH 00/11] soundwire: intel: cleanups/fixes before code reorganization

[Vinod Koul]

On 20-09-22, 01:57, Bard Liao wrote:
> This is a really big cleanup to introduce well defined helpers, prior to
> a larger code reorganization.

Applied, thanks

-- 
~Vinod