Change in coreboot[master]: sb/intel/i82801jx: Use <device/azalia_device.h> registers

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44130 )

Change subject: sb/intel/i82801jx: Use <device/azalia_device.h> registers
......................................................................

sb/intel/i82801jx: Use <device/azalia_device.h> registers

Change-Id: Ic661a1339892dad668ad3f9e68cab70bf380505f
Signed-off-by: Elyes HAOUAS 
---
M src/southbridge/intel/i82801jx/azalia.c
1 file changed, 11 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44130/1

diff --git a/src/southbridge/intel/i82801jx/azalia.c b/src/southbridge/intel/i82801jx/azalia.c
index ba20a6a..b53d641 100644
--- a/src/southbridge/intel/i82801jx/azalia.c
+++ b/src/southbridge/intel/i82801jx/azalia.c
@@ -11,10 +11,6 @@
 #include "chip.h"
 #include "i82801jx.h"

-#define HDA_ICII_REG 0x68
-#define HDA_ICII_BUSY (1 << 0)
-#define HDA_ICII_VALID (1 << 1)
-
 static int set_bits(void *port, u32 mask, u32 val)
 {
    u32 reg32;
@@ -47,15 +43,15 @@
    u32 reg32;

/* Set Bit0 to 0 to enter reset state (BAR + 0x8)[0] */
-	if (set_bits(base + 0x08, 1, 0) == -1)
+	if (set_bits(base + HDA_GCTL_REG, 1, 0) == -1)
    	goto no_codec;

/* Set Bit 0 to 1 to exit reset state (BAR + 0x8)[0] */
-	if (set_bits(base + 0x08, 1, 1) == -1)
+	if (set_bits(base + HDA_GCTL_REG, 1, 1) == -1)
    	goto no_codec;

/* Read in Codec location (BAR + 0xe)[2..0]*/
-	reg32 = read32(base + 0xe);
+	reg32 = read32(base + HDA_STATESTS_REG);
    reg32 &= 0x0f;
    if (!reg32)
    	goto no_codec;
@@ -65,7 +61,7 @@
 no_codec:
    /* Codec Not found */
    /* Put HDA back in reset (BAR + 0x8) [0] */
-	set_bits(base + 0x08, 1, 0);
+	set_bits(base + HDA_GCTL_REG, 1, 0);
    printk(BIOS_DEBUG, "Azalia: No codec!\n");
    return 0;
 }
@@ -118,9 +114,9 @@
    u32 reg32;

/* Send the verb to the codec */
-	reg32 = read32(base + 0x68);
-	reg32 |= (1 << 0) | (1 << 1);
-	write32(base + 0x68, reg32);
+	reg32 = read32(base + HDA_ICII_REG);
+	reg32 |= HDA_ICII_BUSY | HDA_ICII_VALID;
+	write32(base + HDA_ICII_REG, reg32);

/* Use a 50 usec timeout - the Linux kernel uses the same duration */

@@ -149,12 +145,12 @@
    	return;

reg32 = (addr << 28) | 0x000f0000;
-	write32(base + 0x60, reg32);
+	write32(base + HDA_IC_REG, reg32);

if (wait_for_valid(base) == -1)
    	return;

-	reg32 = read32(base + 0x64);
+	reg32 = read32(base + HDA_IR_REG);

/* 2 */
    printk(BIOS_DEBUG, "Azalia: codec viddid: %08x\n", reg32);
@@ -171,7 +167,7 @@
    	if (wait_for_ready(base) == -1)
    		return;

-		write32(base + 0x60, verb[i]);
+		write32(base + HDA_IC_REG, verb[i]);

if (wait_for_valid(base) == -1)
    		return;
@@ -191,7 +187,7 @@
    	if (wait_for_ready(base) == -1)
    		return;

-		write32(base + 0x60, pc_beep_verbs[i]);
+		write32(base + HDA_IC_REG, pc_beep_verbs[i]);

if (wait_for_valid(base) == -1)
    		return;

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: sb/intel/i82801jx: Use <device/azalia_device.h> registers
......................................................................

sb/intel/i82801jx: Use <device/azalia_device.h> registers

Change-Id: Ic661a1339892dad668ad3f9e68cab70bf380505f
Signed-off-by: Elyes HAOUAS 
---
M src/southbridge/intel/i82801jx/azalia.c
1 file changed, 11 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/44130/2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44130 )

