Change in coreboot[master]: soc/intel/common: Set controller state to active in uart init
Usha P has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34447 )
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.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/1
diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c index 9d820ff..e3ed7c5 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(uintptr_t baseaddr,struct device *dev) { + /* Ensure controller is in D0 state */ + lpss_set_power_state(dev, STATE_D0); + /* Take UART out of reset */ lpss_reset_release(baseaddr);
@@ -81,7 +84,7 @@ /* Enable memory access and bus master */ pci_write_config32(dev, PCI_COMMAND, UART_PCI_ENABLE);
- uart_lpss_init(baseaddr); + uart_lpss_init(baseaddr,device); }
struct device *uart_get_device(void) @@ -224,7 +227,7 @@
base = pci_read_config32(dev, PCI_BASE_ADDRESS_0) & ~0xFFF; if (base) - uart_lpss_init(base); + uart_lpss_init(base,dev); } }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 36: static void uart_lpss_init(uintptr_t baseaddr,struct device *dev) space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 39: lpss_set_power_state(dev, STATE_D0); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 39: lpss_set_power_state(dev, STATE_D0); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 87: uart_lpss_init(baseaddr,device); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 230: uart_lpss_init(base,dev); space required after that ',' (ctx:VxV)
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 36: struct device *dev can u make this as first argument ?
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 39: tab here
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 87: uart_lpss_init(baseaddr,device); one space here
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 230: uart_lpss_init(base,dev); one space here
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/+/34447
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.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 36: static void uart_lpss_init(uintptr_t baseaddr,struct device *dev) space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 39: lpss_set_power_state(dev, STATE_D0); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 39: lpss_set_power_state(dev, STATE_D0); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 87: uart_lpss_init(baseaddr,device); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 230: uart_lpss_init(base,dev); space required after that ',' (ctx:VxV)
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/+/34447
to look at the new patch set (#3).
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.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34447/3/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/3/src/soc/intel/common/block/... PS3, Line 87: uart_lpss_init(baseaddr, device); uart_lpss_init(device, baseaddr);
https://review.coreboot.org/c/coreboot/+/34447/3/src/soc/intel/common/block/... PS3, Line 230: uart_lpss_init(base, dev) uart_lpss_init(dev, base);
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/+/34447
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.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 4:
(1 comment)
No bug report anywhere?
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG@11 PS4, Line 11: Please describe the problem in more detail. When is the UART not working currently. How does your test look like?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 4: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG@11 PS4, Line 11: also worth to mention that you have added 1 more argument in uart_init function ?
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 4:
(9 comments)
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 36: struct device *dev
can u make this as first argument ?
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 36: static void uart_lpss_init(uintptr_t baseaddr,struct device *dev)
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 39: lpss_set_power_state(dev, STATE_D0);
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 39: lpss_set_power_state(dev, STATE_D0);
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 39:
tab here
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 87: uart_lpss_init(baseaddr,device);
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 87: uart_lpss_init(baseaddr,device);
one space here
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 230: uart_lpss_init(base,dev);
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/34447/1/src/soc/intel/common/block/... PS1, Line 230: uart_lpss_init(base,dev);
one space here
Done
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34447/4/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/4/src/soc/intel/common/block/... PS4, Line 87: device Can you change this one to dev?
Furquan Shaikh has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Removed Code-Review+2 by Furquan Shaikh furquan@google.com
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/+/34447
to look at the new patch set (#5).
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 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
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/5
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 5:
(10 comments)
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG@11 PS4, Line 11:
also worth to mention that you have added 1 more argument in uart_init function ?
Done
https://review.coreboot.org/c/coreboot/+/34447/4//COMMIT_MSG@11 PS4, Line 11:
Please describe the problem in more detail. When is the UART not working currently. […]
Done
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 36: static void uart_lpss_init(uintptr_t baseaddr,struct device *dev)
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 39: lpss_set_power_state(dev, STATE_D0);
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 39: lpss_set_power_state(dev, STATE_D0);
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 87: uart_lpss_init(baseaddr,device);
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/34447/2/src/soc/intel/common/block/... PS2, Line 230: uart_lpss_init(base,dev);
space required after that ',' (ctx:VxV)
Done
https://review.coreboot.org/c/coreboot/+/34447/3/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/3/src/soc/intel/common/block/... PS3, Line 87: uart_lpss_init(baseaddr, device);
uart_lpss_init(device, baseaddr);
Done
https://review.coreboot.org/c/coreboot/+/34447/3/src/soc/intel/common/block/... PS3, Line 230: uart_lpss_init(base, dev)
uart_lpss_init(dev, base);
Done
https://review.coreboot.org/c/coreboot/+/34447/4/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/4/src/soc/intel/common/block/... PS4, Line 87: device
Can you change this one to dev?
dev is type casted specifically for the usage of __SIMPLE_DEVICE__. uart_common_init function receives the (struct device *device) parameter. The same has been passed to the uart_lpss_init function which is called in general.
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34447/4/src/soc/intel/common/block/... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/34447/4/src/soc/intel/common/block/... PS4, Line 87: device
dev is type casted specifically for the usage of __SIMPLE_DEVICE__. […]
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 5: Code-Review-1
On which platform was it tested? As you touch common code, please test on all affected platforms.
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/+/34447
to look at the new patch set (#6).
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 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 and ICL platforms.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/6
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 6: Code-Review+1
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 6:
Patch Set 5: Code-Review-1
On which platform was it tested? As you touch common code, please test on all affected platforms.
Done
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG@11 PS6, Line 11: One more argument struct device *dev has been added to uart_lpss_init function for the same. please split this in 2 lines.
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG@14 PS6, Line 14: Verify no timeouts seen during UART controller enumeration sequence in CML and ICL platforms. ditto.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 6:
(1 comment)
Patch Set 6:
Patch Set 5: Code-Review-1
On which platform was it tested? As you touch common code, please test on all affected platforms.
Done
Does it work on SPT, KBP, APL, GLK, CNP?
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG@11 PS6, Line 11: One more argument struct device *dev has been added to uart_lpss_init function for the same. please limit the line length in the commits message as per coding guidelines
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 6: -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/+/34447
to look at the new patch set (#7).
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 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 and ICL platforms.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/7
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG@11 PS6, Line 11: One more argument struct device *dev has been added to uart_lpss_init function for the same.
please split this in 2 lines.
Done
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG@11 PS6, Line 11: One more argument struct device *dev has been added to uart_lpss_init function for the same.
please limit the line length in the commits message as per coding guidelines
Done
https://review.coreboot.org/c/coreboot/+/34447/6//COMMIT_MSG@14 PS6, Line 14: Verify no timeouts seen during UART controller enumeration sequence in CML and ICL platforms.
ditto.
Done
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/+/34447
to look at the new patch set (#8).
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 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 and ICL platforms.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e --- 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/47/34447/8
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 8:
Patch Set 6:
(1 comment)
Patch Set 6:
Patch Set 5: Code-Review-1
On which platform was it tested? As you touch common code, please test on all affected platforms.
Done
Does it work on SPT, KBP, APL, GLK, CNP?
The EDS register offsets are same. Hence it will work on those PCH as well.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 8: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 8: Code-Review+2
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
Patch Set 8: Code-Review+2
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34447 )
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 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 and ICL platforms.
Signed-off-by: Usha P usha.p@intel.com Change-Id: I0187267670e1dea3e1d5e83d0b29967724d6063e Reviewed-on: https://review.coreboot.org/c/coreboot/+/34447 Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: V Sowmya v.sowmya@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/common/block/uart/uart.c 1 file changed, 6 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Aamir Bohra: Looks good to me, approved V Sowmya: Looks good to me, approved
Objections: Patrick Rudolph: I would prefer that you didn't submit this
diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c index 9d820ff..82e5df4 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(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);
@@ -81,7 +84,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); }
struct device *uart_get_device(void) @@ -224,7 +227,7 @@
base = pci_read_config32(dev, PCI_BASE_ADDRESS_0) & ~0xFFF; if (base) - uart_lpss_init(base); + uart_lpss_init(dev, base); } }
Christian Walter has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/34447 )
Change subject: soc/intel/common: Set controller state to active in uart init ......................................................................
-
Aamir Bohra (Code Review) -
build bot (Jenkins) (Code Review)
-
Christian Walter (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)
-
Usha P (Code Review)
-
V Sowmya (Code Review)