[coreboot-gerrit] Change in coreboot[master]: Revert "soc/intel/cannonlake: Allow Audio DSP OSC qualification for l...

[Michael Niewöhner (Code Review)]

Attention is currently required from: Shelley Chen, Furquan Shaikh, Tim Wawrzynczak, Aamir Bohra, Patrick Rudolph.
Hello Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Tim Wawrzynczak, Aamir Bohra, Patrick Rudolph,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/57949

to review the following change.

Change subject: Revert "soc/intel/cannonlake: Allow Audio DSP OSC qualification for low power idle"
......................................................................

Revert "soc/intel/cannonlake: Allow Audio DSP OSC qualification for low power idle"

This reverts commit 4d9d3f164de7002dd38f12dc40f9b260f63d2d9b.

Reason for revert:
The commit message does not make clear what the actual problem is or was. The reference to a non-public bug tracker makes it quiet difficult to make any sense of it.

According to the datasheets this means the Audio DSP oscillator is not required to be powered off in order to enter SlpS0/S0ix. However, that bit already defaults to zero and even FSP does not set it. Thus, clearing that bit is not required.

Change-Id: Iff028364fb34f737c3a50809f2df623a0bba7218
---
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/cannonlake/finalize.c
M src/soc/intel/cannonlake/include/soc/pmc.h
3 files changed, 1 insertion(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/57949/1

diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h
index fd37d26..0712146 100644
--- a/src/soc/intel/cannonlake/chip.h
+++ b/src/soc/intel/cannonlake/chip.h
@@ -59,13 +59,8 @@
    uint32_t gen3_dec;
    uint32_t gen4_dec;

-	/* S0ix configuration */
-
    /* Enable S0iX support */
    int s0ix_enable;
-	/* Enable Audio DSP oscillator qualification for S0ix */
-	uint8_t cppmvric2_adsposcdis;
-
    /* Enable DPTF support */
    int dptf_enable;

diff --git a/src/soc/intel/cannonlake/finalize.c b/src/soc/intel/cannonlake/finalize.c
index b2fb9f9..002e8ea 100644
--- a/src/soc/intel/cannonlake/finalize.c
+++ b/src/soc/intel/cannonlake/finalize.c
@@ -91,18 +91,11 @@
    	write8(pmcbase + PCH_PWRM_ACPI_TMR_CTL, reg8);
    }

+	/* Disable XTAL shutdown qualification for low power idle. */
    if (config->s0ix_enable) {
-		/* Disable XTAL shutdown qualification for low power idle. */
    	reg32 = read32(pmcbase + CPPMVRIC);
    	reg32 |= XTALSDQDIS;
    	write32(pmcbase + CPPMVRIC, reg32);
-
-		if (config->cppmvric2_adsposcdis) {
-			/* Enable Audio DSP OSC qualification for S0ix */
-			reg32 = read32(pmcbase + CPPMVRIC2);
-			reg32 &= ~ADSPOSCDIS;
-			write32(pmcbase + CPPMVRIC2, reg32);
-		}
    }

pch_handle_sideband(config);
diff --git a/src/soc/intel/cannonlake/include/soc/pmc.h b/src/soc/intel/cannonlake/include/soc/pmc.h
index fbd366b..252c719 100644
--- a/src/soc/intel/cannonlake/include/soc/pmc.h
+++ b/src/soc/intel/cannonlake/include/soc/pmc.h
@@ -156,9 +156,6 @@
 #define CPPMVRIC			0x1B1C
 #define   XTALSDQDIS			(1 << 22)

-#define CPPMVRIC2			0x1B4C
-#define   ADSPOSCDIS			(1 << 22)
-
 #define IRQ_REG				ACTL
 #define SCI_IRQ_ADJUST			0
 #define ACTL				0x1BD8