[PATCH 1/2] ASoC: soc_pcm_init_runtime_hw: Fix rate_max calculation

[PATCH 1/2] ASoC: soc_pcm_init_runtime_hw: Fix rate_max calculation

[Lars-Peter Clausen]

In order to make sure that the sample rate is in the supported range of both
components the maximum rate of the card should be the minimum of the maximum
rate of each components. There is one special case to consider though, if
max_rate is set to 0 this means there is no maximum specified, so use
min_not_zero() macro which will give use the desired result.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-pcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 170ff96..c4ef880 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -220,7 +220,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
 	struct snd_soc_pcm_stream *cpu_stream)
 {
 	hw->rate_min = max(codec_stream->rate_min, cpu_stream->rate_min);
-	hw->rate_max = max(codec_stream->rate_max, cpu_stream->rate_max);
+	hw->rate_max = min_not_zero(codec_stream->rate_max, cpu_stream->rate_max);
 	hw->channels_min = max(codec_stream->channels_min,
 		cpu_stream->channels_min);
 	hw->channels_max = min(codec_stream->channels_max,
-- 
1.8.0

[PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Lars-Peter Clausen]

snd_pcm_limit_hw_rates() will initialize the minimum and maximum sample rate for
the PCM stream based on the rates specified in the rates field. Since we call
snd_pcm_limit_hw_rates() after soc_pcm_init_runtime_hw() it will essentially
overwrite the min and max rate set in soc_pcm_init_runtime_hw(). This may cause
the minimum or maximum rate to be set to a value outside the range of one of the
components if one of the components sets either SNDRV_PCM_RATE_CONTINUOUS or
SNDRV_PCM_RATE_KNOT and the other component specified a discrete rate via
SNDRV_PCM_RATE_[0-9]* that is outside of the first component's rate range. To
fix this first calculate the minimum and maximum rates using
snd_pcm_limit_hw_rates() and then on top of that apply the contraints specified
in the snd_soc_pcm_stream structs.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-pcm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index c4ef880..22af038 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -215,12 +215,12 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream,
 	}
 }
 
