[1/6] soundwire: fix startup sequence for Intel/Cadence
diff mbox series

Message ID 20190813213227.5163-2-pierre-louis.bossart@linux.intel.com
State New
Headers show
Series
  • soundwire: inits and PM additions for 5.4
Related show

Commit Message

Pierre-Louis Bossart Aug. 13, 2019, 9:32 p.m. UTC
Multiple changes squashed in single patch to avoid tick-tock effect.

1. Per the hardware documentation, all changes to MCP_CONFIG,
MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL need to be validated with a
self-clearing write to MCP_CONFIG_UPDATE. Add a helper and do the
update when the CONFIG is changed.

2. Move interrupt enable after interrupt handler registration

3. Add a new helper to start the hardware bus reset with maximum duration
to make sure the Slave(s) correctly detect the reset pattern and to
ensure electrical conflicts can be resolved.

4. flush command FIFOs

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 84 +++++++++++++++++++++---------
 drivers/soundwire/cadence_master.h |  1 +
 drivers/soundwire/intel.c          | 14 ++++-
 3 files changed, 73 insertions(+), 26 deletions(-)

Comments

Vinod Koul Sept. 4, 2019, 7:11 a.m. UTC | #1
On 13-08-19, 16:32, Pierre-Louis Bossart wrote:
> Multiple changes squashed in single patch to avoid tick-tock effect.
> 
> 1. Per the hardware documentation, all changes to MCP_CONFIG,
> MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL need to be validated with a
> self-clearing write to MCP_CONFIG_UPDATE. Add a helper and do the
> update when the CONFIG is changed.
> 
> 2. Move interrupt enable after interrupt handler registration
> 
> 3. Add a new helper to start the hardware bus reset with maximum duration
> to make sure the Slave(s) correctly detect the reset pattern and to
> ensure electrical conflicts can be resolved.
> 
> 4. flush command FIFOs
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/soundwire/cadence_master.c | 84 +++++++++++++++++++++---------
>  drivers/soundwire/cadence_master.h |  1 +
>  drivers/soundwire/intel.c          | 14 ++++-
>  3 files changed, 73 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> index 502ed4ec8f07..046622e4b264 100644
> --- a/drivers/soundwire/cadence_master.c
> +++ b/drivers/soundwire/cadence_master.c
> @@ -231,6 +231,22 @@ static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value)
>  	return -EAGAIN;
>  }
>  
> +/*
> + * all changes to the MCP_CONFIG, MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL
> + * need to be confirmed with a write to MCP_CONFIG_UPDATE
> + */
> +static int cdns_update_config(struct sdw_cdns *cdns)
> +{
> +	int ret;
> +
> +	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
> +			     CDNS_MCP_CONFIG_UPDATE_BIT);
> +	if (ret < 0)
> +		dev_err(cdns->dev, "Config update timedout\n");
> +
> +	return ret;
> +}
> +
>  /*
>   * debugfs
>   */
> @@ -752,7 +768,42 @@ EXPORT_SYMBOL(sdw_cdns_thread);
>  /*
>   * init routines
>   */
> -static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
> +
> +/**
> + * sdw_cdns_exit_reset() - Program reset parameters and start bus operations
> + * @cdns: Cadence instance
> + */
> +int sdw_cdns_exit_reset(struct sdw_cdns *cdns)
> +{
> +	int ret;
> +
> +	/* program maximum length reset to be safe */
> +	cdns_updatel(cdns, CDNS_MCP_CONTROL,
> +		     CDNS_MCP_CONTROL_RST_DELAY,
> +		     CDNS_MCP_CONTROL_RST_DELAY);
> +
> +	/* use hardware generated reset */
> +	cdns_updatel(cdns, CDNS_MCP_CONTROL,
> +		     CDNS_MCP_CONTROL_HW_RST,
> +		     CDNS_MCP_CONTROL_HW_RST);
> +
> +	/* enable bus operations with clock and data */
> +	cdns_updatel(cdns, CDNS_MCP_CONFIG,
> +		     CDNS_MCP_CONFIG_OP,
> +		     CDNS_MCP_CONFIG_OP_NORMAL);
> +
> +	/* commit changes */
> +	ret = cdns_update_config(cdns);
> +
> +	return ret;

return cdns_update_config() ?


> +}
> +EXPORT_SYMBOL(sdw_cdns_exit_reset);
> +
> +/**
> + * sdw_cdns_enable_interrupt() - Enable SDW interrupts and update config
> + * @cdns: Cadence instance
> + */
> +int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
>  {
>  	u32 mask;
>  
> @@ -784,24 +835,8 @@ static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
>  
>  	cdns_writel(cdns, CDNS_MCP_INTMASK, mask);
>  
> -	return 0;
> -}
> -
> -/**
> - * sdw_cdns_enable_interrupt() - Enable SDW interrupts and update config
> - * @cdns: Cadence instance
> - */
> -int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
> -{
> -	int ret;
> -
> -	_cdns_enable_interrupt(cdns);
> -	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
> -			     CDNS_MCP_CONFIG_UPDATE_BIT);
> -	if (ret < 0)
> -		dev_err(cdns->dev, "Config update timedout\n");
> -
> -	return ret;
> +	/* commit changes */
> +	return cdns_update_config(cdns);
>  }
>  EXPORT_SYMBOL(sdw_cdns_enable_interrupt);
>  
> @@ -975,6 +1010,10 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
>  	cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, CDNS_DEFAULT_SSP_INTERVAL);
>  	cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, CDNS_DEFAULT_SSP_INTERVAL);
>  
> +	/* flush command FIFOs */
> +	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_RST,
> +		     CDNS_MCP_CONTROL_CMD_RST);
> +
>  	/* Set cmd accept mode */
>  	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT,
>  		     CDNS_MCP_CONTROL_CMD_ACCEPT);
> @@ -997,13 +1036,10 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
>  	/* Set cmd mode for Tx and Rx cmds */
>  	val &= ~CDNS_MCP_CONFIG_CMD;
>  
> -	/* Set operation to normal */
> -	val &= ~CDNS_MCP_CONFIG_OP;
> -	val |= CDNS_MCP_CONFIG_OP_NORMAL;
> -
>  	cdns_writel(cdns, CDNS_MCP_CONFIG, val);
>  
> -	return 0;
> +	/* commit changes */
> +	return cdns_update_config(cdns);
>  }
>  EXPORT_SYMBOL(sdw_cdns_init);
>  
> diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
> index 0b72b7094735..1a67728c5000 100644
> --- a/drivers/soundwire/cadence_master.h
> +++ b/drivers/soundwire/cadence_master.h
> @@ -161,6 +161,7 @@ irqreturn_t sdw_cdns_thread(int irq, void *dev_id);
>  int sdw_cdns_init(struct sdw_cdns *cdns);
>  int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
>  		      struct sdw_cdns_stream_config config);
> +int sdw_cdns_exit_reset(struct sdw_cdns *cdns);
>  int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns);
>  
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index 13c54eac0cc3..5f14c6acce80 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c

