Hi Jagan, Thanks for the review.
> -----Original Message----- > From: Jagan Teki <ja...@amarulasolutions.com> > Sent: Tuesday, May 3, 2022 8:32 PM > To: Chin-Ting Kuo <chin-ting_...@aspeedtech.com> > Subject: Re: [PATCH 3/8] spi: aspeed: Add ASPEED SPI controller driver > > On Thu, Apr 14, 2022 at 4:54 PM Chin-Ting Kuo > <chin-ting_...@aspeedtech.com> wrote: > > > > Add ASPEED BMC FMC/SPI memory controller driver with spi-mem interface > > for AST2500 and AST2600 platform. > > > > There are three SPI memory controllers embedded in an ASPEED SoC. > > - FMC: Named as Firmware Memory Controller. After AC on, MCU ROM > > fetches initial device boot image from FMC chip select(CS) 0. > > > > - SPI1: Play the role of a SPI Master controller. Or, there is a > > dedicated path for HOST(X86) to access its BIOS flash mounted > > under BMC. spi-aspeed.c implements the control sequence when > > SPI1 is a SPI master. > > > > - SPI2: It is a pure SPI flash controller. For most scenarios, flashes > > mounted under it are for pure storage purpose. > > > > ASPEED SPI controller supports 1-1-1, 1-1-2 and 1-1-4 SPI flash mode. > > Three types of command mode are supported, normal mode, command > > read/write mode and user mode. > > - Normal mode: Default mode. After power on, normal read command 03h > or > > 13h is used to fetch boot image from SPI flash. > > - AST2500: Only 03h command can be used after power > on > > or reset. > > - AST2600: If FMC04[6:4] is set, 13h command is used, > > otherwise, 03h command. > > The address length is decided by FMC04[2:0]. > > > > - Command mode: SPI controller can send command and address > > automatically when CPU read/write the related > remapped > > or decoded address area. The command used by this > mode > > can be configured by FMC10/14/18[23:16]. Also, the > > address length is decided by FMC04[2:0]. This mode will > > be implemented in the following patch series. > > > > - User mode: It is a traditional and pure SPI operation, where > > SPI transmission is controlled by CPU. It is the main > > mode in this patch. > > > > Each SPI controller in ASPEED SoC has its own decoded address mapping. > > Within each SPI controller decoded address, driver can assign a > > specific address region for each CS of a SPI controller. The decoded > > address cannot overlap to each other. With normal mode and command > > mode, the decoded address accessed by the CPU determines which CS is > active. > > When user mode is adopted, the CS decoded address is a FIFO, CPU can > > send/receive any SPI transmission by accessing the related decoded > > address for the target CS. > > > > Signed-off-by: Chin-Ting Kuo <chin-ting_...@aspeedtech.com> > > --- > > configs/evb-ast2500_defconfig | 13 + configs/evb-ast2600_defconfig > > | 13 + > > drivers/spi/Kconfig | 8 + > > drivers/spi/Makefile | 1 + > > drivers/spi/spi-aspeed.c | 822 > ++++++++++++++++++++++++++++++++++ > > 5 files changed, 857 insertions(+) > > create mode 100644 drivers/spi/spi-aspeed.c > > > > diff --git a/configs/evb-ast2500_defconfig > > b/configs/evb-ast2500_defconfig index 53fe7776e9..f00bded237 100644 > > --- a/configs/evb-ast2500_defconfig > > +++ b/configs/evb-ast2500_defconfig > > @@ -36,6 +36,16 @@ CONFIG_DM_I2C=y > > CONFIG_SYS_I2C_ASPEED=y > > CONFIG_MMC_SDHCI=y > > CONFIG_MMC_SDHCI_ASPEED=y > > +CONFIG_DM_SPI_FLASH=y > > +CONFIG_SPI_FLASH_SFDP_SUPPORT=y > > +CONFIG_SPI_FLASH_GIGADEVICE=y > > +CONFIG_SPI_FLASH_ISSI=y > > +CONFIG_SPI_FLASH_MACRONIX=y > > +CONFIG_SPI_FLASH_SPANSION=y > > +CONFIG_SPI_FLASH_STMICRO=y > > +CONFIG_SPI_FLASH_SST=y > > +CONFIG_SPI_FLASH_WINBOND=y > > +# CONFIG_SPI_FLASH_USE_4K_SECTORS is not set > > CONFIG_PHY_REALTEK=y > > CONFIG_DM_ETH=y > > CONFIG_FTGMAC100=y > > @@ -45,6 +55,9 @@ CONFIG_RAM=y > > CONFIG_DM_RESET=y > > CONFIG_DM_SERIAL=y > > CONFIG_SYS_NS16550=y > > +CONFIG_SPI=y > > +CONFIG_DM_SPI=y > > +CONFIG_SPI_ASPEED=y > > CONFIG_SYSRESET=y > > CONFIG_TIMER=y > > CONFIG_WDT=y > > diff --git a/configs/evb-ast2600_defconfig > > b/configs/evb-ast2600_defconfig index ea75762926..e013976bb0 100644 > > --- a/configs/evb-ast2600_defconfig > > +++ b/configs/evb-ast2600_defconfig > > @@ -66,6 +66,16 @@ CONFIG_MISC=y > > CONFIG_SPL_MISC=y > > CONFIG_MMC_SDHCI=y > > CONFIG_MMC_SDHCI_ASPEED=y > > +CONFIG_DM_SPI_FLASH=y > > +CONFIG_SPI_FLASH_SFDP_SUPPORT=y > > +CONFIG_SPI_FLASH_GIGADEVICE=y > > +CONFIG_SPI_FLASH_ISSI=y > > +CONFIG_SPI_FLASH_MACRONIX=y > > +CONFIG_SPI_FLASH_SPANSION=y > > +CONFIG_SPI_FLASH_STMICRO=y > > +CONFIG_SPI_FLASH_SST=y > > +CONFIG_SPI_FLASH_WINBOND=y > > +# CONFIG_SPI_FLASH_USE_4K_SECTORS is not set > > CONFIG_PHY_REALTEK=y > > CONFIG_DM_ETH=y > > CONFIG_DM_MDIO=y > > @@ -78,6 +88,9 @@ CONFIG_SPL_RAM=y > > CONFIG_DM_RESET=y > > CONFIG_DM_SERIAL=y > > CONFIG_SYS_NS16550=y > > +CONFIG_SPI=y > > +CONFIG_DM_SPI=y > > +CONFIG_SPI_ASPEED=y > > CONFIG_SYSRESET=y > > CONFIG_SPL_SYSRESET=y > > CONFIG_WDT=y > > Separate patches for spi driver and config enablements. Do you mean that the following change should be separated into a new standalone patch? If yes, maybe, I also need to move the remaining changes of this config file to the other new patch. +CONFIG_SPI_ASPEED=y Best Wishes, Chin-Ting