-static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
+static void soc_pcm_init_runtime_hw(struct snd_pcm_runtime *runtime,
 	struct snd_soc_pcm_stream *codec_stream,
 	struct snd_soc_pcm_stream *cpu_stream)
 {
-	hw->rate_min = max(codec_stream->rate_min, cpu_stream->rate_min);
-	hw->rate_max = min_not_zero(codec_stream->rate_max, cpu_stream->rate_max);
+	struct snd_pcm_hardware *hw = &runtime->hw;
+
 	hw->channels_min = max(codec_stream->channels_min,
 		cpu_stream->channels_min);
 	hw->channels_max = min(codec_stream->channels_max,
@@ -233,6 +233,13 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
 	if (cpu_stream->rates
 		& (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))
 		hw->rates |= codec_stream->rates;
+
+	snd_pcm_limit_hw_rates(runtime);
+
+	hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
+	hw->rate_min = max(hw->rate_min, codec_stream->rate_min);
+	hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max);
+	hw->rate_max = min_not_zero(hw->rate_max, codec_stream->rate_max);
 }
 
 /*
@@ -302,15 +309,14 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 
 	/* Check that the codec and cpu DAIs are compatible */
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		soc_pcm_init_runtime_hw(&runtime->hw, &codec_dai_drv->playback,
+		soc_pcm_init_runtime_hw(runtime, &codec_dai_drv->playback,
 			&cpu_dai_drv->playback);
 	} else {
-		soc_pcm_init_runtime_hw(&runtime->hw, &codec_dai_drv->capture,
+		soc_pcm_init_runtime_hw(runtime, &codec_dai_drv->capture,
 			&cpu_dai_drv->capture);
 	}
 
 	ret = -EINVAL;
-	snd_pcm_limit_hw_rates(runtime);
 	if (!runtime->hw.rates) {
 		printk(KERN_ERR "ASoC: %s <-> %s No matching rates\n",
 			codec_dai->name, cpu_dai->name);
-- 
1.8.0

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Takashi Iwai]

At Wed, 27 Nov 2013 09:58:18 +0100,
Lars-Peter Clausen wrote:
> 
> snd_pcm_limit_hw_rates() will initialize the minimum and maximum sample rate for
> the PCM stream based on the rates specified in the rates field. Since we call
> snd_pcm_limit_hw_rates() after soc_pcm_init_runtime_hw() it will essentially
> overwrite the min and max rate set in soc_pcm_init_runtime_hw(). This may cause
> the minimum or maximum rate to be set to a value outside the range of one of the
> components if one of the components sets either SNDRV_PCM_RATE_CONTINUOUS or
> SNDRV_PCM_RATE_KNOT and the other component specified a discrete rate via
> SNDRV_PCM_RATE_[0-9]* that is outside of the first component's rate range. To
> fix this first calculate the minimum and maximum rates using
> snd_pcm_limit_hw_rates() and then on top of that apply the contraints specified
> in the snd_soc_pcm_stream structs.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  sound/soc/soc-pcm.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index c4ef880..22af038 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -215,12 +215,12 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream,
>  	}
>  }
>  
> -static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
> +static void soc_pcm_init_runtime_hw(struct snd_pcm_runtime *runtime,
>  	struct snd_soc_pcm_stream *codec_stream,
>  	struct snd_soc_pcm_stream *cpu_stream)
>  {
> -	hw->rate_min = max(codec_stream->rate_min, cpu_stream->rate_min);
> -	hw->rate_max = min_not_zero(codec_stream->rate_max, cpu_stream->rate_max);
> +	struct snd_pcm_hardware *hw = &runtime->hw;
> +
>  	hw->channels_min = max(codec_stream->channels_min,
>  		cpu_stream->channels_min);
>  	hw->channels_max = min(codec_stream->channels_max,
> @@ -233,6 +233,13 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
>  	if (cpu_stream->rates
>  		& (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))
>  		hw->rates |= codec_stream->rates;
> +
> +	snd_pcm_limit_hw_rates(runtime);

Strictly speaking, snd_pcm_limit_hw_rates() should be applied only in
case without KNOT nor CONTINUOUS flag.  But, I guess this would work
better as is since there might be drivers that don't give proper
rate_min and rate_max but rely on the rate bits, and min_not_zero()
does the right thing in the code below.

The only missing thing is the conflict between CONTINUOUS and KNOT.
Could you put the fix I suggested into this series, too?

In anyway, feel free to take my ack for both patches
    Acked-by: Takashi iwai <tiwai@suse.de>


thanks,

Takashi

> +	hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
> +	hw->rate_min = max(hw->rate_min, codec_stream->rate_min);
> +	hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max);
> +	hw->rate_max = min_not_zero(hw->rate_max, codec_stream->rate_max);
>  }
>  
>  /*
> @@ -302,15 +309,14 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
>  
>  	/* Check that the codec and cpu DAIs are compatible */
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -		soc_pcm_init_runtime_hw(&runtime->hw, &codec_dai_drv->playback,
> +		soc_pcm_init_runtime_hw(runtime, &codec_dai_drv->playback,
>  			&cpu_dai_drv->playback);
>  	} else {
> -		soc_pcm_init_runtime_hw(&runtime->hw, &codec_dai_drv->capture,
> +		soc_pcm_init_runtime_hw(runtime, &codec_dai_drv->capture,
>  			&cpu_dai_drv->capture);
>  	}
>  
>  	ret = -EINVAL;
> -	snd_pcm_limit_hw_rates(runtime);
>  	if (!runtime->hw.rates) {
>  		printk(KERN_ERR "ASoC: %s <-> %s No matching rates\n",
>  			codec_dai->name, cpu_dai->name);
> -- 
> 1.8.0
> 

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Lars-Peter Clausen]

On 11/27/2013 11:31 AM, Takashi Iwai wrote:
> At Wed, 27 Nov 2013 09:58:18 +0100,
> Lars-Peter Clausen wrote:
>>
>> snd_pcm_limit_hw_rates() will initialize the minimum and maximum sample rate for
>> the PCM stream based on the rates specified in the rates field. Since we call
>> snd_pcm_limit_hw_rates() after soc_pcm_init_runtime_hw() it will essentially
>> overwrite the min and max rate set in soc_pcm_init_runtime_hw(). This may cause
>> the minimum or maximum rate to be set to a value outside the range of one of the
>> components if one of the components sets either SNDRV_PCM_RATE_CONTINUOUS or
>> SNDRV_PCM_RATE_KNOT and the other component specified a discrete rate via
>> SNDRV_PCM_RATE_[0-9]* that is outside of the first component's rate range. To
>> fix this first calculate the minimum and maximum rates using
>> snd_pcm_limit_hw_rates() and then on top of that apply the contraints specified
>> in the snd_soc_pcm_stream structs.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  sound/soc/soc-pcm.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
>> index c4ef880..22af038 100644
>> --- a/sound/soc/soc-pcm.c
>> +++ b/sound/soc/soc-pcm.c
>> @@ -215,12 +215,12 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream,
>>  	}
>>  }
>>  
>> -static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
>> +static void soc_pcm_init_runtime_hw(struct snd_pcm_runtime *runtime,
>>  	struct snd_soc_pcm_stream *codec_stream,
>>  	struct snd_soc_pcm_stream *cpu_stream)
>>  {
>> -	hw->rate_min = max(codec_stream->rate_min, cpu_stream->rate_min);
>> -	hw->rate_max = min_not_zero(codec_stream->rate_max, cpu_stream->rate_max);
>> +	struct snd_pcm_hardware *hw = &runtime->hw;
>> +
>>  	hw->channels_min = max(codec_stream->channels_min,
>>  		cpu_stream->channels_min);
>>  	hw->channels_max = min(codec_stream->channels_max,
>> @@ -233,6 +233,13 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
>>  	if (cpu_stream->rates
>>  		& (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))
>>  		hw->rates |= codec_stream->rates;
>> +
>> +	snd_pcm_limit_hw_rates(runtime);
> 
> Strictly speaking, snd_pcm_limit_hw_rates() should be applied only in
> case without KNOT nor CONTINUOUS flag.  But, I guess this would work
> better as is since there might be drivers that don't give proper
> rate_min and rate_max but rely on the rate bits, and min_not_zero()
> does the right thing in the code below.

My thinking as well. There are some driver which do set CONTINUOUS or KNOT,
but don't specify a minimum and maximum rate and probably rely on the fact
that we always call snd_pcm_limit_hw_rates() (E.g. the kirkwood drivers do
this). In my opinion those drivers should be fixed and once that is done we
can add a check that skips snd_pcm_limit_hw_rates() if KNOT or CONTINUOUS is
set. The current behavior isn't optimal e.g. if a driver sets
SNDRV_PCM_RATE_CONTINUOUS and SNDRV_PCM_RATE_8000_192000 and rate_max to
384000, we'd still end up with a maximum rate of 192000 because of
snd_pcm_limit_hw_rates().

> 
> The only missing thing is the conflict between CONTINUOUS and KNOT.
> Could you put the fix I suggested into this series, too?

ok.

- Lars

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Takashi Iwai]

At Wed, 27 Nov 2013 12:18:24 +0100,
Lars-Peter Clausen wrote:
> 
> On 11/27/2013 11:31 AM, Takashi Iwai wrote:
> > At Wed, 27 Nov 2013 09:58:18 +0100,
> > Lars-Peter Clausen wrote:
> >>
> >> snd_pcm_limit_hw_rates() will initialize the minimum and maximum sample rate for
> >> the PCM stream based on the rates specified in the rates field. Since we call
> >> snd_pcm_limit_hw_rates() after soc_pcm_init_runtime_hw() it will essentially
> >> overwrite the min and max rate set in soc_pcm_init_runtime_hw(). This may cause
> >> the minimum or maximum rate to be set to a value outside the range of one of the
> >> components if one of the components sets either SNDRV_PCM_RATE_CONTINUOUS or
> >> SNDRV_PCM_RATE_KNOT and the other component specified a discrete rate via
> >> SNDRV_PCM_RATE_[0-9]* that is outside of the first component's rate range. To
> >> fix this first calculate the minimum and maximum rates using
> >> snd_pcm_limit_hw_rates() and then on top of that apply the contraints specified
> >> in the snd_soc_pcm_stream structs.
> >>
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> >> ---
> >>  sound/soc/soc-pcm.c | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> >> index c4ef880..22af038 100644
> >> --- a/sound/soc/soc-pcm.c
> >> +++ b/sound/soc/soc-pcm.c
> >> @@ -215,12 +215,12 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream,
> >>  	}
> >>  }
> >>  
> >> -static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
> >> +static void soc_pcm_init_runtime_hw(struct snd_pcm_runtime *runtime,
> >>  	struct snd_soc_pcm_stream *codec_stream,
> >>  	struct snd_soc_pcm_stream *cpu_stream)
> >>  {
> >> -	hw->rate_min = max(codec_stream->rate_min, cpu_stream->rate_min);
> >> -	hw->rate_max = min_not_zero(codec_stream->rate_max, cpu_stream->rate_max);
> >> +	struct snd_pcm_hardware *hw = &runtime->hw;
> >> +
> >>  	hw->channels_min = max(codec_stream->channels_min,
> >>  		cpu_stream->channels_min);
> >>  	hw->channels_max = min(codec_stream->channels_max,
> >> @@ -233,6 +233,13 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw,
> >>  	if (cpu_stream->rates
> >>  		& (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))
> >>  		hw->rates |= codec_stream->rates;
> >> +
> >> +	snd_pcm_limit_hw_rates(runtime);
> > 
> > Strictly speaking, snd_pcm_limit_hw_rates() should be applied only in
> > case without KNOT nor CONTINUOUS flag.  But, I guess this would work
> > better as is since there might be drivers that don't give proper
> > rate_min and rate_max but rely on the rate bits, and min_not_zero()
> > does the right thing in the code below.
> 
> My thinking as well. There are some driver which do set CONTINUOUS or KNOT,
> but don't specify a minimum and maximum rate and probably rely on the fact
> that we always call snd_pcm_limit_hw_rates() (E.g. the kirkwood drivers do
> this). In my opinion those drivers should be fixed and once that is done we
> can add a check that skips snd_pcm_limit_hw_rates() if KNOT or CONTINUOUS is
> set. The current behavior isn't optimal e.g. if a driver sets
> SNDRV_PCM_RATE_CONTINUOUS and SNDRV_PCM_RATE_8000_192000 and rate_max to
> 384000, we'd still end up with a maximum rate of 192000 because of
> snd_pcm_limit_hw_rates().

Right.  We should catch all them once, then add a sanity check
something like
	WARN_ON((rates & SNDRV_PCM_RATE_KNOT) && \
		(rates & ~SNDRV_PCM_RATE_KNOT));
	WARN_ON((rates & SNDRV_PCM_RATE_CONTINUOUS) && \
		(rates & ~SNDRV_PCM_RATE_CONTINUOUS));

Takashi

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Mark Brown]

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

On Wed, Nov 27, 2013 at 12:26:29PM +0100, Takashi Iwai wrote:
> Lars-Peter Clausen wrote:

> > My thinking as well. There are some driver which do set CONTINUOUS or KNOT,
> > but don't specify a minimum and maximum rate and probably rely on the fact
> > that we always call snd_pcm_limit_hw_rates() (E.g. the kirkwood drivers do
> > this). In my opinion those drivers should be fixed and once that is done we
> > can add a check that skips snd_pcm_limit_hw_rates() if KNOT or CONTINUOUS is
> > set. The current behavior isn't optimal e.g. if a driver sets
> > SNDRV_PCM_RATE_CONTINUOUS and SNDRV_PCM_RATE_8000_192000 and rate_max to
> > 384000, we'd still end up with a maximum rate of 192000 because of
> > snd_pcm_limit_hw_rates().

> Right.  We should catch all them once, then add a sanity check
> something like

It seems neater if snd_pcm_limit_hw_rates() does the right thing no
matter what the rates are - this centralises the logic for handling this
which reduces the complexity for drivers.

> 	WARN_ON((rates & SNDRV_PCM_RATE_KNOT) && \
> 		(rates & ~SNDRV_PCM_RATE_KNOT));
> 	WARN_ON((rates & SNDRV_PCM_RATE_CONTINUOUS) && \
> 		(rates & ~SNDRV_PCM_RATE_CONTINUOUS));

This does make sense, though we'll need to change the constraint merging
code in ASoC since right now it assumes that combining the masks will do
the merge which won't be the case if you have devices with one of these
flags together with any other flags (either specific rates or the other
non-bitmask one).  It would be nice if we were just throwing the device
constraints into the core constraint code and letting it merge them
together.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Takashi Iwai]

