[coreboot-gerrit] Change in coreboot[master]: mb/google/volteer: New variant for Volteer reworked with Dauntless

[Jes Klinke (Code Review)]

Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46437 )

Change subject: mb/google/volteer: New variant for Volteer reworked with Dauntless
......................................................................

Patch Set 9:

(2 comments)

Thanks for the very useful suggestions.

I have merged the changes into the existing variants/volteer2 directory.  I have not added conditionals to the gpio early initialization list, meaning that both SPI and I2C pads will be configured early for both variants, I doubt that will present a problem.

PTAL

https://review.coreboot.org/c/coreboot/+/46437/6/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/Kconfig:

https://review.coreboot.org/c/coreboot/+/46437/6/src/mainboard/google/voltee... 
PS6, Line 140: volteer2_ti50
> Instead of copying the entire directory, can you still use the volteer2 directory and handle the dif […]

Done

The only actual addition to devicetree was a section that I had copied from one of the older designs using I2C TPM:

chip drivers/i2c/tpm
    	register "hid" = ""GOOG0005""
    	register "irq" = "ACPI_IRQ_EDGE_LOW(GPP_C21_IRQ)"
    	device i2c 50 on end
    end

I assume this is meant to make the kernel know how to communicate with the TPM.  I am surprised though, that I do not see any similar declaration in the SPI section of the existing Volteer devicetree.

I have left it out for now, since for now we will be happy to get coreboot to communicate with the new Dauntless chip, then we can worry about the kernel later.

https://review.coreboot.org/c/coreboot/+/46437/6/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/Kconfig.name:

https://review.coreboot.org/c/coreboot/+/46437/6/src/mainboard/google/voltee... 
PS6, Line 9: MAINBOARD_HAS_SPI_TPM_CR50
> Rather than making each variant select, can this be done in Kconfig using "if BOARD_GOOGLE_VOLTEER2_ […]

Thanks for the suggestion.  I had attempted a top-level if/else/endif, but saw that "else" is not supported by Kconfig in such a context, so I gave up.

With your proposal, the change is much less intrusive.