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.