All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes
@ 2023-01-30 12:06 ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

Hi,

This series adds runtime PM support for Microchip SPDIFRX driver.
Along with it I added few fixes identified while going though the code
and playing with Microchip SPDIFRX controller.

Thank you,
Claudiu Beznea

Claudiu Beznea (8):
  ASoC: mchp-spdifrx: fix controls which rely on rsr register
  ASoC: mchp-spdifrx: fix return value in case completion times out
  ASoC: mchp-spdifrx: fix controls that works with completion mechanism
  ASoC: mchp-spdifrx: disable all interrupts in
    mchp_spdifrx_dai_remove()
  ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
  ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
  ASoC: mchp-spdifrx: add runtime pm support
  ASoC: mchp-spdifrx: document data structures

 sound/soc/atmel/mchp-spdifrx.c | 552 ++++++++++++++++++++++++---------
 1 file changed, 403 insertions(+), 149 deletions(-)

-- 
2.34.1


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

* [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes
@ 2023-01-30 12:06 ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Hi,

This series adds runtime PM support for Microchip SPDIFRX driver.
Along with it I added few fixes identified while going though the code
and playing with Microchip SPDIFRX controller.

Thank you,
Claudiu Beznea

Claudiu Beznea (8):
  ASoC: mchp-spdifrx: fix controls which rely on rsr register
  ASoC: mchp-spdifrx: fix return value in case completion times out
  ASoC: mchp-spdifrx: fix controls that works with completion mechanism
  ASoC: mchp-spdifrx: disable all interrupts in
    mchp_spdifrx_dai_remove()
  ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
  ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
  ASoC: mchp-spdifrx: add runtime pm support
  ASoC: mchp-spdifrx: document data structures

 sound/soc/atmel/mchp-spdifrx.c | 552 ++++++++++++++++++++++++---------
 1 file changed, 403 insertions(+), 149 deletions(-)

-- 
2.34.1


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

* [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes
@ 2023-01-30 12:06 ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Hi,

This series adds runtime PM support for Microchip SPDIFRX driver.
Along with it I added few fixes identified while going though the code
and playing with Microchip SPDIFRX controller.

Thank you,
Claudiu Beznea

Claudiu Beznea (8):
  ASoC: mchp-spdifrx: fix controls which rely on rsr register
  ASoC: mchp-spdifrx: fix return value in case completion times out
  ASoC: mchp-spdifrx: fix controls that works with completion mechanism
  ASoC: mchp-spdifrx: disable all interrupts in
    mchp_spdifrx_dai_remove()
  ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
  ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
  ASoC: mchp-spdifrx: add runtime pm support
  ASoC: mchp-spdifrx: document data structures

 sound/soc/atmel/mchp-spdifrx.c | 552 ++++++++++++++++++++++++---------
 1 file changed, 403 insertions(+), 149 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

The SPDIFRX block is clocked by 2 clocks: peripheral and generic clocks.
Peripheral clock feeds user interface (registers) and generic clock feeds
the receiver.

To enable the receiver the generic clock needs to be enabled and also the
ENABLE bit of MCHP_SPDIFRX_MR register need to be set.

The signal control exported by mchp-spdifrx driver reports wrong status
when the receiver is disabled. This can happen when requesting the signal
and the capture was not previously started. To solve this the receiver
needs to be enabled (by enabling generic clock and setting ENABLE bit of
MR register) before reading the signal status.

As with this fix there are 2 paths now that need to control the generic
clock and ENABLE bit of SPDIFRX_MR register (one path though controls, one
path though configuration) a mutex has been introduced. We can't rely on
subsystem locking as the controls are protected by
struct snd_card::controls_rwsem semaphore and configuration is protected
by a different lock (embedded in snd_pcm_stream_lock_irq()).

The introduction of mutex is also extended to other controls which rely on
SPDIFRX_RSR.ULOCK bit as it has been discovered experimentally that having
both clocks enabled but not the receiver (through ENABLE bit of SPDIFRX.MR)
leads to inconsistent values of SPDIFRX_RSR.ULOCK. Thus on some controls we
rely on software state (dev->trigger_enabled protected by mutex) to
retrieve proper values.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 192 ++++++++++++++++++++++++---------
 1 file changed, 142 insertions(+), 50 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index ec0705cc40fa..2d86e0ec930f 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -233,11 +233,13 @@ struct mchp_spdifrx_dev {
 	struct mchp_spdifrx_mixer_control	control;
 	spinlock_t				blockend_lock;	/* protect access to blockend_refcount */
 	int					blockend_refcount;
+	struct mutex				mlock;
 	struct device				*dev;
 	struct regmap				*regmap;
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				fmt;
+	unsigned int				trigger_enabled;
 	unsigned int				gclk_enabled:1;
 };
 
@@ -353,47 +355,40 @@ static int mchp_spdifrx_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
-	u32 mr;
-	int running;
-	int ret;
-
-	regmap_read(dev->regmap, SPDIFRX_MR, &mr);
-	running = !!(mr & SPDIFRX_MR_RXEN_ENABLE);
+	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (!running) {
-			mr &= ~SPDIFRX_MR_RXEN_MASK;
-			mr |= SPDIFRX_MR_RXEN_ENABLE;
-			/* enable overrun interrupts */
-			regmap_write(dev->regmap, SPDIFRX_IER,
-				     SPDIFRX_IR_OVERRUN);
-		}
+		mutex_lock(&dev->mlock);
+		/* Enable overrun interrupts */
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_OVERRUN);
+
+		/* Enable receiver. */
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_ENABLE);
+		dev->trigger_enabled = true;
+		mutex_unlock(&dev->mlock);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (running) {
-			mr &= ~SPDIFRX_MR_RXEN_MASK;
-			mr |= SPDIFRX_MR_RXEN_DISABLE;
-			/* disable overrun interrupts */
-			regmap_write(dev->regmap, SPDIFRX_IDR,
-				     SPDIFRX_IR_OVERRUN);
-		}
+		mutex_lock(&dev->mlock);
+		/* Disable overrun interrupts */
+		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_OVERRUN);
+
+		/* Disable receiver. */
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_DISABLE);
+		dev->trigger_enabled = false;
+		mutex_unlock(&dev->mlock);
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
-	ret = regmap_write(dev->regmap, SPDIFRX_MR, mr);
-	if (ret) {
-		dev_err(dev->dev, "unable to enable/disable RX: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return ret;
 }
 
 static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
@@ -413,13 +408,6 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	regmap_read(dev->regmap, SPDIFRX_MR, &mr);
-
-	if (mr & SPDIFRX_MR_RXEN_ENABLE) {
-		dev_err(dev->dev, "PCM already running\n");
-		return -EBUSY;
-	}
-
 	if (params_channels(params) != SPDIFRX_CHANNELS) {
 		dev_err(dev->dev, "unsupported number of channels: %d\n",
 			params_channels(params));
@@ -445,6 +433,13 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
+	mutex_lock(&dev->mlock);
+	if (dev->trigger_enabled) {
+		dev_err(dev->dev, "PCM already running\n");
+		ret = -EBUSY;
+		goto unlock;
+	}
+
 	if (dev->gclk_enabled) {
 		clk_disable_unprepare(dev->gclk);
 		dev->gclk_enabled = 0;
@@ -455,19 +450,24 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev,
 			"unable to set gclk min rate: rate %u * ratio %u + 1\n",
 			params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
-		return ret;
+		goto unlock;
 	}
 	ret = clk_prepare_enable(dev->gclk);
 	if (ret) {
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
-		return ret;
+		goto unlock;
 	}
 	dev->gclk_enabled = 1;
 
 	dev_dbg(dev->dev, "GCLK range min set to %d\n",
 		params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
 
-	return regmap_write(dev->regmap, SPDIFRX_MR, mr);
+	ret = regmap_write(dev->regmap, SPDIFRX_MR, mr);
+
+unlock:
+	mutex_unlock(&dev->mlock);
+
+	return ret;
 }
 
 static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
@@ -475,10 +475,12 @@ static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
 {
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
+	mutex_lock(&dev->mlock);
 	if (dev->gclk_enabled) {
 		clk_disable_unprepare(dev->gclk);
 		dev->gclk_enabled = 0;
 	}
+	mutex_unlock(&dev->mlock);
 	return 0;
 }
 
@@ -627,10 +629,24 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	bool ulock_old = ctrl->ulock;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->ulock = !(val & SPDIFRX_RSR_ULOCK);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		ctrl->ulock = !(val & SPDIFRX_RSR_ULOCK);
+	} else {
+		ctrl->ulock = 0;
+	}
+
 	uvalue->value.integer.value[0] = ctrl->ulock;
 
+	mutex_unlock(&dev->mlock);
+
 	return ulock_old != ctrl->ulock;
 }
 
@@ -643,8 +659,22 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	bool badf_old = ctrl->badf;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->badf = !!(val & SPDIFRX_RSR_BADF);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		ctrl->badf = !!(val & SPDIFRX_RSR_BADF);
+	} else {
+		ctrl->badf = 0;
+	}
+
+	mutex_unlock(&dev->mlock);
+
 	uvalue->value.integer.value[0] = ctrl->badf;
 
 	return badf_old != ctrl->badf;
@@ -656,11 +686,48 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
-	u32 val;
+	u32 val = ~0U, loops = 10;
+	int ret;
 	bool signal_old = ctrl->signal;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->signal = !(val & SPDIFRX_RSR_NOSIGNAL);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * To get the signal we need to have receiver enabled. This
+	 * could be enabled also from trigger() function thus we need to
+	 * take care of not disabling the receiver when it runs.
+	 */
+	if (!dev->trigger_enabled) {
+		ret = clk_prepare_enable(dev->gclk);
+		if (ret)
+			goto unlock;
+
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_ENABLE);
+
+		/* Wait for RSR.ULOCK bit. */
+		while (--loops) {
+			regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+			if (!(val & SPDIFRX_RSR_ULOCK))
+				break;
+			usleep_range(100, 150);
+		}
+
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_DISABLE);
+
+		clk_disable_unprepare(dev->gclk);
+	} else {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+	}
+
+unlock:
+	mutex_unlock(&dev->mlock);
+
+	if (!(val & SPDIFRX_RSR_ULOCK))
+		ctrl->signal = !(val & SPDIFRX_RSR_NOSIGNAL);
+	else
+		ctrl->signal = 0;
 	uvalue->value.integer.value[0] = ctrl->signal;
 
 	return signal_old != ctrl->signal;
@@ -685,18 +752,32 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	int rate;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-
-	/* if the receiver is not locked, ISF data is invalid */
-	if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		/* If the receiver is not locked, ISF data is invalid. */
+		if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
+			ucontrol->value.integer.value[0] = 0;
+			goto unlock;
+		}
+	} else {
+		/* Reveicer is not locked, IFS data is invalid. */
 		ucontrol->value.integer.value[0] = 0;
-		return 0;
+		goto unlock;
 	}
 
 	rate = clk_get_rate(dev->gclk);
 
 	ucontrol->value.integer.value[0] = rate / (32 * SPDIFRX_RSR_IFS(val));
 
+unlock:
+	mutex_unlock(&dev->mlock);
 	return 0;
 }
 
@@ -913,7 +994,18 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 			"failed to get the PMC generated clock: %d\n", err);
 		return err;
 	}
+
+	/*
+	 * Signal control need a valid rate on gclk. hw_params() configures
+	 * it propertly but requesting signal before any hw_params() has been
+	 * called lead to invalid value returned for signal. Thus, configure
+	 * gclk at a valid rate, here, in initialization, to simplify the
+	 * control path.
+	 */
+	clk_set_min_rate(dev->gclk, 48000 * SPDIFRX_GCLK_RATIO_MIN + 1);
+
 	spin_lock_init(&dev->blockend_lock);
+	mutex_init(&dev->mlock);
 
 	dev->dev = &pdev->dev;
 	dev->regmap = regmap;
-- 
2.34.1


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

