From ccc6cf1573b92f15cca79b674d555bef0fdf3059 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 18 May 2023 10:42:15 +0800 Subject: soundwire: intel_bus_common: update error log We use __func__ in all calls of sdw_cdns_check_self_clearing_bits(), except in one case. Likely an editing miss when the code was refactored. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20230518024215.164281-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel_bus_common.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/soundwire/intel_bus_common.c') diff --git a/drivers/soundwire/intel_bus_common.c b/drivers/soundwire/intel_bus_common.c index f180e3bea989..b30058e20910 100644 --- a/drivers/soundwire/intel_bus_common.c +++ b/drivers/soundwire/intel_bus_common.c @@ -171,8 +171,7 @@ int intel_start_bus_after_clock_stop(struct sdw_intel *sdw) return ret; } - sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime no_quirks", - true, INTEL_MASTER_RESET_ITERATIONS); + sdw_cdns_check_self_clearing_bits(cdns, __func__, true, INTEL_MASTER_RESET_ITERATIONS); return 0; } -- cgit v1.2.3 From 46b56a5cd2ff496c0707ae3deae849f12b86e534 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 18 May 2023 10:41:17 +0800 Subject: soundwire: intel_bus_common: enable interrupts last 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 Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20230518024119.164160-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel_bus_common.c | 57 ++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 31 deletions(-) (limited to 'drivers/soundwire/intel_bus_common.c') diff --git a/drivers/soundwire/intel_bus_common.c b/drivers/soundwire/intel_bus_common.c index b30058e20910..1e47f4402d1a 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); + ret = sdw_cdns_clock_restart(cdns, false); if (ret < 0) { - dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret); + dev_err(dev, "%s: unable to restart clock: %d\n", __func__, ret); return ret; } - ret = sdw_cdns_clock_restart(cdns, false); + ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { - dev_err(dev, "%s: unable to restart clock: %d\n", __func__, ret); - sdw_cdns_enable_interrupt(cdns, false); + dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret); return ret; } -- cgit v1.2.3 From ffc363d970d1d04be0f12b2252aa21aaef3ecfd0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 18 May 2023 10:41:18 +0800 Subject: soundwire: intel/cadence: update hardware reset sequence Combining hardware reset with the multi-link mode leads to a shortened hardware reset pattern observed on the bus. The updated hardware programming sequence is to first enable the clock with the sync_arm/sync_go pattern, and only in a second step to issue the hardware reset sequence. Since there is no longer a dependency between sync_arm/sync_go and hw_reset, the behavior of sdw_cdns_exit_reset() is changed to wait for the self-clearing CONFIG_UPDATE to go back to zero, Link: https://github.com/thesofproject/linux/issues/4170 Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20230518024119.164160-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 31 ++++++++++++++++++++++++------- drivers/soundwire/cadence_master.h | 3 +++ drivers/soundwire/intel_bus_common.c | 36 ++++++++++++++++++++++++++---------- 3 files changed, 53 insertions(+), 17 deletions(-) (limited to 'drivers/soundwire/intel_bus_common.c') diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 39502bc75712..58686ae50bbf 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -283,6 +283,29 @@ static int cdns_config_update(struct sdw_cdns *cdns) return ret; } +/** + * sdw_cdns_config_update() - Update configurations + * @cdns: Cadence instance + */ +void sdw_cdns_config_update(struct sdw_cdns *cdns) +{ + /* commit changes */ + cdns_writel(cdns, CDNS_MCP_CONFIG_UPDATE, CDNS_MCP_CONFIG_UPDATE_BIT); +} +EXPORT_SYMBOL(sdw_cdns_config_update); + +/** + * sdw_cdns_config_update_set_wait() - wait until configuration update bit is self-cleared + * @cdns: Cadence instance + */ +int sdw_cdns_config_update_set_wait(struct sdw_cdns *cdns) +{ + /* the hardware recommendation is to wait at least 300us */ + return cdns_set_wait(cdns, CDNS_MCP_CONFIG_UPDATE, + CDNS_MCP_CONFIG_UPDATE_BIT, 0); +} +EXPORT_SYMBOL(sdw_cdns_config_update_set_wait); + /* * debugfs */ @@ -1116,13 +1139,7 @@ int sdw_cdns_exit_reset(struct sdw_cdns *cdns) CDNS_MCP_CONTROL_HW_RST); /* commit changes */ - cdns_updatel(cdns, CDNS_MCP_CONFIG_UPDATE, - CDNS_MCP_CONFIG_UPDATE_BIT, - CDNS_MCP_CONFIG_UPDATE_BIT); - - /* don't wait here */ - return 0; - + return cdns_config_update(cdns); } EXPORT_SYMBOL(sdw_cdns_exit_reset); diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 86a450b1cbda..bc84435e420f 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -199,4 +199,7 @@ int cdns_set_sdw_stream(struct snd_soc_dai *dai, void sdw_cdns_check_self_clearing_bits(struct sdw_cdns *cdns, const char *string, bool initial_delay, int reset_iterations); +void sdw_cdns_config_update(struct sdw_cdns *cdns); +int sdw_cdns_config_update_set_wait(struct sdw_cdns *cdns); + #endif /* __SDW_CADENCE_H */ diff --git a/drivers/soundwire/intel_bus_common.c b/drivers/soundwire/intel_bus_common.c index 1e47f4402d1a..e5ac3cc7cb79 100644 --- a/drivers/soundwire/intel_bus_common.c +++ b/drivers/soundwire/intel_bus_common.c @@ -29,11 +29,7 @@ int intel_start_bus(struct sdw_intel *sdw) 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); - return ret; - } + sdw_cdns_config_update(cdns); if (bus->multi_link) { ret = sdw_intel_sync_go(sdw); @@ -43,6 +39,18 @@ int intel_start_bus(struct sdw_intel *sdw) } } + ret = sdw_cdns_config_update_set_wait(cdns); + if (ret < 0) { + dev_err(dev, "%s: CONFIG_UPDATE BIT still set\n", __func__); + 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); + return ret; + } + ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret); @@ -112,11 +120,7 @@ int intel_start_bus_after_reset(struct sdw_intel *sdw) } 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; - } + sdw_cdns_config_update(cdns); if (bus->multi_link) { ret = sdw_intel_sync_go(sdw); @@ -126,6 +130,18 @@ int intel_start_bus_after_reset(struct sdw_intel *sdw) } } + ret = sdw_cdns_config_update_set_wait(cdns); + if (ret < 0) { + dev_err(dev, "%s: CONFIG_UPDATE BIT still set\n", __func__); + 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; + } + ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { dev_err(dev, "cannot enable interrupts during resume\n"); -- cgit v1.2.3