[PATCH 0/4] simple-audio-card codec2codec support

[PATCH 0/4] simple-audio-card codec2codec support

[Samuel Holland]

We are currently using simple-audio-card on the Allwinner A64 SoC.
The digital audio codec there (sun8i-codec) has 3 AIFs, one each for the
CPU, the modem, and Bluetooth. Adding support for the secondary AIFs
requires adding codec2codec DAI links.

Since the modem and bt-sco codec DAI drivers only have one set of
possible PCM parameters (namely, 8kHz mono S16LE), there's no real
need for a machine driver to specify the DAI link configuration. The
parameters for these "simple" DAI links can be chosen automatically.

This series adds a single "codec-to-codec" property to the
simple-audio-card binding, which does exactly what it says. It works out
rather nicely without making the device tree binding too complicated.

The first patch fixes a bug I found while implementing this feature.

I tried to reuse as much code as possible, so the middle two patches
refactor a couple of helper functions to be more generic.

Finally, the last patch adds the new feature and its documentation.

Samuel Holland (4):
  ASoC: codec2codec: avoid invalid/double-free of pcm runtime
  ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly
  ASoC: pcm: Export parameter intersection logic
  ASoC: simple-card: Add support for codec-to-codec dai_links

 .../devicetree/bindings/sound/simple-card.txt |  1 +
 Documentation/sound/soc/codec-to-codec.rst    |  9 ++-
 .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  2 +-
 include/sound/pcm.h                           |  2 +-
 include/sound/simple_card_utils.h             |  1 +
 include/sound/soc.h                           |  3 +
 sound/arm/aaci.c                              |  2 +-
 sound/arm/pxa2xx-ac97.c                       |  2 +-
 sound/core/pcm_misc.c                         | 14 ++---
 sound/firewire/dice/dice-pcm.c                |  2 +-
 sound/firewire/digi00x/digi00x-pcm.c          |  2 +-
 sound/firewire/fireworks/fireworks_pcm.c      |  2 +-
 sound/firewire/motu/motu-pcm.c                |  2 +-
 sound/firewire/tascam/tascam-pcm.c            |  2 +-
 sound/pci/atiixp.c                            |  2 +-
 sound/pci/cs5535audio/cs5535audio_pcm.c       |  4 +-
 sound/pci/hda/hda_controller.c                |  4 +-
 sound/pci/intel8x0.c                          |  2 +-
 sound/pci/sis7019.c                           |  2 +-
 sound/pci/via82xx.c                           |  4 +-
 sound/soc/generic/simple-card-utils.c         | 39 +++++++++++++
 sound/soc/generic/simple-card.c               | 12 ++++
 sound/soc/soc-dapm.c                          |  3 -
 sound/soc/soc-pcm.c                           | 57 +++++++++++++------
 sound/usb/caiaq/audio.c                       |  4 +-
 25 files changed, 130 insertions(+), 49 deletions(-)

-- 
2.24.1

[alsa-devel] [PATCH 0/4] simple-audio-card codec2codec support

[Samuel Holland]

We are currently using simple-audio-card on the Allwinner A64 SoC.
The digital audio codec there (sun8i-codec) has 3 AIFs, one each for the
CPU, the modem, and Bluetooth. Adding support for the secondary AIFs
requires adding codec2codec DAI links.

Since the modem and bt-sco codec DAI drivers only have one set of
possible PCM parameters (namely, 8kHz mono S16LE), there's no real
need for a machine driver to specify the DAI link configuration. The
parameters for these "simple" DAI links can be chosen automatically.

This series adds a single "codec-to-codec" property to the
simple-audio-card binding, which does exactly what it says. It works out
rather nicely without making the device tree binding too complicated.

The first patch fixes a bug I found while implementing this feature.

I tried to reuse as much code as possible, so the middle two patches
refactor a couple of helper functions to be more generic.

Finally, the last patch adds the new feature and its documentation.

Samuel Holland (4):
  ASoC: codec2codec: avoid invalid/double-free of pcm runtime
  ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly
  ASoC: pcm: Export parameter intersection logic
  ASoC: simple-card: Add support for codec-to-codec dai_links

 .../devicetree/bindings/sound/simple-card.txt |  1 +
 Documentation/sound/soc/codec-to-codec.rst    |  9 ++-
 .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  2 +-
 include/sound/pcm.h                           |  2 +-
 include/sound/simple_card_utils.h             |  1 +
 include/sound/soc.h                           |  3 +
 sound/arm/aaci.c                              |  2 +-
 sound/arm/pxa2xx-ac97.c                       |  2 +-
 sound/core/pcm_misc.c                         | 14 ++---
 sound/firewire/dice/dice-pcm.c                |  2 +-
 sound/firewire/digi00x/digi00x-pcm.c          |  2 +-
 sound/firewire/fireworks/fireworks_pcm.c      |  2 +-
 sound/firewire/motu/motu-pcm.c                |  2 +-
 sound/firewire/tascam/tascam-pcm.c            |  2 +-
 sound/pci/atiixp.c                            |  2 +-
 sound/pci/cs5535audio/cs5535audio_pcm.c       |  4 +-
 sound/pci/hda/hda_controller.c                |  4 +-
 sound/pci/intel8x0.c                          |  2 +-
 sound/pci/sis7019.c                           |  2 +-
 sound/pci/via82xx.c                           |  4 +-
 sound/soc/generic/simple-card-utils.c         | 39 +++++++++++++
 sound/soc/generic/simple-card.c               | 12 ++++
 sound/soc/soc-dapm.c                          |  3 -
 sound/soc/soc-pcm.c                           | 57 +++++++++++++------
 sound/usb/caiaq/audio.c                       |  4 +-
 25 files changed, 130 insertions(+), 49 deletions(-)

-- 
2.24.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Samuel Holland]

The PCM runtime was freed during PMU in the case that the event hook
encountered an error. However, it is also unconditionally freed during
PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.

Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
Cc: stable@vger.kernel.org
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 sound/soc/soc-dapm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index b6378f025836..935b5375ecc5 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3888,9 +3888,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 	runtime->rate = params_rate(params);
 
 out:
-	if (ret < 0)
-		kfree(runtime);
-
 	kfree(params);
 	return ret;
 }
-- 
2.24.1

[alsa-devel] [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Samuel Holland]

The PCM runtime was freed during PMU in the case that the event hook
encountered an error. However, it is also unconditionally freed during
PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.

Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
Cc: stable@vger.kernel.org
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 sound/soc/soc-dapm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index b6378f025836..935b5375ecc5 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3888,9 +3888,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 	runtime->rate = params_rate(params);
 
 out:
-	if (ret < 0)
-		kfree(runtime);
-
 	kfree(params);
 	return ret;
 }
-- 
2.24.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/4] ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly

[Samuel Holland]

It can be useful to derive min/max rates of a snd_pcm_hardware without
having a snd_pcm_runtime, such as before constructing an ASoC DAI link.

Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually
need the snd_pcm_runtime. Modify it to take a pointer to hw directly.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c    |  2 +-
 include/sound/pcm.h                                |  2 +-
 sound/arm/aaci.c                                   |  2 +-
 sound/arm/pxa2xx-ac97.c                            |  2 +-
 sound/core/pcm_misc.c                              | 14 +++++++-------
 sound/firewire/dice/dice-pcm.c                     |  2 +-
 sound/firewire/digi00x/digi00x-pcm.c               |  2 +-
 sound/firewire/fireworks/fireworks_pcm.c           |  2 +-
 sound/firewire/motu/motu-pcm.c                     |  2 +-
 sound/firewire/tascam/tascam-pcm.c                 |  2 +-
 sound/pci/atiixp.c                                 |  2 +-
 sound/pci/cs5535audio/cs5535audio_pcm.c            |  4 ++--
 sound/pci/hda/hda_controller.c                     |  4 ++--
 sound/pci/intel8x0.c                               |  2 +-
 sound/pci/sis7019.c                                |  2 +-
 sound/pci/via82xx.c                                |  4 ++--
 sound/soc/soc-pcm.c                                |  4 ++--
 sound/usb/caiaq/audio.c                            |  4 ++--
 18 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
index 2b7539701b42..33f7bcf992a4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
@@ -328,7 +328,7 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
 	if (ret < 0)
 		return ret;
 
-	ret = snd_pcm_limit_hw_rates(runtime);
+	ret = snd_pcm_limit_hw_rates(&runtime->hw);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 8a89fa6fdd5e..203b79d2712a 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1121,7 +1121,7 @@ snd_pcm_kernel_readv(struct snd_pcm_substream *substream,
 	return __snd_pcm_lib_xfer(substream, bufs, false, frames, true);
 }
 
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime);
+int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw);
 unsigned int snd_pcm_rate_to_rate_bit(unsigned int rate);
 unsigned int snd_pcm_rate_bit_to_rate(unsigned int rate_bit);
 unsigned int snd_pcm_rate_mask_intersect(unsigned int rates_a,
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
index b5399b0090a7..5052689247f9 100644
--- a/sound/arm/aaci.c
+++ b/sound/arm/aaci.c
@@ -413,7 +413,7 @@ static int aaci_pcm_open(struct snd_pcm_substream *substream)
 	runtime->private_data = aacirun;
 	runtime->hw = aaci_hw_info;
 	runtime->hw.rates = aacirun->pcm->rates;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		runtime->hw.channels_max = 6;
diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
index acfaf1d4ec25..cfb23073471e 100644
--- a/sound/arm/pxa2xx-ac97.c
+++ b/sound/arm/pxa2xx-ac97.c
@@ -77,7 +77,7 @@ static int pxa2xx_ac97_pcm_open(struct snd_pcm_substream *substream)
 	i = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
 		AC97_RATES_FRONT_DAC : AC97_RATES_ADC;
 	runtime->hw.rates = pxa2xx_ac97_ac97->rates[i];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	platform_ops = substream->pcm->card->dev->platform_data;
 	if (platform_ops && platform_ops->startup) {
diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
index c4eb561d2008..435688855ed0 100644
--- a/sound/core/pcm_misc.c
+++ b/sound/core/pcm_misc.c
@@ -474,25 +474,25 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
 
 /**
  * snd_pcm_limit_hw_rates - determine rate_min/rate_max fields
- * @runtime: the runtime instance
+ * @runtime: the pcm hw instance
  *
  * Determines the rate_min and rate_max fields from the rates bits of
- * the given runtime->hw.
+ * the given hw.
  *
  * Return: Zero if successful.
  */
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime)
+int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw)
 {
 	int i;
 	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
-		if (runtime->hw.rates & (1 << i)) {
-			runtime->hw.rate_min = snd_pcm_known_rates.list[i];
+		if (hw->rates & (1 << i)) {
+			hw->rate_min = snd_pcm_known_rates.list[i];
 			break;
 		}
 	}
 	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
-		if (runtime->hw.rates & (1 << i)) {
-			runtime->hw.rate_max = snd_pcm_known_rates.list[i];
+		if (hw->rates & (1 << i)) {
+			hw->rate_max = snd_pcm_known_rates.list[i];
 			break;
 		}
 	}
diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index be79d659eedf..85941d945067 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -117,7 +117,7 @@ static int limit_channels_and_rates(struct snd_dice *dice,
 		hw->channels_max = max(hw->channels_max, channels);
 	}
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	return 0;
 }
diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
index 57cbce4fd836..0709070ab5af 100644
--- a/sound/firewire/digi00x/digi00x-pcm.c
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -78,7 +78,7 @@ static int pcm_init_hw_params(struct snd_dg00x *dg00x,
 		    SNDRV_PCM_RATE_48000 |
 		    SNDRV_PCM_RATE_88200 |
 		    SNDRV_PCM_RATE_96000;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	err = snd_pcm_hw_rule_add(substream->runtime, 0,
 				  SNDRV_PCM_HW_PARAM_CHANNELS,
diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c
index e69896d748df..2ee8e98ea2b6 100644
--- a/sound/firewire/fireworks/fireworks_pcm.c
+++ b/sound/firewire/fireworks/fireworks_pcm.c
@@ -149,7 +149,7 @@ pcm_init_hw_params(struct snd_efw *efw,
 
 	/* limit rates */
 	runtime->hw.rates = efw->supported_sampling_rate,
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	limit_channels(&runtime->hw, pcm_channels);
 
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
index 005970931030..338eb0572890 100644
--- a/sound/firewire/motu/motu-pcm.c
+++ b/sound/firewire/motu/motu-pcm.c
@@ -92,7 +92,7 @@ static void limit_channels_and_rates(struct snd_motu *motu,
 		hw->channels_max = max(hw->channels_max, pcm_channels);
 	}
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 }
 
 static int init_hw_info(struct snd_motu *motu,
diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c
index 8e9b444c8bff..6722c1a65a42 100644
--- a/sound/firewire/tascam/tascam-pcm.c
+++ b/sound/firewire/tascam/tascam-pcm.c
@@ -35,7 +35,7 @@ static int pcm_init_hw_params(struct snd_tscm *tscm,
 		    SNDRV_PCM_RATE_48000 |
 		    SNDRV_PCM_RATE_88200 |
 		    SNDRV_PCM_RATE_96000;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	return amdtp_tscm_add_pcm_hw_constraints(stream, runtime);
 }
diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
index 1e1ededf8eb2..d7af407306b7 100644
--- a/sound/pci/atiixp.c
+++ b/sound/pci/atiixp.c
@@ -1039,7 +1039,7 @@ static int snd_atiixp_pcm_open(struct snd_pcm_substream *substream,
 	dma->ac97_pcm_type = pcm_type;
 	if (pcm_type >= 0) {
 		runtime->hw.rates = chip->pcms[pcm_type]->rates;
-		snd_pcm_limit_hw_rates(runtime);
+		snd_pcm_limit_hw_rates(&runtime->hw);
 	} else {
 		/* direct SPDIF */
 		runtime->hw.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE;
diff --git a/sound/pci/cs5535audio/cs5535audio_pcm.c b/sound/pci/cs5535audio/cs5535audio_pcm.c
index 4642e5384e83..7ce1664a8148 100644
--- a/sound/pci/cs5535audio/cs5535audio_pcm.c
+++ b/sound/pci/cs5535audio/cs5535audio_pcm.c
@@ -84,7 +84,7 @@ static int snd_cs5535audio_playback_open(struct snd_pcm_substream *substream)
 
 	runtime->hw = snd_cs5535audio_playback;
 	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_FRONT_DAC];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	cs5535au->playback_substream = substream;
 	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_PLAYBACK]);
 	if ((err = snd_pcm_hw_constraint_integer(runtime,
@@ -343,7 +343,7 @@ static int snd_cs5535audio_capture_open(struct snd_pcm_substream *substream)
 
 	runtime->hw = snd_cs5535audio_capture;
 	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_ADC];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	cs5535au->capture_substream = substream;
 	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_CAPTURE]);
 	if ((err = snd_pcm_hw_constraint_integer(runtime,
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index ba56b59b3e17..3728dbfae7b0 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -605,7 +605,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
 	runtime->hw.channels_max = hinfo->channels_max;
 	runtime->hw.formats = hinfo->formats;
 	runtime->hw.rates = hinfo->rates;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 
 	/* avoid wrap-around with wall-clock */
@@ -648,7 +648,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
 		azx_release_device(azx_dev);
 		goto powerdown;
 	}
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	/* sanity check */
 	if (snd_BUG_ON(!runtime->hw.channels_min) ||
 	    snd_BUG_ON(!runtime->hw.channels_max) ||
diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
index 12374ba08ca2..7f85a30291a1 100644
--- a/sound/pci/intel8x0.c
+++ b/sound/pci/intel8x0.c
@@ -1119,7 +1119,7 @@ static int snd_intel8x0_pcm_open(struct snd_pcm_substream *substream, struct ich
 	ichdev->substream = substream;
 	runtime->hw = snd_intel8x0_stream;
 	runtime->hw.rates = ichdev->pcm->rates;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	if (chip->device_type == DEVICE_SIS) {
 		runtime->hw.buffer_bytes_max = 64*1024;
 		runtime->hw.period_bytes_max = 64*1024;
diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
index ef7dd290ae05..4d8e852d438f 100644
--- a/sound/pci/sis7019.c
+++ b/sound/pci/sis7019.c
@@ -681,7 +681,7 @@ static int sis_capture_open(struct snd_pcm_substream *substream)
 	runtime->private_data = voice;
 	runtime->hw = sis_capture_hw_info;
 	runtime->hw.rates = sis->ac97[0]->rates[AC97_RATES_ADC];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
 						9, 0xfff9);
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c
index 30c817b6b635..8a55153a6ea3 100644
--- a/sound/pci/via82xx.c
+++ b/sound/pci/via82xx.c
@@ -1180,7 +1180,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
 	if (chip->spdif_on && viadev->reg_offset == 0x30) {
 		/* DXS#3 and spdif is on */
 		runtime->hw.rates = chip->ac97->rates[AC97_RATES_SPDIF];
-		snd_pcm_limit_hw_rates(runtime);
+		snd_pcm_limit_hw_rates(&runtime->hw);
 	} else if (chip->dxs_fixed && viadev->reg_offset < 0x40) {
 		/* fixed DXS playback rate */
 		runtime->hw.rates = SNDRV_PCM_RATE_48000;
@@ -1195,7 +1195,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
 	} else if (! ratep->rate) {
 		int idx = viadev->direction ? AC97_RATES_ADC : AC97_RATES_FRONT_DAC;
 		runtime->hw.rates = chip->ac97->rates[idx];
-		snd_pcm_limit_hw_rates(runtime);
+		snd_pcm_limit_hw_rates(&runtime->hw);
 	} else {
 		/* a fixed rate */
 		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 01e7bc03d92f..bd7b3cfcfa62 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -416,7 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 		hw->formats = formats & cpu_stream->formats;
 	hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
 	hw->rate_min = max(hw->rate_min, rate_min);
@@ -1951,7 +1951,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
 
 	dpcm_set_fe_runtime(fe_substream);
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	ret = dpcm_apply_symmetry(fe_substream, stream);
 	if (ret < 0) {
diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
index 970eb0865ba3..242f50ebc63a 100644
--- a/sound/usb/caiaq/audio.c
+++ b/sound/usb/caiaq/audio.c
@@ -145,7 +145,7 @@ static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
 
 	dev_dbg(dev, "%s(%p)\n", __func__, substream);
 	substream->runtime->hw = cdev->pcm_info;
-	snd_pcm_limit_hw_rates(substream->runtime);
+	snd_pcm_limit_hw_rates(&substream->runtime->hw);
 
 	return 0;
 }
@@ -243,7 +243,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream)
 		if (runtime->rate == rates[i])
 			cdev->pcm_info.rates = 1 << i;
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	bytes_per_sample = BYTES_PER_SAMPLE;
 	if (cdev->spec.data_alignment >= 2)
-- 
2.24.1

[alsa-devel] [PATCH 2/4] ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly

[Samuel Holland]

It can be useful to derive min/max rates of a snd_pcm_hardware without
having a snd_pcm_runtime, such as before constructing an ASoC DAI link.

Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually
need the snd_pcm_runtime. Modify it to take a pointer to hw directly.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c    |  2 +-
 include/sound/pcm.h                                |  2 +-
 sound/arm/aaci.c                                   |  2 +-
 sound/arm/pxa2xx-ac97.c                            |  2 +-
 sound/core/pcm_misc.c                              | 14 +++++++-------
 sound/firewire/dice/dice-pcm.c                     |  2 +-
 sound/firewire/digi00x/digi00x-pcm.c               |  2 +-
 sound/firewire/fireworks/fireworks_pcm.c           |  2 +-
 sound/firewire/motu/motu-pcm.c                     |  2 +-
 sound/firewire/tascam/tascam-pcm.c                 |  2 +-
 sound/pci/atiixp.c                                 |  2 +-
 sound/pci/cs5535audio/cs5535audio_pcm.c            |  4 ++--
 sound/pci/hda/hda_controller.c                     |  4 ++--
 sound/pci/intel8x0.c                               |  2 +-
 sound/pci/sis7019.c                                |  2 +-
 sound/pci/via82xx.c                                |  4 ++--
 sound/soc/soc-pcm.c                                |  4 ++--
 sound/usb/caiaq/audio.c                            |  4 ++--
 18 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
index 2b7539701b42..33f7bcf992a4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
@@ -328,7 +328,7 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
 	if (ret < 0)
 		return ret;
 
-	ret = snd_pcm_limit_hw_rates(runtime);
+	ret = snd_pcm_limit_hw_rates(&runtime->hw);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 8a89fa6fdd5e..203b79d2712a 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1121,7 +1121,7 @@ snd_pcm_kernel_readv(struct snd_pcm_substream *substream,
 	return __snd_pcm_lib_xfer(substream, bufs, false, frames, true);
 }
 
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime);
+int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw);
 unsigned int snd_pcm_rate_to_rate_bit(unsigned int rate);
 unsigned int snd_pcm_rate_bit_to_rate(unsigned int rate_bit);
 unsigned int snd_pcm_rate_mask_intersect(unsigned int rates_a,
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
index b5399b0090a7..5052689247f9 100644
--- a/sound/arm/aaci.c
+++ b/sound/arm/aaci.c
@@ -413,7 +413,7 @@ static int aaci_pcm_open(struct snd_pcm_substream *substream)
 	runtime->private_data = aacirun;
 	runtime->hw = aaci_hw_info;
 	runtime->hw.rates = aacirun->pcm->rates;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		runtime->hw.channels_max = 6;
diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
index acfaf1d4ec25..cfb23073471e 100644
--- a/sound/arm/pxa2xx-ac97.c
+++ b/sound/arm/pxa2xx-ac97.c
@@ -77,7 +77,7 @@ static int pxa2xx_ac97_pcm_open(struct snd_pcm_substream *substream)
 	i = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
 		AC97_RATES_FRONT_DAC : AC97_RATES_ADC;
 	runtime->hw.rates = pxa2xx_ac97_ac97->rates[i];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	platform_ops = substream->pcm->card->dev->platform_data;
 	if (platform_ops && platform_ops->startup) {
diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
index c4eb561d2008..435688855ed0 100644
--- a/sound/core/pcm_misc.c
+++ b/sound/core/pcm_misc.c
@@ -474,25 +474,25 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
 
 /**
  * snd_pcm_limit_hw_rates - determine rate_min/rate_max fields
- * @runtime: the runtime instance
+ * @runtime: the pcm hw instance
  *
  * Determines the rate_min and rate_max fields from the rates bits of
- * the given runtime->hw.
+ * the given hw.
  *
  * Return: Zero if successful.
  */
-int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime)
+int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw)
 {
 	int i;
 	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
-		if (runtime->hw.rates & (1 << i)) {
-			runtime->hw.rate_min = snd_pcm_known_rates.list[i];
+		if (hw->rates & (1 << i)) {
+			hw->rate_min = snd_pcm_known_rates.list[i];
 			break;
 		}
 	}
 	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
-		if (runtime->hw.rates & (1 << i)) {
-			runtime->hw.rate_max = snd_pcm_known_rates.list[i];
+		if (hw->rates & (1 << i)) {
+			hw->rate_max = snd_pcm_known_rates.list[i];
 			break;
 		}
 	}
diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index be79d659eedf..85941d945067 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -117,7 +117,7 @@ static int limit_channels_and_rates(struct snd_dice *dice,
 		hw->channels_max = max(hw->channels_max, channels);
 	}
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	return 0;
 }
diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
index 57cbce4fd836..0709070ab5af 100644
--- a/sound/firewire/digi00x/digi00x-pcm.c
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -78,7 +78,7 @@ static int pcm_init_hw_params(struct snd_dg00x *dg00x,
 		    SNDRV_PCM_RATE_48000 |
 		    SNDRV_PCM_RATE_88200 |
 		    SNDRV_PCM_RATE_96000;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	err = snd_pcm_hw_rule_add(substream->runtime, 0,
 				  SNDRV_PCM_HW_PARAM_CHANNELS,
diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c
index e69896d748df..2ee8e98ea2b6 100644
--- a/sound/firewire/fireworks/fireworks_pcm.c
+++ b/sound/firewire/fireworks/fireworks_pcm.c
@@ -149,7 +149,7 @@ pcm_init_hw_params(struct snd_efw *efw,
 
 	/* limit rates */
 	runtime->hw.rates = efw->supported_sampling_rate,
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	limit_channels(&runtime->hw, pcm_channels);
 
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
index 005970931030..338eb0572890 100644
--- a/sound/firewire/motu/motu-pcm.c
+++ b/sound/firewire/motu/motu-pcm.c
@@ -92,7 +92,7 @@ static void limit_channels_and_rates(struct snd_motu *motu,
 		hw->channels_max = max(hw->channels_max, pcm_channels);
 	}
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 }
 
 static int init_hw_info(struct snd_motu *motu,
diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c
index 8e9b444c8bff..6722c1a65a42 100644
--- a/sound/firewire/tascam/tascam-pcm.c
+++ b/sound/firewire/tascam/tascam-pcm.c
@@ -35,7 +35,7 @@ static int pcm_init_hw_params(struct snd_tscm *tscm,
 		    SNDRV_PCM_RATE_48000 |
 		    SNDRV_PCM_RATE_88200 |
 		    SNDRV_PCM_RATE_96000;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	return amdtp_tscm_add_pcm_hw_constraints(stream, runtime);
 }
diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
index 1e1ededf8eb2..d7af407306b7 100644
--- a/sound/pci/atiixp.c
+++ b/sound/pci/atiixp.c
@@ -1039,7 +1039,7 @@ static int snd_atiixp_pcm_open(struct snd_pcm_substream *substream,
 	dma->ac97_pcm_type = pcm_type;
 	if (pcm_type >= 0) {
 		runtime->hw.rates = chip->pcms[pcm_type]->rates;
-		snd_pcm_limit_hw_rates(runtime);
+		snd_pcm_limit_hw_rates(&runtime->hw);
 	} else {
 		/* direct SPDIF */
 		runtime->hw.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE;
diff --git a/sound/pci/cs5535audio/cs5535audio_pcm.c b/sound/pci/cs5535audio/cs5535audio_pcm.c
index 4642e5384e83..7ce1664a8148 100644
--- a/sound/pci/cs5535audio/cs5535audio_pcm.c
+++ b/sound/pci/cs5535audio/cs5535audio_pcm.c
@@ -84,7 +84,7 @@ static int snd_cs5535audio_playback_open(struct snd_pcm_substream *substream)
 
 	runtime->hw = snd_cs5535audio_playback;
 	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_FRONT_DAC];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	cs5535au->playback_substream = substream;
 	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_PLAYBACK]);
 	if ((err = snd_pcm_hw_constraint_integer(runtime,
@@ -343,7 +343,7 @@ static int snd_cs5535audio_capture_open(struct snd_pcm_substream *substream)
 
 	runtime->hw = snd_cs5535audio_capture;
 	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_ADC];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	cs5535au->capture_substream = substream;
 	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_CAPTURE]);
 	if ((err = snd_pcm_hw_constraint_integer(runtime,
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index ba56b59b3e17..3728dbfae7b0 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -605,7 +605,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
 	runtime->hw.channels_max = hinfo->channels_max;
 	runtime->hw.formats = hinfo->formats;
 	runtime->hw.rates = hinfo->rates;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 
 	/* avoid wrap-around with wall-clock */
@@ -648,7 +648,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
 		azx_release_device(azx_dev);
 		goto powerdown;
 	}
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	/* sanity check */
 	if (snd_BUG_ON(!runtime->hw.channels_min) ||
 	    snd_BUG_ON(!runtime->hw.channels_max) ||
diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
index 12374ba08ca2..7f85a30291a1 100644
--- a/sound/pci/intel8x0.c
+++ b/sound/pci/intel8x0.c
@@ -1119,7 +1119,7 @@ static int snd_intel8x0_pcm_open(struct snd_pcm_substream *substream, struct ich
 	ichdev->substream = substream;
 	runtime->hw = snd_intel8x0_stream;
 	runtime->hw.rates = ichdev->pcm->rates;
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	if (chip->device_type == DEVICE_SIS) {
 		runtime->hw.buffer_bytes_max = 64*1024;
 		runtime->hw.period_bytes_max = 64*1024;
diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
index ef7dd290ae05..4d8e852d438f 100644
--- a/sound/pci/sis7019.c
+++ b/sound/pci/sis7019.c
@@ -681,7 +681,7 @@ static int sis_capture_open(struct snd_pcm_substream *substream)
 	runtime->private_data = voice;
 	runtime->hw = sis_capture_hw_info;
 	runtime->hw.rates = sis->ac97[0]->rates[AC97_RATES_ADC];
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
 						9, 0xfff9);
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c
index 30c817b6b635..8a55153a6ea3 100644
--- a/sound/pci/via82xx.c
+++ b/sound/pci/via82xx.c
@@ -1180,7 +1180,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
 	if (chip->spdif_on && viadev->reg_offset == 0x30) {
 		/* DXS#3 and spdif is on */
 		runtime->hw.rates = chip->ac97->rates[AC97_RATES_SPDIF];
-		snd_pcm_limit_hw_rates(runtime);
+		snd_pcm_limit_hw_rates(&runtime->hw);
 	} else if (chip->dxs_fixed && viadev->reg_offset < 0x40) {
 		/* fixed DXS playback rate */
 		runtime->hw.rates = SNDRV_PCM_RATE_48000;
@@ -1195,7 +1195,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
 	} else if (! ratep->rate) {
 		int idx = viadev->direction ? AC97_RATES_ADC : AC97_RATES_FRONT_DAC;
 		runtime->hw.rates = chip->ac97->rates[idx];
-		snd_pcm_limit_hw_rates(runtime);
+		snd_pcm_limit_hw_rates(&runtime->hw);
 	} else {
 		/* a fixed rate */
 		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 01e7bc03d92f..bd7b3cfcfa62 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -416,7 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 		hw->formats = formats & cpu_stream->formats;
 	hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(hw);
 
 	hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
 	hw->rate_min = max(hw->rate_min, rate_min);
@@ -1951,7 +1951,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
 
 	dpcm_set_fe_runtime(fe_substream);
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	ret = dpcm_apply_symmetry(fe_substream, stream);
 	if (ret < 0) {
diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
index 970eb0865ba3..242f50ebc63a 100644
--- a/sound/usb/caiaq/audio.c
+++ b/sound/usb/caiaq/audio.c
@@ -145,7 +145,7 @@ static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
 
 	dev_dbg(dev, "%s(%p)\n", __func__, substream);
 	substream->runtime->hw = cdev->pcm_info;
-	snd_pcm_limit_hw_rates(substream->runtime);
+	snd_pcm_limit_hw_rates(&substream->runtime->hw);
 
 	return 0;
 }
@@ -243,7 +243,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream)
 		if (runtime->rate == rates[i])
 			cdev->pcm_info.rates = 1 << i;
 
-	snd_pcm_limit_hw_rates(runtime);
+	snd_pcm_limit_hw_rates(&runtime->hw);
 
 	bytes_per_sample = BYTES_PER_SAMPLE;
 	if (cdev->spec.data_alignment >= 2)
-- 
2.24.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 3/4] ASoC: pcm: Export parameter intersection logic

[Samuel Holland]

The logic to calculate the subset of stream parameters supported by all
DAIs associated with a PCM stream is nontrivial. Export a helper
function so it can be used to set up simple codec2codec DAI links.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 include/sound/soc.h |  3 +++
 sound/soc/soc-pcm.c | 53 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 262896799826..578a8e3e08ca 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -473,6 +473,9 @@ bool snd_soc_runtime_ignore_pmdown_time(struct snd_soc_pcm_runtime *rtd);
 void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream);
 void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream);
 
+int snd_soc_runtime_calc_hw(struct snd_soc_pcm_runtime *rtd,
+			    struct snd_pcm_hardware *hw, int stream);
+
 int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
 	unsigned int dai_fmt);
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index bd7b3cfcfa62..562af1d3f24e 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -348,11 +348,18 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
 	soc_pcm_set_msb(substream, cpu_bits);
 }
 
-static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
+/**
+ * snd_soc_runtime_calc_hw() - Calculate hw limits for a PCM stream
+ * @rtd: ASoC PCM runtime
+ * @hw: PCM hardware parameters (output)
+ * @stream: Direction of the PCM stream
+ *
+ * Calculates the subset of stream parameters supported by all DAIs
+ * associated with the PCM stream.
+ */
+int snd_soc_runtime_calc_hw(struct snd_soc_pcm_runtime *rtd,
+			    struct snd_pcm_hardware *hw, int stream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_pcm_hardware *hw = &runtime->hw;
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai;
 	struct snd_soc_dai_driver *cpu_dai_drv = rtd->cpu_dai->driver;
 	struct snd_soc_dai_driver *codec_dai_drv;
@@ -364,7 +371,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 	u64 formats = ULLONG_MAX;
 	int i;
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
 		cpu_stream = &cpu_dai_drv->playback;
 	else
 		cpu_stream = &cpu_dai_drv->capture;
@@ -377,16 +384,12 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 		 * Otherwise, since the rate, channel, and format values will
 		 * zero in that case, we would have no usable settings left,
 		 * causing the resulting setup to fail.
-		 * At least one CODEC should match, otherwise we should have
-		 * bailed out on a higher level, since there would be no
-		 * CODEC to support the transfer direction in that case.
 		 */
-		if (!snd_soc_dai_stream_valid(codec_dai,
-					      substream->stream))
+		if (!snd_soc_dai_stream_valid(codec_dai, stream))
 			continue;
 
 		codec_dai_drv = codec_dai->driver;
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		if (stream == SNDRV_PCM_STREAM_PLAYBACK)
 			codec_stream = &codec_dai_drv->playback;
 		else
 			codec_stream = &codec_dai_drv->capture;
@@ -398,6 +401,9 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 		rates = snd_pcm_rate_mask_intersect(codec_stream->rates, rates);
 	}
 
+	if (!chan_min)
+		return -EINVAL;
+
 	/*
 	 * chan min/max cannot be enforced if there are multiple CODEC DAIs
 	 * connected to a single CPU DAI, use CPU DAI's directly and let
@@ -410,10 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 
 	hw->channels_min = max(chan_min, cpu_stream->channels_min);
 	hw->channels_max = min(chan_max, cpu_stream->channels_max);
-	if (hw->formats)
-		hw->formats &= formats & cpu_stream->formats;
-	else
-		hw->formats = formats & cpu_stream->formats;
+	hw->formats = formats & cpu_stream->formats;
 	hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
 
 	snd_pcm_limit_hw_rates(hw);
@@ -422,6 +425,26 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 	hw->rate_min = max(hw->rate_min, rate_min);
 	hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max);
 	hw->rate_max = min_not_zero(hw->rate_max, rate_max);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_runtime_calc_hw);
+
+static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_hardware *hw = &substream->runtime->hw;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	u64 formats = hw->formats;
+
+	/*
+	 * At least one CODEC should match, otherwise we should have
+	 * bailed out on a higher level, since there would be no
+	 * CODEC to support the transfer direction in that case.
+	 */
+	snd_soc_runtime_calc_hw(rtd, hw, substream->stream);
+
+	if (formats)
+		hw->formats &= formats;
 }
 
 static int soc_pcm_components_open(struct snd_pcm_substream *substream,
-- 
2.24.1

[alsa-devel] [PATCH 3/4] ASoC: pcm: Export parameter intersection logic

[Samuel Holland]

The logic to calculate the subset of stream parameters supported by all
DAIs associated with a PCM stream is nontrivial. Export a helper
function so it can be used to set up simple codec2codec DAI links.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 include/sound/soc.h |  3 +++
 sound/soc/soc-pcm.c | 53 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 262896799826..578a8e3e08ca 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -473,6 +473,9 @@ bool snd_soc_runtime_ignore_pmdown_time(struct snd_soc_pcm_runtime *rtd);
 void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream);
 void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream);
 
+int snd_soc_runtime_calc_hw(struct snd_soc_pcm_runtime *rtd,
+			    struct snd_pcm_hardware *hw, int stream);
+
 int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
 	unsigned int dai_fmt);
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index bd7b3cfcfa62..562af1d3f24e 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -348,11 +348,18 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
 	soc_pcm_set_msb(substream, cpu_bits);
 }
 
-static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
+/**
+ * snd_soc_runtime_calc_hw() - Calculate hw limits for a PCM stream
+ * @rtd: ASoC PCM runtime
+ * @hw: PCM hardware parameters (output)
+ * @stream: Direction of the PCM stream
+ *
+ * Calculates the subset of stream parameters supported by all DAIs
+ * associated with the PCM stream.
+ */
+int snd_soc_runtime_calc_hw(struct snd_soc_pcm_runtime *rtd,
+			    struct snd_pcm_hardware *hw, int stream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_pcm_hardware *hw = &runtime->hw;
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai;
 	struct snd_soc_dai_driver *cpu_dai_drv = rtd->cpu_dai->driver;
 	struct snd_soc_dai_driver *codec_dai_drv;
@@ -364,7 +371,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 	u64 formats = ULLONG_MAX;
 	int i;
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
 		cpu_stream = &cpu_dai_drv->playback;
 	else
 		cpu_stream = &cpu_dai_drv->capture;
@@ -377,16 +384,12 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 		 * Otherwise, since the rate, channel, and format values will
 		 * zero in that case, we would have no usable settings left,
 		 * causing the resulting setup to fail.
-		 * At least one CODEC should match, otherwise we should have
-		 * bailed out on a higher level, since there would be no
-		 * CODEC to support the transfer direction in that case.
 		 */
-		if (!snd_soc_dai_stream_valid(codec_dai,
-					      substream->stream))
+		if (!snd_soc_dai_stream_valid(codec_dai, stream))
 			continue;
 
 		codec_dai_drv = codec_dai->driver;
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		if (stream == SNDRV_PCM_STREAM_PLAYBACK)
 			codec_stream = &codec_dai_drv->playback;
 		else
 			codec_stream = &codec_dai_drv->capture;
@@ -398,6 +401,9 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 		rates = snd_pcm_rate_mask_intersect(codec_stream->rates, rates);
 	}
 
+	if (!chan_min)
+		return -EINVAL;
+
 	/*
 	 * chan min/max cannot be enforced if there are multiple CODEC DAIs
 	 * connected to a single CPU DAI, use CPU DAI's directly and let
@@ -410,10 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 
 	hw->channels_min = max(chan_min, cpu_stream->channels_min);
 	hw->channels_max = min(chan_max, cpu_stream->channels_max);
-	if (hw->formats)
-		hw->formats &= formats & cpu_stream->formats;
-	else
-		hw->formats = formats & cpu_stream->formats;
+	hw->formats = formats & cpu_stream->formats;
 	hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
 
 	snd_pcm_limit_hw_rates(hw);
@@ -422,6 +425,26 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 	hw->rate_min = max(hw->rate_min, rate_min);
 	hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max);
 	hw->rate_max = min_not_zero(hw->rate_max, rate_max);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_runtime_calc_hw);
+
+static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_hardware *hw = &substream->runtime->hw;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	u64 formats = hw->formats;
+
+	/*
+	 * At least one CODEC should match, otherwise we should have
+	 * bailed out on a higher level, since there would be no
+	 * CODEC to support the transfer direction in that case.
+	 */
+	snd_soc_runtime_calc_hw(rtd, hw, substream->stream);
+
+	if (formats)
+		hw->formats &= formats;
 }
 
 static int soc_pcm_components_open(struct snd_pcm_substream *substream,
-- 
2.24.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 4/4] ASoC: simple-card: Add support for codec-to-codec dai_links

