Change in coreboot[master]: src/soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: src/soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

src/soc/intel/{cnl,icl}: Add support to configure interrupt overrides

Change-Id: If55701b7ad292e3357d0b419bb6168bd2c3e4030
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/Kconfig
M src/soc/intel/cannonlake/fsp_params.c
M src/soc/intel/icelake/Kconfig
M src/soc/intel/icelake/fsp_params.c
4 files changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34349/1

diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig
index 4235b7a..c8dfc13 100644
--- a/src/soc/intel/cannonlake/Kconfig
+++ b/src/soc/intel/cannonlake/Kconfig
@@ -97,6 +97,7 @@
    select SOC_INTEL_COMMON_BLOCK_XHCI_ELOG
    select SOC_INTEL_COMMON_BLOCK_SMM
    select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP
+	select SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE
    select SOC_INTEL_COMMON_PCH_BASE
    select SOC_INTEL_COMMON_NHLT
    select SOC_INTEL_COMMON_RESET
diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c
index fe5b3dd..4eef58e 100644
--- a/src/soc/intel/cannonlake/fsp_params.c
+++ b/src/soc/intel/cannonlake/fsp_params.c
@@ -18,6 +18,7 @@
 #include <device/pci.h>
 #include <fsp/api.h>
 #include <fsp/util.h>
+#include <intelblocks/irq.h>
 #include <intelblocks/lpss.h>
 #include <intelblocks/xdci.h>
 #include <soc/intel/common/vbt.h>
@@ -408,6 +409,11 @@
    	configure_gspi_cs(i, config,
    			&params->SerialIoSpiCsPolarity[0], NULL, NULL);
 #endif
+
+	/* Fill interrupt config override */
+#if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE)
+	get_irq_table((void *)params->DevIntConfigPtr, &params->NumOfDevIntConfig);
+#endif
 }

/* Mainboard GPIO Configuration */
diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig
index a2261a0..d31c8eb 100644
--- a/src/soc/intel/icelake/Kconfig
+++ b/src/soc/intel/icelake/Kconfig
@@ -51,6 +51,7 @@
    select SOC_INTEL_COMMON_BLOCK_SA
    select SOC_INTEL_COMMON_BLOCK_SMM
    select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP
+	select SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE
    select SOC_INTEL_COMMON_PCH_BASE
    select SOC_INTEL_COMMON_RESET
    select SSE2
diff --git a/src/soc/intel/icelake/fsp_params.c b/src/soc/intel/icelake/fsp_params.c
index ed86c27..ee45024 100644
--- a/src/soc/intel/icelake/fsp_params.c
+++ b/src/soc/intel/icelake/fsp_params.c
@@ -18,6 +18,7 @@
 #include <device/pci.h>
 #include <fsp/api.h>
 #include <fsp/util.h>
+#include <intelblocks/irq.h>
 #include <intelblocks/lpss.h>
 #include <intelblocks/xdci.h>
 #include <soc/intel/common/vbt.h>
@@ -241,6 +242,12 @@

params->Heci3Enabled = config->Heci3Enabled;
    params->Device4Enable = config->Device4Enable;
+
+	/* Fill interrupt config override */
+#if CONFIG(SOC_INTEL_COMMON_ITSS_INTERRUPT_OVERRIDE)
+	get_irq_table((void *)params->DevIntConfigPtr, &params->NumOfDevIntConfig);
+#endif
+
 }

/* Mainboard GPIO Configuration */

[Furquan Shaikh (Code Review)]

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

Change subject: src/soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 3:

(1 comment)

Where is the change to asl code? It needs to go in along with this change, else interrupts would be broken.

https://review.coreboot.org/c/coreboot/+/34349/3/src/soc/intel/icelake/fsp_p... 
File src/soc/intel/icelake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/34349/3/src/soc/intel/icelake/fsp_p... 
PS3, Line 248: DevIntConfigPtr
This is not right. The pointer is set to 0 by default. You need to provide a pointer to the table in memory where the config is stored by coreboot.

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

soc/intel/{cnl,icl}: Add support to configure interrupt overrides

Change-Id: If55701b7ad292e3357d0b419bb6168bd2c3e4030
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/Kconfig
M src/soc/intel/cannonlake/fsp_params.c
M src/soc/intel/cannonlake/irq.c
M src/soc/intel/icelake/Kconfig
M src/soc/intel/icelake/fsp_params.c
M src/soc/intel/icelake/irq.c
6 files changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34349/4

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/ir... 
File src/soc/intel/cannonlake/irq.c:

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/ir... 
PS4, Line 80: 	 * Alocate 128 bytes for irq configuration with assumption
'Alocate' may be misspelled - perhaps 'Allocate'?

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/icelake/irq.c 
File src/soc/intel/icelake/irq.c:

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/icelake/irq.c... 
PS4, Line 80: 	 * Alocate 128 bytes for irq configuration with assumption
'Alocate' may be misspelled - perhaps 'Allocate'?

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 4: Code-Review-1