Change subject: sb/intel/i82801jx: Use <device/azalia_device.h> registers
......................................................................

Patch Set 2: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44130 )

Change subject: sb/intel/i82801jx: Use <device/azalia_device.h> registers
......................................................................

Patch Set 3: Code-Review+2

Asus P5QL PRO remains identical.

Note that I would expect the commit authors to check if their changes are reproducible.

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44130 )

Change subject: sb/intel/i82801jx: Use <device/azalia_device.h> registers
......................................................................

sb/intel/i82801jx: Use <device/azalia_device.h> registers

Change-Id: Ic661a1339892dad668ad3f9e68cab70bf380505f
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44130
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/southbridge/intel/i82801jx/azalia.c
1 file changed, 11 insertions(+), 15 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/southbridge/intel/i82801jx/azalia.c b/src/southbridge/intel/i82801jx/azalia.c
index ba20a6a..b53d641 100644
--- a/src/southbridge/intel/i82801jx/azalia.c
+++ b/src/southbridge/intel/i82801jx/azalia.c
@@ -11,10 +11,6 @@
 #include "chip.h"
 #include "i82801jx.h"

-#define HDA_ICII_REG 0x68
-#define HDA_ICII_BUSY (1 << 0)
-#define HDA_ICII_VALID (1 << 1)
-
 static int set_bits(void *port, u32 mask, u32 val)
 {
    u32 reg32;
@@ -47,15 +43,15 @@
    u32 reg32;

/* Set Bit0 to 0 to enter reset state (BAR + 0x8)[0] */
-	if (set_bits(base + 0x08, 1, 0) == -1)
+	if (set_bits(base + HDA_GCTL_REG, 1, 0) == -1)
    	goto no_codec;

/* Set Bit 0 to 1 to exit reset state (BAR + 0x8)[0] */
-	if (set_bits(base + 0x08, 1, 1) == -1)
+	if (set_bits(base + HDA_GCTL_REG, 1, 1) == -1)
    	goto no_codec;

/* Read in Codec location (BAR + 0xe)[2..0]*/
-	reg32 = read32(base + 0xe);
+	reg32 = read32(base + HDA_STATESTS_REG);
    reg32 &= 0x0f;
    if (!reg32)
    	goto no_codec;
@@ -65,7 +61,7 @@
 no_codec:
    /* Codec Not found */
    /* Put HDA back in reset (BAR + 0x8) [0] */
-	set_bits(base + 0x08, 1, 0);
+	set_bits(base + HDA_GCTL_REG, 1, 0);
    printk(BIOS_DEBUG, "Azalia: No codec!\n");
    return 0;
 }
@@ -118,9 +114,9 @@
    u32 reg32;

/* Send the verb to the codec */
-	reg32 = read32(base + 0x68);
-	reg32 |= (1 << 0) | (1 << 1);
-	write32(base + 0x68, reg32);
+	reg32 = read32(base + HDA_ICII_REG);
+	reg32 |= HDA_ICII_BUSY | HDA_ICII_VALID;
+	write32(base + HDA_ICII_REG, reg32);

/* Use a 50 usec timeout - the Linux kernel uses the same duration */

@@ -149,12 +145,12 @@
    	return;

reg32 = (addr << 28) | 0x000f0000;
-	write32(base + 0x60, reg32);
+	write32(base + HDA_IC_REG, reg32);

if (wait_for_valid(base) == -1)
    	return;

-	reg32 = read32(base + 0x64);
+	reg32 = read32(base + HDA_IR_REG);

/* 2 */
    printk(BIOS_DEBUG, "Azalia: codec viddid: %08x\n", reg32);
@@ -171,7 +167,7 @@
    	if (wait_for_ready(base) == -1)
    		return;

-		write32(base + 0x60, verb[i]);
+		write32(base + HDA_IC_REG, verb[i]);

if (wait_for_valid(base) == -1)
    		return;
@@ -191,7 +187,7 @@
    	if (wait_for_ready(base) == -1)
    		return;

-		write32(base + 0x60, pc_beep_verbs[i]);
+		write32(base + HDA_IC_REG, pc_beep_verbs[i]);

if (wait_for_valid(base) == -1)
    		return;