* [PATCH 1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

The SPDIFRX block is clocked by 2 clocks: peripheral and generic clocks.
Peripheral clock feeds user interface (registers) and generic clock feeds
the receiver.

To enable the receiver the generic clock needs to be enabled and also the
ENABLE bit of MCHP_SPDIFRX_MR register need to be set.

The signal control exported by mchp-spdifrx driver reports wrong status
when the receiver is disabled. This can happen when requesting the signal
and the capture was not previously started. To solve this the receiver
needs to be enabled (by enabling generic clock and setting ENABLE bit of
MR register) before reading the signal status.

As with this fix there are 2 paths now that need to control the generic
clock and ENABLE bit of SPDIFRX_MR register (one path though controls, one
path though configuration) a mutex has been introduced. We can't rely on
subsystem locking as the controls are protected by
struct snd_card::controls_rwsem semaphore and configuration is protected
by a different lock (embedded in snd_pcm_stream_lock_irq()).

The introduction of mutex is also extended to other controls which rely on
SPDIFRX_RSR.ULOCK bit as it has been discovered experimentally that having
both clocks enabled but not the receiver (through ENABLE bit of SPDIFRX.MR)
leads to inconsistent values of SPDIFRX_RSR.ULOCK. Thus on some controls we
rely on software state (dev->trigger_enabled protected by mutex) to
retrieve proper values.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 192 ++++++++++++++++++++++++---------
 1 file changed, 142 insertions(+), 50 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index ec0705cc40fa..2d86e0ec930f 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -233,11 +233,13 @@ struct mchp_spdifrx_dev {
 	struct mchp_spdifrx_mixer_control	control;
 	spinlock_t				blockend_lock;	/* protect access to blockend_refcount */
 	int					blockend_refcount;
+	struct mutex				mlock;
 	struct device				*dev;
 	struct regmap				*regmap;
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				fmt;
+	unsigned int				trigger_enabled;
 	unsigned int				gclk_enabled:1;
 };
 
@@ -353,47 +355,40 @@ static int mchp_spdifrx_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
-	u32 mr;
-	int running;
-	int ret;
-
-	regmap_read(dev->regmap, SPDIFRX_MR, &mr);
-	running = !!(mr & SPDIFRX_MR_RXEN_ENABLE);
+	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (!running) {
-			mr &= ~SPDIFRX_MR_RXEN_MASK;
-			mr |= SPDIFRX_MR_RXEN_ENABLE;
-			/* enable overrun interrupts */
-			regmap_write(dev->regmap, SPDIFRX_IER,
-				     SPDIFRX_IR_OVERRUN);
-		}
+		mutex_lock(&dev->mlock);
+		/* Enable overrun interrupts */
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_OVERRUN);
+
+		/* Enable receiver. */
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_ENABLE);
+		dev->trigger_enabled = true;
+		mutex_unlock(&dev->mlock);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (running) {
-			mr &= ~SPDIFRX_MR_RXEN_MASK;
-			mr |= SPDIFRX_MR_RXEN_DISABLE;
-			/* disable overrun interrupts */
-			regmap_write(dev->regmap, SPDIFRX_IDR,
-				     SPDIFRX_IR_OVERRUN);
-		}
+		mutex_lock(&dev->mlock);
+		/* Disable overrun interrupts */
+		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_OVERRUN);
+
+		/* Disable receiver. */
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_DISABLE);
+		dev->trigger_enabled = false;
+		mutex_unlock(&dev->mlock);
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
-	ret = regmap_write(dev->regmap, SPDIFRX_MR, mr);
-	if (ret) {
-		dev_err(dev->dev, "unable to enable/disable RX: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return ret;
 }
 
 static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
@@ -413,13 +408,6 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	regmap_read(dev->regmap, SPDIFRX_MR, &mr);
-
-	if (mr & SPDIFRX_MR_RXEN_ENABLE) {
-		dev_err(dev->dev, "PCM already running\n");
-		return -EBUSY;
-	}
-
 	if (params_channels(params) != SPDIFRX_CHANNELS) {
 		dev_err(dev->dev, "unsupported number of channels: %d\n",
 			params_channels(params));
@@ -445,6 +433,13 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
+	mutex_lock(&dev->mlock);
+	if (dev->trigger_enabled) {
+		dev_err(dev->dev, "PCM already running\n");
+		ret = -EBUSY;
+		goto unlock;
+	}
+
 	if (dev->gclk_enabled) {
 		clk_disable_unprepare(dev->gclk);
 		dev->gclk_enabled = 0;
@@ -455,19 +450,24 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev,
 			"unable to set gclk min rate: rate %u * ratio %u + 1\n",
 			params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
-		return ret;
+		goto unlock;
 	}
 	ret = clk_prepare_enable(dev->gclk);
 	if (ret) {
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
-		return ret;
+		goto unlock;
 	}
 	dev->gclk_enabled = 1;
 
 	dev_dbg(dev->dev, "GCLK range min set to %d\n",
 		params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
 
-	return regmap_write(dev->regmap, SPDIFRX_MR, mr);
+	ret = regmap_write(dev->regmap, SPDIFRX_MR, mr);
+
+unlock:
+	mutex_unlock(&dev->mlock);
+
+	return ret;
 }
 
 static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
@@ -475,10 +475,12 @@ static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
 {
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
+	mutex_lock(&dev->mlock);
 	if (dev->gclk_enabled) {
 		clk_disable_unprepare(dev->gclk);
 		dev->gclk_enabled = 0;
 	}
+	mutex_unlock(&dev->mlock);
 	return 0;
 }
 
@@ -627,10 +629,24 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	bool ulock_old = ctrl->ulock;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->ulock = !(val & SPDIFRX_RSR_ULOCK);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		ctrl->ulock = !(val & SPDIFRX_RSR_ULOCK);
+	} else {
+		ctrl->ulock = 0;
+	}
+
 	uvalue->value.integer.value[0] = ctrl->ulock;
 
+	mutex_unlock(&dev->mlock);
+
 	return ulock_old != ctrl->ulock;
 }
 
@@ -643,8 +659,22 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	bool badf_old = ctrl->badf;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->badf = !!(val & SPDIFRX_RSR_BADF);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		ctrl->badf = !!(val & SPDIFRX_RSR_BADF);
+	} else {
+		ctrl->badf = 0;
+	}
+
+	mutex_unlock(&dev->mlock);
+
 	uvalue->value.integer.value[0] = ctrl->badf;
 
 	return badf_old != ctrl->badf;
@@ -656,11 +686,48 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
-	u32 val;
+	u32 val = ~0U, loops = 10;
+	int ret;
 	bool signal_old = ctrl->signal;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->signal = !(val & SPDIFRX_RSR_NOSIGNAL);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * To get the signal we need to have receiver enabled. This
+	 * could be enabled also from trigger() function thus we need to
+	 * take care of not disabling the receiver when it runs.
+	 */
+	if (!dev->trigger_enabled) {
+		ret = clk_prepare_enable(dev->gclk);
+		if (ret)
+			goto unlock;
+
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_ENABLE);
+
+		/* Wait for RSR.ULOCK bit. */
+		while (--loops) {
+			regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+			if (!(val & SPDIFRX_RSR_ULOCK))
+				break;
+			usleep_range(100, 150);
+		}
+
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_DISABLE);
+
+		clk_disable_unprepare(dev->gclk);
+	} else {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+	}
+
+unlock:
+	mutex_unlock(&dev->mlock);
+
+	if (!(val & SPDIFRX_RSR_ULOCK))
+		ctrl->signal = !(val & SPDIFRX_RSR_NOSIGNAL);
+	else
+		ctrl->signal = 0;
 	uvalue->value.integer.value[0] = ctrl->signal;
 
 	return signal_old != ctrl->signal;
@@ -685,18 +752,32 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	int rate;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-
-	/* if the receiver is not locked, ISF data is invalid */
-	if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		/* If the receiver is not locked, ISF data is invalid. */
+		if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
+			ucontrol->value.integer.value[0] = 0;
+			goto unlock;
+		}
+	} else {
+		/* Reveicer is not locked, IFS data is invalid. */
 		ucontrol->value.integer.value[0] = 0;
-		return 0;
+		goto unlock;
 	}
 
 	rate = clk_get_rate(dev->gclk);
 
 	ucontrol->value.integer.value[0] = rate / (32 * SPDIFRX_RSR_IFS(val));
 
+unlock:
+	mutex_unlock(&dev->mlock);
 	return 0;
 }
 
@@ -913,7 +994,18 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 			"failed to get the PMC generated clock: %d\n", err);
 		return err;
 	}
+
+	/*
+	 * Signal control need a valid rate on gclk. hw_params() configures
+	 * it propertly but requesting signal before any hw_params() has been
+	 * called lead to invalid value returned for signal. Thus, configure
+	 * gclk at a valid rate, here, in initialization, to simplify the
+	 * control path.
+	 */
+	clk_set_min_rate(dev->gclk, 48000 * SPDIFRX_GCLK_RATIO_MIN + 1);
+
 	spin_lock_init(&dev->blockend_lock);
+	mutex_init(&dev->mlock);
 
 	dev->dev = &pdev->dev;
 	dev->regmap = regmap;
-- 
2.34.1


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

* [PATCH 1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

The SPDIFRX block is clocked by 2 clocks: peripheral and generic clocks.
Peripheral clock feeds user interface (registers) and generic clock feeds
the receiver.

To enable the receiver the generic clock needs to be enabled and also the
ENABLE bit of MCHP_SPDIFRX_MR register need to be set.

The signal control exported by mchp-spdifrx driver reports wrong status
when the receiver is disabled. This can happen when requesting the signal
and the capture was not previously started. To solve this the receiver
needs to be enabled (by enabling generic clock and setting ENABLE bit of
MR register) before reading the signal status.

As with this fix there are 2 paths now that need to control the generic
clock and ENABLE bit of SPDIFRX_MR register (one path though controls, one
path though configuration) a mutex has been introduced. We can't rely on
subsystem locking as the controls are protected by
struct snd_card::controls_rwsem semaphore and configuration is protected
by a different lock (embedded in snd_pcm_stream_lock_irq()).

The introduction of mutex is also extended to other controls which rely on
SPDIFRX_RSR.ULOCK bit as it has been discovered experimentally that having
both clocks enabled but not the receiver (through ENABLE bit of SPDIFRX.MR)
leads to inconsistent values of SPDIFRX_RSR.ULOCK. Thus on some controls we
rely on software state (dev->trigger_enabled protected by mutex) to
retrieve proper values.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 192 ++++++++++++++++++++++++---------
 1 file changed, 142 insertions(+), 50 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index ec0705cc40fa..2d86e0ec930f 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -233,11 +233,13 @@ struct mchp_spdifrx_dev {
 	struct mchp_spdifrx_mixer_control	control;
 	spinlock_t				blockend_lock;	/* protect access to blockend_refcount */
 	int					blockend_refcount;
+	struct mutex				mlock;
 	struct device				*dev;
 	struct regmap				*regmap;
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				fmt;
+	unsigned int				trigger_enabled;
 	unsigned int				gclk_enabled:1;
 };
 
@@ -353,47 +355,40 @@ static int mchp_spdifrx_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
-	u32 mr;
-	int running;
-	int ret;
-
-	regmap_read(dev->regmap, SPDIFRX_MR, &mr);
-	running = !!(mr & SPDIFRX_MR_RXEN_ENABLE);
+	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (!running) {
-			mr &= ~SPDIFRX_MR_RXEN_MASK;
-			mr |= SPDIFRX_MR_RXEN_ENABLE;
-			/* enable overrun interrupts */
-			regmap_write(dev->regmap, SPDIFRX_IER,
-				     SPDIFRX_IR_OVERRUN);
-		}
+		mutex_lock(&dev->mlock);
+		/* Enable overrun interrupts */
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_OVERRUN);
+
+		/* Enable receiver. */
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_ENABLE);
+		dev->trigger_enabled = true;
+		mutex_unlock(&dev->mlock);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (running) {
-			mr &= ~SPDIFRX_MR_RXEN_MASK;
-			mr |= SPDIFRX_MR_RXEN_DISABLE;
-			/* disable overrun interrupts */
-			regmap_write(dev->regmap, SPDIFRX_IDR,
-				     SPDIFRX_IR_OVERRUN);
-		}
+		mutex_lock(&dev->mlock);
+		/* Disable overrun interrupts */
+		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_OVERRUN);
+
+		/* Disable receiver. */
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_DISABLE);
+		dev->trigger_enabled = false;
+		mutex_unlock(&dev->mlock);
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
-	ret = regmap_write(dev->regmap, SPDIFRX_MR, mr);
-	if (ret) {
-		dev_err(dev->dev, "unable to enable/disable RX: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return ret;
 }
 
 static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
@@ -413,13 +408,6 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	regmap_read(dev->regmap, SPDIFRX_MR, &mr);
-
-	if (mr & SPDIFRX_MR_RXEN_ENABLE) {
-		dev_err(dev->dev, "PCM already running\n");
-		return -EBUSY;
-	}
-
 	if (params_channels(params) != SPDIFRX_CHANNELS) {
 		dev_err(dev->dev, "unsupported number of channels: %d\n",
 			params_channels(params));
@@ -445,6 +433,13 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
+	mutex_lock(&dev->mlock);
+	if (dev->trigger_enabled) {
+		dev_err(dev->dev, "PCM already running\n");
+		ret = -EBUSY;
+		goto unlock;
+	}
+
 	if (dev->gclk_enabled) {
 		clk_disable_unprepare(dev->gclk);
 		dev->gclk_enabled = 0;
@@ -455,19 +450,24 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev,
 			"unable to set gclk min rate: rate %u * ratio %u + 1\n",
 			params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
-		return ret;
+		goto unlock;
 	}
 	ret = clk_prepare_enable(dev->gclk);
 	if (ret) {
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
-		return ret;
+		goto unlock;
 	}
 	dev->gclk_enabled = 1;
 
 	dev_dbg(dev->dev, "GCLK range min set to %d\n",
 		params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
 
-	return regmap_write(dev->regmap, SPDIFRX_MR, mr);
+	ret = regmap_write(dev->regmap, SPDIFRX_MR, mr);
+
+unlock:
+	mutex_unlock(&dev->mlock);
+
+	return ret;
 }
 
 static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
@@ -475,10 +475,12 @@ static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
 {
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
+	mutex_lock(&dev->mlock);
 	if (dev->gclk_enabled) {
 		clk_disable_unprepare(dev->gclk);
 		dev->gclk_enabled = 0;
 	}
+	mutex_unlock(&dev->mlock);
 	return 0;
 }
 
@@ -627,10 +629,24 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	bool ulock_old = ctrl->ulock;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->ulock = !(val & SPDIFRX_RSR_ULOCK);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		ctrl->ulock = !(val & SPDIFRX_RSR_ULOCK);
+	} else {
+		ctrl->ulock = 0;
+	}
+
 	uvalue->value.integer.value[0] = ctrl->ulock;
 
+	mutex_unlock(&dev->mlock);
+
 	return ulock_old != ctrl->ulock;
 }
 
@@ -643,8 +659,22 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	bool badf_old = ctrl->badf;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->badf = !!(val & SPDIFRX_RSR_BADF);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		ctrl->badf = !!(val & SPDIFRX_RSR_BADF);
+	} else {
+		ctrl->badf = 0;
+	}
+
+	mutex_unlock(&dev->mlock);
+
 	uvalue->value.integer.value[0] = ctrl->badf;
 
 	return badf_old != ctrl->badf;
@@ -656,11 +686,48 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
-	u32 val;
+	u32 val = ~0U, loops = 10;
+	int ret;
 	bool signal_old = ctrl->signal;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-	ctrl->signal = !(val & SPDIFRX_RSR_NOSIGNAL);
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * To get the signal we need to have receiver enabled. This
+	 * could be enabled also from trigger() function thus we need to
+	 * take care of not disabling the receiver when it runs.
+	 */
+	if (!dev->trigger_enabled) {
+		ret = clk_prepare_enable(dev->gclk);
+		if (ret)
+			goto unlock;
+
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_ENABLE);
+
+		/* Wait for RSR.ULOCK bit. */
+		while (--loops) {
+			regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+			if (!(val & SPDIFRX_RSR_ULOCK))
+				break;
+			usleep_range(100, 150);
+		}
+
+		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
+				   SPDIFRX_MR_RXEN_DISABLE);
+
+		clk_disable_unprepare(dev->gclk);
+	} else {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+	}
+
+unlock:
+	mutex_unlock(&dev->mlock);
+
+	if (!(val & SPDIFRX_RSR_ULOCK))
+		ctrl->signal = !(val & SPDIFRX_RSR_NOSIGNAL);
+	else
+		ctrl->signal = 0;
 	uvalue->value.integer.value[0] = ctrl->signal;
 
 	return signal_old != ctrl->signal;
@@ -685,18 +752,32 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 	u32 val;
 	int rate;
 
