On Mon, Jun 06, 2022 at 09:19:10PM +0200, Martin Povišer wrote:
> + /*
> + * Primary FE
> + *
> + * The mclk/fs ratio at 64 for the primary frontend is important
> + * to ensure that the headphones codec's idea of left and right
> + * in a stereo stream over I2S fits in nicely with everyone else's.
> + * (This is until the headphones codec's driver supports
> + * set_tdm_slot.)
> + *
> + * The low mclk/fs ratio precludes transmitting more than two
> + * channels over I2S, but that's okay since there is the secondary
> + * FE for speaker arrays anyway.
> + */
> + .mclk_fs = 64,
> + },
This seems weird - it looks like it's confusing MCLK and the bit clock
for the audio bus. These are two different clocks. Note that it's very
common for devices to require a higher MCLK/fs ratio to deliver the best
audio performance, 256fs is standard.
> + {
> + /*
> + * Secondary FE
> + *
> + * Here we want frames plenty long to be able to drive all
> + * those fancy speaker arrays.
> + */
> + .mclk_fs = 256,
> + }
Same thing here - this is at least confusing MCLK and the bit clock.
> +static bool macaudio_match_kctl_name(const char *pattern, const char *name)
> +{
> + if (pattern[0] == '*') {
> + int namelen, patternlen;
> +
> + pattern++;
> + if (pattern[0] == ' ')
> + pattern++;
> +
> + namelen = strlen(name);
> + patternlen = strlen(pattern);
> +
> + if (namelen > patternlen)
> + name += (namelen - patternlen);
> + }
> +
> + return !strcmp(name, pattern);
> +}
> +
> +static int macaudio_limit_volume(struct snd_soc_card *card,
> + const char *pattern, int max)
> +{
> + struct snd_kcontrol *kctl;
> + struct soc_mixer_control *mc;
> + int found = 0;
> +
> + list_for_each_entry(kctl, &card->snd_card->controls, list) {
> + if (!macaudio_match_kctl_name(pattern, kctl->id.name))
> + continue;
> +
> + found++;
> + dev_dbg(card->dev, "limiting volume on '%s'\n", kctl->id.name);
> +
> + /*
> + * TODO: This doesn't decrease the volume if it's already
> + * above the limit!
> + */
> + mc = (struct soc_mixer_control *)kctl->private_value;
> + if (max <= mc->max)
> + mc->platform_max = max;
> +
> + }
> +
> + return found;
> +}
This shouldn't be open coded in a driver, please factor it out into the
core so we've got an API for "set limit X on control Y" then call that.
Attachment:
signature.asc
Description: PGP signature