At Wed, 27 Nov 2013 12:21:53 +0000,
Mark Brown wrote:
> 
> On Wed, Nov 27, 2013 at 12:26:29PM +0100, Takashi Iwai wrote:
> > Lars-Peter Clausen wrote:
> 
> > > My thinking as well. There are some driver which do set CONTINUOUS or KNOT,
> > > but don't specify a minimum and maximum rate and probably rely on the fact
> > > that we always call snd_pcm_limit_hw_rates() (E.g. the kirkwood drivers do
> > > this). In my opinion those drivers should be fixed and once that is done we
> > > can add a check that skips snd_pcm_limit_hw_rates() if KNOT or CONTINUOUS is
> > > set. The current behavior isn't optimal e.g. if a driver sets
> > > SNDRV_PCM_RATE_CONTINUOUS and SNDRV_PCM_RATE_8000_192000 and rate_max to
> > > 384000, we'd still end up with a maximum rate of 192000 because of
> > > snd_pcm_limit_hw_rates().
> 
> > Right.  We should catch all them once, then add a sanity check
> > something like
> 
> It seems neater if snd_pcm_limit_hw_rates() does the right thing no
> matter what the rates are - this centralises the logic for handling this
> which reduces the complexity for drivers.

Well, the difficult point is to decide what is right, if the given
condition is already wrong (e.g. rate_min/max don't match with the
RATE bits).  We can accept it somehow silently (as of now), but I
think it's better to warn (and/or refuse).

