From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 81F77248BA1 for ; Wed, 15 Jan 2025 10:50:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736938203; cv=none; b=hZvbbB06VNhy2XR6SFEiQFN4g42O+QzmxAsHesCwQRYD2X24sN+vdUGudViVecHHZY/5bk+wDnqadyf2WaxvIlmHSi2A0F57V0UhuzIAl6+4aJRIeLIMxe74DTPxvfq+l0yyBAE+Ks1OfRqlZcJ44lol+XgCdOSYq1kHqO2TbiY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736938203; c=relaxed/simple; bh=kmRUx+T9z5hOdY53lps5YelMiPZ/R3uuXcRBKA9ZxK4=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=enSZlx4+EJBCpMdQkom+Vu/JcKSbxZap9YxDfYhVtm4z+CEFgTQCguq58/3QUVgtgaEGUSbhRNaoTOOGjgeqS/Dg1XV1ahX72EkNZe5Q6PU88Nu6Qz1UU8Li9x+2ukEr9d9hme74KoRvIPXOc0GD42YgrekkMTJ+1dFu220f6E0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=P1HrxVrf; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="P1HrxVrf" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 29FE9C4CEE1; Wed, 15 Jan 2025 10:50:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736938203; bh=kmRUx+T9z5hOdY53lps5YelMiPZ/R3uuXcRBKA9ZxK4=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=P1HrxVrfTQ7oAQTq/YgYe8rkzguo/akbWeQxglq7lPnPMITTwkI6MIDhTTzhRbnbS MzCNHtH5KDBrI/O1rVFWYO/uBayzC2uX8kxdSS0VTE3SVBNI5p8JYZJqr5yVb5m+03 jno6V7THXeB1Lr6vNDHmbUFxwCmtDk0Qgjj327Xnzmfb2F5+eNCaevuNTZoosYtVIo 4/JVcts/92iteghjbsNYtVLSKUTfnBDlo9TbHyKOMIseyvH6ezHeDgbOENP9QMgCHF yvsEu0XDYD5+WYmTq/U8jBzfyBTppMA8a52T5YoMUTW7faLJ640a1VcgHo6q10zTNA O29JUl3g+xA2A== From: Pratyush Yadav To: Tudor Ambarus Cc: Pratyush Yadav , Michael Walle , Miquel Raynal , Richard Weinberger , Vignesh Raghavendra , Cheng Ming Lin , Alexander Stein , linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, alvinzhou@mxic.com.tw, leoyu@mxic.com.tw, Cheng Ming Lin Subject: Re: [PATCH] Revert "mtd: spi-nor: core: replace dummy buswidth from addr to data" In-Reply-To: <878f66c3-83a4-4f83-88d8-ea2917d82d76@linaro.org> (Tudor Ambarus's message of "Wed, 15 Jan 2025 10:29:25 +0000") References: <20250115101659.55882-1-pratyush@kernel.org> <878f66c3-83a4-4f83-88d8-ea2917d82d76@linaro.org> Date: Wed, 15 Jan 2025 10:50:00 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Wed, Jan 15 2025, Tudor Ambarus wrote: > On 1/15/25 10:16 AM, Pratyush Yadav wrote: >> This reverts commit 98d1fb94ce75f39febd456d6d3cbbe58b6678795. The commit > > "The commit" on a new paragraph please ACK, will do when applying. > >> uses data nbits instead of addr nbits for dummy phase. This causes a >> regression for all boards where spi-tx-bus-width is smaller than >> spi-rx-bus-width. It is a common pattern for boards to have >> spi-tx-bus-width == 1 and spi-rx-bus-width > 1. The regression causes >> all reads with a dummy phase to become unavailable for such boards, >> leading to a usually slower 0-dummy-cycle read being selected. >> >> Most controllers' supports_op hooks call spi_mem_default_supports_op(). >> In spi_mem_default_supports_op(), spi_mem_check_buswidth() is called to >> check if the buswidths for the op can actually be supported by the >> board's wiring. This wiring information comes from (among other things) >> the spi-{tx,rx}-bus-width DT properties. Based on these properties, >> SPI_TX_* or SPI_RX_* flags are set by of_spi_parse_dt(). >> spi_mem_check_buswidth() then uses these flags to make the decision >> whether an op can be supported by the board's wiring (in a way, >> indirectly checking against spi-{rx,tx}-bus-width). >> >> Now the tricky bit here is that spi_mem_check_buswidth() does: >> >> if (op->dummy.nbytes && >> spi_check_buswidth_req(mem, op->dummy.buswidth, true)) >> return false; >> >> The true argument to spi_check_buswidth_req() means the op is treated as >> a TX op. For a board that has say 1-bit TX and 4-bit RX, a 4-bit dummy >> TX is considered as unsupported, and the op gets rejected. >> >> The commit being reverted uses the data buswidth for dummy buswidth. So >> for reads, the RX buswidth gets used for the dummy phase, uncovering >> this issue. In reality, a dummy phase is neither RX nor TX. As the name >> suggests, these are just dummy cycles that send or receive no data, and >> thus don't really need to have any buswidth at all. >> >> Ideally, dummy phases should not be checked against the board's wiring >> capabilities at all, and should only be sanity-checked for having a sane >> buswidth value. Since we are now at rc7 and such a change might >> introduce many unexpected bugs, revert the commit for now. It can be >> sent out later along with the spi_mem_check_buswidth() fix. >> >> Reported-by: Alexander Stein >> Closes: https://lore.kernel.org/linux-mtd/3342163.44csPzL39Z@steina-w/ > > fixes tag please Hmm, I thought a fixes for a revert might be pointless since we have the commit hash in the message anyway. I don't have a strong opinion, so will add the below when applying: Fixes: 98d1fb94ce75 ("mtd: spi-nor: core: replace dummy buswidth from addr to data") > > with that: > Reviewed-by: Tudor Ambarus Thanks! -- Regards, Pratyush Yadav