[S] Change in coreboot[main]: soc/intel/common/block/acpi: Add GPE1 blocks to ACPI FADT table

[Subrata Banik (Code Review)]

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams.

Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84103?usp=email )

Change subject: soc/intel/common/block/acpi: Add GPE1 blocks to ACPI FADT table
......................................................................

Patch Set 7:

(4 comments)

File src/soc/intel/common/block/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/84103/comment/6af78459_cda92e90?usp... :
PS5, Line 87: 	  devices.
> sure. will do this. thx.

Acknowledged

File src/soc/intel/common/block/acpi/acpi.c:

https://review.coreboot.org/c/coreboot/+/84103/comment/9e0c716b_9344f78c?usp... :
PS5, Line 29: GPE1_STS
> Subrata,
>
> One thing left is the case when GPE1 is support but we intend to use GPE0 with this Kconfig not selected, the defines here is duplicated. I add the #ifndef to support this case in the next patchset. Pls let me know what you think. thx.
>
> I also run into the same GPE1 defines issue on the next CL https://review.coreboot.org/c/coreboot/+/84104. I am doing the same for pmclib.c there.

if you don't intend to use GPE1 then ideally you don't select `SOC_INTEL_COMMON_BLOCK_ACPI_USE_GPE1` there could be two config

- SOC_INTEL_COMMON_BLOCK_ACPI_USE_GPE1
- SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1

where `_USE_GPE1` should be dependent on `_HAVE_GPE1`.

Assume a case, when PTL SoC selects `SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1` and Main board can still choose not to use GPE1 hence, main board won't be choosing `_USE_GPE1`.

Does that make sense to you ?

to support this design, please use below logic.

```
#if !CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
 /* NOTE: For SoCs that don't have support for GPE1, add a dummy entry here for common code */
#define GPE1_STS(x) (0x0 + ((x) * 4))
#define GPE1_REG_MAX 0
#endif

```

File src/soc/intel/common/block/acpi/acpi.c:

https://review.coreboot.org/c/coreboot/+/84103/comment/50a29b42_1e50c3d7?usp... :
PS7, Line 121: 	if (CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_USE_GPE1)) {
```
        fadt->gpe1_blk =  GPE1_STS(0) ? (pmbase + GPE1_STS(0)) :  GPE1_STS(0);
```

we don't need redundant guarding using if/else around Kconfig. Therefore, the suggested above code changes.

Also, it's better to assign gpe1_blk to zero for SoC platform that doesn't have support for GPE1.

https://review.coreboot.org/c/coreboot/+/84103/comment/ea58f07b_39278535?usp... :
PS7, Line 128:  8
can you please explain why this is `8` I assume this is like `GPE1_REG_MAX` aka 4 and then two sets like STATUS and ENABLE bits?