-	regmap_read(dev->regmap, SPDIFRX_RSR, &val);
-
-	/* if the receiver is not locked, ISF data is invalid */
-	if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
+	 * and the receiver is disabled. Thus we take into account the
+	 * dev->trigger_enabled here to return a real status.
+	 */
+	if (dev->trigger_enabled) {
+		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
+		/* If the receiver is not locked, ISF data is invalid. */
+		if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
+			ucontrol->value.integer.value[0] = 0;
+			goto unlock;
+		}
+	} else {
+		/* Reveicer is not locked, IFS data is invalid. */
 		ucontrol->value.integer.value[0] = 0;
-		return 0;
+		goto unlock;
 	}
 
 	rate = clk_get_rate(dev->gclk);
 
 	ucontrol->value.integer.value[0] = rate / (32 * SPDIFRX_RSR_IFS(val));
 
+unlock:
+	mutex_unlock(&dev->mlock);
 	return 0;
 }
 
@@ -913,7 +994,18 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 			"failed to get the PMC generated clock: %d\n", err);
 		return err;
 	}
+
+	/*
+	 * Signal control need a valid rate on gclk. hw_params() configures
+	 * it propertly but requesting signal before any hw_params() has been
+	 * called lead to invalid value returned for signal. Thus, configure
+	 * gclk at a valid rate, here, in initialization, to simplify the
+	 * control path.
+	 */
+	clk_set_min_rate(dev->gclk, 48000 * SPDIFRX_GCLK_RATIO_MIN + 1);
+
 	spin_lock_init(&dev->blockend_lock);
+	mutex_init(&dev->mlock);
 
 	dev->dev = &pdev->dev;
 	dev->regmap = regmap;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/8] ASoC: mchp-spdifrx: fix return value in case completion times out
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

wait_for_completion_interruptible_timeout() returns 0 in case of
timeout. Check this into account when returning from function.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 2d86e0ec930f..7f359371b31b 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -524,9 +524,10 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 	ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
 							msecs_to_jiffies(100));
 	/* IP might not be started or valid stream might not be present */
-	if (ret < 0) {
+	if (ret <= 0) {
 		dev_dbg(dev->dev, "channel status for channel %d timeout\n",
 			channel);
+		return ret ? : -ETIMEDOUT;
 	}
 
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
@@ -580,7 +581,7 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 		dev_dbg(dev->dev, "user data for channel %d timeout\n",
 			channel);
 		mchp_spdifrx_isr_blockend_dis(dev);
-		return ret;
+		return ret ? : -ETIMEDOUT;
 	}
 
 	spin_lock_irqsave(&user_data->lock, flags);
-- 
2.34.1


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

* [PATCH 2/8] ASoC: mchp-spdifrx: fix return value in case completion times out
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

wait_for_completion_interruptible_timeout() returns 0 in case of
timeout. Check this into account when returning from function.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 2d86e0ec930f..7f359371b31b 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -524,9 +524,10 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 	ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
 							msecs_to_jiffies(100));
 	/* IP might not be started or valid stream might not be present */
-	if (ret < 0) {
+	if (ret <= 0) {
 		dev_dbg(dev->dev, "channel status for channel %d timeout\n",
 			channel);
+		return ret ? : -ETIMEDOUT;
 	}
 
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
@@ -580,7 +581,7 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 		dev_dbg(dev->dev, "user data for channel %d timeout\n",
 			channel);
 		mchp_spdifrx_isr_blockend_dis(dev);
-		return ret;
+		return ret ? : -ETIMEDOUT;
 	}
 
 	spin_lock_irqsave(&user_data->lock, flags);
-- 
2.34.1


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

* [PATCH 2/8] ASoC: mchp-spdifrx: fix return value in case completion times out
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

wait_for_completion_interruptible_timeout() returns 0 in case of
timeout. Check this into account when returning from function.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 2d86e0ec930f..7f359371b31b 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -524,9 +524,10 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 	ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
 							msecs_to_jiffies(100));
 	/* IP might not be started or valid stream might not be present */
-	if (ret < 0) {
+	if (ret <= 0) {
 		dev_dbg(dev->dev, "channel status for channel %d timeout\n",
 			channel);
+		return ret ? : -ETIMEDOUT;
 	}
 
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
@@ -580,7 +581,7 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 		dev_dbg(dev->dev, "user data for channel %d timeout\n",
 			channel);
 		mchp_spdifrx_isr_blockend_dis(dev);
-		return ret;
+		return ret ? : -ETIMEDOUT;
 	}
 
 	spin_lock_irqsave(&user_data->lock, flags);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

Channel status get and channel subcode get controls relies on data
returned by controls when certain IRQs are raised. To achieve that
completions are used b/w controls and interrupt service routine. The
concurrent accesses to these controls are protected by
struct snd_card::controls_rwsem.

Issues identified:
- reinit_completion() may be called while waiting for completion
  which should be avoided
- in case of multiple threads waiting, the complete() call in interrupt
  will signal only one waiting thread per interrupt which may lead to
  timeout for the others
- in case of channel status get as the CSC interrupt is not refcounted
  ISR may disable interrupt for threads that were just enabled it.

To solve these the access to controls were protected by a mutex. Along
with this there is no need for spinlock to protect the software cache
reads/updates b/w controls and ISR as the update is happening only when
requested from control, and only one reader can reach the control.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 143 ++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 66 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 7f359371b31b..31ffaaf46dec 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -217,7 +217,6 @@ struct mchp_spdifrx_ch_stat {
 struct mchp_spdifrx_user_data {
 	unsigned char data[SPDIFRX_UD_BITS / 8];
 	struct completion done;
-	spinlock_t lock;	/* protect access to user data */
 };
 
 struct mchp_spdifrx_mixer_control {
@@ -231,8 +230,6 @@ struct mchp_spdifrx_mixer_control {
 struct mchp_spdifrx_dev {
 	struct snd_dmaengine_dai_dma_data	capture;
 	struct mchp_spdifrx_mixer_control	control;
-	spinlock_t				blockend_lock;	/* protect access to blockend_refcount */
-	int					blockend_refcount;
 	struct mutex				mlock;
 	struct device				*dev;
 	struct regmap				*regmap;
@@ -277,37 +274,11 @@ static void mchp_spdifrx_channel_user_data_read(struct mchp_spdifrx_dev *dev,
 	}
 }
 
-/* called from non-atomic context only */
-static void mchp_spdifrx_isr_blockend_en(struct mchp_spdifrx_dev *dev)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->blockend_lock, flags);
-	dev->blockend_refcount++;
-	/* don't enable BLOCKEND interrupt if it's already enabled */
-	if (dev->blockend_refcount == 1)
-		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_BLOCKEND);
-	spin_unlock_irqrestore(&dev->blockend_lock, flags);
-}
-
-/* called from atomic/non-atomic context */
-static void mchp_spdifrx_isr_blockend_dis(struct mchp_spdifrx_dev *dev)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->blockend_lock, flags);
-	dev->blockend_refcount--;
-	/* don't enable BLOCKEND interrupt if it's already enabled */
-	if (dev->blockend_refcount == 0)
-		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
-	spin_unlock_irqrestore(&dev->blockend_lock, flags);
-}
-
 static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 {
 	struct mchp_spdifrx_dev *dev = dev_id;
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
-	u32 sr, imr, pending, idr = 0;
+	u32 sr, imr, pending;
 	irqreturn_t ret = IRQ_NONE;
 	int ch;
 
@@ -322,13 +293,10 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 
 	if (pending & SPDIFRX_IR_BLOCKEND) {
 		for (ch = 0; ch < SPDIFRX_CHANNELS; ch++) {
-			spin_lock(&ctrl->user_data[ch].lock);
 			mchp_spdifrx_channel_user_data_read(dev, ch);
-			spin_unlock(&ctrl->user_data[ch].lock);
-
 			complete(&ctrl->user_data[ch].done);
 		}
-		mchp_spdifrx_isr_blockend_dis(dev);
+		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
 		ret = IRQ_HANDLED;
 	}
 
@@ -336,7 +304,7 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 		if (pending & SPDIFRX_IR_CSC(ch)) {
 			mchp_spdifrx_channel_status_read(dev, ch);
 			complete(&ctrl->ch_stat[ch].done);
-			idr |= SPDIFRX_IR_CSC(ch);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(ch));
 			ret = IRQ_HANDLED;
 		}
 	}
@@ -346,8 +314,6 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
-	regmap_write(dev->regmap, SPDIFRX_IDR, idr);
-
 	return ret;
 }
 
@@ -517,23 +483,51 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 {
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	struct mchp_spdifrx_ch_stat *ch_stat = &ctrl->ch_stat[channel];
-	int ret;
+	int ret = 0;
+
+	mutex_lock(&dev->mlock);
 
-	regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_CSC(channel));
-	/* check for new data available */
-	ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
-							msecs_to_jiffies(100));
-	/* IP might not be started or valid stream might not be present */
-	if (ret <= 0) {
-		dev_dbg(dev->dev, "channel status for channel %d timeout\n",
-			channel);
-		return ret ? : -ETIMEDOUT;
+	/*
+	 * We may reach this point with both clocks enabled but the receiver
+	 * still disabled. To void waiting for completion and return with
+	 * timeout check the dev->trigger_enabled.
+	 *
+	 * To retrieve data:
+	 * - if the receiver is enabled CSC IRQ will update the data in software
+	 *   caches (ch_stat->data)
+	 * - otherwise we just update it here the software caches with latest
+	 *   available information and return it; in this case we don't need
+	 *   spin locking as the IRQ is disabled and will not be raised from
+	 *   anywhere else.
+	 */
+
+	if (dev->trigger_enabled) {
+		reinit_completion(&ch_stat->done);
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_CSC(channel));
+		/* Check for new data available */
+		ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
+								msecs_to_jiffies(100));
+		/* Valid stream might not be present */
+		if (ret <= 0) {
+			dev_dbg(dev->dev, "channel status for channel %d timeout\n",
+				channel);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(channel));
+			ret = ret ? : -ETIMEDOUT;
+			goto unlock;
+		} else {
+			ret = 0;
+		}
+	} else {
+		/* Update software cache with latest channel status. */
+		mchp_spdifrx_channel_status_read(dev, channel);
 	}
 
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
 	       sizeof(ch_stat->data));
 
-	return 0;
+unlock:
+	mutex_unlock(&dev->mlock);
+	return ret;
 }
 
 static int mchp_spdifrx_cs1_get(struct snd_kcontrol *kcontrol,
@@ -567,29 +561,49 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 				       int channel,
 				       struct snd_ctl_elem_value *uvalue)
 {
-	unsigned long flags;
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	struct mchp_spdifrx_user_data *user_data = &ctrl->user_data[channel];
-	int ret;
+	int ret = 0;
+
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * We may reach this point with both clocks enabled but the receiver
+	 * still disabled. To void waiting for completion to just timeout we
+	 * check here the dev->trigger_enabled flag.
+	 *
+	 * To retrieve data:
+	 * - if the receiver is enabled we need to wait for blockend IRQ to read
+	 *   data to and update it for us in software caches
+	 * - otherwise reading the SPDIFRX_CHUD() registers is enough.
+	 */
 
-	reinit_completion(&user_data->done);
-	mchp_spdifrx_isr_blockend_en(dev);
-	ret = wait_for_completion_interruptible_timeout(&user_data->done,
-							msecs_to_jiffies(100));
-	/* IP might not be started or valid stream might not be present */
-	if (ret <= 0) {
-		dev_dbg(dev->dev, "user data for channel %d timeout\n",
-			channel);
-		mchp_spdifrx_isr_blockend_dis(dev);
-		return ret ? : -ETIMEDOUT;
+	if (dev->trigger_enabled) {
+		reinit_completion(&user_data->done);
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_BLOCKEND);
+		ret = wait_for_completion_interruptible_timeout(&user_data->done,
+								msecs_to_jiffies(100));
+		/* Valid stream might not be present. */
+		if (ret <= 0) {
+			dev_dbg(dev->dev, "user data for channel %d timeout\n",
+				channel);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
+			ret = ret ? : -ETIMEDOUT;
+			goto unlock;
+		} else {
+			ret = 0;
+		}
+	} else {
+		/* Update software cache with last available data. */
+		mchp_spdifrx_channel_user_data_read(dev, channel);
 	}
 
-	spin_lock_irqsave(&user_data->lock, flags);
 	memcpy(uvalue->value.iec958.subcode, user_data->data,
 	       sizeof(user_data->data));
-	spin_unlock_irqrestore(&user_data->lock, flags);
 
-	return 0;
+unlock:
+	mutex_unlock(&dev->mlock);
+	return ret;
 }
 
 static int mchp_spdifrx_subcode_ch1_get(struct snd_kcontrol *kcontrol,
@@ -890,11 +904,9 @@ static int mchp_spdifrx_dai_probe(struct snd_soc_dai *dai)
 		     SPDIFRX_MR_AUTORST_NOACTION |
 		     SPDIFRX_MR_PACK_DISABLED);
 
-	dev->blockend_refcount = 0;
 	for (ch = 0; ch < SPDIFRX_CHANNELS; ch++) {
 		init_completion(&ctrl->ch_stat[ch].done);
 		init_completion(&ctrl->user_data[ch].done);
-		spin_lock_init(&ctrl->user_data[ch].lock);
 	}
 
 	/* Add controls */
@@ -1005,7 +1017,6 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 	 */
 	clk_set_min_rate(dev->gclk, 48000 * SPDIFRX_GCLK_RATIO_MIN + 1);
 
-	spin_lock_init(&dev->blockend_lock);
 	mutex_init(&dev->mlock);
 
 	dev->dev = &pdev->dev;
-- 
2.34.1


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

