21.05.2021 16:12, Jon Hunter пишет:
On 20/05/2021 18:50, Dmitry Osipenko wrote:
Squash all machine drivers into a single-universal one. This reduces code duplication, eases addition of a new drivers and upgrades older code to a modern Linux kernel APIs.
Suggested-by: Jonathan Hunter jonathanh@nvidia.com Co-developed-by: Ion Agorria ion@agorria.com Signed-off-by: Ion Agorria ion@agorria.com Co-developed-by: Svyatoslav Ryhel clamor95@gmail.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Signed-off-by: Dmitry Osipenko digetx@gmail.com
sound/soc/tegra/Kconfig | 12 + sound/soc/tegra/Makefile | 18 +- sound/soc/tegra/tegra20_ac97.c | 1 - sound/soc/tegra/tegra_alc5632.c | 260 ---------- sound/soc/tegra/tegra_asoc_machine.c | 732 +++++++++++++++++++++++++++ sound/soc/tegra/tegra_asoc_machine.h | 45 ++ sound/soc/tegra/tegra_max98090.c | 277 ---------- sound/soc/tegra/tegra_rt5640.c | 223 -------- sound/soc/tegra/tegra_rt5677.c | 325 ------------ sound/soc/tegra/tegra_sgtl5000.c | 212 -------- sound/soc/tegra/tegra_wm8753.c | 186 ------- sound/soc/tegra/tegra_wm8903.c | 358 +++---------- sound/soc/tegra/tegra_wm9712.c | 167 ------ sound/soc/tegra/trimslice.c | 173 ------- 14 files changed, 862 insertions(+), 2127 deletions(-) delete mode 100644 sound/soc/tegra/tegra_alc5632.c create mode 100644 sound/soc/tegra/tegra_asoc_machine.c create mode 100644 sound/soc/tegra/tegra_asoc_machine.h delete mode 100644 sound/soc/tegra/tegra_max98090.c delete mode 100644 sound/soc/tegra/tegra_rt5640.c delete mode 100644 sound/soc/tegra/tegra_rt5677.c delete mode 100644 sound/soc/tegra/tegra_sgtl5000.c delete mode 100644 sound/soc/tegra/tegra_wm8753.c delete mode 100644 sound/soc/tegra/tegra_wm9712.c delete mode 100644 sound/soc/tegra/trimslice.c
...
+static unsigned int tegra_max98090_mclk_rate(unsigned int srate) +{
Minor comment, but I wonder if there is a better name for the above function? This function is using a fixed rate as opposed to scaling it with sample rate which can be common and not really specific to the max98090 codec.
I'll rename it in v3, thank you for suggestion.
- unsigned int mclk;
- switch (srate) {
- case 8000:
- case 16000:
- case 24000:
- case 32000:
- case 48000:
- case 64000:
- case 96000:
mclk = 12288000;break;- case 11025:
- case 22050:
- case 44100:
- case 88200:
mclk = 11289600;break;- default:
mclk = 12000000;break;- }
- return mclk;
+}
+unsigned int tegra_asoc_machine_mclk_rate(unsigned int srate) +{
- unsigned int mclk;
- switch (srate) {
- case 64000:
- case 88200:
- case 96000:
mclk = 128 * srate;break;- default:
mclk = 256 * srate;break;- }
- /* FIXME: Codec only requires >= 3MHz if OSR==0 */
- while (mclk < 6000000)
mclk *= 2;So this appears to be specific to the wm8903 codec or at least this is where it came from. And given that the switch statement is not complete in terms of the sample rates (ie. only has a subset), I am wondering if set should keep this specific to the wm8903 codec?
The RT5631 codec of Asus Transformers will re-use this function.
IIUC, the default switch-case works properly for all rates below 64KHz, at least I haven't had any problems with it. Could you please clarify why you are saying that the switch statement appears to be incomplete?
- return mclk;
+} +EXPORT_SYMBOL_GPL(tegra_asoc_machine_mclk_rate);> + +static int tegra_machine_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
- struct snd_soc_card *card = rtd->card;
- struct tegra_machine *machine = snd_soc_card_get_drvdata(card);
- unsigned int srate = params_rate(params);
- unsigned int mclk = machine->asoc->mclk_rate(srate);
- const unsigned int clk_id = 0;
- int err;
- err = tegra_asoc_utils_set_rate(&machine->util_data, srate, mclk);
- if (err < 0) {
dev_err(card->dev, "Can't configure clocks: %d\n", err);return err;- }
- err = snd_soc_dai_set_sysclk(codec_dai, clk_id, mclk, SND_SOC_CLOCK_IN);
Looks like clk_id is always 0. Most likely all the clock ids passed are 0 by default but I wonder if we should not assume this in case something changes in the future?
Initially I had the same thought and even made the clk_id customizable, but then decided that for now it will be cleaner to hardcode ID to 0 since it will be very easy to customize the ID if will become necessary.
None of the currently supported devices use a different ID. I see now that the older Galaxy Tab 10 may need to use ID=1, so perhaps indeed it won't hurt to make it customizable already. I'll reconsider it for v3.
Thank you for the review.