[v1,04/10] ASoC: tegra: Support RT5631 by machine driver
Commit Message
Add Realtek ALC5631/RT5631 codec support to the Tegra ASoC machine driver.
The RT5631 codec is found on devices like ASUS Transformer TF201, TF700T
and other Tegra-based Android tablets.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Signed-off-by: Ion Agorria <ion@agorria.com>
---
sound/soc/tegra/Kconfig | 9 ++++
sound/soc/tegra/tegra_asoc_machine.c | 74 ++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
Comments
On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote:
> diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c
> index 78faa8bcae27..607800ec07a6 100644
> --- a/sound/soc/tegra/tegra_asoc_machine.c
> +++ b/sound/soc/tegra/tegra_asoc_machine.c
> @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = {
> };
>
> /* Mic Jack */
This comment doesn't make sense now. It was never super useful, though.
Just delete it.
> +static int headset_check(void *data)
> +{
> + struct tegra_machine *machine = (struct tegra_machine *)data;
> +
> + /* Detect mic insertion only if 3.5 jack is in */
> + if (gpiod_get_value_cansleep(machine->gpiod_hp_det) &&
> + gpiod_get_value_cansleep(machine->gpiod_mic_det))
> + return SND_JACK_MICROPHONE;
> +
> + return 0;
> +}
>
> static struct snd_soc_jack tegra_machine_mic_jack;
>
> @@ -183,8 +194,15 @@ int tegra_asoc_machine_init(struct snd_soc_pcm_runtime *rtd)
regards,
dan carpenter
On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote:
> Add Realtek ALC5631/RT5631 codec support to the Tegra ASoC machine driver.
> The RT5631 codec is found on devices like ASUS Transformer TF201, TF700T
> and other Tegra-based Android tablets.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Ion Agorria <ion@agorria.com>
Your signoff should be last if you're the one sending this.
> +static unsigned int tegra_machine_mclk_rate_6mhz(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;
It feels like this is complicated enough and looks like the
clocking is flexible enough that it might be easier to just have
a table of values or otherwise enumerate standard rates, seeing
the code I feel like I need to worry about what happens if we
pick a clock rate over 6MHz (the loop could give a value over
that), and it's not clear why we have the switch statement rather
than just starting at a multiple of 128 and looping an extra time.
I suspect there's going to be no meaningful downside for having
the clock held at over 3MHz on a tablet form factor, the usual
issue would be power consumption but between the larger battery
size you tend to have on a tablet and the power draw of the
screen if that's on it's likely to be into the noise practially
speaking.
вт, 21 лют. 2023 р. о 21:32 Dan Carpenter <error27@gmail.com> пише:
>
> On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote:
> > diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c
> > index 78faa8bcae27..607800ec07a6 100644
> > --- a/sound/soc/tegra/tegra_asoc_machine.c
> > +++ b/sound/soc/tegra/tegra_asoc_machine.c
> > @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = {
> > };
> >
> > /* Mic Jack */
>
> This comment doesn't make sense now. It was never super useful, though.
> Just delete it.
It does. Headset is Mic Jack + Headphones combined. headset_check function
performs check for a Mic Jack component in plugged Jack 3.5
> > +static int headset_check(void *data)
> > +{
> > + struct tegra_machine *machine = (struct tegra_machine *)data;
> > +
> > + /* Detect mic insertion only if 3.5 jack is in */
> > + if (gpiod_get_value_cansleep(machine->gpiod_hp_det) &&
> > + gpiod_get_value_cansleep(machine->gpiod_mic_det))
> > + return SND_JACK_MICROPHONE;
> > +
> > + return 0;
> > +}
> >
> > static struct snd_soc_jack tegra_machine_mic_jack;
> >
> > @@ -183,8 +194,15 @@ int tegra_asoc_machine_init(struct snd_soc_pcm_runtime *rtd)
>
> regards,
> dan carpenter
ср, 22 лют. 2023 р. о 00:23 Mark Brown <broonie@kernel.org> пише:
>
> On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote:
>
> > Add Realtek ALC5631/RT5631 codec support to the Tegra ASoC machine driver.
> > The RT5631 codec is found on devices like ASUS Transformer TF201, TF700T
> > and other Tegra-based Android tablets.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Ion Agorria <ion@agorria.com>
>
> Your signoff should be last if you're the one sending this.
Thanks
> > +static unsigned int tegra_machine_mclk_rate_6mhz(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;
>
> It feels like this is complicated enough and looks like the
> clocking is flexible enough that it might be easier to just have
> a table of values or otherwise enumerate standard rates, seeing
> the code I feel like I need to worry about what happens if we
> pick a clock rate over 6MHz (the loop could give a value over
> that), and it's not clear why we have the switch statement rather
> than just starting at a multiple of 128 and looping an extra time.
>
> I suspect there's going to be no meaningful downside for having
> the clock held at over 3MHz on a tablet form factor, the usual
> issue would be power consumption but between the larger battery
> size you tend to have on a tablet and the power draw of the
> screen if that's on it's likely to be into the noise practially
> speaking.
This is how downstream handled mclk rate for RT5631.
On Wed, Feb 22, 2023 at 10:00:58AM +0200, Svyatoslav Ryhel wrote:
> ср, 22 лют. 2023 р. о 00:23 Mark Brown <broonie@kernel.org> пише:
> > It feels like this is complicated enough and looks like the
> > clocking is flexible enough that it might be easier to just have
> > a table of values or otherwise enumerate standard rates, seeing
> > the code I feel like I need to worry about what happens if we
> > pick a clock rate over 6MHz (the loop could give a value over
> > that), and it's not clear why we have the switch statement rather
> > than just starting at a multiple of 128 and looping an extra time.
> > I suspect there's going to be no meaningful downside for having
> > the clock held at over 3MHz on a tablet form factor, the usual
> > issue would be power consumption but between the larger battery
> > size you tend to have on a tablet and the power draw of the
> > screen if that's on it's likely to be into the noise practially
> > speaking.
> This is how downstream handled mclk rate for RT5631.
That doesn't mean it shouldn't be fixed or improved.
On Wed, Feb 22, 2023 at 09:55:52AM +0200, Svyatoslav Ryhel wrote:
> вт, 21 лют. 2023 р. о 21:32 Dan Carpenter <error27@gmail.com> пише:
> >
> > On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote:
> > > diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c
> > > index 78faa8bcae27..607800ec07a6 100644
> > > --- a/sound/soc/tegra/tegra_asoc_machine.c
> > > +++ b/sound/soc/tegra/tegra_asoc_machine.c
> > > @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = {
> > > };
> > >
> > > /* Mic Jack */
> >
> > This comment doesn't make sense now. It was never super useful, though.
> > Just delete it.
>
> It does. Headset is Mic Jack + Headphones combined. headset_check function
> performs check for a Mic Jack component in plugged Jack 3.5
>
I feel if we need to discuess what a comment means or if it even means
anything then that's a useless comment by definition.
regards,
dan carpenter
On Wed, Feb 22, 2023 at 04:28:09PM +0300, Dan Carpenter wrote:
> On Wed, Feb 22, 2023 at 09:55:52AM +0200, Svyatoslav Ryhel wrote:
> > вт, 21 лют. 2023 р. о 21:32 Dan Carpenter <error27@gmail.com> пише:
> > > > /* Mic Jack */
> > > This comment doesn't make sense now. It was never super useful, though.
> > > Just delete it.
> > It does. Headset is Mic Jack + Headphones combined. headset_check function
> > performs check for a Mic Jack component in plugged Jack 3.5
> I feel if we need to discuess what a comment means or if it even means
> anything then that's a useless comment by definition.
If the device doesn't have a distinct mic jack then it's not ideal to
talk about there being one (as opposed to the microphone on the headset
jack).
@@ -189,6 +189,15 @@ config SND_SOC_TEGRA_AUDIO_GRAPH_CARD
config SND_SOC_TEGRA_MACHINE_DRV
tristate
+config SND_SOC_TEGRA_RT5631
+ tristate "SoC Audio support for Tegra boards using an RT5631 codec"
+ depends on SND_SOC_TEGRA && I2C && GPIOLIB
+ select SND_SOC_TEGRA_MACHINE_DRV
+ select SND_SOC_RT5631
+ help
+ Say Y or M here if you want to add support for SoC audio on Tegra
+ boards using the RT5631 codec, such as Transformer.
+
config SND_SOC_TEGRA_RT5640
tristate "SoC Audio support for Tegra boards using an RT5640 codec"
depends on I2C && GPIOLIB
@@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = {
};
/* Mic Jack */
+static int headset_check(void *data)
+{
+ struct tegra_machine *machine = (struct tegra_machine *)data;
+
+ /* Detect mic insertion only if 3.5 jack is in */
+ if (gpiod_get_value_cansleep(machine->gpiod_hp_det) &&
+ gpiod_get_value_cansleep(machine->gpiod_mic_det))
+ return SND_JACK_MICROPHONE;
+
+ return 0;
+}
static struct snd_soc_jack tegra_machine_mic_jack;
@@ -183,8 +194,15 @@ int tegra_asoc_machine_init(struct snd_soc_pcm_runtime *rtd)
return err;
}
+ tegra_machine_mic_jack_gpio.data = machine;
tegra_machine_mic_jack_gpio.desc = machine->gpiod_mic_det;
+ if (of_property_read_bool(card->dev->of_node,
+ "nvidia,coupled-mic-hp-det")) {
+ tegra_machine_mic_jack_gpio.desc = machine->gpiod_hp_det;
+ tegra_machine_mic_jack_gpio.jack_status_check = headset_check;
+ };
+
err = snd_soc_jack_add_gpios(&tegra_machine_mic_jack, 1,
&tegra_machine_mic_jack_gpio);
if (err)
@@ -238,6 +256,27 @@ static unsigned int tegra_machine_mclk_rate_12mhz(unsigned int srate)
return mclk;
}
+static unsigned int tegra_machine_mclk_rate_6mhz(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;
+
+ return mclk;
+}
+
static int tegra_machine_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
@@ -865,6 +904,40 @@ static const struct tegra_asoc_data tegra_rt5632_data = {
.add_headset_jack = true,
};
+/* RT5631 machine */
+
+SND_SOC_DAILINK_DEFS(rt5631_hifi,
+ DAILINK_COMP_ARRAY(COMP_EMPTY()),
+ DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "rt5631-hifi")),
+ DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+static struct snd_soc_dai_link tegra_rt5631_dai = {
+ .name = "RT5631",
+ .stream_name = "RT5631 PCM",
+ .init = tegra_asoc_machine_init,
+ .dai_fmt = SND_SOC_DAIFMT_I2S |
+ SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+ SND_SOC_DAILINK_REG(rt5631_hifi),
+};
+
+static struct snd_soc_card snd_soc_tegra_rt5631 = {
+ .components = "codec:rt5631",
+ .dai_link = &tegra_rt5631_dai,
+ .num_links = 1,
+ .fully_routed = true,
+};
+
+static const struct tegra_asoc_data tegra_rt5631_data = {
+ .mclk_rate = tegra_machine_mclk_rate_6mhz,
+ .card = &snd_soc_tegra_rt5631,
+ .add_common_dapm_widgets = true,
+ .add_common_controls = true,
+ .add_common_snd_ops = true,
+ .add_mic_jack = true,
+ .add_hp_jack = true,
+};
+
static const struct of_device_id tegra_machine_of_match[] = {
{ .compatible = "nvidia,tegra-audio-trimslice", .data = &tegra_trimslice_data },
{ .compatible = "nvidia,tegra-audio-max98090", .data = &tegra_max98090_data },
@@ -874,6 +947,7 @@ static const struct of_device_id tegra_machine_of_match[] = {
{ .compatible = "nvidia,tegra-audio-rt5677", .data = &tegra_rt5677_data },
{ .compatible = "nvidia,tegra-audio-rt5640", .data = &tegra_rt5640_data },
{ .compatible = "nvidia,tegra-audio-alc5632", .data = &tegra_rt5632_data },
+ { .compatible = "nvidia,tegra-audio-rt5631", .data = &tegra_rt5631_data },
{},
};
MODULE_DEVICE_TABLE(of, tegra_machine_of_match);