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.