[Samuel Holland]

For now we assume there are only a few sets of valid PCM stream
parameters, to avoid needing to specify them in the device tree. We
also assume they are the same in both directions. We calculate the
common subset of parameters, and then the existing code automatically
chooses the highest quality of the remaining values.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../devicetree/bindings/sound/simple-card.txt |  1 +
 Documentation/sound/soc/codec-to-codec.rst    |  9 ++++-
 include/sound/simple_card_utils.h             |  1 +
 sound/soc/generic/simple-card-utils.c         | 39 +++++++++++++++++++
 sound/soc/generic/simple-card.c               | 12 ++++++
 5 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 79954cd6e37b..18a62e404a30 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -63,6 +63,7 @@ Optional dai-link subnode properties:
 - mclk-fs             			: Multiplication factor between stream
 					  rate and codec mclk, applied only for
 					  the dai-link.
+- codec-to-codec			: Indicates a codec-to-codec dai-link.
 
 For backward compatibility the frame-master and bitclock-master
 properties can be used as booleans in codec subnode to indicate if the
diff --git a/Documentation/sound/soc/codec-to-codec.rst b/Documentation/sound/soc/codec-to-codec.rst
index 810109d7500d..efe0a8c07933 100644
--- a/Documentation/sound/soc/codec-to-codec.rst
+++ b/Documentation/sound/soc/codec-to-codec.rst
@@ -104,5 +104,10 @@ Make sure to name your corresponding cpu and codec playback and capture
 dai names ending with "Playback" and "Capture" respectively as dapm core
 will link and power those dais based on the name.
 
