[M] Change in coreboot[main]: soc/intel/common/block: Add support for watchdog

[Marek Maślanka (Code Review)]

Attention is currently required from: Jakub Czapiga, Michał Żygowski, Subrata Banik.

Marek Maślanka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79909?usp=email )

Change subject: soc/intel/common/block: Add support for watchdog
......................................................................

Patch Set 13:

(8 comments)

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

https://review.coreboot.org/c/coreboot/+/79909/comment/41488cb1_b0fe0436 :
PS12, Line 395: wdat
> missed the NULL check in the code ?

I added checking against the NULL, just in case

https://review.coreboot.org/c/coreboot/+/79909/comment/cf3d2b27_f67f0ce3 :
PS12, Line 397: tcobase
> what if this address is zero like TCO is not enabled ?

Now, it's checked and handled

https://review.coreboot.org/c/coreboot/+/79909/comment/6018259d_b431745b :
PS12, Line 410: // write countdown
> ` […]

Done

https://review.coreboot.org/c/coreboot/+/79909/comment/826fd3e1_2829571b :
PS12, Line 461: 11
> what is the meaning of the hardcoded value ?

I replaced it with a function that takes the offset from a given field register definition to make it more clear and less error-prone.

https://review.coreboot.org/c/coreboot/+/79909/comment/3eb694a1_4b948f99 :
PS12, Line 468: 	entry->action = ACPI_WDAT_SET_RUNNING_STATE;
              : 	entry->instruction = ACPI_WDAT_WRITE_VALUE | ACPI_WDAT_PRESERVE_REGISTER;
              : 	entry->mask = 0x01;
              : 	entry->register_region.space_id = ACPI_ADDRESS_SPACE_IO;
              : 	entry->register_region.addrl = tcobase + TCO1_CNT;
              : 	entry->register_region.access_size = ACPI_WDAT_ACCESS_SIZE_WORD;
              : 	entry->register_region.bit_offset = 11;
> can this be a helper function like […]

I would prefer to keep it consistent with filling in other entries instead of creating separate functions for that.

File src/soc/intel/common/block/smbus/tco.c:

https://review.coreboot.org/c/coreboot/+/79909/comment/dfb2d8fd_f232529f :
PS12, Line 79: {
> single statement, don't need braces

Done

https://review.coreboot.org/c/coreboot/+/79909/comment/4501edc8_52629f8a :
PS12, Line 148: __weak
> why weak ?

right, needless

https://review.coreboot.org/c/coreboot/+/79909/comment/70de1e51_7a23ec02 :
PS12, Line 149: u32
> uint32_t

Done