As I have said in the past it doesnt help having a patch touching two
components. The patch is titled cadence!

> @@ -1043,8 +1043,6 @@ static int intel_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_init;
>  
> -	ret = sdw_cdns_enable_interrupt(&sdw->cdns);
> -
>  	/* Read the PDI config and initialize cadence PDI */
>  	intel_pdi_init(sdw, &config);
>  	ret = sdw_cdns_pdi_init(&sdw->cdns, config);
> @@ -1062,6 +1060,18 @@ static int intel_probe(struct platform_device *pdev)
>  		goto err_init;
>  	}
>  
> +	ret = sdw_cdns_enable_interrupt(&sdw->cdns);
> +	if (ret < 0) {
> +		dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
> +		goto err_init;
> +	}
> +
> +	ret = sdw_cdns_exit_reset(&sdw->cdns);
> +	if (ret < 0) {
> +		dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
> +		goto err_init;

Don't you want to disable interrupts at least... before you return
error? err_init does bus cleanup and not controller one
Pierre-Louis Bossart Sept. 4, 2019, 1:16 p.m. UTC | #2
On 9/4/19 2:11 AM, Vinod Koul wrote:
> On 13-08-19, 16:32, Pierre-Louis Bossart wrote:
>> Multiple changes squashed in single patch to avoid tick-tock effect.
>>
>> 1. Per the hardware documentation, all changes to MCP_CONFIG,
>> MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL need to be validated with a
>> self-clearing write to MCP_CONFIG_UPDATE. Add a helper and do the
>> update when the CONFIG is changed.
>>
>> 2. Move interrupt enable after interrupt handler registration
>>
>> 3. Add a new helper to start the hardware bus reset with maximum duration
>> to make sure the Slave(s) correctly detect the reset pattern and to
>> ensure electrical conflicts can be resolved.
>>
>> 4. flush command FIFOs
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   drivers/soundwire/cadence_master.c | 84 +++++++++++++++++++++---------
>>   drivers/soundwire/cadence_master.h |  1 +
>>   drivers/soundwire/intel.c          | 14 ++++-
>>   3 files changed, 73 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
>> index 502ed4ec8f07..046622e4b264 100644
>> --- a/drivers/soundwire/cadence_master.c
>> +++ b/drivers/soundwire/cadence_master.c
>> @@ -231,6 +231,22 @@ static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value)
>>   	return -EAGAIN;
>>   }
>>   
>> +/*
>> + * all changes to the MCP_CONFIG, MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL
>> + * need to be confirmed with a write to MCP_CONFIG_UPDATE
>> + */
>> +static int cdns_update_config(struct sdw_cdns *cdns)
>> +{
>> +	int ret;
>> +
>> +	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
>> +			     CDNS_MCP_CONFIG_UPDATE_BIT);
>> +	if (ret < 0)
>> +		dev_err(cdns->dev, "Config update timedout\n");
>> +
>> +	return ret;
>> +}
>> +
>>   /*
>>    * debugfs
>>    */
>> @@ -752,7 +768,42 @@ EXPORT_SYMBOL(sdw_cdns_thread);
>>   /*
>>    * init routines
>>    */
>> -static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
>> +
>> +/**
>> + * sdw_cdns_exit_reset() - Program reset parameters and start bus operations
>> + * @cdns: Cadence instance
>> + */
>> +int sdw_cdns_exit_reset(struct sdw_cdns *cdns)
>> +{
>> +	int ret;
>> +
>> +	/* program maximum length reset to be safe */
>> +	cdns_updatel(cdns, CDNS_MCP_CONTROL,
>> +		     CDNS_MCP_CONTROL_RST_DELAY,
>> +		     CDNS_MCP_CONTROL_RST_DELAY);
>> +
>> +	/* use hardware generated reset */
>> +	cdns_updatel(cdns, CDNS_MCP_CONTROL,
>> +		     CDNS_MCP_CONTROL_HW_RST,
>> +		     CDNS_MCP_CONTROL_HW_RST);
>> +
>> +	/* enable bus operations with clock and data */
>> +	cdns_updatel(cdns, CDNS_MCP_CONFIG,
>> +		     CDNS_MCP_CONFIG_OP,
>> +		     CDNS_MCP_CONFIG_OP_NORMAL);
>> +
>> +	/* commit changes */
>> +	ret = cdns_update_config(cdns);
>> +
>> +	return ret;
> 
> return cdns_update_config() ?

ok

> 
> 
>> +}
>> +EXPORT_SYMBOL(sdw_cdns_exit_reset);
>> +
>> +/**
>> + * sdw_cdns_enable_interrupt() - Enable SDW interrupts and update config
>> + * @cdns: Cadence instance
>> + */
>> +int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
>>   {
>>   	u32 mask;
>>   
>> @@ -784,24 +835,8 @@ static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
>>   
>>   	cdns_writel(cdns, CDNS_MCP_INTMASK, mask);
>>   
>> -	return 0;
>> -}
>> -
>> -/**
>> - * sdw_cdns_enable_interrupt() - Enable SDW interrupts and update config
>> - * @cdns: Cadence instance
>> - */
>> -int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
>> -{
>> -	int ret;
>> -
>> -	_cdns_enable_interrupt(cdns);
>> -	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
>> -			     CDNS_MCP_CONFIG_UPDATE_BIT);
>> -	if (ret < 0)
>> -		dev_err(cdns->dev, "Config update timedout\n");
>> -
>> -	return ret;
>> +	/* commit changes */
>> +	return cdns_update_config(cdns);
>>   }
>>   EXPORT_SYMBOL(sdw_cdns_enable_interrupt);
>>   
>> @@ -975,6 +1010,10 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
>>   	cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, CDNS_DEFAULT_SSP_INTERVAL);
>>   	cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, CDNS_DEFAULT_SSP_INTERVAL);
>>   
>> +	/* flush command FIFOs */
>> +	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_RST,
>> +		     CDNS_MCP_CONTROL_CMD_RST);
>> +
>>   	/* Set cmd accept mode */
>>   	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT,
>>   		     CDNS_MCP_CONTROL_CMD_ACCEPT);
>> @@ -997,13 +1036,10 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
>>   	/* Set cmd mode for Tx and Rx cmds */
>>   	val &= ~CDNS_MCP_CONFIG_CMD;
>>   
>> -	/* Set operation to normal */
>> -	val &= ~CDNS_MCP_CONFIG_OP;
>> -	val |= CDNS_MCP_CONFIG_OP_NORMAL;
>> -
>>   	cdns_writel(cdns, CDNS_MCP_CONFIG, val);
>>   
>> -	return 0;
>> +	/* commit changes */
>> +	return cdns_update_config(cdns);
>>   }
>>   EXPORT_SYMBOL(sdw_cdns_init);
>>   
>> diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
>> index 0b72b7094735..1a67728c5000 100644
>> --- a/drivers/soundwire/cadence_master.h
>> +++ b/drivers/soundwire/cadence_master.h
>> @@ -161,6 +161,7 @@ irqreturn_t sdw_cdns_thread(int irq, void *dev_id);
>>   int sdw_cdns_init(struct sdw_cdns *cdns);
>>   int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
>>   		      struct sdw_cdns_stream_config config);
>> +int sdw_cdns_exit_reset(struct sdw_cdns *cdns);
>>   int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns);
>>   
>>   #ifdef CONFIG_DEBUG_FS
>> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
>> index 13c54eac0cc3..5f14c6acce80 100644
>> --- a/drivers/soundwire/intel.c
>> +++ b/drivers/soundwire/intel.c
> 
> As I have said in the past it doesnt help having a patch touching two
> components. The patch is titled cadence!