-Note that in current device tree there is no way to mark a dai_link
-as codec to codec. However, it may change in future.
+A dai_link in a "simple-audio-card" can be marked as codec to codec in
+the device tree by adding the "codec-to-codec" property. The dai_link
+will be initialized with the subset of stream parameters (channels,
+format, sample rate) supported by all DAIs on the link. Since there is
+no way to provide these parameters in the device tree, this is mostly
+useful for communication with simple fixed-function codecs, such as a
+Bluetooth controller or cellular modem.
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index bbdd1542d6f1..80b60237b3cd 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -49,6 +49,7 @@ struct asoc_simple_priv {
 		struct asoc_simple_data adata;
 		struct snd_soc_codec_conf *codec_conf;
 		unsigned int mclk_fs;
+		bool codec_to_codec;
 	} *dai_props;
 	struct asoc_simple_jack hp_jack;
 	struct asoc_simple_jack mic_jack;
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 9b794775df53..2de4cfead790 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -331,6 +331,41 @@ static int asoc_simple_init_dai(struct snd_soc_dai *dai,
 	return 0;
 }
 
+static int asoc_simple_init_dai_link_params(struct snd_soc_pcm_runtime *rtd,
+					    struct simple_dai_props *dai_props)
+{
+	struct snd_soc_dai_link *dai_link = rtd->dai_link;
+	struct snd_soc_pcm_stream *params;
+	struct snd_pcm_hardware hw;
+	int ret;
+
+	if (!dai_props->codec_to_codec)
+		return 0;
+
+	/* Assumes the hardware capabilities are the same in both directions */
+	ret = snd_soc_runtime_calc_hw(rtd, &hw, SNDRV_PCM_STREAM_PLAYBACK);
+	if (ret < 0) {
+		dev_err(rtd->dev, "simple-card: no valid dai_link params\n");
+		return ret;
+	}
+
+	params = devm_kzalloc(rtd->dev, sizeof(*params), GFP_KERNEL);
+	if (!params)
+		return -ENOMEM;
+
+	params->formats = hw.formats;
+	params->rates = hw.rates;
+	params->rate_min = hw.rate_min;
+	params->rate_max = hw.rate_max;
+	params->channels_min = hw.channels_min;
+	params->channels_max = hw.channels_max;
+
+	dai_link->params = params;
+	dai_link->num_params = 1;
+
+	return 0;
+}
+
 int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card);
@@ -347,6 +382,10 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
 	if (ret < 0)
 		return ret;
 
+	ret = asoc_simple_init_dai_link_params(rtd, dai_props);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(asoc_simple_dai_init);
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 55e9f8800b3e..179ab4482d10 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -77,6 +77,16 @@ static int asoc_simple_parse_dai(struct device_node *node,
 	return 0;
 }
 
+static void simple_parse_codec_to_codec(struct device_node *node,
+					struct simple_dai_props *props,
+					char *prefix)
+{
+	char prop[128];
+
+	snprintf(prop, sizeof(prop), "%scodec-to-codec", prefix);
+	props->codec_to_codec = of_property_read_bool(node, prop);
+}
+
 static void simple_parse_convert(struct device *dev,
 				 struct device_node *np,
 				 struct asoc_simple_data *adata)
@@ -217,6 +227,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 					     "prefix");
 	}
 
+	simple_parse_codec_to_codec(node, dai_props, prefix);
 	simple_parse_convert(dev, np, &dai_props->adata);
 	simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
 
@@ -292,6 +303,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 	if (ret < 0)
 		goto dai_link_of_err;
 
+	simple_parse_codec_to_codec(node, dai_props, prefix);
 	simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
 
 	ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu);
-- 
2.24.1

[alsa-devel] [PATCH 4/4] ASoC: simple-card: Add support for codec-to-codec dai_links

[Samuel Holland]

For now we assume there are only a few sets of valid PCM stream
parameters, to avoid needing to specify them in the device tree. We
also assume they are the same in both directions. We calculate the
common subset of parameters, and then the existing code automatically
chooses the highest quality of the remaining values.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../devicetree/bindings/sound/simple-card.txt |  1 +
 Documentation/sound/soc/codec-to-codec.rst    |  9 ++++-
 include/sound/simple_card_utils.h             |  1 +
 sound/soc/generic/simple-card-utils.c         | 39 +++++++++++++++++++
 sound/soc/generic/simple-card.c               | 12 ++++++
 5 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 79954cd6e37b..18a62e404a30 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -63,6 +63,7 @@ Optional dai-link subnode properties:
 - mclk-fs             			: Multiplication factor between stream
 					  rate and codec mclk, applied only for
 					  the dai-link.
+- codec-to-codec			: Indicates a codec-to-codec dai-link.
 
 For backward compatibility the frame-master and bitclock-master
 properties can be used as booleans in codec subnode to indicate if the
diff --git a/Documentation/sound/soc/codec-to-codec.rst b/Documentation/sound/soc/codec-to-codec.rst
index 810109d7500d..efe0a8c07933 100644
--- a/Documentation/sound/soc/codec-to-codec.rst
+++ b/Documentation/sound/soc/codec-to-codec.rst
@@ -104,5 +104,10 @@ Make sure to name your corresponding cpu and codec playback and capture
 dai names ending with "Playback" and "Capture" respectively as dapm core
 will link and power those dais based on the name.
 
-Note that in current device tree there is no way to mark a dai_link
-as codec to codec. However, it may change in future.
+A dai_link in a "simple-audio-card" can be marked as codec to codec in
+the device tree by adding the "codec-to-codec" property. The dai_link
+will be initialized with the subset of stream parameters (channels,
+format, sample rate) supported by all DAIs on the link. Since there is
+no way to provide these parameters in the device tree, this is mostly
+useful for communication with simple fixed-function codecs, such as a
+Bluetooth controller or cellular modem.
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index bbdd1542d6f1..80b60237b3cd 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -49,6 +49,7 @@ struct asoc_simple_priv {
 		struct asoc_simple_data adata;
 		struct snd_soc_codec_conf *codec_conf;
 		unsigned int mclk_fs;
+		bool codec_to_codec;
 	} *dai_props;
 	struct asoc_simple_jack hp_jack;
 	struct asoc_simple_jack mic_jack;
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 9b794775df53..2de4cfead790 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -331,6 +331,41 @@ static int asoc_simple_init_dai(struct snd_soc_dai *dai,
 	return 0;
 }
 
+static int asoc_simple_init_dai_link_params(struct snd_soc_pcm_runtime *rtd,
+					    struct simple_dai_props *dai_props)
+{
+	struct snd_soc_dai_link *dai_link = rtd->dai_link;
+	struct snd_soc_pcm_stream *params;
+	struct snd_pcm_hardware hw;
+	int ret;
+
+	if (!dai_props->codec_to_codec)
+		return 0;
+
+	/* Assumes the hardware capabilities are the same in both directions */
+	ret = snd_soc_runtime_calc_hw(rtd, &hw, SNDRV_PCM_STREAM_PLAYBACK);
+	if (ret < 0) {
+		dev_err(rtd->dev, "simple-card: no valid dai_link params\n");
+		return ret;
+	}
+
+	params = devm_kzalloc(rtd->dev, sizeof(*params), GFP_KERNEL);
+	if (!params)
+		return -ENOMEM;
+
+	params->formats = hw.formats;
+	params->rates = hw.rates;
+	params->rate_min = hw.rate_min;
+	params->rate_max = hw.rate_max;
+	params->channels_min = hw.channels_min;
+	params->channels_max = hw.channels_max;
+
+	dai_link->params = params;
+	dai_link->num_params = 1;
+
+	return 0;
+}
+
 int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card);
@@ -347,6 +382,10 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
 	if (ret < 0)
 		return ret;
 
+	ret = asoc_simple_init_dai_link_params(rtd, dai_props);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(asoc_simple_dai_init);
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 55e9f8800b3e..179ab4482d10 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -77,6 +77,16 @@ static int asoc_simple_parse_dai(struct device_node *node,
 	return 0;
 }
 
