[alsa-devel] [PATCH] ASoC: wm8741: Remove unneeded startup() callback
Do not apply rate constraints in the startup() callback. The machine driver can change the sysclk and hence the supported frame rates in its hw_params(). This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow master clock switching").
Signed-off-by: Sergej Sawazki ce3a@gmx.de --- sound/soc/codecs/wm8741.c | 15 --------------- 1 file changed, 15 deletions(-)
diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c index b8c1940..d6e540a 100644 --- a/sound/soc/codecs/wm8741.c +++ b/sound/soc/codecs/wm8741.c @@ -176,20 +176,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = { .list = rates_36864, };
-static int wm8741_startup(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_codec *codec = dai->codec; - struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec); - - if (wm8741->sysclk) - snd_pcm_hw_constraint_list(substream->runtime, 0, - SNDRV_PCM_HW_PARAM_RATE, - wm8741->sysclk_constraints); - - return 0; -} - static int wm8741_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -360,7 +346,6 @@ static int wm8741_set_dai_fmt(struct snd_soc_dai *codec_dai, SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
static const struct snd_soc_dai_ops wm8741_dai_ops = { - .startup = wm8741_startup, .hw_params = wm8741_hw_params, .set_sysclk = wm8741_set_dai_sysclk, .set_fmt = wm8741_set_dai_fmt,
On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote:
Do not apply rate constraints in the startup() callback. The machine driver can change the sysclk and hence the supported frame rates in its hw_params(). This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow master clock switching").
Signed-off-by: Sergej Sawazki ce3a@gmx.de
sound/soc/codecs/wm8741.c | 15 --------------- 1 file changed, 15 deletions(-)
diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c index b8c1940..d6e540a 100644 --- a/sound/soc/codecs/wm8741.c +++ b/sound/soc/codecs/wm8741.c @@ -176,20 +176,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = { .list = rates_36864, };
-static int wm8741_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)-{
- struct snd_soc_codec *codec = dai->codec;
- struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec);
- if (wm8741->sysclk)
snd_pcm_hw_constraint_list(substream->runtime, 0,SNDRV_PCM_HW_PARAM_RATE,wm8741->sysclk_constraints);- return 0;
-}
This function should not be removed it is performing a useful function. If a sysclk has already been configured by the machine driver then we should inform user-space of the rates we can support from that clock. Without this that information is not available to user-space, so instead of user-space being able to resample appropriately it would just error out from hw_params.
Are you perhaps missing a call to clear the sysclk from your machine driver? Usually a dai_set_sysclk call with a rate of zero, also I find if it is a machine driver supporting multiple rates you are best to use ignore_pmdown_time on the DAI link, assuming the devices don't have long bring up times.
Thanks, Charles
On Mon, 30 Jan 2017 09:22:59 +0000, Charles Keepax wrote:
On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote:
Do not apply rate constraints in the startup() callback. The machine driver can change the sysclk and hence the supported frame rates in its hw_params(). This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow master clock switching").
This function should not be removed it is performing a useful function. If a sysclk has already been configured by the machine driver then we should inform user-space of the rates we can support from that clock. Without this that information is not available to user-space, so instead of user-space being able to resample appropriately it would just error out from hw_params.
Are you perhaps missing a call to clear the sysclk from your machine driver? Usually a dai_set_sysclk call with a rate of zero, also I find if it is a machine driver supporting multiple rates you are best to use ignore_pmdown_time on the DAI link, assuming the devices don't have long bring up times.
After clearing the sysclk, the codec should 'pretend' to support all rates, right? (no sysclk -> no rate constrains)
After applying rate constraints using snd_pcm_hw_constraint_list(...), what would be a clean way to remove the constraints from the rates parameter?
Thanks, Sergej
On Mon, Jan 30, 2017 at 11:56:52PM +0100, Sergej Sawazki wrote:
On Mon, 30 Jan 2017 09:22:59 +0000, Charles Keepax wrote:
On Sat, Jan 28, 2017 at 12:35:45AM +0100, Sergej Sawazki wrote:
Do not apply rate constraints in the startup() callback. The machine driver can change the sysclk and hence the supported frame rates in its hw_params(). This callback is unneeded since commit e369bd006fd6 ("ASoC: wm8741: Allow master clock switching").
This function should not be removed it is performing a useful function. If a sysclk has already been configured by the machine driver then we should inform user-space of the rates we can support from that clock. Without this that information is not available to user-space, so instead of user-space being able to resample appropriately it would just error out from hw_params.
Are you perhaps missing a call to clear the sysclk from your machine driver? Usually a dai_set_sysclk call with a rate of zero, also I find if it is a machine driver supporting multiple rates you are best to use ignore_pmdown_time on the DAI link, assuming the devices don't have long bring up times.
After clearing the sysclk, the codec should 'pretend' to support all rates, right? (no sysclk -> no rate constrains)
Indeed yes, so a normal pattern would often be setting a sysclk rate in set_bias_level(on the way up)/hw_params and clearing it in set_bias_level(on the way down). That way whilst the device is powered up it is fixed at a certain rate so as not to disrupt any running audio but whilst powered down you are free to start it up at any rate.
After applying rate constraints using snd_pcm_hw_constraint_list(...), what would be a clean way to remove the constraints from the rates parameter?
I am not sure I fully follow here, normally one would close the stream and open a new one at the new rate. Hence no need to clear the constraits as its a new stream. Are you intending to keep the stream open but reconfigure it for new rates?
Thanks, Charles
On Tue, 31 Jan 2017 09:49:31 +0000, Charles Keepax wrote:
On Mon, Jan 30, 2017 at 11:56:52PM +0100, Sergej Sawazki wrote:
After clearing the sysclk, the codec should 'pretend' to support all rates, right? (no sysclk -> no rate constrains)
Indeed yes, so a normal pattern would often be setting a sysclk rate in set_bias_level(on the way up)/hw_params and clearing it in set_bias_level(on the way down). That way whilst the device is powered up it is fixed at a certain rate so as not to disrupt any running audio but whilst powered down you are free to start it up at any rate.
Thanks Charles, it is clear now, sorry for the noise.
Sergej
participants (2)
-
Charles Keepax -
Sergej Sawazki