no, it's called 'fix startup sequence for Intel/Cadence' and it's 
intentional to modify the two parts in a single patch.
I tried to split this but it's very difficult due to the dependencies. 
If you look at all the initial comments from early July people were lost 
when I did these changes in steps. I try to keep patches local and 
self-contained, but in this case it's nearly impossible or makes the 
review very hard for no good reason.

> 
>> @@ -1043,8 +1043,6 @@ static int intel_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto err_init;
>>   
>> -	ret = sdw_cdns_enable_interrupt(&sdw->cdns);
>> -
>>   	/* Read the PDI config and initialize cadence PDI */
>>   	intel_pdi_init(sdw, &config);
>>   	ret = sdw_cdns_pdi_init(&sdw->cdns, config);
>> @@ -1062,6 +1060,18 @@ static int intel_probe(struct platform_device *pdev)
>>   		goto err_init;
>>   	}
>>   
>> +	ret = sdw_cdns_enable_interrupt(&sdw->cdns);
>> +	if (ret < 0) {
>> +		dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
>> +		goto err_init;
>> +	}
>> +
>> +	ret = sdw_cdns_exit_reset(&sdw->cdns);
>> +	if (ret < 0) {
>> +		dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
>> +		goto err_init;
> 
> Don't you want to disable interrupts at least... before you return
> error? err_init does bus cleanup and not controller one

yes good point, let me look at this.
Pierre-Louis Bossart Sept. 16, 2019, 6:14 p.m. UTC | #3
>>> @@ -1043,8 +1043,6 @@ static int intel_probe(struct platform_device 
>>> *pdev)
>>>       if (ret)
>>>           goto err_init;
>>> -    ret = sdw_cdns_enable_interrupt(&sdw->cdns);
>>> -
>>>       /* Read the PDI config and initialize cadence PDI */
>>>       intel_pdi_init(sdw, &config);
>>>       ret = sdw_cdns_pdi_init(&sdw->cdns, config);
>>> @@ -1062,6 +1060,18 @@ static int intel_probe(struct platform_device 
>>> *pdev)
>>>           goto err_init;
>>>       }
>>> +    ret = sdw_cdns_enable_interrupt(&sdw->cdns);
>>> +    if (ret < 0) {
>>> +        dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
>>> +        goto err_init;
>>> +    }
>>> +
>>> +    ret = sdw_cdns_exit_reset(&sdw->cdns);
>>> +    if (ret < 0) {
>>> +        dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
>>> +        goto err_init;
>>
>> Don't you want to disable interrupts at least... before you return
>> error? err_init does bus cleanup and not controller one
> 
> yes good point, let me look at this.

The existing code has no interrupt disable sequence.

I will add this improved error handling in a follow-up patch, after the 
capability to disable interrupts is added.

Patch
diff mbox series

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 502ed4ec8f07..046622e4b264 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -231,6 +231,22 @@  static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value)
 	return -EAGAIN;
 }
 