* [PATCH 3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Channel status get and channel subcode get controls relies on data
returned by controls when certain IRQs are raised. To achieve that
completions are used b/w controls and interrupt service routine. The
concurrent accesses to these controls are protected by
struct snd_card::controls_rwsem.

Issues identified:
- reinit_completion() may be called while waiting for completion
  which should be avoided
- in case of multiple threads waiting, the complete() call in interrupt
  will signal only one waiting thread per interrupt which may lead to
  timeout for the others
- in case of channel status get as the CSC interrupt is not refcounted
  ISR may disable interrupt for threads that were just enabled it.

To solve these the access to controls were protected by a mutex. Along
with this there is no need for spinlock to protect the software cache
reads/updates b/w controls and ISR as the update is happening only when
requested from control, and only one reader can reach the control.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 143 ++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 66 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 7f359371b31b..31ffaaf46dec 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -217,7 +217,6 @@ struct mchp_spdifrx_ch_stat {
 struct mchp_spdifrx_user_data {
 	unsigned char data[SPDIFRX_UD_BITS / 8];
 	struct completion done;
-	spinlock_t lock;	/* protect access to user data */
 };
 
 struct mchp_spdifrx_mixer_control {
@@ -231,8 +230,6 @@ struct mchp_spdifrx_mixer_control {
 struct mchp_spdifrx_dev {
 	struct snd_dmaengine_dai_dma_data	capture;
 	struct mchp_spdifrx_mixer_control	control;
-	spinlock_t				blockend_lock;	/* protect access to blockend_refcount */
-	int					blockend_refcount;
 	struct mutex				mlock;
 	struct device				*dev;
 	struct regmap				*regmap;
@@ -277,37 +274,11 @@ static void mchp_spdifrx_channel_user_data_read(struct mchp_spdifrx_dev *dev,
 	}
 }
 
-/* called from non-atomic context only */
-static void mchp_spdifrx_isr_blockend_en(struct mchp_spdifrx_dev *dev)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->blockend_lock, flags);
-	dev->blockend_refcount++;
-	/* don't enable BLOCKEND interrupt if it's already enabled */
-	if (dev->blockend_refcount == 1)
-		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_BLOCKEND);
-	spin_unlock_irqrestore(&dev->blockend_lock, flags);
-}
-
-/* called from atomic/non-atomic context */
-static void mchp_spdifrx_isr_blockend_dis(struct mchp_spdifrx_dev *dev)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->blockend_lock, flags);
-	dev->blockend_refcount--;
-	/* don't enable BLOCKEND interrupt if it's already enabled */
-	if (dev->blockend_refcount == 0)
-		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
-	spin_unlock_irqrestore(&dev->blockend_lock, flags);
-}
-
 static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 {
 	struct mchp_spdifrx_dev *dev = dev_id;
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
-	u32 sr, imr, pending, idr = 0;
+	u32 sr, imr, pending;
 	irqreturn_t ret = IRQ_NONE;
 	int ch;
 
@@ -322,13 +293,10 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 
 	if (pending & SPDIFRX_IR_BLOCKEND) {
 		for (ch = 0; ch < SPDIFRX_CHANNELS; ch++) {
-			spin_lock(&ctrl->user_data[ch].lock);
 			mchp_spdifrx_channel_user_data_read(dev, ch);
-			spin_unlock(&ctrl->user_data[ch].lock);
-
 			complete(&ctrl->user_data[ch].done);
 		}
-		mchp_spdifrx_isr_blockend_dis(dev);
+		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
 		ret = IRQ_HANDLED;
 	}
 
@@ -336,7 +304,7 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 		if (pending & SPDIFRX_IR_CSC(ch)) {
 			mchp_spdifrx_channel_status_read(dev, ch);
 			complete(&ctrl->ch_stat[ch].done);
-			idr |= SPDIFRX_IR_CSC(ch);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(ch));
 			ret = IRQ_HANDLED;
 		}
 	}
@@ -346,8 +314,6 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
-	regmap_write(dev->regmap, SPDIFRX_IDR, idr);
-
 	return ret;
 }
 
@@ -517,23 +483,51 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 {
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	struct mchp_spdifrx_ch_stat *ch_stat = &ctrl->ch_stat[channel];
-	int ret;
+	int ret = 0;
+
+	mutex_lock(&dev->mlock);
 
-	regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_CSC(channel));
-	/* check for new data available */
-	ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
-							msecs_to_jiffies(100));
-	/* IP might not be started or valid stream might not be present */
-	if (ret <= 0) {
-		dev_dbg(dev->dev, "channel status for channel %d timeout\n",
-			channel);
-		return ret ? : -ETIMEDOUT;
+	/*
+	 * We may reach this point with both clocks enabled but the receiver
+	 * still disabled. To void waiting for completion and return with
+	 * timeout check the dev->trigger_enabled.
+	 *
+	 * To retrieve data:
+	 * - if the receiver is enabled CSC IRQ will update the data in software
+	 *   caches (ch_stat->data)
+	 * - otherwise we just update it here the software caches with latest
+	 *   available information and return it; in this case we don't need
+	 *   spin locking as the IRQ is disabled and will not be raised from
+	 *   anywhere else.
+	 */
+
+	if (dev->trigger_enabled) {
+		reinit_completion(&ch_stat->done);
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_CSC(channel));
+		/* Check for new data available */
+		ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
+								msecs_to_jiffies(100));
+		/* Valid stream might not be present */
+		if (ret <= 0) {
+			dev_dbg(dev->dev, "channel status for channel %d timeout\n",
+				channel);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(channel));
+			ret = ret ? : -ETIMEDOUT;
+			goto unlock;
+		} else {
+			ret = 0;
+		}
+	} else {
+		/* Update software cache with latest channel status. */
+		mchp_spdifrx_channel_status_read(dev, channel);
 	}
 
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
 	       sizeof(ch_stat->data));
 
-	return 0;
+unlock:
+	mutex_unlock(&dev->mlock);
+	return ret;
 }
 
 static int mchp_spdifrx_cs1_get(struct snd_kcontrol *kcontrol,
@@ -567,29 +561,49 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 				       int channel,
 				       struct snd_ctl_elem_value *uvalue)
 {
-	unsigned long flags;
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	struct mchp_spdifrx_user_data *user_data = &ctrl->user_data[channel];
-	int ret;
+	int ret = 0;
+
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * We may reach this point with both clocks enabled but the receiver
+	 * still disabled. To void waiting for completion to just timeout we
+	 * check here the dev->trigger_enabled flag.
+	 *
+	 * To retrieve data:
+	 * - if the receiver is enabled we need to wait for blockend IRQ to read
+	 *   data to and update it for us in software caches
+	 * - otherwise reading the SPDIFRX_CHUD() registers is enough.
+	 */
 
-	reinit_completion(&user_data->done);
-	mchp_spdifrx_isr_blockend_en(dev);
-	ret = wait_for_completion_interruptible_timeout(&user_data->done,
-							msecs_to_jiffies(100));
-	/* IP might not be started or valid stream might not be present */
-	if (ret <= 0) {
-		dev_dbg(dev->dev, "user data for channel %d timeout\n",
-			channel);
-		mchp_spdifrx_isr_blockend_dis(dev);
-		return ret ? : -ETIMEDOUT;
+	if (dev->trigger_enabled) {
+		reinit_completion(&user_data->done);
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_BLOCKEND);
+		ret = wait_for_completion_interruptible_timeout(&user_data->done,
+								msecs_to_jiffies(100));
+		/* Valid stream might not be present. */
+		if (ret <= 0) {
+			dev_dbg(dev->dev, "user data for channel %d timeout\n",
+				channel);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
+			ret = ret ? : -ETIMEDOUT;
+			goto unlock;
+		} else {
+			ret = 0;
+		}
+	} else {
+		/* Update software cache with last available data. */
+		mchp_spdifrx_channel_user_data_read(dev, channel);
 	}
 
-	spin_lock_irqsave(&user_data->lock, flags);
 	memcpy(uvalue->value.iec958.subcode, user_data->data,
 	       sizeof(user_data->data));
-	spin_unlock_irqrestore(&user_data->lock, flags);
 
-	return 0;
+unlock:
+	mutex_unlock(&dev->mlock);
+	return ret;
 }
 
 static int mchp_spdifrx_subcode_ch1_get(struct snd_kcontrol *kcontrol,
@@ -890,11 +904,9 @@ static int mchp_spdifrx_dai_probe(struct snd_soc_dai *dai)
 		     SPDIFRX_MR_AUTORST_NOACTION |
 		     SPDIFRX_MR_PACK_DISABLED);
 
-	dev->blockend_refcount = 0;
 	for (ch = 0; ch < SPDIFRX_CHANNELS; ch++) {
 		init_completion(&ctrl->ch_stat[ch].done);
 		init_completion(&ctrl->user_data[ch].done);
-		spin_lock_init(&ctrl->user_data[ch].lock);
 	}
 
 	/* Add controls */
@@ -1005,7 +1017,6 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 	 */
 	clk_set_min_rate(dev->gclk, 48000 * SPDIFRX_GCLK_RATIO_MIN + 1);
 
-	spin_lock_init(&dev->blockend_lock);
 	mutex_init(&dev->mlock);
 
 	dev->dev = &pdev->dev;
-- 
2.34.1


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

* [PATCH 3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Channel status get and channel subcode get controls relies on data
returned by controls when certain IRQs are raised. To achieve that
completions are used b/w controls and interrupt service routine. The
concurrent accesses to these controls are protected by
struct snd_card::controls_rwsem.

Issues identified:
- reinit_completion() may be called while waiting for completion
  which should be avoided
- in case of multiple threads waiting, the complete() call in interrupt
  will signal only one waiting thread per interrupt which may lead to
  timeout for the others
- in case of channel status get as the CSC interrupt is not refcounted
  ISR may disable interrupt for threads that were just enabled it.

To solve these the access to controls were protected by a mutex. Along
with this there is no need for spinlock to protect the software cache
reads/updates b/w controls and ISR as the update is happening only when
requested from control, and only one reader can reach the control.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 143 ++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 66 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 7f359371b31b..31ffaaf46dec 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -217,7 +217,6 @@ struct mchp_spdifrx_ch_stat {
 struct mchp_spdifrx_user_data {
 	unsigned char data[SPDIFRX_UD_BITS / 8];
 	struct completion done;
-	spinlock_t lock;	/* protect access to user data */
 };
 
 struct mchp_spdifrx_mixer_control {
@@ -231,8 +230,6 @@ struct mchp_spdifrx_mixer_control {
 struct mchp_spdifrx_dev {
 	struct snd_dmaengine_dai_dma_data	capture;
 	struct mchp_spdifrx_mixer_control	control;
-	spinlock_t				blockend_lock;	/* protect access to blockend_refcount */
-	int					blockend_refcount;
 	struct mutex				mlock;
 	struct device				*dev;
 	struct regmap				*regmap;
@@ -277,37 +274,11 @@ static void mchp_spdifrx_channel_user_data_read(struct mchp_spdifrx_dev *dev,
 	}
 }
 
-/* called from non-atomic context only */
-static void mchp_spdifrx_isr_blockend_en(struct mchp_spdifrx_dev *dev)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->blockend_lock, flags);
-	dev->blockend_refcount++;
-	/* don't enable BLOCKEND interrupt if it's already enabled */
-	if (dev->blockend_refcount == 1)
-		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_BLOCKEND);
-	spin_unlock_irqrestore(&dev->blockend_lock, flags);
-}
-
-/* called from atomic/non-atomic context */
-static void mchp_spdifrx_isr_blockend_dis(struct mchp_spdifrx_dev *dev)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->blockend_lock, flags);
-	dev->blockend_refcount--;
-	/* don't enable BLOCKEND interrupt if it's already enabled */
-	if (dev->blockend_refcount == 0)
-		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
-	spin_unlock_irqrestore(&dev->blockend_lock, flags);
-}
-
 static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 {
 	struct mchp_spdifrx_dev *dev = dev_id;
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
-	u32 sr, imr, pending, idr = 0;
+	u32 sr, imr, pending;
 	irqreturn_t ret = IRQ_NONE;
 	int ch;
 
@@ -322,13 +293,10 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 
 	if (pending & SPDIFRX_IR_BLOCKEND) {
 		for (ch = 0; ch < SPDIFRX_CHANNELS; ch++) {
-			spin_lock(&ctrl->user_data[ch].lock);
 			mchp_spdifrx_channel_user_data_read(dev, ch);
-			spin_unlock(&ctrl->user_data[ch].lock);
-
 			complete(&ctrl->user_data[ch].done);
 		}
-		mchp_spdifrx_isr_blockend_dis(dev);
+		regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
 		ret = IRQ_HANDLED;
 	}
 
@@ -336,7 +304,7 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 		if (pending & SPDIFRX_IR_CSC(ch)) {
 			mchp_spdifrx_channel_status_read(dev, ch);
 			complete(&ctrl->ch_stat[ch].done);
-			idr |= SPDIFRX_IR_CSC(ch);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(ch));
 			ret = IRQ_HANDLED;
 		}
 	}
@@ -346,8 +314,6 @@ static irqreturn_t mchp_spdif_interrupt(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
-	regmap_write(dev->regmap, SPDIFRX_IDR, idr);
-
 	return ret;
 }
 
@@ -517,23 +483,51 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 {
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	struct mchp_spdifrx_ch_stat *ch_stat = &ctrl->ch_stat[channel];
-	int ret;
+	int ret = 0;
+
+	mutex_lock(&dev->mlock);
 
-	regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_CSC(channel));
-	/* check for new data available */
-	ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
-							msecs_to_jiffies(100));
-	/* IP might not be started or valid stream might not be present */
-	if (ret <= 0) {
-		dev_dbg(dev->dev, "channel status for channel %d timeout\n",
-			channel);
-		return ret ? : -ETIMEDOUT;
+	/*
+	 * We may reach this point with both clocks enabled but the receiver
+	 * still disabled. To void waiting for completion and return with
+	 * timeout check the dev->trigger_enabled.
+	 *
+	 * To retrieve data:
+	 * - if the receiver is enabled CSC IRQ will update the data in software
+	 *   caches (ch_stat->data)
+	 * - otherwise we just update it here the software caches with latest
+	 *   available information and return it; in this case we don't need
+	 *   spin locking as the IRQ is disabled and will not be raised from
+	 *   anywhere else.
+	 */
+
+	if (dev->trigger_enabled) {
+		reinit_completion(&ch_stat->done);
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_CSC(channel));
+		/* Check for new data available */
+		ret = wait_for_completion_interruptible_timeout(&ch_stat->done,
+								msecs_to_jiffies(100));
+		/* Valid stream might not be present */
+		if (ret <= 0) {
+			dev_dbg(dev->dev, "channel status for channel %d timeout\n",
+				channel);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(channel));
+			ret = ret ? : -ETIMEDOUT;
+			goto unlock;
+		} else {
+			ret = 0;
+		}
+	} else {
+		/* Update software cache with latest channel status. */
+		mchp_spdifrx_channel_status_read(dev, channel);
 	}
 
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
 	       sizeof(ch_stat->data));
 