> > 	WARN_ON((rates & SNDRV_PCM_RATE_KNOT) && \
> > 		(rates & ~SNDRV_PCM_RATE_KNOT));
> > 	WARN_ON((rates & SNDRV_PCM_RATE_CONTINUOUS) && \
> > 		(rates & ~SNDRV_PCM_RATE_CONTINUOUS));
> 
> This does make sense, though we'll need to change the constraint merging
> code in ASoC since right now it assumes that combining the masks will do
> the merge which won't be the case if you have devices with one of these
> flags together with any other flags (either specific rates or the other
> non-bitmask one).  It would be nice if we were just throwing the device
> constraints into the core constraint code and letting it merge them
> together.

I think Lars' fix will handle such a case nicely now; at least,
merging between RATE bits and rate_min/max is solved, as long as the
given conditions are consistent.

But yes, we can certainly put some sanity checks and more handling
code into a core stuff.  But the drivers giving wrong parameters must
be fixed in anyway.  IIRC, even ac97_codec.c does something
misaligned.  Warnings would be a help for hunting such ones, and I
didn't intend it being more than that in WARN_ON() above.


thanks,

Takashi

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Mark Brown]

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

On Wed, Nov 27, 2013 at 02:13:21PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > It seems neater if snd_pcm_limit_hw_rates() does the right thing no
> > matter what the rates are - this centralises the logic for handling this
> > which reduces the complexity for drivers.