+static void simple_parse_codec_to_codec(struct device_node *node,
+					struct simple_dai_props *props,
+					char *prefix)
+{
+	char prop[128];
+
+	snprintf(prop, sizeof(prop), "%scodec-to-codec", prefix);
+	props->codec_to_codec = of_property_read_bool(node, prop);
+}
+
 static void simple_parse_convert(struct device *dev,
 				 struct device_node *np,
 				 struct asoc_simple_data *adata)
@@ -217,6 +227,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
 					     "prefix");
 	}
 
+	simple_parse_codec_to_codec(node, dai_props, prefix);
 	simple_parse_convert(dev, np, &dai_props->adata);
 	simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
 
@@ -292,6 +303,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
 	if (ret < 0)
 		goto dai_link_of_err;
 
+	simple_parse_codec_to_codec(node, dai_props, prefix);
 	simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
 
 	ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu);
-- 
2.24.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/4] ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly

[Takashi Iwai]

On Thu, 13 Feb 2020 07:11:45 +0100,
Samuel Holland wrote:
> 
> It can be useful to derive min/max rates of a snd_pcm_hardware without
> having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
> 
> Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually
> need the snd_pcm_runtime. Modify it to take a pointer to hw directly.

I prefer adding a new function and change snd_pcm_limit_hw_rates() to
static inline just calling the new one with &runtime->hw, instead of
touching so many callers site.


thanks,

Takashi

> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  .../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c    |  2 +-
>  include/sound/pcm.h                                |  2 +-
>  sound/arm/aaci.c                                   |  2 +-
>  sound/arm/pxa2xx-ac97.c                            |  2 +-
>  sound/core/pcm_misc.c                              | 14 +++++++-------
>  sound/firewire/dice/dice-pcm.c                     |  2 +-
>  sound/firewire/digi00x/digi00x-pcm.c               |  2 +-
>  sound/firewire/fireworks/fireworks_pcm.c           |  2 +-
>  sound/firewire/motu/motu-pcm.c                     |  2 +-
>  sound/firewire/tascam/tascam-pcm.c                 |  2 +-
>  sound/pci/atiixp.c                                 |  2 +-
>  sound/pci/cs5535audio/cs5535audio_pcm.c            |  4 ++--
>  sound/pci/hda/hda_controller.c                     |  4 ++--
>  sound/pci/intel8x0.c                               |  2 +-
>  sound/pci/sis7019.c                                |  2 +-
>  sound/pci/via82xx.c                                |  4 ++--
>  sound/soc/soc-pcm.c                                |  4 ++--
>  sound/usb/caiaq/audio.c                            |  4 ++--
>  18 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index 2b7539701b42..33f7bcf992a4 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -328,7 +328,7 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = snd_pcm_limit_hw_rates(runtime);
> +	ret = snd_pcm_limit_hw_rates(&runtime->hw);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 8a89fa6fdd5e..203b79d2712a 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -1121,7 +1121,7 @@ snd_pcm_kernel_readv(struct snd_pcm_substream *substream,
>  	return __snd_pcm_lib_xfer(substream, bufs, false, frames, true);
>  }
>  
> -int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime);
> +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw);
>  unsigned int snd_pcm_rate_to_rate_bit(unsigned int rate);
>  unsigned int snd_pcm_rate_bit_to_rate(unsigned int rate_bit);
>  unsigned int snd_pcm_rate_mask_intersect(unsigned int rates_a,
> diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
> index b5399b0090a7..5052689247f9 100644
> --- a/sound/arm/aaci.c
> +++ b/sound/arm/aaci.c
> @@ -413,7 +413,7 @@ static int aaci_pcm_open(struct snd_pcm_substream *substream)
>  	runtime->private_data = aacirun;
>  	runtime->hw = aaci_hw_info;
>  	runtime->hw.rates = aacirun->pcm->rates;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		runtime->hw.channels_max = 6;
> diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
> index acfaf1d4ec25..cfb23073471e 100644
> --- a/sound/arm/pxa2xx-ac97.c
> +++ b/sound/arm/pxa2xx-ac97.c
> @@ -77,7 +77,7 @@ static int pxa2xx_ac97_pcm_open(struct snd_pcm_substream *substream)
>  	i = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
>  		AC97_RATES_FRONT_DAC : AC97_RATES_ADC;
>  	runtime->hw.rates = pxa2xx_ac97_ac97->rates[i];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	platform_ops = substream->pcm->card->dev->platform_data;
>  	if (platform_ops && platform_ops->startup) {
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index c4eb561d2008..435688855ed0 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -474,25 +474,25 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
>  
>  /**
>   * snd_pcm_limit_hw_rates - determine rate_min/rate_max fields
> - * @runtime: the runtime instance
> + * @runtime: the pcm hw instance
>   *
>   * Determines the rate_min and rate_max fields from the rates bits of
> - * the given runtime->hw.
> + * the given hw.
>   *
>   * Return: Zero if successful.
>   */
> -int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime)
> +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw)
>  {
>  	int i;
>  	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
> -		if (runtime->hw.rates & (1 << i)) {
> -			runtime->hw.rate_min = snd_pcm_known_rates.list[i];
> +		if (hw->rates & (1 << i)) {
> +			hw->rate_min = snd_pcm_known_rates.list[i];
>  			break;
>  		}
>  	}
>  	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
> -		if (runtime->hw.rates & (1 << i)) {
> -			runtime->hw.rate_max = snd_pcm_known_rates.list[i];
> +		if (hw->rates & (1 << i)) {
> +			hw->rate_max = snd_pcm_known_rates.list[i];
>  			break;
>  		}
>  	}
> diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
> index be79d659eedf..85941d945067 100644
> --- a/sound/firewire/dice/dice-pcm.c
> +++ b/sound/firewire/dice/dice-pcm.c
> @@ -117,7 +117,7 @@ static int limit_channels_and_rates(struct snd_dice *dice,
>  		hw->channels_max = max(hw->channels_max, channels);
>  	}
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	return 0;
>  }
> diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
> index 57cbce4fd836..0709070ab5af 100644
> --- a/sound/firewire/digi00x/digi00x-pcm.c
> +++ b/sound/firewire/digi00x/digi00x-pcm.c
> @@ -78,7 +78,7 @@ static int pcm_init_hw_params(struct snd_dg00x *dg00x,
>  		    SNDRV_PCM_RATE_48000 |
>  		    SNDRV_PCM_RATE_88200 |
>  		    SNDRV_PCM_RATE_96000;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	err = snd_pcm_hw_rule_add(substream->runtime, 0,
>  				  SNDRV_PCM_HW_PARAM_CHANNELS,
> diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c
> index e69896d748df..2ee8e98ea2b6 100644
> --- a/sound/firewire/fireworks/fireworks_pcm.c
> +++ b/sound/firewire/fireworks/fireworks_pcm.c
> @@ -149,7 +149,7 @@ pcm_init_hw_params(struct snd_efw *efw,
>  
>  	/* limit rates */
>  	runtime->hw.rates = efw->supported_sampling_rate,
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	limit_channels(&runtime->hw, pcm_channels);
>  
> diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
> index 005970931030..338eb0572890 100644
> --- a/sound/firewire/motu/motu-pcm.c
> +++ b/sound/firewire/motu/motu-pcm.c
> @@ -92,7 +92,7 @@ static void limit_channels_and_rates(struct snd_motu *motu,
>  		hw->channels_max = max(hw->channels_max, pcm_channels);
>  	}
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  }
>  
>  static int init_hw_info(struct snd_motu *motu,
> diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c
> index 8e9b444c8bff..6722c1a65a42 100644
> --- a/sound/firewire/tascam/tascam-pcm.c
> +++ b/sound/firewire/tascam/tascam-pcm.c
> @@ -35,7 +35,7 @@ static int pcm_init_hw_params(struct snd_tscm *tscm,
>  		    SNDRV_PCM_RATE_48000 |
>  		    SNDRV_PCM_RATE_88200 |
>  		    SNDRV_PCM_RATE_96000;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	return amdtp_tscm_add_pcm_hw_constraints(stream, runtime);
>  }
> diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
> index 1e1ededf8eb2..d7af407306b7 100644
> --- a/sound/pci/atiixp.c
> +++ b/sound/pci/atiixp.c
> @@ -1039,7 +1039,7 @@ static int snd_atiixp_pcm_open(struct snd_pcm_substream *substream,
>  	dma->ac97_pcm_type = pcm_type;
>  	if (pcm_type >= 0) {
>  		runtime->hw.rates = chip->pcms[pcm_type]->rates;
> -		snd_pcm_limit_hw_rates(runtime);
> +		snd_pcm_limit_hw_rates(&runtime->hw);
>  	} else {
>  		/* direct SPDIF */
>  		runtime->hw.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE;
> diff --git a/sound/pci/cs5535audio/cs5535audio_pcm.c b/sound/pci/cs5535audio/cs5535audio_pcm.c
> index 4642e5384e83..7ce1664a8148 100644
> --- a/sound/pci/cs5535audio/cs5535audio_pcm.c
> +++ b/sound/pci/cs5535audio/cs5535audio_pcm.c
> @@ -84,7 +84,7 @@ static int snd_cs5535audio_playback_open(struct snd_pcm_substream *substream)
>  
>  	runtime->hw = snd_cs5535audio_playback;
>  	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_FRONT_DAC];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	cs5535au->playback_substream = substream;
>  	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_PLAYBACK]);
>  	if ((err = snd_pcm_hw_constraint_integer(runtime,
> @@ -343,7 +343,7 @@ static int snd_cs5535audio_capture_open(struct snd_pcm_substream *substream)
>  
>  	runtime->hw = snd_cs5535audio_capture;
>  	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_ADC];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	cs5535au->capture_substream = substream;
>  	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_CAPTURE]);
>  	if ((err = snd_pcm_hw_constraint_integer(runtime,
> diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
> index ba56b59b3e17..3728dbfae7b0 100644
> --- a/sound/pci/hda/hda_controller.c
> +++ b/sound/pci/hda/hda_controller.c
> @@ -605,7 +605,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
>  	runtime->hw.channels_max = hinfo->channels_max;
>  	runtime->hw.formats = hinfo->formats;
>  	runtime->hw.rates = hinfo->rates;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
>  
>  	/* avoid wrap-around with wall-clock */
> @@ -648,7 +648,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
>  		azx_release_device(azx_dev);
>  		goto powerdown;
>  	}
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	/* sanity check */
>  	if (snd_BUG_ON(!runtime->hw.channels_min) ||
>  	    snd_BUG_ON(!runtime->hw.channels_max) ||
> diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
> index 12374ba08ca2..7f85a30291a1 100644
> --- a/sound/pci/intel8x0.c
> +++ b/sound/pci/intel8x0.c
> @@ -1119,7 +1119,7 @@ static int snd_intel8x0_pcm_open(struct snd_pcm_substream *substream, struct ich
>  	ichdev->substream = substream;
>  	runtime->hw = snd_intel8x0_stream;
>  	runtime->hw.rates = ichdev->pcm->rates;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	if (chip->device_type == DEVICE_SIS) {
>  		runtime->hw.buffer_bytes_max = 64*1024;
>  		runtime->hw.period_bytes_max = 64*1024;
> diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
> index ef7dd290ae05..4d8e852d438f 100644
> --- a/sound/pci/sis7019.c
> +++ b/sound/pci/sis7019.c
> @@ -681,7 +681,7 @@ static int sis_capture_open(struct snd_pcm_substream *substream)
>  	runtime->private_data = voice;
>  	runtime->hw = sis_capture_hw_info;
>  	runtime->hw.rates = sis->ac97[0]->rates[AC97_RATES_ADC];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
>  						9, 0xfff9);
>  	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
> diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c
> index 30c817b6b635..8a55153a6ea3 100644
> --- a/sound/pci/via82xx.c
> +++ b/sound/pci/via82xx.c
> @@ -1180,7 +1180,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
>  	if (chip->spdif_on && viadev->reg_offset == 0x30) {
>  		/* DXS#3 and spdif is on */
>  		runtime->hw.rates = chip->ac97->rates[AC97_RATES_SPDIF];
> -		snd_pcm_limit_hw_rates(runtime);
> +		snd_pcm_limit_hw_rates(&runtime->hw);
>  	} else if (chip->dxs_fixed && viadev->reg_offset < 0x40) {
>  		/* fixed DXS playback rate */
>  		runtime->hw.rates = SNDRV_PCM_RATE_48000;
> @@ -1195,7 +1195,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
>  	} else if (! ratep->rate) {
>  		int idx = viadev->direction ? AC97_RATES_ADC : AC97_RATES_FRONT_DAC;
>  		runtime->hw.rates = chip->ac97->rates[idx];
> -		snd_pcm_limit_hw_rates(runtime);
> +		snd_pcm_limit_hw_rates(&runtime->hw);
>  	} else {
>  		/* a fixed rate */
>  		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 01e7bc03d92f..bd7b3cfcfa62 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -416,7 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
>  		hw->formats = formats & cpu_stream->formats;
>  	hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
>  	hw->rate_min = max(hw->rate_min, rate_min);
> @@ -1951,7 +1951,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
>  	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
>  
>  	dpcm_set_fe_runtime(fe_substream);
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	ret = dpcm_apply_symmetry(fe_substream, stream);
>  	if (ret < 0) {
> diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
> index 970eb0865ba3..242f50ebc63a 100644
> --- a/sound/usb/caiaq/audio.c
> +++ b/sound/usb/caiaq/audio.c
> @@ -145,7 +145,7 @@ static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
>  
>  	dev_dbg(dev, "%s(%p)\n", __func__, substream);
>  	substream->runtime->hw = cdev->pcm_info;
> -	snd_pcm_limit_hw_rates(substream->runtime);
> +	snd_pcm_limit_hw_rates(&substream->runtime->hw);
>  
>  	return 0;
>  }
> @@ -243,7 +243,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream)
>  		if (runtime->rate == rates[i])
>  			cdev->pcm_info.rates = 1 << i;
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	bytes_per_sample = BYTES_PER_SAMPLE;
>  	if (cdev->spec.data_alignment >= 2)
> -- 
> 2.24.1
> 

Re: [alsa-devel] [PATCH 2/4] ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly

[Takashi Iwai]

On Thu, 13 Feb 2020 07:11:45 +0100,
Samuel Holland wrote:
> 
> It can be useful to derive min/max rates of a snd_pcm_hardware without
> having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
> 
> Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually
> need the snd_pcm_runtime. Modify it to take a pointer to hw directly.

I prefer adding a new function and change snd_pcm_limit_hw_rates() to
static inline just calling the new one with &runtime->hw, instead of
touching so many callers site.


thanks,

Takashi

> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  .../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c    |  2 +-
>  include/sound/pcm.h                                |  2 +-
>  sound/arm/aaci.c                                   |  2 +-
>  sound/arm/pxa2xx-ac97.c                            |  2 +-
>  sound/core/pcm_misc.c                              | 14 +++++++-------
>  sound/firewire/dice/dice-pcm.c                     |  2 +-
>  sound/firewire/digi00x/digi00x-pcm.c               |  2 +-
>  sound/firewire/fireworks/fireworks_pcm.c           |  2 +-
>  sound/firewire/motu/motu-pcm.c                     |  2 +-
>  sound/firewire/tascam/tascam-pcm.c                 |  2 +-
>  sound/pci/atiixp.c                                 |  2 +-
>  sound/pci/cs5535audio/cs5535audio_pcm.c            |  4 ++--
>  sound/pci/hda/hda_controller.c                     |  4 ++--
>  sound/pci/intel8x0.c                               |  2 +-
>  sound/pci/sis7019.c                                |  2 +-
>  sound/pci/via82xx.c                                |  4 ++--
>  sound/soc/soc-pcm.c                                |  4 ++--
>  sound/usb/caiaq/audio.c                            |  4 ++--
>  18 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> index 2b7539701b42..33f7bcf992a4 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
> @@ -328,7 +328,7 @@ static int dw_hdmi_open(struct snd_pcm_substream *substream)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = snd_pcm_limit_hw_rates(runtime);
> +	ret = snd_pcm_limit_hw_rates(&runtime->hw);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 8a89fa6fdd5e..203b79d2712a 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -1121,7 +1121,7 @@ snd_pcm_kernel_readv(struct snd_pcm_substream *substream,
>  	return __snd_pcm_lib_xfer(substream, bufs, false, frames, true);
>  }
>  
> -int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime);
> +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw);
>  unsigned int snd_pcm_rate_to_rate_bit(unsigned int rate);
>  unsigned int snd_pcm_rate_bit_to_rate(unsigned int rate_bit);
>  unsigned int snd_pcm_rate_mask_intersect(unsigned int rates_a,
> diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
> index b5399b0090a7..5052689247f9 100644
> --- a/sound/arm/aaci.c
> +++ b/sound/arm/aaci.c
> @@ -413,7 +413,7 @@ static int aaci_pcm_open(struct snd_pcm_substream *substream)
>  	runtime->private_data = aacirun;
>  	runtime->hw = aaci_hw_info;
>  	runtime->hw.rates = aacirun->pcm->rates;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		runtime->hw.channels_max = 6;
> diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
> index acfaf1d4ec25..cfb23073471e 100644
> --- a/sound/arm/pxa2xx-ac97.c
> +++ b/sound/arm/pxa2xx-ac97.c
> @@ -77,7 +77,7 @@ static int pxa2xx_ac97_pcm_open(struct snd_pcm_substream *substream)
>  	i = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
>  		AC97_RATES_FRONT_DAC : AC97_RATES_ADC;
>  	runtime->hw.rates = pxa2xx_ac97_ac97->rates[i];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	platform_ops = substream->pcm->card->dev->platform_data;
>  	if (platform_ops && platform_ops->startup) {
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index c4eb561d2008..435688855ed0 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -474,25 +474,25 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
>  
>  /**
>   * snd_pcm_limit_hw_rates - determine rate_min/rate_max fields
> - * @runtime: the runtime instance
> + * @runtime: the pcm hw instance
>   *
>   * Determines the rate_min and rate_max fields from the rates bits of
> - * the given runtime->hw.
> + * the given hw.
>   *
>   * Return: Zero if successful.
>   */
> -int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime)
> +int snd_pcm_limit_hw_rates(struct snd_pcm_hardware *hw)
>  {
>  	int i;
>  	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
> -		if (runtime->hw.rates & (1 << i)) {
> -			runtime->hw.rate_min = snd_pcm_known_rates.list[i];
> +		if (hw->rates & (1 << i)) {
> +			hw->rate_min = snd_pcm_known_rates.list[i];
>  			break;
>  		}
>  	}
>  	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
> -		if (runtime->hw.rates & (1 << i)) {
> -			runtime->hw.rate_max = snd_pcm_known_rates.list[i];
> +		if (hw->rates & (1 << i)) {
> +			hw->rate_max = snd_pcm_known_rates.list[i];
>  			break;
>  		}
>  	}
> diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
> index be79d659eedf..85941d945067 100644
> --- a/sound/firewire/dice/dice-pcm.c
> +++ b/sound/firewire/dice/dice-pcm.c
> @@ -117,7 +117,7 @@ static int limit_channels_and_rates(struct snd_dice *dice,
>  		hw->channels_max = max(hw->channels_max, channels);
>  	}
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	return 0;
>  }
> diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
> index 57cbce4fd836..0709070ab5af 100644
> --- a/sound/firewire/digi00x/digi00x-pcm.c
> +++ b/sound/firewire/digi00x/digi00x-pcm.c
> @@ -78,7 +78,7 @@ static int pcm_init_hw_params(struct snd_dg00x *dg00x,
>  		    SNDRV_PCM_RATE_48000 |
>  		    SNDRV_PCM_RATE_88200 |
>  		    SNDRV_PCM_RATE_96000;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	err = snd_pcm_hw_rule_add(substream->runtime, 0,
>  				  SNDRV_PCM_HW_PARAM_CHANNELS,
> diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c
> index e69896d748df..2ee8e98ea2b6 100644
> --- a/sound/firewire/fireworks/fireworks_pcm.c
> +++ b/sound/firewire/fireworks/fireworks_pcm.c
> @@ -149,7 +149,7 @@ pcm_init_hw_params(struct snd_efw *efw,
>  
>  	/* limit rates */
>  	runtime->hw.rates = efw->supported_sampling_rate,
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	limit_channels(&runtime->hw, pcm_channels);
>  
> diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
> index 005970931030..338eb0572890 100644
> --- a/sound/firewire/motu/motu-pcm.c
> +++ b/sound/firewire/motu/motu-pcm.c
> @@ -92,7 +92,7 @@ static void limit_channels_and_rates(struct snd_motu *motu,
>  		hw->channels_max = max(hw->channels_max, pcm_channels);
>  	}
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  }
>  
>  static int init_hw_info(struct snd_motu *motu,
> diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c
> index 8e9b444c8bff..6722c1a65a42 100644
> --- a/sound/firewire/tascam/tascam-pcm.c
> +++ b/sound/firewire/tascam/tascam-pcm.c
> @@ -35,7 +35,7 @@ static int pcm_init_hw_params(struct snd_tscm *tscm,
>  		    SNDRV_PCM_RATE_48000 |
>  		    SNDRV_PCM_RATE_88200 |
>  		    SNDRV_PCM_RATE_96000;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	return amdtp_tscm_add_pcm_hw_constraints(stream, runtime);
>  }
> diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
> index 1e1ededf8eb2..d7af407306b7 100644
> --- a/sound/pci/atiixp.c
> +++ b/sound/pci/atiixp.c
> @@ -1039,7 +1039,7 @@ static int snd_atiixp_pcm_open(struct snd_pcm_substream *substream,
>  	dma->ac97_pcm_type = pcm_type;
>  	if (pcm_type >= 0) {
>  		runtime->hw.rates = chip->pcms[pcm_type]->rates;
> -		snd_pcm_limit_hw_rates(runtime);
> +		snd_pcm_limit_hw_rates(&runtime->hw);
>  	} else {
>  		/* direct SPDIF */
>  		runtime->hw.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE;
> diff --git a/sound/pci/cs5535audio/cs5535audio_pcm.c b/sound/pci/cs5535audio/cs5535audio_pcm.c
> index 4642e5384e83..7ce1664a8148 100644
> --- a/sound/pci/cs5535audio/cs5535audio_pcm.c
> +++ b/sound/pci/cs5535audio/cs5535audio_pcm.c
> @@ -84,7 +84,7 @@ static int snd_cs5535audio_playback_open(struct snd_pcm_substream *substream)
>  
>  	runtime->hw = snd_cs5535audio_playback;
>  	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_FRONT_DAC];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	cs5535au->playback_substream = substream;
>  	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_PLAYBACK]);
>  	if ((err = snd_pcm_hw_constraint_integer(runtime,
> @@ -343,7 +343,7 @@ static int snd_cs5535audio_capture_open(struct snd_pcm_substream *substream)
>  
>  	runtime->hw = snd_cs5535audio_capture;
>  	runtime->hw.rates = cs5535au->ac97->rates[AC97_RATES_ADC];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	cs5535au->capture_substream = substream;
>  	runtime->private_data = &(cs5535au->dmas[CS5535AUDIO_DMA_CAPTURE]);
>  	if ((err = snd_pcm_hw_constraint_integer(runtime,
> diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
> index ba56b59b3e17..3728dbfae7b0 100644
> --- a/sound/pci/hda/hda_controller.c
> +++ b/sound/pci/hda/hda_controller.c
> @@ -605,7 +605,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
>  	runtime->hw.channels_max = hinfo->channels_max;
>  	runtime->hw.formats = hinfo->formats;
>  	runtime->hw.rates = hinfo->rates;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
>  
>  	/* avoid wrap-around with wall-clock */
> @@ -648,7 +648,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream)
>  		azx_release_device(azx_dev);
>  		goto powerdown;
>  	}
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	/* sanity check */
>  	if (snd_BUG_ON(!runtime->hw.channels_min) ||
>  	    snd_BUG_ON(!runtime->hw.channels_max) ||
> diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
> index 12374ba08ca2..7f85a30291a1 100644
> --- a/sound/pci/intel8x0.c
> +++ b/sound/pci/intel8x0.c
> @@ -1119,7 +1119,7 @@ static int snd_intel8x0_pcm_open(struct snd_pcm_substream *substream, struct ich
>  	ichdev->substream = substream;
>  	runtime->hw = snd_intel8x0_stream;
>  	runtime->hw.rates = ichdev->pcm->rates;
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	if (chip->device_type == DEVICE_SIS) {
>  		runtime->hw.buffer_bytes_max = 64*1024;
>  		runtime->hw.period_bytes_max = 64*1024;
> diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
> index ef7dd290ae05..4d8e852d438f 100644
> --- a/sound/pci/sis7019.c
> +++ b/sound/pci/sis7019.c
> @@ -681,7 +681,7 @@ static int sis_capture_open(struct snd_pcm_substream *substream)
>  	runtime->private_data = voice;
>  	runtime->hw = sis_capture_hw_info;
>  	runtime->hw.rates = sis->ac97[0]->rates[AC97_RATES_ADC];
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
>  						9, 0xfff9);
>  	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
> diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c
> index 30c817b6b635..8a55153a6ea3 100644
> --- a/sound/pci/via82xx.c
> +++ b/sound/pci/via82xx.c
> @@ -1180,7 +1180,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
>  	if (chip->spdif_on && viadev->reg_offset == 0x30) {
>  		/* DXS#3 and spdif is on */
>  		runtime->hw.rates = chip->ac97->rates[AC97_RATES_SPDIF];
> -		snd_pcm_limit_hw_rates(runtime);
> +		snd_pcm_limit_hw_rates(&runtime->hw);
>  	} else if (chip->dxs_fixed && viadev->reg_offset < 0x40) {
>  		/* fixed DXS playback rate */
>  		runtime->hw.rates = SNDRV_PCM_RATE_48000;
> @@ -1195,7 +1195,7 @@ static int snd_via82xx_pcm_open(struct via82xx *chip, struct viadev *viadev,
>  	} else if (! ratep->rate) {
>  		int idx = viadev->direction ? AC97_RATES_ADC : AC97_RATES_FRONT_DAC;
>  		runtime->hw.rates = chip->ac97->rates[idx];
> -		snd_pcm_limit_hw_rates(runtime);
> +		snd_pcm_limit_hw_rates(&runtime->hw);
>  	} else {
>  		/* a fixed rate */
>  		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 01e7bc03d92f..bd7b3cfcfa62 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -416,7 +416,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
>  		hw->formats = formats & cpu_stream->formats;
>  	hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(hw);
>  
>  	hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
>  	hw->rate_min = max(hw->rate_min, rate_min);
> @@ -1951,7 +1951,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
>  	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
>  
>  	dpcm_set_fe_runtime(fe_substream);
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	ret = dpcm_apply_symmetry(fe_substream, stream);
>  	if (ret < 0) {
> diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
> index 970eb0865ba3..242f50ebc63a 100644
> --- a/sound/usb/caiaq/audio.c
> +++ b/sound/usb/caiaq/audio.c
> @@ -145,7 +145,7 @@ static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
>  
>  	dev_dbg(dev, "%s(%p)\n", __func__, substream);
>  	substream->runtime->hw = cdev->pcm_info;
> -	snd_pcm_limit_hw_rates(substream->runtime);
> +	snd_pcm_limit_hw_rates(&substream->runtime->hw);
>  
>  	return 0;
>  }
> @@ -243,7 +243,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream)
>  		if (runtime->rate == rates[i])
>  			cdev->pcm_info.rates = 1 << i;
>  
> -	snd_pcm_limit_hw_rates(runtime);
> +	snd_pcm_limit_hw_rates(&runtime->hw);
>  
>  	bytes_per_sample = BYTES_PER_SAMPLE;
>  	if (cdev->spec.data_alignment >= 2)
> -- 
> 2.24.1
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Jerome Brunet]

On Thu 13 Feb 2020 at 07:11, Samuel Holland <samuel@sholland.org> wrote:

> The PCM runtime was freed during PMU in the case that the event hook
> encountered an error. However, it is also unconditionally freed during
> PMD. Avoid a double-free by dropping the call to kfree in the PMU
> hook.

Oh ... Thanks for finding this.

I thought that a widget which has failed PMU would not go through PMD,
but It seems the return value dapm_seq_check_event is not checked.

This brings another question/problem:
A link which has failed in PMU, could try in PMD to hw_free/shutdown a
dai which has not gone through startup/hw_params, right ?

>
> Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
> Cc: stable@vger.kernel.org
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  sound/soc/soc-dapm.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index b6378f025836..935b5375ecc5 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -3888,9 +3888,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
>  	runtime->rate = params_rate(params);
>  
>  out:
> -	if (ret < 0)
> -		kfree(runtime);
> -
>  	kfree(params);
>  	return ret;
>  }

Re: [alsa-devel] [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Jerome Brunet]

On Thu 13 Feb 2020 at 07:11, Samuel Holland <samuel@sholland.org> wrote:

> The PCM runtime was freed during PMU in the case that the event hook
> encountered an error. However, it is also unconditionally freed during
> PMD. Avoid a double-free by dropping the call to kfree in the PMU
> hook.

Oh ... Thanks for finding this.

I thought that a widget which has failed PMU would not go through PMD,
but It seems the return value dapm_seq_check_event is not checked.

This brings another question/problem:
A link which has failed in PMU, could try in PMD to hw_free/shutdown a
dai which has not gone through startup/hw_params, right ?

>
> Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
> Cc: stable@vger.kernel.org
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  sound/soc/soc-dapm.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index b6378f025836..935b5375ecc5 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -3888,9 +3888,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
>  	runtime->rate = params_rate(params);
>  
>  out:
> -	if (ret < 0)
> -		kfree(runtime);
> -
>  	kfree(params);
>  	return ret;
>  }

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 4/4] ASoC: simple-card: Add support for codec-to-codec dai_links

[Jerome Brunet]

On Thu 13 Feb 2020 at 07:11, Samuel Holland <samuel@sholland.org> wrote:

> For now we assume there are only a few sets of valid PCM stream
> parameters, to avoid needing to specify them in the device tree. We
> also assume they are the same in both directions. We calculate the
> common subset of parameters, and then the existing code automatically
> chooses the highest quality of the remaining values.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  .../devicetree/bindings/sound/simple-card.txt |  1 +
>  Documentation/sound/soc/codec-to-codec.rst    |  9 ++++-
>  include/sound/simple_card_utils.h             |  1 +
>  sound/soc/generic/simple-card-utils.c         | 39 +++++++++++++++++++
>  sound/soc/generic/simple-card.c               | 12 ++++++
>  5 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index 79954cd6e37b..18a62e404a30 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
> @@ -63,6 +63,7 @@ Optional dai-link subnode properties:
>  - mclk-fs             			: Multiplication factor between stream
>  					  rate and codec mclk, applied only for
>  					  the dai-link.
> +- codec-to-codec			: Indicates a codec-to-codec
> dai-link.

I wonder if such property could be viewed as a Linux implementation
detail ?

Similar discussion around DPCM:

* https://lore.kernel.org/linux-devicetree/20191014115635.GB4826@sirena.co.uk/#t
* https://alsa-devel.alsa-project.narkive.com/NCs4MsqL/simle-card-dt-style-for-dpcm#post4

Should the card figure out the codec-to-codec links on its own or is it
something generic enough to put in DT ?

In the Amlogic card driver, I'm using the compatible string of the
device to figure out if CPU is a CODEC.
It is ad-hoc and, to be honest, I'm not entirely happy with this
solution but I could not figure out a better one yet.

>  
>  For backward compatibility the frame-master and bitclock-master
>  properties can be used as booleans in codec subnode to indicate if the
> diff --git a/Documentation/sound/soc/codec-to-codec.rst b/Documentation/sound/soc/codec-to-codec.rst
> index 810109d7500d..efe0a8c07933 100644
> --- a/Documentation/sound/soc/codec-to-codec.rst
> +++ b/Documentation/sound/soc/codec-to-codec.rst
> @@ -104,5 +104,10 @@ Make sure to name your corresponding cpu and codec playback and capture
>  dai names ending with "Playback" and "Capture" respectively as dapm core
>  will link and power those dais based on the name.
>  
> -Note that in current device tree there is no way to mark a dai_link
> -as codec to codec. However, it may change in future.
> +A dai_link in a "simple-audio-card" can be marked as codec to codec in
> +the device tree by adding the "codec-to-codec" property. The dai_link
> +will be initialized with the subset of stream parameters (channels,
> +format, sample rate) supported by all DAIs on the link. Since there is
> +no way to provide these parameters in the device tree, this is mostly
> +useful for communication with simple fixed-function codecs, such as a
> +Bluetooth controller or cellular modem.
> diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
> index bbdd1542d6f1..80b60237b3cd 100644
> --- a/include/sound/simple_card_utils.h
> +++ b/include/sound/simple_card_utils.h
> @@ -49,6 +49,7 @@ struct asoc_simple_priv {
>  		struct asoc_simple_data adata;
>  		struct snd_soc_codec_conf *codec_conf;
>  		unsigned int mclk_fs;
> +		bool codec_to_codec;
>  	} *dai_props;
>  	struct asoc_simple_jack hp_jack;
>  	struct asoc_simple_jack mic_jack;
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index 9b794775df53..2de4cfead790 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -331,6 +331,41 @@ static int asoc_simple_init_dai(struct snd_soc_dai *dai,
>  	return 0;
>  }
>  
> +static int asoc_simple_init_dai_link_params(struct snd_soc_pcm_runtime *rtd,
> +					    struct simple_dai_props *dai_props)
> +{
> +	struct snd_soc_dai_link *dai_link = rtd->dai_link;
> +	struct snd_soc_pcm_stream *params;
> +	struct snd_pcm_hardware hw;
> +	int ret;
> +
> +	if (!dai_props->codec_to_codec)
> +		return 0;
> +
> +	/* Assumes the hardware capabilities are the same in both directions */
> +	ret = snd_soc_runtime_calc_hw(rtd, &hw, SNDRV_PCM_STREAM_PLAYBACK);
> +	if (ret < 0) {
> +		dev_err(rtd->dev, "simple-card: no valid dai_link params\n");
> +		return ret;
> +	}
> +
> +	params = devm_kzalloc(rtd->dev, sizeof(*params), GFP_KERNEL);
> +	if (!params)
> +		return -ENOMEM;
> +
> +	params->formats = hw.formats;
> +	params->rates = hw.rates;
> +	params->rate_min = hw.rate_min;
> +	params->rate_max = hw.rate_max;
> +	params->channels_min = hw.channels_min;
> +	params->channels_max = hw.channels_max;
> +
> +	dai_link->params = params;
> +	dai_link->num_params = 1;
> +
> +	return 0;
> +}
> +
>  int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
>  {
>  	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card);
> @@ -347,6 +382,10 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = asoc_simple_init_dai_link_params(rtd, dai_props);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(asoc_simple_dai_init);
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 55e9f8800b3e..179ab4482d10 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -77,6 +77,16 @@ static int asoc_simple_parse_dai(struct device_node *node,
>  	return 0;
>  }
>  
> +static void simple_parse_codec_to_codec(struct device_node *node,
> +					struct simple_dai_props *props,
> +					char *prefix)
> +{
> +	char prop[128];
> +
> +	snprintf(prop, sizeof(prop), "%scodec-to-codec", prefix);
> +	props->codec_to_codec = of_property_read_bool(node, prop);
> +}
> +
>  static void simple_parse_convert(struct device *dev,
>  				 struct device_node *np,
>  				 struct asoc_simple_data *adata)
> @@ -217,6 +227,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
>  					     "prefix");
>  	}
>  
> +	simple_parse_codec_to_codec(node, dai_props, prefix);
>  	simple_parse_convert(dev, np, &dai_props->adata);
>  	simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
>  
> @@ -292,6 +303,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
>  	if (ret < 0)
>  		goto dai_link_of_err;
>  
> +	simple_parse_codec_to_codec(node, dai_props, prefix);
>  	simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
>  
>  	ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu);

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Add support for codec-to-codec dai_links

[Jerome Brunet]

On Thu 13 Feb 2020 at 07:11, Samuel Holland <samuel@sholland.org> wrote:

> For now we assume there are only a few sets of valid PCM stream
> parameters, to avoid needing to specify them in the device tree. We
> also assume they are the same in both directions. We calculate the
> common subset of parameters, and then the existing code automatically
> chooses the highest quality of the remaining values.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  .../devicetree/bindings/sound/simple-card.txt |  1 +
>  Documentation/sound/soc/codec-to-codec.rst    |  9 ++++-
>  include/sound/simple_card_utils.h             |  1 +
>  sound/soc/generic/simple-card-utils.c         | 39 +++++++++++++++++++
>  sound/soc/generic/simple-card.c               | 12 ++++++
>  5 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index 79954cd6e37b..18a62e404a30 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
> @@ -63,6 +63,7 @@ Optional dai-link subnode properties:
>  - mclk-fs             			: Multiplication factor between stream
>  					  rate and codec mclk, applied only for
>  					  the dai-link.
> +- codec-to-codec			: Indicates a codec-to-codec
> dai-link.

I wonder if such property could be viewed as a Linux implementation
detail ?

Similar discussion around DPCM:

* https://lore.kernel.org/linux-devicetree/20191014115635.GB4826@sirena.co.uk/#t
* https://alsa-devel.alsa-project.narkive.com/NCs4MsqL/simle-card-dt-style-for-dpcm#post4

Should the card figure out the codec-to-codec links on its own or is it
something generic enough to put in DT ?

In the Amlogic card driver, I'm using the compatible string of the
device to figure out if CPU is a CODEC.
It is ad-hoc and, to be honest, I'm not entirely happy with this
solution but I could not figure out a better one yet.

>  
>  For backward compatibility the frame-master and bitclock-master
>  properties can be used as booleans in codec subnode to indicate if the
> diff --git a/Documentation/sound/soc/codec-to-codec.rst b/Documentation/sound/soc/codec-to-codec.rst
> index 810109d7500d..efe0a8c07933 100644
> --- a/Documentation/sound/soc/codec-to-codec.rst
> +++ b/Documentation/sound/soc/codec-to-codec.rst
> @@ -104,5 +104,10 @@ Make sure to name your corresponding cpu and codec playback and capture
>  dai names ending with "Playback" and "Capture" respectively as dapm core
>  will link and power those dais based on the name.
>  
> -Note that in current device tree there is no way to mark a dai_link
> -as codec to codec. However, it may change in future.
> +A dai_link in a "simple-audio-card" can be marked as codec to codec in
> +the device tree by adding the "codec-to-codec" property. The dai_link
> +will be initialized with the subset of stream parameters (channels,
> +format, sample rate) supported by all DAIs on the link. Since there is
> +no way to provide these parameters in the device tree, this is mostly
> +useful for communication with simple fixed-function codecs, such as a
> +Bluetooth controller or cellular modem.
> diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
> index bbdd1542d6f1..80b60237b3cd 100644
> --- a/include/sound/simple_card_utils.h
> +++ b/include/sound/simple_card_utils.h
> @@ -49,6 +49,7 @@ struct asoc_simple_priv {
>  		struct asoc_simple_data adata;
>  		struct snd_soc_codec_conf *codec_conf;
>  		unsigned int mclk_fs;
> +		bool codec_to_codec;
>  	} *dai_props;
>  	struct asoc_simple_jack hp_jack;
>  	struct asoc_simple_jack mic_jack;
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index 9b794775df53..2de4cfead790 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -331,6 +331,41 @@ static int asoc_simple_init_dai(struct snd_soc_dai *dai,
>  	return 0;
>  }
>  
> +static int asoc_simple_init_dai_link_params(struct snd_soc_pcm_runtime *rtd,
> +					    struct simple_dai_props *dai_props)
> +{
> +	struct snd_soc_dai_link *dai_link = rtd->dai_link;
> +	struct snd_soc_pcm_stream *params;
> +	struct snd_pcm_hardware hw;
> +	int ret;
> +
> +	if (!dai_props->codec_to_codec)
> +		return 0;
> +
> +	/* Assumes the hardware capabilities are the same in both directions */
> +	ret = snd_soc_runtime_calc_hw(rtd, &hw, SNDRV_PCM_STREAM_PLAYBACK);
> +	if (ret < 0) {
> +		dev_err(rtd->dev, "simple-card: no valid dai_link params\n");
> +		return ret;
> +	}
> +
> +	params = devm_kzalloc(rtd->dev, sizeof(*params), GFP_KERNEL);
> +	if (!params)
> +		return -ENOMEM;
> +
> +	params->formats = hw.formats;
> +	params->rates = hw.rates;
> +	params->rate_min = hw.rate_min;
> +	params->rate_max = hw.rate_max;
> +	params->channels_min = hw.channels_min;
> +	params->channels_max = hw.channels_max;
> +
> +	dai_link->params = params;
> +	dai_link->num_params = 1;
> +
> +	return 0;
> +}
> +
>  int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
>  {
>  	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card);
> @@ -347,6 +382,10 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = asoc_simple_init_dai_link_params(rtd, dai_props);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(asoc_simple_dai_init);
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 55e9f8800b3e..179ab4482d10 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -77,6 +77,16 @@ static int asoc_simple_parse_dai(struct device_node *node,
>  	return 0;
>  }
>  
> +static void simple_parse_codec_to_codec(struct device_node *node,
> +					struct simple_dai_props *props,
> +					char *prefix)
> +{
> +	char prop[128];
> +
> +	snprintf(prop, sizeof(prop), "%scodec-to-codec", prefix);
> +	props->codec_to_codec = of_property_read_bool(node, prop);
> +}
> +
>  static void simple_parse_convert(struct device *dev,
>  				 struct device_node *np,
>  				 struct asoc_simple_data *adata)
> @@ -217,6 +227,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
>  					     "prefix");
>  	}
>  
> +	simple_parse_codec_to_codec(node, dai_props, prefix);
>  	simple_parse_convert(dev, np, &dai_props->adata);
>  	simple_parse_mclk_fs(top, np, codec, dai_props, prefix);
>  
> @@ -292,6 +303,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
>  	if (ret < 0)
>  		goto dai_link_of_err;
>  
> +	simple_parse_codec_to_codec(node, dai_props, prefix);
>  	simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix);
>  
>  	ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu);

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

On Thu, Feb 13, 2020 at 09:37:18AM +0100, Jerome Brunet wrote:

> This brings another question/problem:
> A link which has failed in PMU, could try in PMD to hw_free/shutdown a
> dai which has not gone through startup/hw_params, right ?

I think so, yes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [alsa-devel] [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 255 bytes --]

On Thu, Feb 13, 2020 at 09:37:18AM +0100, Jerome Brunet wrote:

> This brings another question/problem:
> A link which has failed in PMU, could try in PMD to hw_free/shutdown a
> dai which has not gone through startup/hw_params, right ?

I think so, yes.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 4/4] ASoC: simple-card: Add support for codec-to-codec dai_links

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Thu, Feb 13, 2020 at 10:18:53AM +0100, Jerome Brunet wrote:
> On Thu 13 Feb 2020 at 07:11, Samuel Holland <samuel@sholland.org> wrote:

> > +- codec-to-codec			: Indicates a codec-to-codec
> > dai-link.

> I wonder if such property could be viewed as a Linux implementation
> detail ?

Yes, it is.

> Should the card figure out the codec-to-codec links on its own or is it
> something generic enough to put in DT ?

We should be able to figure it out by ourselves, we already have a link
between two CODECs - we should be able to infer that it is in fact a
link between two CODECs with the information we already have, probably
by looking at the two devices that are referenced.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Add support for codec-to-codec dai_links

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 682 bytes --]

On Thu, Feb 13, 2020 at 10:18:53AM +0100, Jerome Brunet wrote:
> On Thu 13 Feb 2020 at 07:11, Samuel Holland <samuel@sholland.org> wrote:

> > +- codec-to-codec			: Indicates a codec-to-codec
> > dai-link.

> I wonder if such property could be viewed as a Linux implementation
> detail ?

Yes, it is.

> Should the card figure out the codec-to-codec links on its own or is it
> something generic enough to put in DT ?

We should be able to figure it out by ourselves, we already have a link
between two CODECs - we should be able to infer that it is in fact a
link between two CODECs with the information we already have, probably
by looking at the two devices that are referenced.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Applied "ASoC: codec2codec: avoid invalid/double-free of pcm runtime" to the asoc tree

[Mark Brown]

The patch

   ASoC: codec2codec: avoid invalid/double-free of pcm runtime

has been applied to the asoc tree at

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

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

From b6570fdb96edf45bcf71884bd2644bd73d348d1a Mon Sep 17 00:00:00 2001
From: Samuel Holland <samuel@sholland.org>
Date: Thu, 13 Feb 2020 00:11:44 -0600
Subject: [PATCH] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

The PCM runtime was freed during PMU in the case that the event hook
encountered an error. However, it is also unconditionally freed during
PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.

Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
Cc: stable@vger.kernel.org
Signed-off-by: Samuel Holland <samuel@sholland.org>
Link: https://lore.kernel.org/r/20200213061147.29386-2-samuel@sholland.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-dapm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index bc20ad9abf8b..8b24396675ec 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3916,9 +3916,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 	runtime->rate = params_rate(params);
 
 out:
-	if (ret < 0)
-		kfree(runtime);
-
 	kfree(params);
 	return ret;
 }
