linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset
@ 2023-02-24 15:33 Martin Povišer
  2023-02-24 15:33 ` [PATCH 2/3] ASoC: apple: mca: Fix SERDES reset sequence Martin Povišer
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Martin Povišer @ 2023-02-24 15:33 UTC (permalink / raw)
  To: Martin Povišer, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai
  Cc: asahi, alsa-devel, linux-kernel

From within the early trigger we are doing a reset of the SERDES unit,
but the final status read is on a bad address. Add the missing SERDES
unit offset in calculation of the address.

Fixes: 3df5d0d97289 ("ASoC: apple: mca: Start new platform driver")
Signed-off-by: Martin Povišer <povik+lin@cutebit.org>
---
 sound/soc/apple/mca.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/apple/mca.c b/sound/soc/apple/mca.c
index 24381c42eb54..9cceeb259952 100644
--- a/sound/soc/apple/mca.c
+++ b/sound/soc/apple/mca.c
@@ -210,7 +210,7 @@ static void mca_fe_early_trigger(struct snd_pcm_substream *substream, int cmd,
 			   SERDES_CONF_SOME_RST);
 		readl_relaxed(cl->base + serdes_conf);
 		mca_modify(cl, serdes_conf, SERDES_STATUS_RST, 0);
-		WARN_ON(readl_relaxed(cl->base + REG_SERDES_STATUS) &
+		WARN_ON(readl_relaxed(cl->base + serdes_unit + REG_SERDES_STATUS) &
 			SERDES_STATUS_RST);
 		break;
 	default:
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] ASoC: apple: mca: Fix SERDES reset sequence
  2023-02-24 15:33 [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Martin Povišer
@ 2023-02-24 15:33 ` Martin Povišer
  2023-02-24 15:33 ` [PATCH 3/3] ASoC: apple: mca: Improve handling of unavailable DMA channels Martin Povišer
  2023-02-24 18:19 ` [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Martin Povišer @ 2023-02-24 15:33 UTC (permalink / raw)
  To: Martin Povišer, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai
  Cc: asahi, alsa-devel, linux-kernel

Fix the reset sequence of reads and writes that we invoke from within
the early trigger. It looks like there never was a SERDES_CONF_SOME_RST
bit that should be involved in the reset sequence, and its presence in
the driver code is a mistake from earlier.

Instead, the reset sequence should go as follows: We should switch the
the SERDES unit's SYNC_SEL mux to the value of 7 (so outside the range
of 1...6 representing cluster's SYNCGEN units), then raise the RST bit
in SERDES_STATUS and wait for it to clear.

Properly resetting the SERDES unit fixes frame desynchronization hazard
in case of long frames (longer than 4 used slots). The desynchronization
manifests itself by rotating the PCM channels.

Fixes: 3df5d0d97289 ("ASoC: apple: mca: Start new platform driver")
Signed-off-by: Martin Povišer <povik+lin@cutebit.org>
---
 sound/soc/apple/mca.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/sound/soc/apple/mca.c b/sound/soc/apple/mca.c
index 9cceeb259952..aea08c7b2ee8 100644
--- a/sound/soc/apple/mca.c
+++ b/sound/soc/apple/mca.c
@@ -101,7 +101,6 @@
 #define SERDES_CONF_UNK3	BIT(14)
 #define SERDES_CONF_NO_DATA_FEEDBACK	BIT(15)
 #define SERDES_CONF_SYNC_SEL	GENMASK(18, 16)
-#define SERDES_CONF_SOME_RST	BIT(19)
 #define REG_TX_SERDES_BITSTART	0x08
 #define REG_RX_SERDES_BITSTART	0x0c
 #define REG_TX_SERDES_SLOTMASK	0x0c
@@ -203,15 +202,24 @@ static void mca_fe_early_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		mca_modify(cl, serdes_conf, SERDES_CONF_SYNC_SEL,
+			   FIELD_PREP(SERDES_CONF_SYNC_SEL, 0));
+		mca_modify(cl, serdes_conf, SERDES_CONF_SYNC_SEL,
+			   FIELD_PREP(SERDES_CONF_SYNC_SEL, 7));
 		mca_modify(cl, serdes_unit + REG_SERDES_STATUS,
 			   SERDES_STATUS_EN | SERDES_STATUS_RST,
 			   SERDES_STATUS_RST);
-		mca_modify(cl, serdes_conf, SERDES_CONF_SOME_RST,
-			   SERDES_CONF_SOME_RST);
-		readl_relaxed(cl->base + serdes_conf);
-		mca_modify(cl, serdes_conf, SERDES_STATUS_RST, 0);
+		/*
+		 * Experiments suggest that it takes at most ~1 us
+		 * for the bit to clear, so wait 2 us for good measure.
+		 */
+		udelay(2);
 		WARN_ON(readl_relaxed(cl->base + serdes_unit + REG_SERDES_STATUS) &
 			SERDES_STATUS_RST);
+		mca_modify(cl, serdes_conf, SERDES_CONF_SYNC_SEL,
+			   FIELD_PREP(SERDES_CONF_SYNC_SEL, 0));
+		mca_modify(cl, serdes_conf, SERDES_CONF_SYNC_SEL,
+			   FIELD_PREP(SERDES_CONF_SYNC_SEL, cl->no + 1));
 		break;
 	default:
 		break;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] ASoC: apple: mca: Improve handling of unavailable DMA channels
  2023-02-24 15:33 [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Martin Povišer
  2023-02-24 15:33 ` [PATCH 2/3] ASoC: apple: mca: Fix SERDES reset sequence Martin Povišer
@ 2023-02-24 15:33 ` Martin Povišer
  2023-02-24 18:19 ` [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Martin Povišer @ 2023-02-24 15:33 UTC (permalink / raw)
  To: Martin Povišer, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai
  Cc: asahi, alsa-devel, linux-kernel

When we fail to obtain a DMA channel, don't return a blanket -EINVAL,
instead return the original error code if there's one. This makes
deferring work as it should. Also don't print an error message for
-EPROBE_DEFER.

Fixes: 4ec8179c212f ("ASoC: apple: mca: Postpone requesting of DMA channels")
Signed-off-by: Martin Povišer <povik+lin@cutebit.org>
---
 sound/soc/apple/mca.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/soc/apple/mca.c b/sound/soc/apple/mca.c
index aea08c7b2ee8..64750db9b963 100644
--- a/sound/soc/apple/mca.c
+++ b/sound/soc/apple/mca.c
@@ -950,10 +950,17 @@ static int mca_pcm_new(struct snd_soc_component *component,
 		chan = mca_request_dma_channel(cl, i);
 
 		if (IS_ERR_OR_NULL(chan)) {
+			mca_pcm_free(component, rtd->pcm);
+
+			if (chan && PTR_ERR(chan) == -EPROBE_DEFER)
+				return PTR_ERR(chan);
+
 			dev_err(component->dev, "unable to obtain DMA channel (stream %d cluster %d): %pe\n",
 				i, cl->no, chan);
-			mca_pcm_free(component, rtd->pcm);
-			return -EINVAL;
+
+			if (!chan)
+				return -EINVAL;
+			return PTR_ERR(chan);
 		}
 
 		cl->dma_chans[i] = chan;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset
  2023-02-24 15:33 [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Martin Povišer
  2023-02-24 15:33 ` [PATCH 2/3] ASoC: apple: mca: Fix SERDES reset sequence Martin Povišer
  2023-02-24 15:33 ` [PATCH 3/3] ASoC: apple: mca: Improve handling of unavailable DMA channels Martin Povišer
@ 2023-02-24 18:19 ` Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2023-02-24 18:19 UTC (permalink / raw)
  To: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Martin Povišer
  Cc: asahi, alsa-devel, linux-kernel

On Fri, 24 Feb 2023 16:33:00 +0100, Martin Povišer wrote:
> From within the early trigger we are doing a reset of the SERDES unit,
> but the final status read is on a bad address. Add the missing SERDES
> unit offset in calculation of the address.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/3] ASoC: apple: mca: Fix final status read on SERDES reset
      commit: aaf5f0d76b6e1870e3674408de2b13a92a4d4059
[2/3] ASoC: apple: mca: Fix SERDES reset sequence
      commit: d8b3e396088d787771f19fd3b7949e080dc31d6f
[3/3] ASoC: apple: mca: Improve handling of unavailable DMA channels
      commit: fb1847cc460c127b12720119eae5f438ffc62e85

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-24 18:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-24 15:33 [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Martin Povišer
2023-02-24 15:33 ` [PATCH 2/3] ASoC: apple: mca: Fix SERDES reset sequence Martin Povišer
2023-02-24 15:33 ` [PATCH 3/3] ASoC: apple: mca: Improve handling of unavailable DMA channels Martin Povišer
2023-02-24 18:19 ` [PATCH 1/3] ASoC: apple: mca: Fix final status read on SERDES reset Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).