Re: [PATCH] ASoC: Intel: sof_rt5682: Add rt1015p speaker amp support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux