On 9/14/21 10:45 PM, Tom Rini wrote:
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.

Probably check a couple of board schematics and SoC datasheets to build up your own statistics ? In Linux it is 0 because that's likely what most of the systems expect it to be, and the few odd ones just set it to 3 explicitly there (or enable CPOL/CPHA as needed). My statistics indicates that I keep running into "oh, here it is also 3, and it should be 0" recently.

I don't have any of the am335x/atmel boards easily available, so I cannot check those, hence the CC list.

Reply via email to