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.