Change in coreboot[master]: soc/intel/common: Set controller state to active in uart init

[Usha P (Code Review)]

Usha P has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

soc/intel/common: Set controller state to active in uart init

Set the controller state to D0 during the uart init sequence, this
ensures the controller is up and active.

One more argument "const struct device *dev" has been added
to uart_lpss_init function for the same.

BUG=b:135941367
TEST=Verify no timeouts seen during UART controller enumeration
     sequence in CML, ICl and APL platforms

Change-Id: Ie91b502a38d1a40a3dea3711b015b7a5b7ede2db
Signed-off-by: Usha P 
---
M src/soc/intel/common/block/uart/uart.c
1 file changed, 6 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34810/1

diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c
index f556aed..364835d 100644
--- a/src/soc/intel/common/block/uart/uart.c
+++ b/src/soc/intel/common/block/uart/uart.c
@@ -33,8 +33,11 @@
 extern const struct uart_gpio_pad_config uart_gpio_pads[];
 extern const int uart_max_index;

-static void uart_lpss_init(uintptr_t baseaddr)
+static void uart_lpss_init(const struct device *dev, uintptr_t baseaddr)
 {
+	/* Ensure controller is in D0 state */
+	lpss_set_power_state(dev, STATE_D0);
+
    /* Take UART out of reset */
    lpss_reset_release(baseaddr);

@@ -79,7 +82,7 @@
    /* Enable memory access and bus master */
    pci_write_config32(dev, PCI_COMMAND, UART_PCI_ENABLE);

-	uart_lpss_init(baseaddr);
+	uart_lpss_init(device, baseaddr);
 }

const struct device *uart_get_device(void)
@@ -231,7 +234,7 @@

base = pci_read_config32(dev, PCI_BASE_ADDRESS_0) & ~0xFFF;
    	if (base)
-			uart_lpss_init(base);
+			uart_lpss_init(dev, base);
    }
 }

[Usha P (Code Review)]

Hello Patrick Rudolph, Subrata Banik, Meera Ravindranath, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

soc/intel/common: Set controller state to active in uart init

Set the controller state to D0 during the uart init sequence, this
ensures the controller is up and active.

One more argument "const struct device *dev" has been added
to uart_lpss_init function.

BUG=b:135941367
TEST=Verify no timeouts seen during UART controller enumeration
     sequence in CML, ICl and APL platforms

Change-Id: Ie91b502a38d1a40a3dea3711b015b7a5b7ede2db
Signed-off-by: Usha P 
---
M src/soc/intel/common/block/uart/uart.c
1 file changed, 6 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34810/2

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 3: Code-Review+2

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 3: Code-Review+2

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 3: -Code-Review

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34810/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34810/3//COMMIT_MSG@17 
PS3, Line 17: ICl
ICL

[Usha P (Code Review)]

Hello Patrick Rudolph, Subrata Banik, Meera Ravindranath, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

soc/intel/common: Set controller state to active in uart init

Set the controller state to D0 during the uart init sequence, this
ensures the controller is up and active.

One more argument "const struct device *dev" has been added
to uart_lpss_init function.

BUG=b:135941367
TEST=Verify no timeouts seen during UART controller enumeration
     sequence in CML, ICL and APL platforms

Change-Id: Ie91b502a38d1a40a3dea3711b015b7a5b7ede2db
Signed-off-by: Usha P 
---
M src/soc/intel/common/block/uart/uart.c
1 file changed, 6 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/34810/4

[Usha P (Code Review)]

Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34810/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34810/3//COMMIT_MSG@17 
PS3, Line 17: ICl
> ICL

Done

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 4: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34810/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34810/4//COMMIT_MSG@16 
PS4, Line 16: erify no timeouts seen during UART controller enumeration
            :      sequence in CML, ICL and APL platforms
            : 
Also no boot failures or no console issues?

[Usha P (Code Review)]

Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34810/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34810/4//COMMIT_MSG@16 
PS4, Line 16: erify no timeouts seen during UART controller enumeration
            :      sequence in CML, ICL and APL platforms
            :
> Also no boot failures or no console issues?

Yes. We have not observed any issues.

[Usha P (Code Review)]

Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34810/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34810/4//COMMIT_MSG@16 
PS4, Line 16: erify no timeouts seen during UART controller enumeration
            :      sequence in CML, ICL and APL platforms
            :
> Yes. We have not observed any issues.

Done

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

Patch Set 4: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34810 )

Change subject: soc/intel/common: Set controller state to active in uart init
......................................................................

soc/intel/common: Set controller state to active in uart init

Set the controller state to D0 during the uart init sequence, this
ensures the controller is up and active.

One more argument "const struct device *dev" has been added
to uart_lpss_init function.

BUG=b:135941367
TEST=Verify no timeouts seen during UART controller enumeration
     sequence in CML, ICL and APL platforms

Change-Id: Ie91b502a38d1a40a3dea3711b015b7a5b7ede2db
Signed-off-by: Usha P 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34810
Reviewed-by: Subrata Banik 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Aamir Bohra 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/common/block/uart/uart.c
1 file changed, 6 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Subrata Banik: Looks good to me, approved
  Aamir Bohra: Looks good to me, approved

diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c
index f556aed..364835d 100644
--- a/src/soc/intel/common/block/uart/uart.c
+++ b/src/soc/intel/common/block/uart/uart.c
@@ -33,8 +33,11 @@
 extern const struct uart_gpio_pad_config uart_gpio_pads[];
 extern const int uart_max_index;

-static void uart_lpss_init(uintptr_t baseaddr)
+static void uart_lpss_init(const struct device *dev, uintptr_t baseaddr)
 {
+	/* Ensure controller is in D0 state */
+	lpss_set_power_state(dev, STATE_D0);
+
    /* Take UART out of reset */
    lpss_reset_release(baseaddr);

@@ -79,7 +82,7 @@
    /* Enable memory access and bus master */
    pci_write_config32(dev, PCI_COMMAND, UART_PCI_ENABLE);

-	uart_lpss_init(baseaddr);
+	uart_lpss_init(device, baseaddr);
 }

const struct device *uart_get_device(void)
@@ -231,7 +234,7 @@

base = pci_read_config32(dev, PCI_BASE_ADDRESS_0) & ~0xFFF;
    	if (base)
-			uart_lpss_init(base);
+			uart_lpss_init(dev, base);
    }
 }