Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31213
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
soc/intel/cannonlake: Add max uart config
This patch adds SOC_INTEL_UART_DEV_MAX for cannonlake soc.
Change-Id: I7a04558869f966288fae00c8a0a75433b36c9735 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/31213/1
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 0a1cff0..3e8ace3 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -187,6 +187,10 @@ default 4 if SOC_INTEL_CANNONLAKE_PCH_H default 6
+config SOC_INTEL_UART_DEV_MAX + int + default 3 + # Clock divider parameters for 115200 baud rate config SOC_INTEL_COMMON_LPSS_UART_CLK_M_VAL hex
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX Why is this required? It is used only in icelake:
# git grep SOC_INTEL_UART_DEV_MAX src/soc/intel/icelake/Kconfig:config SOC_INTEL_UART_DEV_MAX src/soc/intel/icelake/chip.h: uint8_t SerialIoUartMode[CONFIG_SOC_INTEL_UART_DEV_MAX];
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
Why is this required? It is used only in icelake: […]
this is just to prep for CML up steaming work. we will start by next week sometime.
As CML will select CNP PCH hence we will use same CNL soc codebase.
Adding this kconfig will help us for internal development
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
this is just to prep for CML up steaming work. we will start by next week sometime. […]
Any reason why it has to be a Kconfig option and not a simple macro?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
Any reason why it has to be a Kconfig option and not a simple macro?
its almost same kind of FSP UPD change as ICL where we will fill those LPSS UPDs as below
for (int i = 0; i < CONFIG_SOC_INTEL_I2C_DEV_MAX; i++) params->SerialIoI2cMode[i] = config->SerialIoI2cMode[i];
for (int i = 0; i < CONFIG_SOC_INTEL_COMMON_BLOCK_GSPI_MAX; i++) params->SerialIoSpiMode[i] = config->SerialIoGSpiMode[i];
for (int i = 0; i < CONFIG_SOC_INTEL_UART_DEV_MAX; i++) params->SerialIoUartMode[i] = config->SerialIoUartMode[i];
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
its almost same kind of FSP UPD change as ICL where we will fill those LPSS UPDs as below […]
Right. I meant does it really need to be a Kconfig option though? Doing something like "#define SOC_INTEL_UART_DEV_MAX 3" would be good enough?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
Right. […]
yes, can be done, bt if i recall it correctly then Aaron in UART common code review had explicitly asked to make it Kconfig rather macro.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
yes, can be done, bt if i recall it correctly then Aaron in UART common code review had explicitly a […]
Also kconfig has better readability over macro, hence in my personal choice i really like kconfig. :)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
Also kconfig has better readability over macro, hence in my personal choice i really like kconfig. […]
Common code implementation is different from this. In common code, you want each SoC to provide this info to common code using Kconfig. IIUC, here, the usage is pretty much restricted to defining size of SerialIoUartMode in chip.h.
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Abandoned
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31213 )
Change subject: soc/intel/cannonlake: Add max uart config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31213/1/src/soc/intel/cannonlake/Kconfig@190 PS1, Line 190: SOC_INTEL_UART_DEV_MAX
Common code implementation is different from this. […]
i will abandon this CL and add it with CFL changes as and when required
-
Furquan Shaikh (Code Review) -
Subrata Banik (Code Review)