-- 
2.20.1

[alsa-devel] Applied "ASoC: codec2codec: avoid invalid/double-free of pcm runtime" to the asoc tree

[Mark Brown]

The patch

   ASoC: codec2codec: avoid invalid/double-free of pcm runtime

has been applied to the asoc tree at

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

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

From b6570fdb96edf45bcf71884bd2644bd73d348d1a Mon Sep 17 00:00:00 2001
From: Samuel Holland <samuel@sholland.org>
Date: Thu, 13 Feb 2020 00:11:44 -0600
Subject: [PATCH] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

The PCM runtime was freed during PMU in the case that the event hook
encountered an error. However, it is also unconditionally freed during
PMD. Avoid a double-free by dropping the call to kfree in the PMU hook.

Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
Cc: stable@vger.kernel.org
Signed-off-by: Samuel Holland <samuel@sholland.org>
Link: https://lore.kernel.org/r/20200213061147.29386-2-samuel@sholland.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-dapm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index bc20ad9abf8b..8b24396675ec 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3916,9 +3916,6 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 	runtime->rate = params_rate(params);
 
 out:
-	if (ret < 0)
-		kfree(runtime);
-
 	kfree(params);
 	return ret;
 }
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Jerome Brunet]

On Thu 13 Feb 2020 at 12:37, Mark Brown <broonie@kernel.org> wrote:

