lkml.org 
[lkml]   [2021]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support
From
Date
I am not a big fan of the code partition you've selected.

> +void sof_rt1015p_set_share_en_spk(void)
> +{
> + /* Two amps share one en pin so there is only one device in acpi
> + * table
> + */
> + rt1015p_quirk |= RT1015P_SHARE_EN_SPK;
> +}

This is a function now used in the machine driver, see below [1]
This adds a new interface between machine and realtek common code, which
we are trying to move to a module with well-defined APIs.

> +void sof_rt1015p_dai_link(struct snd_soc_dai_link *link)
> +{
> + link->codecs = rt1015p_dai_link_components;
> + if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
> + link->num_codecs = 1;
> + else
> + link->num_codecs = ARRAY_SIZE(rt1015p_dai_link_components);
> + link->init = rt1015p_init;
> + link->ops = &rt1015p_ops;
> +}
> +
> +void sof_rt1015p_codec_conf(struct snd_soc_card *card)
> +{
> + if (rt1015p_quirk & RT1015P_SHARE_EN_SPK)
> + return;
> +
> + card->codec_conf = rt1015p_codec_confs;
> + card->num_configs = ARRAY_SIZE(rt1015p_codec_confs);
> +}

could we not handle this quirk inside one of the two dai_link or
codec_conf functions above?
The machine driver does not care about this RT1015P_SHARE_EN_SPK quirk,
it's only used in those two functions so should be set in this scope.
There's no need to make it visible to the machine driver, is there?

> + /* setup amp quirk if any */
> + if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
> + /* There may be only one device instance if two amps
> + * sharing one en pin
> + */
> + if (!acpi_dev_present("RTL1015", "1", -1)) {
> + dev_dbg(&pdev->dev, "Device %s not exist\n",
> + RT1015P_DEV1_NAME);
> + sof_rt1015p_set_share_en_spk();
> + }
> + }
> +

[1]

I see no problem using the _UID (Unique ID) to check if a second
amplifier is present or not. This seems to follow the ACPI spec Section
6.1.12, as long as "the _UID must be unique across all devices with
either a common _HID or _CID"


> + else if (sof_rt5682_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT)
> + sof_rt1015p_codec_conf(&sof_audio_card_rt5682);

\
 
 \ /
  Last update: 2021-03-15 16:33    [W:0.060 / U:0.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site