[PATCH 0/4] ASoC/soundwire: log actual PING status on resume issues
we've been stuck with problems in the dual-amplifier configurations where one of the two devices seems to become UNATTACHED and never regains sync, see https://github.com/thesofproject/linux/issues/3638.
This is a rather infrequent issue that may happen once or twice per month, but still it remains a concern.
One possibility is that the device does lose sync but somehow our hardware detection fails to see it resync.
This series just adds a basic read directly from the PING frames to help confirm if yes/no the device regain sync.
The change is mainly on soundwire. @Mark, Could you ack the ASoC patch if it looks good to you?
Pierre-Louis Bossart (4): soundwire: add read_ping_status helper definition in manager ops soundwire: intel/cadence: expose PING status in manager ops soundwire: add sdw_show_ping_status() helper ASoC: codecs: show PING status on resume failures
drivers/soundwire/bus.c | 32 ++++++++++++++++++++++++++++++ drivers/soundwire/cadence_master.c | 8 ++++++++ drivers/soundwire/cadence_master.h | 2 ++ drivers/soundwire/intel.c | 1 + include/linux/soundwire/sdw.h | 5 +++++ sound/soc/codecs/max98373-sdw.c | 2 ++ sound/soc/codecs/rt1308-sdw.c | 2 ++ sound/soc/codecs/rt1316-sdw.c | 2 ++ sound/soc/codecs/rt5682-sdw.c | 2 ++ sound/soc/codecs/rt700-sdw.c | 2 ++ sound/soc/codecs/rt711-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdw.c | 2 ++ 13 files changed, 64 insertions(+)
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The existing manager ops provide callbacks to transfer read/write commands, but don't allow for direct access to PING status register. This is accessible in all existing IP, and would help diagnose timeouts or resume issues by reporting the 'true' status instead of the internal status reported by the IP.
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 --- include/linux/soundwire/sdw.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index a2b31d25ea27..a85cf829bb77 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -839,6 +839,8 @@ struct sdw_defer { * @set_bus_conf: Set the bus configuration * @pre_bank_switch: Callback for pre bank switch * @post_bank_switch: Callback for post bank switch + * @read_ping_status: Read status from PING frames, reported with two bits per Device. + * Bits 31:24 are reserved. */ struct sdw_master_ops { int (*read_prop)(struct sdw_bus *bus); @@ -855,6 +857,7 @@ struct sdw_master_ops { struct sdw_bus_params *params); int (*pre_bank_switch)(struct sdw_bus *bus); int (*post_bank_switch)(struct sdw_bus *bus); + u32 (*read_ping_status)(struct sdw_bus *bus);
};
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Simple indirection to existing register.
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/cadence_master.c | 8 ++++++++ drivers/soundwire/cadence_master.h | 2 ++ drivers/soundwire/intel.c | 1 + 3 files changed, 11 insertions(+)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 4fbb19557f5e..615b0b63a3e1 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -756,6 +756,14 @@ cdns_reset_page_addr(struct sdw_bus *bus, unsigned int dev_num) } EXPORT_SYMBOL(cdns_reset_page_addr);
+u32 cdns_read_ping_status(struct sdw_bus *bus) +{ + struct sdw_cdns *cdns = bus_to_cdns(bus); + + return cdns_readl(cdns, CDNS_MCP_SLAVE_STAT); +} +EXPORT_SYMBOL(cdns_read_ping_status); + /* * IRQ handling */ diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 595d72c15d97..ca9e805bab88 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -177,6 +177,8 @@ enum sdw_command_response cdns_xfer_msg_defer(struct sdw_bus *bus, struct sdw_msg *msg, struct sdw_defer *defer);
+u32 cdns_read_ping_status(struct sdw_bus *bus); + int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params);
int cdns_set_sdw_stream(struct snd_soc_dai *dai, diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 25b27cd1be1d..e1e943396e36 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1255,6 +1255,7 @@ static struct sdw_master_ops sdw_intel_ops = { .set_bus_conf = cdns_bus_conf, .pre_bank_switch = intel_pre_bank_switch, .post_bank_switch = intel_post_bank_switch, + .read_ping_status = cdns_read_ping_status, };
static int intel_init(struct sdw_intel *sdw)
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This helper provides an optional delay parameter to wait for devices to resync in case of errors, and checks that devices are indeed attached on the bus.
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/bus.c | 32 ++++++++++++++++++++++++++++++++ include/linux/soundwire/sdw.h | 2 ++ 2 files changed, 34 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 2772973eebb1..0a99ac791c7e 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -300,6 +300,38 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg) return ret; }
+/** + * sdw_show_ping_status() - Direct report of PING status, to be used by Peripheral drivers + * @bus: SDW bus + * @sync_delay: Delay before reading status + */ +void sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay) +{ + u32 status; + + if (!bus->ops->read_ping_status) + return; + + /* + * wait for peripheral to sync if desired. 10-15ms should be more than + * enough in most cases. + */ + if (sync_delay) + usleep_range(10000, 15000); + + mutex_lock(&bus->msg_lock); + + status = bus->ops->read_ping_status(bus); + + mutex_unlock(&bus->msg_lock); + + if (!status) + dev_warn(bus->dev, "%s: no peripherals attached\n", __func__); + else + dev_dbg(bus->dev, "PING status: %#x\n", status); +} +EXPORT_SYMBOL(sdw_show_ping_status); + /** * sdw_transfer_defer() - Asynchronously transfer message to a SDW Slave device * @bus: SDW bus diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index a85cf829bb77..9e4537f409c2 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -926,6 +926,8 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, struct fwnode_handle *fwnode); void sdw_bus_master_delete(struct sdw_bus *bus);
+void sdw_show_ping_status(struct sdw_bus *bus, bool sync_delay); + /** * sdw_port_config: Master or Slave Port configuration *
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This helper should help identify cases where devices fall off the bus and don't resync.
BugLink: https://github.com/thesofproject/linux/issues/3638 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 --- sound/soc/codecs/max98373-sdw.c | 2 ++ sound/soc/codecs/rt1308-sdw.c | 2 ++ sound/soc/codecs/rt1316-sdw.c | 2 ++ sound/soc/codecs/rt5682-sdw.c | 2 ++ sound/soc/codecs/rt700-sdw.c | 2 ++ sound/soc/codecs/rt711-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdw.c | 2 ++ 8 files changed, 16 insertions(+)
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c index f47e956d4f55..7bed8e146b78 100644 --- a/sound/soc/codecs/max98373-sdw.c +++ b/sound/soc/codecs/max98373-sdw.c @@ -281,6 +281,8 @@ static __maybe_unused int max98373_resume(struct device *dev) msecs_to_jiffies(MAX98373_PROBE_TIMEOUT)); if (!time) { dev_err(dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index 1c11b42dd76e..0049c6c66855 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -727,6 +727,8 @@ static int __maybe_unused rt1308_dev_resume(struct device *dev) msecs_to_jiffies(RT1308_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c index 60baa9ff1907..34ca2b77ee4f 100644 --- a/sound/soc/codecs/rt1316-sdw.c +++ b/sound/soc/codecs/rt1316-sdw.c @@ -712,6 +712,8 @@ static int __maybe_unused rt1316_dev_resume(struct device *dev) msecs_to_jiffies(RT1316_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 248257a2e4e0..b4e722bb7e25 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -790,6 +790,8 @@ static int __maybe_unused rt5682_dev_resume(struct device *dev) msecs_to_jiffies(RT5682_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index bda594899664..132e60f72c03 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -538,6 +538,8 @@ static int __maybe_unused rt700_dev_resume(struct device *dev) msecs_to_jiffies(RT700_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c index aaf5af153d3f..d1d8327cf72f 100644 --- a/sound/soc/codecs/rt711-sdca-sdw.c +++ b/sound/soc/codecs/rt711-sdca-sdw.c @@ -442,6 +442,8 @@ static int __maybe_unused rt711_sdca_dev_resume(struct device *dev) msecs_to_jiffies(RT711_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt715-sdca-sdw.c b/sound/soc/codecs/rt715-sdca-sdw.c index 0ecd2948f7aa..796b5e982b8a 100644 --- a/sound/soc/codecs/rt715-sdca-sdw.c +++ b/sound/soc/codecs/rt715-sdca-sdw.c @@ -233,6 +233,8 @@ static int __maybe_unused rt715_dev_resume(struct device *dev) msecs_to_jiffies(RT715_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Enumeration not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c index a7b21b03c08b..42b8c176b821 100644 --- a/sound/soc/codecs/rt715-sdw.c +++ b/sound/soc/codecs/rt715-sdw.c @@ -551,6 +551,8 @@ static int __maybe_unused rt715_dev_resume(struct device *dev) msecs_to_jiffies(RT715_PROBE_TIMEOUT)); if (!time) { dev_err(&slave->dev, "Initialization not complete, timed out\n"); + sdw_show_ping_status(slave->bus, true); + return -ETIMEDOUT; }
On Thu, Jul 14, 2022 at 09:10:43AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This helper should help identify cases where devices fall off the bus and don't resync.
Acked-by: Mark Brown broonie@kernel.org
If this is applied to the Soundwire tree it might be good to get a pull request in case of conflicts, though I guess they are relatively unlikely here.
Hey Mark,
On 14-07-22, 20:21, Mark Brown wrote:
On Thu, Jul 14, 2022 at 09:10:43AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
This helper should help identify cases where devices fall off the bus and don't resync.
Acked-by: Mark Brown broonie@kernel.org
If this is applied to the Soundwire tree it might be good to get a pull request in case of conflicts, though I guess they are relatively unlikely here.
It is bit late for sdw tree (we are closed for this cycle now), if you are okay, pls feel free to take thru ASoC tree:
You may use: git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git tags/soundwire-5.20-rc1
On 14-07-22, 09:10, Bard Liao wrote:
we've been stuck with problems in the dual-amplifier configurations where one of the two devices seems to become UNATTACHED and never regains sync, see https://github.com/thesofproject/linux/issues/3638.
This is a rather infrequent issue that may happen once or twice per month, but still it remains a concern.
One possibility is that the device does lose sync but somehow our hardware detection fails to see it resync.
This series just adds a basic read directly from the PING frames to help confirm if yes/no the device regain sync.
The change is mainly on soundwire. @Mark, Could you ack the ASoC patch if it looks good to you?
Mark,
The series lgtm, feel free to merge thru ASoC tree with:
Acked-By: Vinod Koul vkoul@kernel.org
Pierre-Louis Bossart (4): soundwire: add read_ping_status helper definition in manager ops soundwire: intel/cadence: expose PING status in manager ops soundwire: add sdw_show_ping_status() helper ASoC: codecs: show PING status on resume failures
drivers/soundwire/bus.c | 32 ++++++++++++++++++++++++++++++ drivers/soundwire/cadence_master.c | 8 ++++++++ drivers/soundwire/cadence_master.h | 2 ++ drivers/soundwire/intel.c | 1 + include/linux/soundwire/sdw.h | 5 +++++ sound/soc/codecs/max98373-sdw.c | 2 ++ sound/soc/codecs/rt1308-sdw.c | 2 ++ sound/soc/codecs/rt1316-sdw.c | 2 ++ sound/soc/codecs/rt5682-sdw.c | 2 ++ sound/soc/codecs/rt700-sdw.c | 2 ++ sound/soc/codecs/rt711-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdw.c | 2 ++ 13 files changed, 64 insertions(+)
-- 2.25.1
-----Original Message----- From: Vinod Koul vkoul@kernel.org Sent: Monday, July 18, 2022 1:45 PM To: Bard Liao yung-chuan.liao@linux.intel.com Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; tiwai@suse.de; broonie@kernel.org; pierre-louis.bossart@linux.intel.com; Liao, Bard bard.liao@intel.com Subject: Re: [PATCH 0/4] ASoC/soundwire: log actual PING status on resume issues
On 14-07-22, 09:10, Bard Liao wrote:
we've been stuck with problems in the dual-amplifier configurations where one of the two devices seems to become UNATTACHED and never regains
sync,
see https://github.com/thesofproject/linux/issues/3638.
This is a rather infrequent issue that may happen once or twice per month, but still it remains a concern.
One possibility is that the device does lose sync but somehow our
hardware
detection fails to see it resync.
This series just adds a basic read directly from the PING frames to help confirm if yes/no the device regain sync.
The change is mainly on soundwire. @Mark, Could you ack the ASoC patch if it looks good to you?
Mark,
The series lgtm, feel free to merge thru ASoC tree with:
Acked-By: Vinod Koul vkoul@kernel.org
Hi Mark/Vinod,
Both of you are acked. Can this series be merged now?
Thanks.
Pierre-Louis Bossart (4): soundwire: add read_ping_status helper definition in manager ops soundwire: intel/cadence: expose PING status in manager ops soundwire: add sdw_show_ping_status() helper ASoC: codecs: show PING status on resume failures
drivers/soundwire/bus.c | 32 ++++++++++++++++++++++++++++++ drivers/soundwire/cadence_master.c | 8 ++++++++ drivers/soundwire/cadence_master.h | 2 ++ drivers/soundwire/intel.c | 1 + include/linux/soundwire/sdw.h | 5 +++++ sound/soc/codecs/max98373-sdw.c | 2 ++ sound/soc/codecs/rt1308-sdw.c | 2 ++ sound/soc/codecs/rt1316-sdw.c | 2 ++ sound/soc/codecs/rt5682-sdw.c | 2 ++ sound/soc/codecs/rt700-sdw.c | 2 ++ sound/soc/codecs/rt711-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdw.c | 2 ++ 13 files changed, 64 insertions(+)
-- 2.25.1
-- ~Vinod
On Wed, Aug 17, 2022 at 06:48:56AM +0000, Liao, Bard wrote:
This series just adds a basic read directly from the PING frames to help confirm if yes/no the device regain sync.
The change is mainly on soundwire. @Mark, Could you ack the ASoC patch if it looks good to you?
The series lgtm, feel free to merge thru ASoC tree with:
Acked-By: Vinod Koul vkoul@kernel.org
Both of you are acked. Can this series be merged now?
I guess I'll apply it - given that it's mainly a Soundwire change I would have expected it to go via Soundwire as you'd suggested.
On Thu, 14 Jul 2022 09:10:39 +0800, Bard Liao wrote:
we've been stuck with problems in the dual-amplifier configurations where one of the two devices seems to become UNATTACHED and never regains sync, see https://github.com/thesofproject/linux/issues/3638.
This is a rather infrequent issue that may happen once or twice per month, but still it remains a concern.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] soundwire: add read_ping_status helper definition in manager ops commit: 874de459488b8741afc0e9888d39f2e15a962b3d [2/4] soundwire: intel/cadence: expose PING status in manager ops commit: 133547a1ef16cbdadb5c0023e5917924ae326dcc [3/4] soundwire: add sdw_show_ping_status() helper commit: 79fe02cb7547fcc09e83b850cfd32896d7dc6289 [4/4] ASoC: codecs: show PING status on resume failures commit: 917df025e1db1286afb6e46914ae3e8b40241568
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (4)
-
Bard Liao -
Liao, Bard -
Mark Brown -
Vinod Koul