> On Thu, Feb 13, 2020 at 09:37:18AM +0100, Jerome Brunet wrote:
>
>> This brings another question/problem:
>> A link which has failed in PMU, could try in PMD to hw_free/shutdown a
>> dai which has not gone through startup/hw_params, right ?
>
> I think so, yes.

Maybe this can be solved using the dai active counts which the
codec-to-codec event is not updating. I'll try to come up with
something.

Re: [alsa-devel] [PATCH 1/4] ASoC: codec2codec: avoid invalid/double-free of pcm runtime

[Jerome Brunet]

On Thu 13 Feb 2020 at 12:37, Mark Brown <broonie@kernel.org> wrote:

> On Thu, Feb 13, 2020 at 09:37:18AM +0100, Jerome Brunet wrote:
>
>> This brings another question/problem:
>> A link which has failed in PMU, could try in PMD to hw_free/shutdown a
>> dai which has not gone through startup/hw_params, right ?
>
> I think so, yes.

Maybe this can be solved using the dai active counts which the
codec-to-codec event is not updating. I'll try to come up with
something.


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/4] ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly

[Samuel Holland]

On 2/13/20 12:30 AM, Takashi Iwai wrote:
> On Thu, 13 Feb 2020 07:11:45 +0100,
> Samuel Holland wrote:
>>
>> It can be useful to derive min/max rates of a snd_pcm_hardware without
>> having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
>>
>> Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually
>> need the snd_pcm_runtime. Modify it to take a pointer to hw directly.
> 
> I prefer adding a new function and change snd_pcm_limit_hw_rates() to
> static inline just calling the new one with &runtime->hw, instead of
> touching so many callers site.

I agree. I will definitely do that for v2.

Thanks,
Samuel

> thanks,
> 
> Takashi
> 
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>

Re: [alsa-devel] [PATCH 2/4] ALSA: pcm: Make snd_pcm_limit_hw_rates take hw directly

[Samuel Holland]

On 2/13/20 12:30 AM, Takashi Iwai wrote:
> On Thu, 13 Feb 2020 07:11:45 +0100,
> Samuel Holland wrote:
>>
>> It can be useful to derive min/max rates of a snd_pcm_hardware without
>> having a snd_pcm_runtime, such as before constructing an ASoC DAI link.
>>
>> Since snd_pcm_limit_hw_rates only uses runtime->hw, it does not actually
>> need the snd_pcm_runtime. Modify it to take a pointer to hw directly.
> 
> I prefer adding a new function and change snd_pcm_limit_hw_rates() to
> static inline just calling the new one with &runtime->hw, instead of
> touching so many callers site.

I agree. I will definitely do that for v2.

Thanks,
Samuel

> thanks,
> 
> Takashi
> 
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel