2022-08-08 23:46:02

by Martin Povišer

[permalink] [raw]
Subject: [PATCH 0/3] ASoC platform driver for Apple MCA

Hi,

for review I am posting another version of the ASoC platform driver to be
used on Apple M1/M2 platforms (some previous version was attached to the
macaudio RFC v2 [0]).

Martin

Changes since [0]:
- addition of locking (extra commit)
- transition to set_bclk_ratio (instead of getting the bclk ratio from set_sysclk)
- using shared reset control and documenting the reset in binding
- formatting, comments, and a minor fix to hw driving

[0] https://lore.kernel.org/asahi/[email protected]/

Martin Povišer (3):
dt-bindings: sound: Add Apple MCA I2S transceiver
ASoC: apple: mca: Start new platform driver
ASoC: apple: mca: Add locks on foreign cluster access

.../devicetree/bindings/sound/apple,mca.yaml | 109 ++
MAINTAINERS | 8 +
sound/soc/Kconfig | 1 +
sound/soc/Makefile | 1 +
sound/soc/apple/Kconfig | 9 +
sound/soc/apple/Makefile | 3 +
sound/soc/apple/mca.c | 1173 +++++++++++++++++
7 files changed, 1304 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/apple,mca.yaml
create mode 100644 sound/soc/apple/Kconfig
create mode 100644 sound/soc/apple/Makefile
create mode 100644 sound/soc/apple/mca.c

--
2.33.0


2022-08-08 23:50:16

by Martin Povišer

[permalink] [raw]
Subject: [PATCH 3/3] ASoC: apple: mca: Add locks on foreign cluster access

In DAI ops, accesses to the native cluster (of the DAI), and to data of
clusters related to it by a DPCM frontend-backend link, should have
been synchronized by the 'pcm_mutex' lock at ASoC level.

What is not covered are the 'port_driver' accesses on foreign clusters
to which the current cluster has no a priori relation, so fill in
locking for that. (This should only matter in bizarre configurations of
sharing one MCA peripheral between ASoC cards.)

Signed-off-by: Martin Povišer <[email protected]>
---
sound/soc/apple/mca.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/sound/soc/apple/mca.c b/sound/soc/apple/mca.c
index ab41fd1a2444..1e2464e89d1c 100644
--- a/sound/soc/apple/mca.c
+++ b/sound/soc/apple/mca.c
@@ -159,6 +159,9 @@ struct mca_data {
struct reset_control *rstc;
struct device_link *pd_link;

+ /* Mutex for accessing port_driver of foreign clusters */
+ struct mutex port_mutex;
+
int nclusters;
struct mca_cluster clusters[];
};
@@ -297,16 +300,21 @@ static bool mca_fe_clocks_in_use(struct mca_cluster *cl)
struct mca_cluster *be_cl;
int stream, i;

+ mutex_lock(&mca->port_mutex);
for (i = 0; i < mca->nclusters; i++) {
be_cl = &mca->clusters[i];

if (be_cl->port_driver != cl->no)
continue;

- for_each_pcm_streams(stream)
- if (be_cl->clocks_in_use[stream])
+ for_each_pcm_streams(stream) {
+ if (be_cl->clocks_in_use[stream]) {
+ mutex_unlock(&mca->port_mutex);
return true;
+ }
+ }
}
+ mutex_unlock(&mca->port_mutex);
return false;
}

@@ -331,8 +339,10 @@ static int mca_be_prepare(struct snd_pcm_substream *substream,
*/
if (!mca_fe_clocks_in_use(fe_cl)) {
ret = mca_fe_enable_clocks(fe_cl);
- if (ret < 0)
+ if (ret < 0) {
+ mutex_unlock(&mca->port_mutex);
return ret;
+ }
}

cl->clocks_in_use[substream->stream] = true;
@@ -350,6 +360,11 @@ static int mca_be_hw_free(struct snd_pcm_substream *substream,
if (cl->port_driver < 0)
return -EINVAL;

+ /*
+ * We are operating on a foreign cluster here, but since we
+ * belong to the same PCM, accesses should have been
+ * synchronized at ASoC level.
+ */
fe_cl = &mca->clusters[cl->port_driver];
if (!mca_fe_clocks_in_use(fe_cl))
return 0; /* Nothing to do */
@@ -722,7 +737,9 @@ static int mca_be_startup(struct snd_pcm_substream *substream,
cl->base + REG_PORT_CLOCK_SEL);
writel_relaxed(PORT_DATA_SEL_TXA(fe_cl->no),
cl->base + REG_PORT_DATA_SEL);
+ mutex_lock(&mca->port_mutex);
cl->port_driver = fe_cl->no;
+ mutex_unlock(&mca->port_mutex);
cl->port_started[substream->stream] = true;

return 0;
@@ -732,6 +749,7 @@ static void mca_be_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct mca_cluster *cl = mca_dai_to_cluster(dai);
+ struct mca_data *mca = cl->host;

cl->port_started[substream->stream] = false;

@@ -742,7 +760,9 @@ static void mca_be_shutdown(struct snd_pcm_substream *substream,
*/
writel_relaxed(0, cl->base + REG_PORT_ENABLES);
writel_relaxed(0, cl->base + REG_PORT_DATA_SEL);
+ mutex_lock(&mca->port_mutex);
cl->port_driver = -1;
+ mutex_unlock(&mca->port_mutex);
}
}

@@ -963,6 +983,7 @@ static int apple_mca_probe(struct platform_device *pdev)
return -ENOMEM;
mca->dev = &pdev->dev;
mca->nclusters = nclusters;
+ mutex_init(&mca->port_mutex);
platform_set_drvdata(pdev, mca);
clusters = mca->clusters;

--
2.33.0

2022-08-09 07:33:55

by Martin Povišer

[permalink] [raw]
Subject: Re: [PATCH 3/3] ASoC: apple: mca: Add locks on foreign cluster access


> @@ -331,8 +339,10 @@ static int mca_be_prepare(struct snd_pcm_substream *substream,
> */
> if (!mca_fe_clocks_in_use(fe_cl)) {
> ret = mca_fe_enable_clocks(fe_cl);
> - if (ret < 0)
> + if (ret < 0) {
> + mutex_unlock(&mca->port_mutex);
> return ret;
> + }
> }

Stray unlock here

--
Martin