On Tue, Sep 14, 2021 at 10:10:22PM +0200, Marek Vasut wrote: > On 9/14/21 9:19 PM, Tom Rini wrote: > > On Tue, Sep 14, 2021 at 08:28:24PM +0200, Marek Vasut wrote: > > > > > Before e2e95e5e254 ("spi: Update speed/mode on change") most systems > > > silently defaulted to SF bus mode 0. Now the mode is always updated, > > > which causes breakage. It seems most SF which are used as boot media > > > operate in bus mode 0, so switch that as the default. > > > > > > This should fix booting at least on Altera SoCFPGA, ST STM32, Xilinx > > > ZynqMP, NXP iMX and Rockchip SoCs, which recently ran into trouble > > > with mode 3. Marvell Kirkwood and Xilinx microblaze need to be checked > > > as those might need mode 3. > > > > > > Signed-off-by: Marek Vasut <ma...@denx.de> > > > Cc: Aleksandar Gerasimovski > > > <aleksandar.gerasimov...@hitachi-powergrids.com> > > > Cc: Andreas Biessmann <andr...@biessmann.org> > > > Cc: Eugen Hristev <eugen.hris...@microchip.com> > > > Cc: Michal Simek <michal.si...@xilinx.com> > > > Cc: Patrice Chotard <patrice.chot...@foss.st.com> > > > Cc: Patrick Delaunay <patrick.delau...@foss.st.com> > > > Cc: Peng Fan <peng....@nxp.com> > > > Cc: Siew Chin Lim <elly.siew.chin....@intel.com> > > > Cc: Tom Rini <tr...@konsulko.com> > > > Cc: Valentin Longchamp <valentin.longch...@hitachi-powergrids.com> > > > Cc: Vignesh Raghavendra <vigne...@ti.com> > > > > So, some background. With commit 88e34e5ff76b ("spl: replace > > CONFIG_SPL_SPI_* with CONFIG_SF_DEFAULT_*") CONFIG_SF_DEFAULT_MODE and > > related moved from having their default value of SPI_MODE_3 (which > > evaluates to 3) be defined in common/cmd_sf.c if not otherwise defined, > > to include/spi_flash.h, and a more visible global default than it was > > before. At that time, the following platforms either set or implied > > SPI_MODE_3 should be used: > > alt am335x_boneblack am335x_boneblack_vboot am335x_evm_norboot > > am335x_evm_nor am335x_evm am335x_evm_spiboot am335x_evm_usbspl > > am43xx_evm am43xx_evm_qspiboot at91sam9n12ek_mmc at91sam9n12ek_nandflash > > at91sam9n12ek_spiflash at91sam9x5ek_dataflash at91sam9x5ek_mmc > > at91sam9x5ek_nandflash at91sam9x5ek_spiflash atstk1004 bf506f-ezkit > > bf537-stamp cam_enc_4xx coreboot-x86 d2net_v2 da830evm da850_am18xxevm > > da850evm_direct_nor da850evm dra7xx_evm dra7xx_evm_qspiboot > > dra7xx_evm_uart3 draco dreamplug dxr2 ea20 ecovec ethernut5 gplugd > > inetspace_v2 k2e_evm k2hk_evm kmcoge5un km_kirkwood_128m16 > > km_kirkwood_pci km_kirkwood kmnusa kmsugp1 kmsuv31 koelsch lager lschlv2 > > lsxhl M53017EVB M54455EVB mgcoge3un mv88f6281gtw_ge net2big_v2 > > netspace_lite_v2 netspace_max_v2 netspace_mini_v2 netspace_v2 > > nios2-generic pcm051_rev1 pcm051_rev3 portl2 pxm2 rut sama5d3xek_mmc > > sama5d3xek_nandflash sama5d3xek_spiflash sh7785lcr tseries_spi vf610twr > > zynq_zc770_xm010 > > > > Of those platforms, I have handy dra7xx_evm (also am335x_evm but that > > needs flipping some DIP switches) which has SPI flash by default. With > > current tip of tree, I did "sf probe 0 76800000 3" and a few cycles of > > reading the whole of flash and crc32'ing it, and getting the same result > > back. So, SPI_MODE_3 seems correct / function here, and likely so on > > the rest of the TI platforms listed above. > > > > With commit 14453fbfadc2 ("Convert CONFIG_SF_DEFAULT_* to Kconfig") we > > migrate these values to Kconfig, and a quick spot-check shows that yes, > > the migration did not change any values. > > > > A big change between 88e34e5ff76b and 14453fbfadc2 is that a ton of > > platforms have been added (it was 5 years, after all) and those > > platforms seem to be where the problems reside. I'm not sure if or why > > SPI_MODE_3 doesn't work on so many new platforms, but I would prefer to > > see "default 0 if ARCH_SOCFPGA || ARCH_STM32 || .." as needed to switch > > the default to SPI_MODE_0 if there's not a more fundamental problem to > > solve on some platforms such that SPI_MODE_3 could / should be enabled. > > I rather suspect mode 3 is special case for atmel/am335x and co. and maybe > marvell kirkwood from the list above. So shouldn't we default to 0 and > special-case the two or three instead ?
I guess my question boils down to why is that the right default? My argument that 3 is the right default is that it's what it was in introduction back in b6368467e6a9 ("SPI Flash: Add "sf" command"). But references to something else such as the Linux Kernel defaults to mode 0 or some SPI specs saying you should default to mode 0 or something else would be much appreciated. -- Tom
signature.asc
Description: PGP signature