[coreboot-gerrit] Change in coreboot[master]: mb/google/asurada: Fixup BOOT_DEVICE_SPI_FLASH_BUS default value

[Hung-Te Lin (Code Review)]

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44452 )

Change subject: mb/google/asurada: Fixup BOOT_DEVICE_SPI_FLASH_BUS default value
......................................................................

Patch Set 1: Code-Review-1

(1 comment)

Are you sure this is really going to work?

The spi_ctrlr_bus_map works by matching bus number. If you define flash_bus to 0, then then bus_map cannot identify if you are going to init SPI NOR or SPI.

Take a look at 8173, they use the trick: (8173 uses TPM on I2C so there's only one SPI for EC):

CONFIG_SPI_BUS_NUMBER = 1
 CONFIG_BOOT_DEVICE_SPI_FLASH_BUS = 9

const struct spi_ctrlr_buses spi_ctrlr_bus_map[] = {
        {
                .ctrlr = &spi_ctrlr,
                .bus_start = 0,
                .bus_end = SPI_BUS_NUMBER - 1,
        },
        {
                .ctrlr = &spi_flash_ctrlr,
                .bus_start = CONFIG_BOOT_DEVICE_SPI_FLASH_BUS,
                .bus_end = CONFIG_BOOT_DEVICE_SPI_FLASH_BUS,
        },
 };

So the CONFIG_BOOT_DEVICE_SPI_FLASH_BUS is a virtual number for targeting SPI-NOR.

As a result, I think the right config for 8192 is

https://review.coreboot.org/c/coreboot/+/44452/1/src/mainboard/google/asurad... 
File src/mainboard/google/asurada/Kconfig:

https://review.coreboot.org/c/coreboot/+/44452/1/src/mainboard/google/asurad... 
PS1, Line 46: 0
Are you sure this is really going to work?

The spi_ctrlr_bus_map works by matching bus number. If you define flash_bus to 0, then then bus_map cannot identify if you are going to init SPI NOR or SPI.

Take a look at 8173, they use the trick: (8173 uses TPM on I2C so there's only one SPI for EC):

SPI_BUS_NUMBER = 1
 CONFIG_BOOT_DEVICE_SPI_FLASH_BUS = 9

const struct spi_ctrlr_buses spi_ctrlr_bus_map[] = {
        {
                .ctrlr = &spi_ctrlr,
                .bus_start = 0,
                .bus_end = SPI_BUS_NUMBER - 1,
        },
        {
                .ctrlr = &spi_flash_ctrlr,
                .bus_start = CONFIG_BOOT_DEVICE_SPI_FLASH_BUS,
                .bus_end = CONFIG_BOOT_DEVICE_SPI_FLASH_BUS,
        },
 };

So the CONFIG_BOOT_DEVICE_SPI_FLASH_BUS is a virtual number for targeting SPI-NOR.

As a result, I think the right config for 8192 is to follow same rule, and add a comment explaining that:

# On MT8192 the SPI flash is actually using a SPI-NOR controller with its own bus.
 # The number here should be a virtual value as (SPI_BUS_NUMBER + 1).
 default 9