+/*
+ * all changes to the MCP_CONFIG, MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL
+ * need to be confirmed with a write to MCP_CONFIG_UPDATE
+ */
+static int cdns_update_config(struct sdw_cdns *cdns)
+{
+	int ret;
+
+	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
+			     CDNS_MCP_CONFIG_UPDATE_BIT);
+	if (ret < 0)
+		dev_err(cdns->dev, "Config update timedout\n");
+
+	return ret;
+}
+
 /*
  * debugfs
  */
@@ -752,7 +768,42 @@  EXPORT_SYMBOL(sdw_cdns_thread);
 /*
  * init routines
  */
-static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
+
+/**
+ * sdw_cdns_exit_reset() - Program reset parameters and start bus operations
+ * @cdns: Cadence instance
+ */
+int sdw_cdns_exit_reset(struct sdw_cdns *cdns)
+{
+	int ret;
+
+	/* program maximum length reset to be safe */
+	cdns_updatel(cdns, CDNS_MCP_CONTROL,
+		     CDNS_MCP_CONTROL_RST_DELAY,
+		     CDNS_MCP_CONTROL_RST_DELAY);
+
+	/* use hardware generated reset */
+	cdns_updatel(cdns, CDNS_MCP_CONTROL,
+		     CDNS_MCP_CONTROL_HW_RST,
+		     CDNS_MCP_CONTROL_HW_RST);
+
+	/* enable bus operations with clock and data */
+	cdns_updatel(cdns, CDNS_MCP_CONFIG,
+		     CDNS_MCP_CONFIG_OP,
+		     CDNS_MCP_CONFIG_OP_NORMAL);
+
+	/* commit changes */
+	ret = cdns_update_config(cdns);
+
+	return ret;
+}
+EXPORT_SYMBOL(sdw_cdns_exit_reset);
+
+/**
+ * sdw_cdns_enable_interrupt() - Enable SDW interrupts and update config
+ * @cdns: Cadence instance
+ */
+int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
 {
 	u32 mask;
 
@@ -784,24 +835,8 @@  static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
 
 	cdns_writel(cdns, CDNS_MCP_INTMASK, mask);
 
-	return 0;
-}
-
-/**
- * sdw_cdns_enable_interrupt() - Enable SDW interrupts and update config
- * @cdns: Cadence instance
- */
-int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
-{
-	int ret;
-
-	_cdns_enable_interrupt(cdns);
-	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
-			     CDNS_MCP_CONFIG_UPDATE_BIT);
-	if (ret < 0)
-		dev_err(cdns->dev, "Config update timedout\n");
-
-	return ret;
+	/* commit changes */
+	return cdns_update_config(cdns);
 }
 EXPORT_SYMBOL(sdw_cdns_enable_interrupt);
 