-	return 0;
+unlock:
+	mutex_unlock(&dev->mlock);
+	return ret;
 }
 
 static int mchp_spdifrx_cs1_get(struct snd_kcontrol *kcontrol,
@@ -567,29 +561,49 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 				       int channel,
 				       struct snd_ctl_elem_value *uvalue)
 {
-	unsigned long flags;
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	struct mchp_spdifrx_user_data *user_data = &ctrl->user_data[channel];
-	int ret;
+	int ret = 0;
+
+	mutex_lock(&dev->mlock);
+
+	/*
+	 * We may reach this point with both clocks enabled but the receiver
+	 * still disabled. To void waiting for completion to just timeout we
+	 * check here the dev->trigger_enabled flag.
+	 *
+	 * To retrieve data:
+	 * - if the receiver is enabled we need to wait for blockend IRQ to read
+	 *   data to and update it for us in software caches
+	 * - otherwise reading the SPDIFRX_CHUD() registers is enough.
+	 */
 
-	reinit_completion(&user_data->done);
-	mchp_spdifrx_isr_blockend_en(dev);
-	ret = wait_for_completion_interruptible_timeout(&user_data->done,
-							msecs_to_jiffies(100));
-	/* IP might not be started or valid stream might not be present */
-	if (ret <= 0) {
-		dev_dbg(dev->dev, "user data for channel %d timeout\n",
-			channel);
-		mchp_spdifrx_isr_blockend_dis(dev);
-		return ret ? : -ETIMEDOUT;
+	if (dev->trigger_enabled) {
+		reinit_completion(&user_data->done);
+		regmap_write(dev->regmap, SPDIFRX_IER, SPDIFRX_IR_BLOCKEND);
+		ret = wait_for_completion_interruptible_timeout(&user_data->done,
+								msecs_to_jiffies(100));
+		/* Valid stream might not be present. */
+		if (ret <= 0) {
+			dev_dbg(dev->dev, "user data for channel %d timeout\n",
+				channel);
+			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
+			ret = ret ? : -ETIMEDOUT;
+			goto unlock;
+		} else {
+			ret = 0;
+		}
+	} else {
+		/* Update software cache with last available data. */
+		mchp_spdifrx_channel_user_data_read(dev, channel);
 	}
 
-	spin_lock_irqsave(&user_data->lock, flags);
 	memcpy(uvalue->value.iec958.subcode, user_data->data,
 	       sizeof(user_data->data));
-	spin_unlock_irqrestore(&user_data->lock, flags);
 
-	return 0;
+unlock:
+	mutex_unlock(&dev->mlock);
+	return ret;
 }
 
 static int mchp_spdifrx_subcode_ch1_get(struct snd_kcontrol *kcontrol,
@@ -890,11 +904,9 @@ static int mchp_spdifrx_dai_probe(struct snd_soc_dai *dai)
 		     SPDIFRX_MR_AUTORST_NOACTION |
 		     SPDIFRX_MR_PACK_DISABLED);
 
-	dev->blockend_refcount = 0;
 	for (ch = 0; ch < SPDIFRX_CHANNELS; ch++) {
 		init_completion(&ctrl->ch_stat[ch].done);
 		init_completion(&ctrl->user_data[ch].done);
-		spin_lock_init(&ctrl->user_data[ch].lock);
 	}
 
 	/* Add controls */
@@ -1005,7 +1017,6 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 	 */
 	clk_set_min_rate(dev->gclk, 48000 * SPDIFRX_GCLK_RATIO_MIN + 1);
 
-	spin_lock_init(&dev->blockend_lock);
 	mutex_init(&dev->mlock);
 
 	dev->dev = &pdev->dev;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove()
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

CSC interrupts which might be used in controls are on bits 8 and 9 of
SPDIFRX_IDR register. Thus disable all the interrupts that are exported
by driver.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 31ffaaf46dec..b81fc77728df 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -921,7 +921,7 @@ static int mchp_spdifrx_dai_remove(struct snd_soc_dai *dai)
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
 	/* Disable interrupts */
-	regmap_write(dev->regmap, SPDIFRX_IDR, 0xFF);
+	regmap_write(dev->regmap, SPDIFRX_IDR, GENMASK(14, 0));
 
 	clk_disable_unprepare(dev->pclk);
 
-- 
2.34.1


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

* [PATCH 4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove()
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

CSC interrupts which might be used in controls are on bits 8 and 9 of
SPDIFRX_IDR register. Thus disable all the interrupts that are exported
by driver.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 31ffaaf46dec..b81fc77728df 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -921,7 +921,7 @@ static int mchp_spdifrx_dai_remove(struct snd_soc_dai *dai)
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
 	/* Disable interrupts */
-	regmap_write(dev->regmap, SPDIFRX_IDR, 0xFF);
+	regmap_write(dev->regmap, SPDIFRX_IDR, GENMASK(14, 0));
 
 	clk_disable_unprepare(dev->pclk);
 
-- 
2.34.1


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

* [PATCH 4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove()
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

CSC interrupts which might be used in controls are on bits 8 and 9 of
SPDIFRX_IDR register. Thus disable all the interrupts that are exported
by driver.

Fixes: ef265c55c1ac ("ASoC: mchp-spdifrx: add driver for SPDIF RX")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 31ffaaf46dec..b81fc77728df 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -921,7 +921,7 @@ static int mchp_spdifrx_dai_remove(struct snd_soc_dai *dai)
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
 	/* Disable interrupts */
-	regmap_write(dev->regmap, SPDIFRX_IDR, 0xFF);
+	regmap_write(dev->regmap, SPDIFRX_IDR, GENMASK(14, 0));
 
 	clk_disable_unprepare(dev->pclk);
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

clk_get_rate() returns an unsigned long. Use a variable of type
unsigned long to store it.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index b81fc77728df..90b2fb3a9844 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -764,8 +764,8 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
+	unsigned long rate;
 	u32 val;
-	int rate;
 
 	mutex_lock(&dev->mlock);
 
-- 
2.34.1


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

* [PATCH 5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

clk_get_rate() returns an unsigned long. Use a variable of type
unsigned long to store it.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index b81fc77728df..90b2fb3a9844 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -764,8 +764,8 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
+	unsigned long rate;
 	u32 val;
-	int rate;
 
 	mutex_lock(&dev->mlock);
 
-- 
2.34.1


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

* [PATCH 5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

clk_get_rate() returns an unsigned long. Use a variable of type
unsigned long to store it.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index b81fc77728df..90b2fb3a9844 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -764,8 +764,8 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
+	unsigned long rate;
 	u32 val;
-	int rate;
 
 	mutex_lock(&dev->mlock);
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Remove member fmt of struct mchp_spdifrx_dev as it is not used anywhere.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 90b2fb3a9844..46fff31321f3 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -235,7 +235,6 @@ struct mchp_spdifrx_dev {
 	struct regmap				*regmap;
 	struct clk				*pclk;
 	struct clk				*gclk;
-	unsigned int				fmt;
 	unsigned int				trigger_enabled;
 	unsigned int				gclk_enabled:1;
 };
-- 
2.34.1


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

* [PATCH 6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

Remove member fmt of struct mchp_spdifrx_dev as it is not used anywhere.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 90b2fb3a9844..46fff31321f3 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -235,7 +235,6 @@ struct mchp_spdifrx_dev {
 	struct regmap				*regmap;
 	struct clk				*pclk;
 	struct clk				*gclk;
-	unsigned int				fmt;
 	unsigned int				trigger_enabled;
 	unsigned int				gclk_enabled:1;
 };
-- 
2.34.1


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

* [PATCH 6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Remove member fmt of struct mchp_spdifrx_dev as it is not used anywhere.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 90b2fb3a9844..46fff31321f3 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -235,7 +235,6 @@ struct mchp_spdifrx_dev {
 	struct regmap				*regmap;
 	struct clk				*pclk;
 	struct clk				*gclk;
-	unsigned int				fmt;
 	unsigned int				trigger_enabled;
 	unsigned int				gclk_enabled:1;
 };
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 7/8] ASoC: mchp-spdifrx: add runtime pm support
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Add runtime PM support for Microchip SPDIFRX driver. On runtime suspend
the clocks are disabled and regmap is set in caching mode. On runtime
resume the clocks are enabled and regmap is synced with the device.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 210 ++++++++++++++++++++++++++-------
 1 file changed, 166 insertions(+), 44 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 46fff31321f3..796d4ec2b2b1 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -9,6 +9,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/spinlock.h>
 
@@ -192,6 +193,43 @@ static bool mchp_spdifrx_precious_reg(struct device *dev, unsigned int reg)
 	}
 }
 
+static bool mchp_spdifrx_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SPDIFRX_IMR:
+	case SPDIFRX_ISR:
+	case SPDIFRX_RSR:
+	case SPDIFRX_CHSR(0, 0):
+	case SPDIFRX_CHSR(0, 1):
+	case SPDIFRX_CHSR(0, 2):
+	case SPDIFRX_CHSR(0, 3):
+	case SPDIFRX_CHSR(0, 4):
+	case SPDIFRX_CHSR(0, 5):
+	case SPDIFRX_CHUD(0, 0):
+	case SPDIFRX_CHUD(0, 1):
+	case SPDIFRX_CHUD(0, 2):
+	case SPDIFRX_CHUD(0, 3):
+	case SPDIFRX_CHUD(0, 4):
+	case SPDIFRX_CHUD(0, 5):
+	case SPDIFRX_CHSR(1, 0):
+	case SPDIFRX_CHSR(1, 1):
+	case SPDIFRX_CHSR(1, 2):
+	case SPDIFRX_CHSR(1, 3):
+	case SPDIFRX_CHSR(1, 4):
+	case SPDIFRX_CHSR(1, 5):
+	case SPDIFRX_CHUD(1, 0):
+	case SPDIFRX_CHUD(1, 1):
+	case SPDIFRX_CHUD(1, 2):
+	case SPDIFRX_CHUD(1, 3):
+	case SPDIFRX_CHUD(1, 4):
+	case SPDIFRX_CHUD(1, 5):
+	case SPDIFRX_VERSION:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static const struct regmap_config mchp_spdifrx_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -200,6 +238,8 @@ static const struct regmap_config mchp_spdifrx_regmap_config = {
 	.readable_reg = mchp_spdifrx_readable_reg,
 	.writeable_reg = mchp_spdifrx_writeable_reg,
 	.precious_reg = mchp_spdifrx_precious_reg,
+	.volatile_reg = mchp_spdifrx_volatile_reg,
+	.cache_type = REGCACHE_FLAT,
 };
 
 #define SPDIFRX_GCLK_RATIO_MIN	(12 * 64)
@@ -236,7 +276,6 @@ struct mchp_spdifrx_dev {
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				trigger_enabled;
-	unsigned int				gclk_enabled:1;
 };
 
 static void mchp_spdifrx_channel_status_read(struct mchp_spdifrx_dev *dev,
@@ -405,16 +444,17 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		goto unlock;
 	}
 
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
+	/* GCLK is enabled by runtime PM. */
+	clk_disable_unprepare(dev->gclk);
+
 	ret = clk_set_min_rate(dev->gclk, params_rate(params) *
 					  SPDIFRX_GCLK_RATIO_MIN + 1);
 	if (ret) {
 		dev_err(dev->dev,
 			"unable to set gclk min rate: rate %u * ratio %u + 1\n",
 			params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
+		/* Restore runtime PM state. */
+		clk_prepare_enable(dev->gclk);
 		goto unlock;
 	}
 	ret = clk_prepare_enable(dev->gclk);
@@ -422,7 +462,6 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
 		goto unlock;
 	}
-	dev->gclk_enabled = 1;
 
 	dev_dbg(dev->dev, "GCLK range min set to %d\n",
 		params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
@@ -435,24 +474,9 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
-				struct snd_soc_dai *dai)
-{
-	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
-
-	mutex_lock(&dev->mlock);
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
-	mutex_unlock(&dev->mlock);
-	return 0;
-}
-
 static const struct snd_soc_dai_ops mchp_spdifrx_dai_ops = {
 	.trigger	= mchp_spdifrx_trigger,
 	.hw_params	= mchp_spdifrx_hw_params,
-	.hw_free	= mchp_spdifrx_hw_free,
 };
 
 #define MCHP_SPDIF_RATES	SNDRV_PCM_RATE_8000_192000
@@ -486,6 +510,10 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * We may reach this point with both clocks enabled but the receiver
 	 * still disabled. To void waiting for completion and return with
@@ -512,7 +540,7 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 				channel);
 			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(channel));
 			ret = ret ? : -ETIMEDOUT;
-			goto unlock;
+			goto pm_runtime_put;
 		} else {
 			ret = 0;
 		}
@@ -524,6 +552,9 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
 	       sizeof(ch_stat->data));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
 	return ret;
@@ -566,6 +597,10 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * We may reach this point with both clocks enabled but the receiver
 	 * still disabled. To void waiting for completion to just timeout we
@@ -588,7 +623,7 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 				channel);
 			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
 			ret = ret ? : -ETIMEDOUT;
-			goto unlock;
+			goto pm_runtime_put;
 		} else {
 			ret = 0;
 		}
@@ -600,6 +635,9 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 	memcpy(uvalue->value.iec958.subcode, user_data->data,
 	       sizeof(user_data->data));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
 	return ret;
@@ -641,10 +679,15 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	u32 val;
+	int ret;
 	bool ulock_old = ctrl->ulock;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -659,6 +702,9 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 
 	uvalue->value.integer.value[0] = ctrl->ulock;
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+unlock:
 	mutex_unlock(&dev->mlock);
 
 	return ulock_old != ctrl->ulock;
@@ -671,10 +717,15 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	u32 val;
+	int ret;
 	bool badf_old = ctrl->badf;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -687,6 +738,9 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 		ctrl->badf = 0;
 	}
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+unlock:
 	mutex_unlock(&dev->mlock);
 
 	uvalue->value.integer.value[0] = ctrl->badf;
@@ -706,16 +760,16 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * To get the signal we need to have receiver enabled. This
 	 * could be enabled also from trigger() function thus we need to
 	 * take care of not disabling the receiver when it runs.
 	 */
 	if (!dev->trigger_enabled) {
-		ret = clk_prepare_enable(dev->gclk);
-		if (ret)
-			goto unlock;
-
 		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
 				   SPDIFRX_MR_RXEN_ENABLE);
 
@@ -729,12 +783,13 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 
 		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
 				   SPDIFRX_MR_RXEN_DISABLE);
-
-		clk_disable_unprepare(dev->gclk);
 	} else {
 		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
 	}
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+
 unlock:
 	mutex_unlock(&dev->mlock);
 
@@ -765,9 +820,14 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	unsigned long rate;
 	u32 val;