(1 comment)

ACPI would be wrong with an override. Please use southbridge/intel/common/acpi_pirq_gen.c to automatically generate it.

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/fs... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/fs... 
PS4, Line 410: DevIntConfigPtr
Could you point to some documentation on the format? It's not in coreboot nor in any public FSP integration guide.

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 4:

(1 comment)
> Patch Set 3:
>
> (1 comment)
>
> Where is the change to asl code? It needs to go in along with this change, else interrupts would be broken.

Ack. Will add that implementation over this.

https://review.coreboot.org/c/coreboot/+/34349/3/src/soc/intel/icelake/fsp_p... 
File src/soc/intel/icelake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/34349/3/src/soc/intel/icelake/fsp_p... 
PS3, Line 248: DevIntConfigPtr
> This is not right. The pointer is set to 0 by default. […]

Yes, updated the implementation to just pass the pointer to FSP to irq table populated in coreboot.

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/fs... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/fs... 
PS4, Line 410: DevIntConfigPtr
> Could you point to some documentation on the format? It's not in coreboot nor in any public FSP inte […]

This UPD holds the pointer to array of SI_PCH_DEVICE_INTERRUPT_CONFIG for PCI IRQ configuration , this struct is documented in integration guide.

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins),

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

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

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

soc/intel/{cnl,icl}: Add support to configure interrupt overrides

Change-Id: If55701b7ad292e3357d0b419bb6168bd2c3e4030
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/Kconfig
M src/soc/intel/cannonlake/fsp_params.c
M src/soc/intel/cannonlake/irq.c
M src/soc/intel/icelake/Kconfig
M src/soc/intel/icelake/fsp_params.c
M src/soc/intel/icelake/irq.c
6 files changed, 62 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34349/5

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins),

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

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

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

soc/intel/{cnl,icl}: Add support to configure interrupt overrides

Change-Id: If55701b7ad292e3357d0b419bb6168bd2c3e4030
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
M src/soc/intel/cannonlake/irq.c
M src/soc/intel/icelake/fsp_params.c
M src/soc/intel/icelake/irq.c
4 files changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34349/6

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins),

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

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

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

soc/intel/{cnl,icl}: Add support to configure interrupt overrides

Change-Id: If55701b7ad292e3357d0b419bb6168bd2c3e4030
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
M src/soc/intel/cannonlake/irq.c
M src/soc/intel/icelake/fsp_params.c
M src/soc/intel/icelake/irq.c
4 files changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34349/7

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins),

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

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

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

soc/intel/{cnl,icl}: Add support to configure interrupt overrides

Change-Id: If55701b7ad292e3357d0b419bb6168bd2c3e4030
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
M src/soc/intel/cannonlake/irq.c
M src/soc/intel/icelake/fsp_params.c
M src/soc/intel/icelake/irq.c
4 files changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/34349/10

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Lean Sheng Tan (Code Review)]

Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 15:

there is a merge conflit

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 15: Code-Review-1

(1 comment)

https://review.coreboot.org/c/coreboot/+/34349/15//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34349/15//COMMIT_MSG@7 
PS15, Line 7: overrides
What are you overriding? You read the current configuration from registers and pass that on to FSP? Please explain more thoroughly what you are doing and why this is needed?

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 15: Code-Review-2

(3 comments)

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/fsp_... 
File src/soc/intel/icelake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/fsp_... 
PS15, Line 245: DevIntConfigPtr
What format does it take, What PCI functions should be allow to pass, what are the constraints? Without properly documentating this it is impossible to review this CL.

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/fsp_... 
PS15, Line 246: soc_get_irq_config
Check for NULL?

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/irq.... 
File src/soc/intel/icelake/irq.c:

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/irq.... 
PS15, Line 95: irq_config
This is bad. The scope of the variables is only inside this function!

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34349 )

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/irq.... 
File src/soc/intel/icelake/irq.c:

https://review.coreboot.org/c/coreboot/+/34349/15/src/soc/intel/icelake/irq.... 
PS15, Line 95: irq_config
This is a function-level static variable (line 85)

Change in coreboot[master]: soc/intel/{cnl,icl}: Add support to configure interrupt overrides

[Aamir Bohra (Code Review)]

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

Change subject: soc/intel/{cnl,icl}: Add support to configure interrupt overrides
......................................................................

Patch Set 16:

This change is ready for review.