[PATCH] ALSA: hda: clarify comments on SCF changes

[PATCH] ALSA: hda: clarify comments on SCF changes

[Pierre-Louis Bossart]

The commit 1f9d3d98694b1 ("ALSA: hda - set intel audio clock to a
proper value") added a number of misleading comments.

There is no ability to detect if an SCF value was set or not, what the
code does is prevent the use of the 6MHz audio clock represented by
the value 0 in LCTL.SCF. Changing the SCF settings does require the
link to be power-cycled, but in all other cases the link is powered
automatically when exiting reset. In other words, the power-cycle is
an exception to the rule that the HDaudio legacy driver does not need
to program SPA/CPA bits.

In addition, the SCF related changes are only relevant for the first
link.

No functionality change, only comment clarifications.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
---
 sound/pci/hda/hda_intel.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 6ff19dd0d10c..92f66bbc5c91 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -485,8 +485,8 @@ static int intel_ml_lctl_set_power(struct azx *chip, int state)
 	int timeout;
 
 	/*
-	 * the codecs are sharing the first link setting by default
-	 * If other links are enabled for stream, they need similar fix
+	 * Changes to LCTL.SCF are only needed for the first multi-link dealing
+	 * with external codecs
 	 */
 	val = readl(bus->mlcap + AZX_ML_BASE + AZX_REG_ML_LCTL);
 	val &= ~AZX_ML_LCTL_SPA;
@@ -513,7 +513,7 @@ static void intel_init_lctl(struct azx *chip)
 
 	/* 0. check lctl register value is correct or not */
 	val = readl(bus->mlcap + AZX_ML_BASE + AZX_REG_ML_LCTL);
-	/* if SCF is already set, let's use it */
+	/* only perform additional configurations if the SCF is initially based on 6MHz */
 	if ((val & AZX_ML_LCTL_SCF) != 0)
 		return;
 
@@ -531,7 +531,7 @@ static void intel_init_lctl(struct azx *chip)
 	if (ret)
 		goto set_spa;
 
-	/* 2. update SCF to select a properly audio clock*/
+	/* 2. update SCF to select an audio clock different from 6MHz */
 	val &= ~AZX_ML_LCTL_SCF;
 	val |= intel_get_lctl_scf(chip);
 	writel(val, bus->mlcap + AZX_ML_BASE + AZX_REG_ML_LCTL);
-- 
2.34.1

Re: [PATCH] ALSA: hda: clarify comments on SCF changes

[Takashi Iwai]

On Mon, 31 Oct 2022 20:55:05 +0100,
Pierre-Louis Bossart wrote:
> 
> The commit 1f9d3d98694b1 ("ALSA: hda - set intel audio clock to a
> proper value") added a number of misleading comments.
> 
> There is no ability to detect if an SCF value was set or not, what the
> code does is prevent the use of the 6MHz audio clock represented by
> the value 0 in LCTL.SCF. Changing the SCF settings does require the
> link to be power-cycled, but in all other cases the link is powered
> automatically when exiting reset. In other words, the power-cycle is
> an exception to the rule that the HDaudio legacy driver does not need
> to program SPA/CPA bits.
> 
> In addition, the SCF related changes are only relevant for the first
> link.
> 
> No functionality change, only comment clarifications.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang@intel.com>

Thanks, applied.


Takashi