+	int ret;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -778,21 +838,24 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 		/* If the receiver is not locked, ISF data is invalid. */
 		if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
 			ucontrol->value.integer.value[0] = 0;
-			goto unlock;
+			goto pm_runtime_put;
 		}
 	} else {
 		/* Reveicer is not locked, IFS data is invalid. */
 		ucontrol->value.integer.value[0] = 0;
-		goto unlock;
+		goto pm_runtime_put;
 	}
 
 	rate = clk_get_rate(dev->gclk);
 
 	ucontrol->value.integer.value[0] = rate / (32 * SPDIFRX_RSR_IFS(val));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
-	return 0;
+	return ret;
 }
 
 static struct snd_kcontrol_new mchp_spdifrx_ctrls[] = {
@@ -882,14 +945,6 @@ static int mchp_spdifrx_dai_probe(struct snd_soc_dai *dai)
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	int ch;
-	int err;
-
-	err = clk_prepare_enable(dev->pclk);
-	if (err) {
-		dev_err(dev->dev,
-			"failed to enable the peripheral clock: %d\n", err);
-		return err;
-	}
 
 	snd_soc_dai_init_dma_data(dai, NULL, &dev->capture);
 
@@ -922,8 +977,6 @@ static int mchp_spdifrx_dai_remove(struct snd_soc_dai *dai)
 	/* Disable interrupts */
 	regmap_write(dev->regmap, SPDIFRX_IDR, GENMASK(14, 0));
 
-	clk_disable_unprepare(dev->pclk);
-
 	return 0;
 }
 
@@ -954,6 +1007,48 @@ static const struct of_device_id mchp_spdifrx_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mchp_spdifrx_dt_ids);
 
+static int mchp_spdifrx_runtime_suspend(struct device *dev)
+{
+	struct mchp_spdifrx_dev *spdifrx = dev_get_drvdata(dev);
+
+	regcache_cache_only(spdifrx->regmap, true);
+	clk_disable_unprepare(spdifrx->gclk);
+	clk_disable_unprepare(spdifrx->pclk);
+
+	return 0;
+}
+
+static int mchp_spdifrx_runtime_resume(struct device *dev)
+{
+	struct mchp_spdifrx_dev *spdifrx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(spdifrx->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(spdifrx->gclk);
+	if (ret)
+		goto disable_pclk;
+
+	regcache_cache_only(spdifrx->regmap, false);
+	regcache_mark_dirty(spdifrx->regmap);
+	ret = regcache_sync(spdifrx->regmap);
+	if (ret) {
+		regcache_cache_only(spdifrx->regmap, true);
+		clk_disable_unprepare(spdifrx->gclk);
+disable_pclk:
+		clk_disable_unprepare(spdifrx->pclk);
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops mchp_spdifrx_pm_ops = {
+	RUNTIME_PM_OPS(mchp_spdifrx_runtime_suspend, mchp_spdifrx_runtime_resume,
+		       NULL)
+};
+
 static int mchp_spdifrx_probe(struct platform_device *pdev)
 {
 	struct mchp_spdifrx_dev *dev;
@@ -1022,13 +1117,20 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 	dev->regmap = regmap;
 	platform_set_drvdata(pdev, dev);
 
+	pm_runtime_enable(dev->dev);
+	if (!pm_runtime_enabled(dev->dev)) {
+		err = mchp_spdifrx_runtime_resume(dev->dev);
+		if (err)
+			goto pm_runtime_disable;
+	}
+
 	dev->capture.addr	= (dma_addr_t)mem->start + SPDIFRX_RHR;
 	dev->capture.maxburst	= 1;
 
 	err = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 	if (err) {
 		dev_err(&pdev->dev, "failed to register PCM: %d\n", err);
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	err = devm_snd_soc_register_component(&pdev->dev,
@@ -1036,20 +1138,40 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 					      &mchp_spdifrx_dai, 1);
 	if (err) {
 		dev_err(&pdev->dev, "fail to register dai\n");
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	regmap_read(regmap, SPDIFRX_VERSION, &vers);
 	dev_info(&pdev->dev, "hw version: %#lx\n", vers & SPDIFRX_VERSION_MASK);
 
 	return 0;
+
+pm_runtime_suspend:
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdifrx_runtime_suspend(dev->dev);
+pm_runtime_disable:
+	pm_runtime_disable(dev->dev);
+	return err;
+}
+
+static int mchp_spdifrx_remove(struct platform_device *pdev)
+{
+	struct mchp_spdifrx_dev *dev = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(dev->dev);
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdifrx_runtime_suspend(dev->dev);
+
+	return 0;
 }
 
 static struct platform_driver mchp_spdifrx_driver = {
 	.probe	= mchp_spdifrx_probe,
+	.remove = mchp_spdifrx_remove,
 	.driver	= {
 		.name	= "mchp_spdifrx",
 		.of_match_table = of_match_ptr(mchp_spdifrx_dt_ids),
+		.pm	= pm_ptr(&mchp_spdifrx_pm_ops),
 	},
 };
 
-- 
2.34.1


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

* [PATCH 7/8] ASoC: mchp-spdifrx: add runtime pm support
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

Add runtime PM support for Microchip SPDIFRX driver. On runtime suspend
the clocks are disabled and regmap is set in caching mode. On runtime
resume the clocks are enabled and regmap is synced with the device.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 210 ++++++++++++++++++++++++++-------
 1 file changed, 166 insertions(+), 44 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 46fff31321f3..796d4ec2b2b1 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -9,6 +9,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/spinlock.h>
 
@@ -192,6 +193,43 @@ static bool mchp_spdifrx_precious_reg(struct device *dev, unsigned int reg)
 	}
 }
 
+static bool mchp_spdifrx_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SPDIFRX_IMR:
+	case SPDIFRX_ISR:
+	case SPDIFRX_RSR:
+	case SPDIFRX_CHSR(0, 0):
+	case SPDIFRX_CHSR(0, 1):
+	case SPDIFRX_CHSR(0, 2):
+	case SPDIFRX_CHSR(0, 3):
+	case SPDIFRX_CHSR(0, 4):
+	case SPDIFRX_CHSR(0, 5):
+	case SPDIFRX_CHUD(0, 0):
+	case SPDIFRX_CHUD(0, 1):
+	case SPDIFRX_CHUD(0, 2):
+	case SPDIFRX_CHUD(0, 3):
+	case SPDIFRX_CHUD(0, 4):
+	case SPDIFRX_CHUD(0, 5):
+	case SPDIFRX_CHSR(1, 0):
+	case SPDIFRX_CHSR(1, 1):
+	case SPDIFRX_CHSR(1, 2):
+	case SPDIFRX_CHSR(1, 3):
+	case SPDIFRX_CHSR(1, 4):
+	case SPDIFRX_CHSR(1, 5):
+	case SPDIFRX_CHUD(1, 0):
+	case SPDIFRX_CHUD(1, 1):
+	case SPDIFRX_CHUD(1, 2):
+	case SPDIFRX_CHUD(1, 3):
+	case SPDIFRX_CHUD(1, 4):
+	case SPDIFRX_CHUD(1, 5):
+	case SPDIFRX_VERSION:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static const struct regmap_config mchp_spdifrx_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -200,6 +238,8 @@ static const struct regmap_config mchp_spdifrx_regmap_config = {
 	.readable_reg = mchp_spdifrx_readable_reg,
 	.writeable_reg = mchp_spdifrx_writeable_reg,
 	.precious_reg = mchp_spdifrx_precious_reg,
+	.volatile_reg = mchp_spdifrx_volatile_reg,
+	.cache_type = REGCACHE_FLAT,
 };
 
 #define SPDIFRX_GCLK_RATIO_MIN	(12 * 64)
@@ -236,7 +276,6 @@ struct mchp_spdifrx_dev {
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				trigger_enabled;
-	unsigned int				gclk_enabled:1;
 };
 
 static void mchp_spdifrx_channel_status_read(struct mchp_spdifrx_dev *dev,
@@ -405,16 +444,17 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		goto unlock;
 	}
 
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
+	/* GCLK is enabled by runtime PM. */
+	clk_disable_unprepare(dev->gclk);
+
 	ret = clk_set_min_rate(dev->gclk, params_rate(params) *
 					  SPDIFRX_GCLK_RATIO_MIN + 1);
 	if (ret) {
 		dev_err(dev->dev,
 			"unable to set gclk min rate: rate %u * ratio %u + 1\n",
 			params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
+		/* Restore runtime PM state. */
+		clk_prepare_enable(dev->gclk);
 		goto unlock;
 	}
 	ret = clk_prepare_enable(dev->gclk);
@@ -422,7 +462,6 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
 		goto unlock;
 	}
-	dev->gclk_enabled = 1;
 
 	dev_dbg(dev->dev, "GCLK range min set to %d\n",
 		params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
@@ -435,24 +474,9 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
-				struct snd_soc_dai *dai)
-{
-	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
-
-	mutex_lock(&dev->mlock);
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
-	mutex_unlock(&dev->mlock);
-	return 0;
-}
-
 static const struct snd_soc_dai_ops mchp_spdifrx_dai_ops = {
 	.trigger	= mchp_spdifrx_trigger,
 	.hw_params	= mchp_spdifrx_hw_params,
-	.hw_free	= mchp_spdifrx_hw_free,
 };
 
 #define MCHP_SPDIF_RATES	SNDRV_PCM_RATE_8000_192000
@@ -486,6 +510,10 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * We may reach this point with both clocks enabled but the receiver
 	 * still disabled. To void waiting for completion and return with
@@ -512,7 +540,7 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 				channel);
 			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(channel));
 			ret = ret ? : -ETIMEDOUT;
-			goto unlock;
+			goto pm_runtime_put;
 		} else {
 			ret = 0;
 		}
@@ -524,6 +552,9 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
 	       sizeof(ch_stat->data));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
 	return ret;
@@ -566,6 +597,10 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * We may reach this point with both clocks enabled but the receiver
 	 * still disabled. To void waiting for completion to just timeout we
@@ -588,7 +623,7 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 				channel);
 			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
 			ret = ret ? : -ETIMEDOUT;
-			goto unlock;
+			goto pm_runtime_put;
 		} else {
 			ret = 0;
 		}
@@ -600,6 +635,9 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 	memcpy(uvalue->value.iec958.subcode, user_data->data,
 	       sizeof(user_data->data));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
 	return ret;
@@ -641,10 +679,15 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	u32 val;
+	int ret;
 	bool ulock_old = ctrl->ulock;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -659,6 +702,9 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 
 	uvalue->value.integer.value[0] = ctrl->ulock;
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+unlock:
 	mutex_unlock(&dev->mlock);
 
 	return ulock_old != ctrl->ulock;
@@ -671,10 +717,15 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	u32 val;
+	int ret;
 	bool badf_old = ctrl->badf;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -687,6 +738,9 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 		ctrl->badf = 0;
 	}
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+unlock:
 	mutex_unlock(&dev->mlock);
 
 	uvalue->value.integer.value[0] = ctrl->badf;
@@ -706,16 +760,16 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * To get the signal we need to have receiver enabled. This
 	 * could be enabled also from trigger() function thus we need to
 	 * take care of not disabling the receiver when it runs.
 	 */
 	if (!dev->trigger_enabled) {
-		ret = clk_prepare_enable(dev->gclk);
-		if (ret)
-			goto unlock;
-
 		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
 				   SPDIFRX_MR_RXEN_ENABLE);
 
@@ -729,12 +783,13 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 
 		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
 				   SPDIFRX_MR_RXEN_DISABLE);
-
-		clk_disable_unprepare(dev->gclk);
 	} else {
 		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
 	}
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+
 unlock:
 	mutex_unlock(&dev->mlock);
 
@@ -765,9 +820,14 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	unsigned long rate;
 	u32 val;
+	int ret;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -778,21 +838,24 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 		/* If the receiver is not locked, ISF data is invalid. */
 		if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
 			ucontrol->value.integer.value[0] = 0;
-			goto unlock;
+			goto pm_runtime_put;
 		}
 	} else {
 		/* Reveicer is not locked, IFS data is invalid. */
 		ucontrol->value.integer.value[0] = 0;
-		goto unlock;
+		goto pm_runtime_put;
 	}
 
 	rate = clk_get_rate(dev->gclk);
 
 	ucontrol->value.integer.value[0] = rate / (32 * SPDIFRX_RSR_IFS(val));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
-	return 0;
+	return ret;
 }
 
 static struct snd_kcontrol_new mchp_spdifrx_ctrls[] = {
@@ -882,14 +945,6 @@ static int mchp_spdifrx_dai_probe(struct snd_soc_dai *dai)
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	int ch;
-	int err;
-
-	err = clk_prepare_enable(dev->pclk);
-	if (err) {
-		dev_err(dev->dev,
-			"failed to enable the peripheral clock: %d\n", err);
-		return err;
-	}
 
 	snd_soc_dai_init_dma_data(dai, NULL, &dev->capture);
 
@@ -922,8 +977,6 @@ static int mchp_spdifrx_dai_remove(struct snd_soc_dai *dai)
 	/* Disable interrupts */
 	regmap_write(dev->regmap, SPDIFRX_IDR, GENMASK(14, 0));
 
-	clk_disable_unprepare(dev->pclk);
-
 	return 0;
 }
 
@@ -954,6 +1007,48 @@ static const struct of_device_id mchp_spdifrx_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mchp_spdifrx_dt_ids);
 
+static int mchp_spdifrx_runtime_suspend(struct device *dev)
+{
+	struct mchp_spdifrx_dev *spdifrx = dev_get_drvdata(dev);
+
+	regcache_cache_only(spdifrx->regmap, true);
+	clk_disable_unprepare(spdifrx->gclk);
+	clk_disable_unprepare(spdifrx->pclk);
+
+	return 0;
+}
+
+static int mchp_spdifrx_runtime_resume(struct device *dev)
+{
+	struct mchp_spdifrx_dev *spdifrx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(spdifrx->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(spdifrx->gclk);
+	if (ret)
+		goto disable_pclk;
+
+	regcache_cache_only(spdifrx->regmap, false);
+	regcache_mark_dirty(spdifrx->regmap);
+	ret = regcache_sync(spdifrx->regmap);
+	if (ret) {
+		regcache_cache_only(spdifrx->regmap, true);
+		clk_disable_unprepare(spdifrx->gclk);
+disable_pclk:
+		clk_disable_unprepare(spdifrx->pclk);
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops mchp_spdifrx_pm_ops = {
+	RUNTIME_PM_OPS(mchp_spdifrx_runtime_suspend, mchp_spdifrx_runtime_resume,
+		       NULL)
+};
+
 static int mchp_spdifrx_probe(struct platform_device *pdev)
 {
 	struct mchp_spdifrx_dev *dev;
@@ -1022,13 +1117,20 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 	dev->regmap = regmap;
 	platform_set_drvdata(pdev, dev);
 
+	pm_runtime_enable(dev->dev);
+	if (!pm_runtime_enabled(dev->dev)) {
+		err = mchp_spdifrx_runtime_resume(dev->dev);
+		if (err)
+			goto pm_runtime_disable;
+	}
+
 	dev->capture.addr	= (dma_addr_t)mem->start + SPDIFRX_RHR;
 	dev->capture.maxburst	= 1;
 
 	err = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 	if (err) {
 		dev_err(&pdev->dev, "failed to register PCM: %d\n", err);
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	err = devm_snd_soc_register_component(&pdev->dev,
@@ -1036,20 +1138,40 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 					      &mchp_spdifrx_dai, 1);
 	if (err) {
 		dev_err(&pdev->dev, "fail to register dai\n");
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	regmap_read(regmap, SPDIFRX_VERSION, &vers);
 	dev_info(&pdev->dev, "hw version: %#lx\n", vers & SPDIFRX_VERSION_MASK);
 
 	return 0;
+
+pm_runtime_suspend:
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdifrx_runtime_suspend(dev->dev);
+pm_runtime_disable:
+	pm_runtime_disable(dev->dev);
+	return err;
+}
+
+static int mchp_spdifrx_remove(struct platform_device *pdev)
+{
+	struct mchp_spdifrx_dev *dev = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(dev->dev);
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdifrx_runtime_suspend(dev->dev);
+
+	return 0;
 }
 
 static struct platform_driver mchp_spdifrx_driver = {
 	.probe	= mchp_spdifrx_probe,
+	.remove = mchp_spdifrx_remove,
 	.driver	= {
 		.name	= "mchp_spdifrx",
 		.of_match_table = of_match_ptr(mchp_spdifrx_dt_ids),
+		.pm	= pm_ptr(&mchp_spdifrx_pm_ops),
 	},
 };
 
-- 
2.34.1


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

* [PATCH 7/8] ASoC: mchp-spdifrx: add runtime pm support
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Add runtime PM support for Microchip SPDIFRX driver. On runtime suspend
the clocks are disabled and regmap is set in caching mode. On runtime
resume the clocks are enabled and regmap is synced with the device.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 210 ++++++++++++++++++++++++++-------
 1 file changed, 166 insertions(+), 44 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 46fff31321f3..796d4ec2b2b1 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -9,6 +9,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/spinlock.h>
 
@@ -192,6 +193,43 @@ static bool mchp_spdifrx_precious_reg(struct device *dev, unsigned int reg)
 	}
 }
 
+static bool mchp_spdifrx_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SPDIFRX_IMR:
+	case SPDIFRX_ISR:
+	case SPDIFRX_RSR:
+	case SPDIFRX_CHSR(0, 0):
+	case SPDIFRX_CHSR(0, 1):
+	case SPDIFRX_CHSR(0, 2):
+	case SPDIFRX_CHSR(0, 3):
+	case SPDIFRX_CHSR(0, 4):
+	case SPDIFRX_CHSR(0, 5):
+	case SPDIFRX_CHUD(0, 0):
+	case SPDIFRX_CHUD(0, 1):
+	case SPDIFRX_CHUD(0, 2):
+	case SPDIFRX_CHUD(0, 3):
+	case SPDIFRX_CHUD(0, 4):
+	case SPDIFRX_CHUD(0, 5):
+	case SPDIFRX_CHSR(1, 0):
+	case SPDIFRX_CHSR(1, 1):
+	case SPDIFRX_CHSR(1, 2):
+	case SPDIFRX_CHSR(1, 3):
+	case SPDIFRX_CHSR(1, 4):
+	case SPDIFRX_CHSR(1, 5):
+	case SPDIFRX_CHUD(1, 0):
+	case SPDIFRX_CHUD(1, 1):
+	case SPDIFRX_CHUD(1, 2):
+	case SPDIFRX_CHUD(1, 3):
+	case SPDIFRX_CHUD(1, 4):
+	case SPDIFRX_CHUD(1, 5):
+	case SPDIFRX_VERSION:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static const struct regmap_config mchp_spdifrx_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -200,6 +238,8 @@ static const struct regmap_config mchp_spdifrx_regmap_config = {
 	.readable_reg = mchp_spdifrx_readable_reg,
 	.writeable_reg = mchp_spdifrx_writeable_reg,
 	.precious_reg = mchp_spdifrx_precious_reg,
+	.volatile_reg = mchp_spdifrx_volatile_reg,
+	.cache_type = REGCACHE_FLAT,
 };
 
 #define SPDIFRX_GCLK_RATIO_MIN	(12 * 64)
@@ -236,7 +276,6 @@ struct mchp_spdifrx_dev {
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				trigger_enabled;
-	unsigned int				gclk_enabled:1;
 };
 
 static void mchp_spdifrx_channel_status_read(struct mchp_spdifrx_dev *dev,
@@ -405,16 +444,17 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		goto unlock;
 	}
 
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
+	/* GCLK is enabled by runtime PM. */
+	clk_disable_unprepare(dev->gclk);
+
 	ret = clk_set_min_rate(dev->gclk, params_rate(params) *
 					  SPDIFRX_GCLK_RATIO_MIN + 1);
 	if (ret) {
 		dev_err(dev->dev,
 			"unable to set gclk min rate: rate %u * ratio %u + 1\n",
 			params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
+		/* Restore runtime PM state. */
+		clk_prepare_enable(dev->gclk);
 		goto unlock;
 	}
 	ret = clk_prepare_enable(dev->gclk);
@@ -422,7 +462,6 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
 		goto unlock;
 	}
-	dev->gclk_enabled = 1;
 
 	dev_dbg(dev->dev, "GCLK range min set to %d\n",
 		params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
@@ -435,24 +474,9 @@ static int mchp_spdifrx_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static int mchp_spdifrx_hw_free(struct snd_pcm_substream *substream,
-				struct snd_soc_dai *dai)
-{
-	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
-
-	mutex_lock(&dev->mlock);
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
-	mutex_unlock(&dev->mlock);
-	return 0;
-}
-
 static const struct snd_soc_dai_ops mchp_spdifrx_dai_ops = {
 	.trigger	= mchp_spdifrx_trigger,
 	.hw_params	= mchp_spdifrx_hw_params,
-	.hw_free	= mchp_spdifrx_hw_free,
 };
 
 #define MCHP_SPDIF_RATES	SNDRV_PCM_RATE_8000_192000
@@ -486,6 +510,10 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * We may reach this point with both clocks enabled but the receiver
 	 * still disabled. To void waiting for completion and return with
@@ -512,7 +540,7 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 				channel);
 			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_CSC(channel));
 			ret = ret ? : -ETIMEDOUT;
-			goto unlock;
+			goto pm_runtime_put;
 		} else {
 			ret = 0;
 		}
@@ -524,6 +552,9 @@ static int mchp_spdifrx_cs_get(struct mchp_spdifrx_dev *dev,
 	memcpy(uvalue->value.iec958.status, ch_stat->data,
 	       sizeof(ch_stat->data));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
 	return ret;
@@ -566,6 +597,10 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * We may reach this point with both clocks enabled but the receiver
 	 * still disabled. To void waiting for completion to just timeout we
@@ -588,7 +623,7 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 				channel);
 			regmap_write(dev->regmap, SPDIFRX_IDR, SPDIFRX_IR_BLOCKEND);
 			ret = ret ? : -ETIMEDOUT;
-			goto unlock;
+			goto pm_runtime_put;
 		} else {
 			ret = 0;
 		}
@@ -600,6 +635,9 @@ static int mchp_spdifrx_subcode_ch_get(struct mchp_spdifrx_dev *dev,
 	memcpy(uvalue->value.iec958.subcode, user_data->data,
 	       sizeof(user_data->data));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
 	return ret;
@@ -641,10 +679,15 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	u32 val;
+	int ret;
 	bool ulock_old = ctrl->ulock;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -659,6 +702,9 @@ static int mchp_spdifrx_ulock_get(struct snd_kcontrol *kcontrol,
 
 	uvalue->value.integer.value[0] = ctrl->ulock;
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+unlock:
 	mutex_unlock(&dev->mlock);
 
 	return ulock_old != ctrl->ulock;
@@ -671,10 +717,15 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	u32 val;
+	int ret;
 	bool badf_old = ctrl->badf;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -687,6 +738,9 @@ static int mchp_spdifrx_badf_get(struct snd_kcontrol *kcontrol,
 		ctrl->badf = 0;
 	}
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+unlock:
 	mutex_unlock(&dev->mlock);
 
 	uvalue->value.integer.value[0] = ctrl->badf;
@@ -706,16 +760,16 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * To get the signal we need to have receiver enabled. This
 	 * could be enabled also from trigger() function thus we need to
 	 * take care of not disabling the receiver when it runs.
 	 */
 	if (!dev->trigger_enabled) {
-		ret = clk_prepare_enable(dev->gclk);
-		if (ret)
-			goto unlock;
-
 		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
 				   SPDIFRX_MR_RXEN_ENABLE);
 
@@ -729,12 +783,13 @@ static int mchp_spdifrx_signal_get(struct snd_kcontrol *kcontrol,
 
 		regmap_update_bits(dev->regmap, SPDIFRX_MR, SPDIFRX_MR_RXEN_MASK,
 				   SPDIFRX_MR_RXEN_DISABLE);
-
-		clk_disable_unprepare(dev->gclk);
 	} else {
 		regmap_read(dev->regmap, SPDIFRX_RSR, &val);
 	}
 
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+
 unlock:
 	mutex_unlock(&dev->mlock);
 
@@ -765,9 +820,14 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	unsigned long rate;
 	u32 val;
+	int ret;
 
 	mutex_lock(&dev->mlock);
 
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto unlock;
+
 	/*
 	 * The RSR.ULOCK has wrong value if both pclk and gclk are enabled
 	 * and the receiver is disabled. Thus we take into account the
@@ -778,21 +838,24 @@ static int mchp_spdifrx_rate_get(struct snd_kcontrol *kcontrol,
 		/* If the receiver is not locked, ISF data is invalid. */
 		if (val & SPDIFRX_RSR_ULOCK || !(val & SPDIFRX_RSR_IFS_MASK)) {
 			ucontrol->value.integer.value[0] = 0;
-			goto unlock;
+			goto pm_runtime_put;
 		}
 	} else {
 		/* Reveicer is not locked, IFS data is invalid. */
 		ucontrol->value.integer.value[0] = 0;
-		goto unlock;
+		goto pm_runtime_put;
 	}
 
 	rate = clk_get_rate(dev->gclk);
 
 	ucontrol->value.integer.value[0] = rate / (32 * SPDIFRX_RSR_IFS(val));
 
+pm_runtime_put:
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 unlock:
 	mutex_unlock(&dev->mlock);
-	return 0;
+	return ret;
 }
 
 static struct snd_kcontrol_new mchp_spdifrx_ctrls[] = {
@@ -882,14 +945,6 @@ static int mchp_spdifrx_dai_probe(struct snd_soc_dai *dai)
 	struct mchp_spdifrx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdifrx_mixer_control *ctrl = &dev->control;
 	int ch;
-	int err;
-
-	err = clk_prepare_enable(dev->pclk);
-	if (err) {
-		dev_err(dev->dev,
-			"failed to enable the peripheral clock: %d\n", err);
-		return err;
-	}
 
 	snd_soc_dai_init_dma_data(dai, NULL, &dev->capture);
 
@@ -922,8 +977,6 @@ static int mchp_spdifrx_dai_remove(struct snd_soc_dai *dai)
 	/* Disable interrupts */
 	regmap_write(dev->regmap, SPDIFRX_IDR, GENMASK(14, 0));
 
-	clk_disable_unprepare(dev->pclk);
-
 	return 0;
 }
 
@@ -954,6 +1007,48 @@ static const struct of_device_id mchp_spdifrx_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mchp_spdifrx_dt_ids);
 