> Well, the difficult point is to decide what is right, if the given
> condition is already wrong (e.g. rate_min/max don't match with the
> RATE bits).  We can accept it somehow silently (as of now), but I
> think it's better to warn (and/or refuse).

I agree it's better to warn if it's wrong, I was more responding to the
comment about it not being quite correct to call snd_pcm_limit_hw_rates()
with _KNOT or _CONTINUOUS.

> I think Lars' fix will handle such a case nicely now; at least,
> merging between RATE bits and rate_min/max is solved, as long as the
> given conditions are consistent.

Yes, it should work for the time being.  It just seems silly to have
more than one set of merging code, it just increases the chances we'll
get stuff wrong - switching to use the core constraint merging code
should avoid the possibility of such errors in future.

> But yes, we can certainly put some sanity checks and more handling
> code into a core stuff.  But the drivers giving wrong parameters must
> be fixed in anyway.  IIRC, even ac97_codec.c does something
> misaligned.  Warnings would be a help for hunting such ones, and I
> didn't intend it being more than that in WARN_ON() above.

I'm talking about the code in ASoC that combines the constraints from
the devices on the link here rather than the drivers, as you say the
drivers ought to have correct constraints standalone.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Lars-Peter Clausen]

On 11/27/2013 03:49 PM, Mark Brown wrote:
[...]
>> I think Lars' fix will handle such a case nicely now; at least,
>> merging between RATE bits and rate_min/max is solved, as long as the
>> given conditions are consistent.
> 
> Yes, it should work for the time being.  It just seems silly to have
> more than one set of merging code, it just increases the chances we'll
> get stuff wrong - switching to use the core constraint merging code
> should avoid the possibility of such errors in future.

There isn't such code in the ALSA core yet, but I guess it makes sense to
add it.

- Lars

Re: [PATCH 2/2] ASoC: pcm: Always honor DAI min and max sample rate constraints

[Mark Brown]

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

On Wed, Nov 27, 2013 at 09:58:18AM +0100, Lars-Peter Clausen wrote:
> snd_pcm_limit_hw_rates() will initialize the minimum and maximum sample rate for
> the PCM stream based on the rates specified in the rates field. Since we call
> snd_pcm_limit_hw_rates() after soc_pcm_init_runtime_hw() it will essentially

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH 1/2] ASoC: soc_pcm_init_runtime_hw: Fix rate_max calculation

[Mark Brown]

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

On Wed, Nov 27, 2013 at 09:58:17AM +0100, Lars-Peter Clausen wrote:
> In order to make sure that the sample rate is in the supported range of both
> components the maximum rate of the card should be the minimum of the maximum
> rate of each components. There is one special case to consider though, if
> max_rate is set to 0 this means there is no maximum specified, so use
> min_not_zero() macro which will give use the desired result.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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