From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C81CC433F5 for ; Tue, 14 Sep 2021 21:44:41 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A7E5F60F46 for ; Tue, 14 Sep 2021 21:44:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A7E5F60F46 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B6A128032E; Tue, 14 Sep 2021 23:44:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="YCpZbUMI"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9EE1680224; Tue, 14 Sep 2021 23:44:35 +0200 (CEST) Received: from mail-qk1-x730.google.com (mail-qk1-x730.google.com [IPv6:2607:f8b0:4864:20::730]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id EA72580224 for ; Tue, 14 Sep 2021 23:44:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qk1-x730.google.com with SMTP id m21so1101295qkm.13 for ; Tue, 14 Sep 2021 14:44:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=8uVZ1J067Cb7jl8wwTAp41Ye9XSKPzyoBvIMded37yc=; b=YCpZbUMIgXT9vqE8D8vI+Y9J4pVukbBZlxX4FPF0ELl7/zi0Sq5A1SNzjL6+gMJHdc AgDMLupF7CXm46UCsg6B6YiQj+dR+YaYtWE4VCINOxd2p43zoUvZYySmhMdGCbWKzs2T Ns1a4H5RtF+5wQE0801KLzZOZg1Ml8zkUzZ+4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=8uVZ1J067Cb7jl8wwTAp41Ye9XSKPzyoBvIMded37yc=; b=yzR1ImlidxrEbfSZ++tXDem+mgE9ybhD5A5Le/xW1wjGoDbBR6K5wAINGIpJvYND41 B+Q5O4WLTuklzm2LQCinzrOCHNq6kBLj+lNGZXaS3Z5jiSROrGXghj+F6KAuCIpqR3XJ dOhgNymC1ssyEdCXtyquj7+dJHfStvIEDsarMVcyNBCKxrrfEiY7j2EYjKRmdmGGoDem PQ51nYtZX5T2Vocur02wgK9rJqdqxpuIusrZSmM/sfYJFCi8k4JVur1H+lPXtXWXPaNE n6J7OZTkKhKk9o/1jOd2careEBbSabLON/5tBhiBhxbu6HrbInDvXKbOvfUpEbKkA0xn 6CiA== X-Gm-Message-State: AOAM532/gw6aup+vSwgUCHLPUeuwNvTWroLiUGcvcpA4HzGZWGqER2HG h/nAycYd2iQ2H3maB/szgGDg3w== X-Google-Smtp-Source: ABdhPJxuH9HmRLdJV7pMTrK/wk+Z2NSZaKPo5RIClqPfubXu3nDcrcqDZEmwHLaAGyeRoJGo0ZxPLw== X-Received: by 2002:a37:66cb:: with SMTP id a194mr7041890qkc.345.1631655868625; Tue, 14 Sep 2021 14:44:28 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-bdd8-2e2f-c73c-581f.res6.spectrum.com. [2603:6081:7b01:cbda:bdd8:2e2f:c73c:581f]) by smtp.gmail.com with ESMTPSA id 90sm6861038qte.89.2021.09.14.14.44.27 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 14 Sep 2021 14:44:27 -0700 (PDT) Date: Tue, 14 Sep 2021 17:44:25 -0400 From: Tom Rini To: Marek Vasut Cc: u-boot@lists.denx.de, Aleksandar Gerasimovski , Andreas Biessmann , Eugen Hristev , Michal Simek , Patrice Chotard , Patrick Delaunay , Peng Fan , Siew Chin Lim , Valentin Longchamp , Vignesh Raghavendra Subject: Re: [RFC][PATCH] mtd: spi: Set CONFIG_SF_DEFAULT_MODE default to 0 Message-ID: <20210914214425.GL12964@bill-the-cat> References: <20210914182824.139667-1-marex@denx.de> <20210914191942.GJ12964@bill-the-cat> <895ddad9-92e8-1940-be1d-8d55fb1c6008@denx.de> <20210914204531.GK12964@bill-the-cat> <4d242cc1-9f48-7dec-9edf-9c6121b4d1d0@denx.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="FrUuTWcY1UW1Vh+U" Content-Disposition: inline In-Reply-To: <4d242cc1-9f48-7dec-9edf-9c6121b4d1d0@denx.de> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean --FrUuTWcY1UW1Vh+U Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 14, 2021 at 11:30:01PM +0200, Marek Vasut wrote: > 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: > > > >=20 > > > > > Before e2e95e5e254 ("spi: Update speed/mode on change") most syst= ems > > > > > silently defaulted to SF bus mode 0. Now the mode is always updat= ed, > > > > > which causes breakage. It seems most SF which are used as boot me= dia > > > > > operate in bus mode 0, so switch that as the default. > > > > >=20 > > > > > This should fix booting at least on Altera SoCFPGA, ST STM32, Xil= inx > > > > > ZynqMP, NXP iMX and Rockchip SoCs, which recently ran into trouble > > > > > with mode 3. Marvell Kirkwood and Xilinx microblaze need to be ch= ecked > > > > > as those might need mode 3. > > > > >=20 > > > > > Signed-off-by: Marek Vasut > > > > > Cc: Aleksandar Gerasimovski > > > > > Cc: Andreas Biessmann > > > > > Cc: Eugen Hristev > > > > > Cc: Michal Simek > > > > > Cc: Patrice Chotard > > > > > Cc: Patrick Delaunay > > > > > Cc: Peng Fan > > > > > Cc: Siew Chin Lim > > > > > Cc: Tom Rini > > > > > Cc: Valentin Longchamp > > > > > Cc: Vignesh Raghavendra > > > >=20 > > > > 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 defi= ned, > > > > to include/spi_flash.h, and a more visible global default than it w= as > > > > 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_nand= flash > > > > at91sam9n12ek_spiflash at91sam9x5ek_dataflash at91sam9x5ek_mmc > > > > at91sam9x5ek_nandflash at91sam9x5ek_spiflash atstk1004 bf506f-ezkit > > > > bf537-stamp cam_enc_4xx coreboot-x86 d2net_v2 da830evm da850_am18xx= evm > > > > 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 ls= chlv2 > > > > 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 vf61= 0twr > > > > zynq_zc770_xm010 > > > >=20 > > > > Of those platforms, I have handy dra7xx_evm (also am335x_evm but th= at > > > > 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 r= esult > > > > back. So, SPI_MODE_3 seems correct / function here, and likely so = on > > > > the rest of the TI platforms listed above. > > > >=20 > > > > 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. > > > >=20 > > > > 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 prefe= r to > > > > see "default 0 if ARCH_SOCFPGA || ARCH_STM32 || .." as needed to sw= itch > > > > 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 enab= led. > > >=20 > > > 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 ? > >=20 > > 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. >=20 > 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. >=20 > I don't have any of the am335x/atmel boards easily available, so I cannot > check those, hence the CC list. Sigh, OK. Lets look at the kernel and there instead of SPI_MODE_N we have spi-cpol and spi-cpha as properties. A quick git grep -l spi-cp arch/arm arch/arm64 shows where we likely do and do not need to use something other than mode 0. So yes, mode 0 as a default makes sense, so long as we also don't break the easily found boards that do need mode 3 (or, mode 1 or 2, as of v5.19 it is true there's 59 matches for each property, but it's not always both, sometimes it's one or the other). --=20 Tom --FrUuTWcY1UW1Vh+U Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmFBF7MACgkQFHw5/5Y0 tyxuyAv/f8505ZMNG3+hVNupIWhro9ZFKGEu05ZamLaBIvVZeqe1Zj6urV1cncNW +k1bu/a+d9qczZjKkIlYJvUe8VLcAIf3VAmvLz0eqQRoc1QRnCgsHphjYti6BmLB bAuUxtPiqZdfCYpnCtl35ZyAKUEUQeOQ+GIYMHTL024J5F3kd32NTihv8T90y81r dvxhikskFnghkOq8dM1kWPbJHsvZiWexwsA9f56jZEpEBtrp9I7P7XFy//LUFg6/ hJomaqi+zB4vjydMwuodgySODDKtMHuHvbZgxrpnwrI4ZjI5C1da8ADfVVyMj6Hu +iTLbkKEJSF3CQ6uGyqJ4fu8nth28fT+gY7X90F8Wdn4qoG7jbedIgKsfU7F9x/1 vk3YS0DU7zTl240E6IDQuOmze/v7Y8Nzcx/hpXKbshtHLvKprfulOlgKEIRlU2Rf 1HDM+/VHue8lRw1tDQlf8ixI3h5qnJl1fXsAQFHMR9xcNrEdb7GiYmxQSuZNFYuF ghn+VBn7 =wiL3 -----END PGP SIGNATURE----- --FrUuTWcY1UW1Vh+U--