+static int mchp_spdifrx_runtime_suspend(struct device *dev)
+{
+	struct mchp_spdifrx_dev *spdifrx = dev_get_drvdata(dev);
+
+	regcache_cache_only(spdifrx->regmap, true);
+	clk_disable_unprepare(spdifrx->gclk);
+	clk_disable_unprepare(spdifrx->pclk);
+
+	return 0;
+}
+
+static int mchp_spdifrx_runtime_resume(struct device *dev)
+{
+	struct mchp_spdifrx_dev *spdifrx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(spdifrx->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(spdifrx->gclk);
+	if (ret)
+		goto disable_pclk;
+
+	regcache_cache_only(spdifrx->regmap, false);
+	regcache_mark_dirty(spdifrx->regmap);
+	ret = regcache_sync(spdifrx->regmap);
+	if (ret) {
+		regcache_cache_only(spdifrx->regmap, true);
+		clk_disable_unprepare(spdifrx->gclk);
+disable_pclk:
+		clk_disable_unprepare(spdifrx->pclk);
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops mchp_spdifrx_pm_ops = {
+	RUNTIME_PM_OPS(mchp_spdifrx_runtime_suspend, mchp_spdifrx_runtime_resume,
+		       NULL)
+};
+
 static int mchp_spdifrx_probe(struct platform_device *pdev)
 {
 	struct mchp_spdifrx_dev *dev;
@@ -1022,13 +1117,20 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 	dev->regmap = regmap;
 	platform_set_drvdata(pdev, dev);
 
+	pm_runtime_enable(dev->dev);
+	if (!pm_runtime_enabled(dev->dev)) {
+		err = mchp_spdifrx_runtime_resume(dev->dev);
+		if (err)
+			goto pm_runtime_disable;
+	}
+
 	dev->capture.addr	= (dma_addr_t)mem->start + SPDIFRX_RHR;
 	dev->capture.maxburst	= 1;
 
 	err = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 	if (err) {
 		dev_err(&pdev->dev, "failed to register PCM: %d\n", err);
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	err = devm_snd_soc_register_component(&pdev->dev,
@@ -1036,20 +1138,40 @@ static int mchp_spdifrx_probe(struct platform_device *pdev)
 					      &mchp_spdifrx_dai, 1);
 	if (err) {
 		dev_err(&pdev->dev, "fail to register dai\n");
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	regmap_read(regmap, SPDIFRX_VERSION, &vers);
 	dev_info(&pdev->dev, "hw version: %#lx\n", vers & SPDIFRX_VERSION_MASK);
 
 	return 0;
+
+pm_runtime_suspend:
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdifrx_runtime_suspend(dev->dev);
+pm_runtime_disable:
+	pm_runtime_disable(dev->dev);
+	return err;
+}
+
+static int mchp_spdifrx_remove(struct platform_device *pdev)
+{
+	struct mchp_spdifrx_dev *dev = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(dev->dev);
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdifrx_runtime_suspend(dev->dev);
+
+	return 0;
 }
 
 static struct platform_driver mchp_spdifrx_driver = {
 	.probe	= mchp_spdifrx_probe,
+	.remove = mchp_spdifrx_remove,
 	.driver	= {
 		.name	= "mchp_spdifrx",
 		.of_match_table = of_match_ptr(mchp_spdifrx_dt_ids),
+		.pm	= pm_ptr(&mchp_spdifrx_pm_ops),
 	},
 };
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 8/8] ASoC: mchp-spdifrx: document data structures
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-30 12:06   ` Claudiu Beznea
  -1 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Document data structures used by mchp-spdifrx driver.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 796d4ec2b2b1..dab5d93de329 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -249,16 +249,34 @@ static const struct regmap_config mchp_spdifrx_regmap_config = {
 
 #define SPDIFRX_CHANNELS	2
 
+/**
+ * struct mchp_spdifrx_ch_stat: MCHP SPDIFRX channel status
+ * @data: channel status bits
+ * @done: completion to signal channel status bits acquisition done
+ */
 struct mchp_spdifrx_ch_stat {
 	unsigned char data[SPDIFRX_CS_BITS / 8];
 	struct completion done;
 };
 
+/**
+ * struct mchp_spdifrx_user_data: MCHP SPDIFRX user data
+ * @data: user data bits
+ * @done: completion to signal user data bits acquisition done
+ */
 struct mchp_spdifrx_user_data {
 	unsigned char data[SPDIFRX_UD_BITS / 8];
 	struct completion done;
 };
 
+/**
+ * struct mchp_spdifrx_mixer_control: MCHP SPDIFRX mixer control data structure
+ * @ch_stat: array of channel statuses
+ * @user_data: array of user data
+ * @ulock: ulock bit status
+ * @badf: badf bit status
+ * @signal: signal bit status
+ */
 struct mchp_spdifrx_mixer_control {
 	struct mchp_spdifrx_ch_stat ch_stat[SPDIFRX_CHANNELS];
 	struct mchp_spdifrx_user_data user_data[SPDIFRX_CHANNELS];
@@ -267,6 +285,17 @@ struct mchp_spdifrx_mixer_control {
 	bool signal;
 };
 
+/**
+ * struct mchp_spdifrx_dev: MCHP SPDIFRX device data structure
+ * @capture: DAI DMA configuration data
+ * @control: mixer controls
+ * @mlock: mutex to protect concurency b/w configuration and control APIs
+ * @dev: struct device
+ * @regmap: regmap for this device
+ * @pclk: peripheral clock
+ * @gclk: generic clock
+ * @trigger_enabled: true if enabled though trigger() ops
+ */
 struct mchp_spdifrx_dev {
 	struct snd_dmaengine_dai_dma_data	capture;
 	struct mchp_spdifrx_mixer_control	control;
-- 
2.34.1


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

* [PATCH 8/8] ASoC: mchp-spdifrx: document data structures
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-kernel, linux-arm-kernel, Claudiu Beznea

Document data structures used by mchp-spdifrx driver.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 796d4ec2b2b1..dab5d93de329 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -249,16 +249,34 @@ static const struct regmap_config mchp_spdifrx_regmap_config = {
 
 #define SPDIFRX_CHANNELS	2
 
+/**
+ * struct mchp_spdifrx_ch_stat: MCHP SPDIFRX channel status
+ * @data: channel status bits
+ * @done: completion to signal channel status bits acquisition done
+ */
 struct mchp_spdifrx_ch_stat {
 	unsigned char data[SPDIFRX_CS_BITS / 8];
 	struct completion done;
 };
 
+/**
+ * struct mchp_spdifrx_user_data: MCHP SPDIFRX user data
+ * @data: user data bits
+ * @done: completion to signal user data bits acquisition done
+ */
 struct mchp_spdifrx_user_data {
 	unsigned char data[SPDIFRX_UD_BITS / 8];
 	struct completion done;
 };
 
+/**
+ * struct mchp_spdifrx_mixer_control: MCHP SPDIFRX mixer control data structure
+ * @ch_stat: array of channel statuses
+ * @user_data: array of user data
+ * @ulock: ulock bit status
+ * @badf: badf bit status
+ * @signal: signal bit status
+ */
 struct mchp_spdifrx_mixer_control {
 	struct mchp_spdifrx_ch_stat ch_stat[SPDIFRX_CHANNELS];
 	struct mchp_spdifrx_user_data user_data[SPDIFRX_CHANNELS];
@@ -267,6 +285,17 @@ struct mchp_spdifrx_mixer_control {
 	bool signal;
 };
 
+/**
+ * struct mchp_spdifrx_dev: MCHP SPDIFRX device data structure
+ * @capture: DAI DMA configuration data
+ * @control: mixer controls
+ * @mlock: mutex to protect concurency b/w configuration and control APIs
+ * @dev: struct device
+ * @regmap: regmap for this device
+ * @pclk: peripheral clock
+ * @gclk: generic clock
+ * @trigger_enabled: true if enabled though trigger() ops
+ */
 struct mchp_spdifrx_dev {
 	struct snd_dmaengine_dai_dma_data	capture;
 	struct mchp_spdifrx_mixer_control	control;
-- 
2.34.1


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

* [PATCH 8/8] ASoC: mchp-spdifrx: document data structures
@ 2023-01-30 12:06   ` Claudiu Beznea
  0 siblings, 0 replies; 30+ messages in thread
From: Claudiu Beznea @ 2023-01-30 12:06 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, nicolas.ferre, alexandre.belloni
  Cc: alsa-devel, linux-arm-kernel, linux-kernel, Claudiu Beznea

Document data structures used by mchp-spdifrx driver.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdifrx.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/sound/soc/atmel/mchp-spdifrx.c b/sound/soc/atmel/mchp-spdifrx.c
index 796d4ec2b2b1..dab5d93de329 100644
--- a/sound/soc/atmel/mchp-spdifrx.c
+++ b/sound/soc/atmel/mchp-spdifrx.c
@@ -249,16 +249,34 @@ static const struct regmap_config mchp_spdifrx_regmap_config = {
 
 #define SPDIFRX_CHANNELS	2
 
+/**
+ * struct mchp_spdifrx_ch_stat: MCHP SPDIFRX channel status
+ * @data: channel status bits
+ * @done: completion to signal channel status bits acquisition done
+ */
 struct mchp_spdifrx_ch_stat {
 	unsigned char data[SPDIFRX_CS_BITS / 8];
 	struct completion done;
 };
 
+/**
+ * struct mchp_spdifrx_user_data: MCHP SPDIFRX user data
+ * @data: user data bits
+ * @done: completion to signal user data bits acquisition done
+ */
 struct mchp_spdifrx_user_data {
 	unsigned char data[SPDIFRX_UD_BITS / 8];
 	struct completion done;
 };
 
+/**
+ * struct mchp_spdifrx_mixer_control: MCHP SPDIFRX mixer control data structure
+ * @ch_stat: array of channel statuses
+ * @user_data: array of user data
+ * @ulock: ulock bit status
+ * @badf: badf bit status
+ * @signal: signal bit status
+ */
 struct mchp_spdifrx_mixer_control {
 	struct mchp_spdifrx_ch_stat ch_stat[SPDIFRX_CHANNELS];
 	struct mchp_spdifrx_user_data user_data[SPDIFRX_CHANNELS];
@@ -267,6 +285,17 @@ struct mchp_spdifrx_mixer_control {
 	bool signal;
 };
 
+/**
+ * struct mchp_spdifrx_dev: MCHP SPDIFRX device data structure
+ * @capture: DAI DMA configuration data
+ * @control: mixer controls
+ * @mlock: mutex to protect concurency b/w configuration and control APIs
+ * @dev: struct device
+ * @regmap: regmap for this device
+ * @pclk: peripheral clock
+ * @gclk: generic clock
+ * @trigger_enabled: true if enabled though trigger() ops
+ */
 struct mchp_spdifrx_dev {
 	struct snd_dmaengine_dai_dma_data	capture;
 	struct mchp_spdifrx_mixer_control	control;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes
  2023-01-30 12:06 ` Claudiu Beznea
  (?)
@ 2023-01-31 14:32   ` Mark Brown
  -1 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2023-01-31 14:32 UTC (permalink / raw)
  To: lgirdwood, perex, tiwai, nicolas.ferre, alexandre.belloni,
	Claudiu Beznea
  Cc: alsa-devel, linux-arm-kernel, linux-kernel

On Mon, 30 Jan 2023 14:06:39 +0200, Claudiu Beznea wrote:
> This series adds runtime PM support for Microchip SPDIFRX driver.
> Along with it I added few fixes identified while going though the code
> and playing with Microchip SPDIFRX controller.
> 
> Thank you,
> Claudiu Beznea
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register
      commit: fa09fa60385abbf99342494b280da8b4aebbc0e9
[2/8] ASoC: mchp-spdifrx: fix return value in case completion times out
      commit: a4c4161d6eae3ef5f486d1638ef452d9bc1376b0
[3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism
      commit: d3681df44e856aab523a6eb7ba15b5e41efcbb1c
[4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove()
      commit: aaecdc32b7e35b4f9b457fb3509414aa9a932589
[5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
      commit: 36187a67ab931eae8b7d13d80fccd097971b7bac
[6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
      commit: ddce4aeccacb6f575cbfad623da5f0deb2592baf
[7/8] ASoC: mchp-spdifrx: add runtime pm support
      commit: c7db2a59438959bc881bc5722abf0d0a38681c2b
[8/8] ASoC: mchp-spdifrx: document data structures
      commit: 514d7f9df3f409cbb0ad59e726b4923d83251e4f

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] 30+ messages in thread

* Re: [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes
@ 2023-01-31 14:32   ` Mark Brown
  0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2023-01-31 14:32 UTC (permalink / raw)
  To: lgirdwood, perex, tiwai, nicolas.ferre, alexandre.belloni,
	Claudiu Beznea
  Cc: alsa-devel, linux-kernel, linux-arm-kernel

On Mon, 30 Jan 2023 14:06:39 +0200, Claudiu Beznea wrote:
> This series adds runtime PM support for Microchip SPDIFRX driver.
> Along with it I added few fixes identified while going though the code
> and playing with Microchip SPDIFRX controller.
> 
> Thank you,
> Claudiu Beznea
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register
      commit: fa09fa60385abbf99342494b280da8b4aebbc0e9
[2/8] ASoC: mchp-spdifrx: fix return value in case completion times out
      commit: a4c4161d6eae3ef5f486d1638ef452d9bc1376b0
[3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism
      commit: d3681df44e856aab523a6eb7ba15b5e41efcbb1c
[4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove()
      commit: aaecdc32b7e35b4f9b457fb3509414aa9a932589
[5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
      commit: 36187a67ab931eae8b7d13d80fccd097971b7bac
[6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
      commit: ddce4aeccacb6f575cbfad623da5f0deb2592baf
[7/8] ASoC: mchp-spdifrx: add runtime pm support
      commit: c7db2a59438959bc881bc5722abf0d0a38681c2b
[8/8] ASoC: mchp-spdifrx: document data structures
      commit: 514d7f9df3f409cbb0ad59e726b4923d83251e4f

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] 30+ messages in thread

* Re: [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes
@ 2023-01-31 14:32   ` Mark Brown
  0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2023-01-31 14:32 UTC (permalink / raw)
  To: lgirdwood, perex, tiwai, nicolas.ferre, alexandre.belloni,
	Claudiu Beznea
  Cc: alsa-devel, linux-arm-kernel, linux-kernel

On Mon, 30 Jan 2023 14:06:39 +0200, Claudiu Beznea wrote:
> This series adds runtime PM support for Microchip SPDIFRX driver.
> Along with it I added few fixes identified while going though the code
> and playing with Microchip SPDIFRX controller.
> 
> Thank you,
> Claudiu Beznea
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register
      commit: fa09fa60385abbf99342494b280da8b4aebbc0e9
[2/8] ASoC: mchp-spdifrx: fix return value in case completion times out
      commit: a4c4161d6eae3ef5f486d1638ef452d9bc1376b0
[3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism
      commit: d3681df44e856aab523a6eb7ba15b5e41efcbb1c
[4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove()
      commit: aaecdc32b7e35b4f9b457fb3509414aa9a932589
[5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value
      commit: 36187a67ab931eae8b7d13d80fccd097971b7bac
[6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member
      commit: ddce4aeccacb6f575cbfad623da5f0deb2592baf
[7/8] ASoC: mchp-spdifrx: add runtime pm support
      commit: c7db2a59438959bc881bc5722abf0d0a38681c2b
[8/8] ASoC: mchp-spdifrx: document data structures
      commit: 514d7f9df3f409cbb0ad59e726b4923d83251e4f

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2023-01-31 14:33 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30 12:06 [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes Claudiu Beznea
2023-01-30 12:06 ` Claudiu Beznea
2023-01-30 12:06 ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 1/8] ASoC: mchp-spdifrx: fix controls which rely on rsr register Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 2/8] ASoC: mchp-spdifrx: fix return value in case completion times out Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 3/8] ASoC: mchp-spdifrx: fix controls that works with completion mechanism Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 4/8] ASoC: mchp-spdifrx: disable all interrupts in mchp_spdifrx_dai_remove() Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 5/8] ASoC: mchp-spdifrx: use unsigned long to store clk_get_rate() value Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 6/8] ASoC: mchp-spdifrx: remove struct mchp_spdifrx_dev::fmt member Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 7/8] ASoC: mchp-spdifrx: add runtime pm support Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06 ` [PATCH 8/8] ASoC: mchp-spdifrx: document data structures Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-30 12:06   ` Claudiu Beznea
2023-01-31 14:32 ` [PATCH 0/8] ASoC: mchp-spdifrx: add runtime PM support and fixes Mark Brown
2023-01-31 14:32   ` Mark Brown
2023-01-31 14:32   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.