@@ -975,6 +1010,10 @@  int sdw_cdns_init(struct sdw_cdns *cdns)
 	cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, CDNS_DEFAULT_SSP_INTERVAL);
 	cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, CDNS_DEFAULT_SSP_INTERVAL);
 
+	/* flush command FIFOs */
+	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_RST,
+		     CDNS_MCP_CONTROL_CMD_RST);
+
 	/* Set cmd accept mode */
 	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT,
 		     CDNS_MCP_CONTROL_CMD_ACCEPT);
@@ -997,13 +1036,10 @@  int sdw_cdns_init(struct sdw_cdns *cdns)
 	/* Set cmd mode for Tx and Rx cmds */
 	val &= ~CDNS_MCP_CONFIG_CMD;
 
-	/* Set operation to normal */
-	val &= ~CDNS_MCP_CONFIG_OP;
-	val |= CDNS_MCP_CONFIG_OP_NORMAL;
-
 	cdns_writel(cdns, CDNS_MCP_CONFIG, val);
 
-	return 0;
+	/* commit changes */
+	return cdns_update_config(cdns);
 }
 EXPORT_SYMBOL(sdw_cdns_init);
 
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index 0b72b7094735..1a67728c5000 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -161,6 +161,7 @@  irqreturn_t sdw_cdns_thread(int irq, void *dev_id);
 int sdw_cdns_init(struct sdw_cdns *cdns);
 int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
 		      struct sdw_cdns_stream_config config);
+int sdw_cdns_exit_reset(struct sdw_cdns *cdns);
 int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns);
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 13c54eac0cc3..5f14c6acce80 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1043,8 +1043,6 @@  static int intel_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_init;
 
-	ret = sdw_cdns_enable_interrupt(&sdw->cdns);
-
 	/* Read the PDI config and initialize cadence PDI */
 	intel_pdi_init(sdw, &config);
 	ret = sdw_cdns_pdi_init(&sdw->cdns, config);
@@ -1062,6 +1060,18 @@  static int intel_probe(struct platform_device *pdev)
 		goto err_init;
 	}
 
+	ret = sdw_cdns_enable_interrupt(&sdw->cdns);
+	if (ret < 0) {
+		dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
+		goto err_init;
+	}
+
+	ret = sdw_cdns_exit_reset(&sdw->cdns);
+	if (ret < 0) {
+		dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
+		goto err_init;
+	}
+
 	/* Register DAIs */
 	ret = intel_register_dai(sdw);
 	if (ret) {