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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id A7934C433EF for ; Mon, 11 Jul 2022 15:52:25 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6519E8401B; Mon, 11 Jul 2022 17:52:23 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id CAA8284038; Mon, 11 Jul 2022 17:52:21 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 1B1F083FCC for ; Mon, 11 Jul 2022 17:52:19 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CA2731596; Mon, 11 Jul 2022 08:52:18 -0700 (PDT) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9C08D3F792; Mon, 11 Jul 2022 08:52:17 -0700 (PDT) Date: Mon, 11 Jul 2022 16:52:15 +0100 From: Andre Przywara To: Jagan Teki , Vignesh R , Simon Glass , Tom Rini , U-Boot Mailing List Cc: Icenowy Zheng Subject: U-Boot SPI DM framework issues Message-ID: <20220711165215.218e21ae@donnerap.cambridge.arm.com> Organization: ARM X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.6 at phobos.denx.de X-Virus-Status: Clean Hi, while trying to debug some nasty SPI flash issue on sunxi, I came across some oddities in the SPI DM framework, and wanted to check some things, since I am not sure whether I miss things here: - When I do a simple "sf probe" call, I see *two* calls to the .claim_bus callback: the first one from: drivers/mtd/spi/sf_probe.c:spi_flash_probe_slave(), which is expected, but then also, shortly afterwards, a second call from: drivers/spi/spi-mem.c:spi_mem_exec_op(). After the probe operation finished, I see the corresponding two spi_release_bus() calls. Seeing *two* claim calls in that nested way looks rather odd, and the name *claim* sounds like it should be only one slave device/driver being able to, well, claim the bus. From digging through the code it looks like we call spi_mem_exec_op() only from *inside* drivers that already claimed the bus, so shall we drop the call inside spi_mem_exec_op()? Allowing those nested calls has some consequences for the implementation of .claim_bus in the controller drivers, as it does not sound indicated to enabled/disable hardware in there. Enabling clock gates *again* might not be harmful per se, but if the first .release_bus call already disables clocks and resets, this might cause problems. - In many controller drivers I see the .of_to_plat implementation checking for a "spi-max-frequency" property in the *controller's* DT node, mostly with a fallback value, and using that frequency as an upper bus frequency limit for the whole controller operation. But looking at the official DT bindings, this property is supposed to be a *slave device* property, and indeed checking a few dozen .dts file I see it only being used in *child* nodes of SPI controllers, never in controller nodes. There is some proper usage in drivers/spi/spi-uclass.c:spi_slave_of_to_plat(), but to me it looks like the bus node code is wrong and should be removed, from the other invocation in spi-uclass.c and all the controller drivers. For sunxi (and probably others) this has the consequence of limiting the whole bus to some "default" frequency limit, which is rather low: #define SUN4I_SPI_DEFAULT_RATE 1000000 plat->max_hz = fdtdec_get_int(gd->fdt_blob, node, "spi-max-frequency", SUN4I_SPI_DEFAULT_RATE); So we never go beyond this 1 MHz, even though the SPI flash chips explicitly advertise 40MHz or more in their child nodes. Please let me know if I am missing something here! If not, I am happy to send some patches aiming at fixing those things. Cheers, Andre