[XL] Change in coreboot[main]: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC

[Name of user not set (Code Review)]

Attention is currently required from: Angel Pons, Elyes Haouas, Felix Singer, Jérémy Compostella, Shuo Liu, Vasiliy Khoruzhick.

[e-mail] has posted comments on this change by [e-mail]. ( https://review.coreboot.org/c/coreboot/+/83321?usp=email )

Change subject: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC
......................................................................

Patch Set 38:

(8 comments)

File src/soc/intel/snowridge/Makefile.mk:

https://review.coreboot.org/c/coreboot/+/83321/comment/99d0c359_7bb3e612?usp... :
PS28, Line 47: CONFIG_DRIVERS_UART_8250MEM
> Okay, then simply adding a Kconfig option for HSUART should suffice. […]

I've changed to use `CONFIG_HIGH_SPEED_UART`.

File src/soc/intel/snowridge/acpi.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/c77d8439_bcdcc71f?usp... :
PS28, Line 170: 		if (domain_id == 2) {
> Can we have is_dlb_domain(), is_pcie_domain() as if checker?

I updated the code to use device tree alias `dev_dlb_sa_ptr`.

https://review.coreboot.org/c/coreboot/+/83321/comment/99dc2ad3_51bd5286?usp... :
PS28, Line 334: 	struct device *dev = NULL, *pch_domain_sa = NULL, *cpu_domain_sa = NULL;
> You may be able to get a reference to these devices using chipset devicetree aliases. […]

Thanks for your reminder, I've updated all the code that finding device to using device tree alias.

File src/soc/intel/snowridge/acpi.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/f85ac9aa_c5c95faa?usp... :
PS33, Line 95: 		index++;
> I think this index is only used to enumerate the order in which CPUs are found, as it is only used t […]

The increment here is redundant, I will remove it.

File src/soc/intel/snowridge/acpi/pcie.asl:

https://review.coreboot.org/c/coreboot/+/83321/comment/7095a19e_43bb94e2?usp... :
PS28, Line 101: Switch (ToInteger (Arg0)) {
              : 		/* PCIe Root Port 1 */
              : 		Case (Package() { 1 }) {
              : 			If (PICM) {
              : 				Return (IQAA)
              : 			} Else {
              : 				Return (IQAP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 2 */
              : 		Case (Package() { 2 }) {
              : 			If (PICM) {
              : 				Return (IQBA)
              : 			} Else {
              : 				Return (IQBP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 3 */
              : 		Case (Package() { 3 }) {
              : 			If (PICM) {
              : 				Return (IQCA)
              : 			} Else {
              : 				Return (IQCP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 4 */
              : 		Case (Package() { 4 }) {
              : 			If (PICM) {
              : 				Return (IQDA)
              : 			} Else {
              : 				Return (IQDP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 5 */
              : 		Case (Package() { 5 }) {
              : 			If (PICM) {
              : 				Return (IQEA)
              : 			} Else {
              : 				Return (IQEP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 6 */
              : 		Case (Package() { 6 }) {
              : 			If (PICM) {
              : 				Return (IQFA)
              : 			} Else {
              : 				Return (IQFP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 7 */
              : 		Case (Package() { 7 }) {
              : 			If (PICM) {
              : 				Return (IQGA)
              : 			} Else {
              : 				Return (IQGP)
              : 			}
              : 		}
              : 
              : 		/* PCIe Root Port 8 */
              : 		Case (Package() { 8 }) {
              : 			If (PICM) {
              : 				Return (IQHA)
              : 			} Else {
              : 				Return (IQHP)
              : 			}
              : 		}
              : 
              : 		Default {
              : 			If (PICM) {
              : 				Return (IQDA)
              : 			} Else {
              : 				Return (IQDP)
              : 			}
              : 		}
              : 	}
> Could this be replaced by package indexing? […]

I changed it to package indexing and move the method local variable to global to avoid insufficient code.

File src/soc/intel/snowridge/chip.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/bb3e7856_b2f3d2c7?usp... :
PS15, Line 323: 		res = new_resource(dev, IOINDEX_SUBTRACTIVE(index++, 0));
> Marking domain resource as `IORESOURCE_SUBTRACTIVE` have no effects during PCI resource allocation,  […]

I just removed the subtractive flag of domain resources.

File src/soc/intel/snowridge/hqm.c:

https://review.coreboot.org/c/coreboot/+/83321/comment/0296418f_e2622cbe?usp... :
PS28, Line 16: 	pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
> Setting the bus master enable bit of a PCI endpoint device allows the device to issue memory request […]

I just find coreboot has the `pci_dev_request_bus_master()` function, so I replaced the manually setting code with it.

File src/soc/intel/snowridge/include/soc/pci_ids.h:

PS28:
> There's a common `src/include/pci_ids. […]

Ok, please see patch https://review.coreboot.org/c/coreboot/+/84782.