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 E88BD20767A; Tue, 14 Jan 2025 19:44:30 +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=1736883871; cv=none; b=e1OXZaKNnHzGRZW7H7em/45xRKNUol0wHUm90htY09NDj0SRTdSX99jlZ9rzGuWwWBm3Vzg1OjRfBlZiWZWbA+K/bjzfT+u3ro4YiTTVI0r4jqfEaYG7biF1hvrPcynUiRFPSsoKJLP9Tgp9jwDb1Y3WrADQ5EufjUzYhQAVhB8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736883871; c=relaxed/simple; bh=QNhJNcQvlfjgdhjEkkoHb79mlnawyevNw4Pmp2t2UCQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ujYVikuxg0n84uIvWfBCm6Jpc5punpZWOfcwhxWxZDrkw1F6GqDQD3nyUoLg+FBEQE+EqgCM3v+UBk3P77gdNSzc7yvPFVnurLJjUg6LgBEu76OcbeM05hpnqdYgielL+dLQwUYSmQZGri+kHNWS6SK5AA8F/pDwZHfLcwXMkIc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=tQX4r/MG; 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="tQX4r/MG" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 21014C4CEDD; Tue, 14 Jan 2025 19:44:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736883870; bh=QNhJNcQvlfjgdhjEkkoHb79mlnawyevNw4Pmp2t2UCQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=tQX4r/MGQRt0j0vma95QWsnLJoYTy7h9PGouJqLNvc49tJEAHsCAngIJZEEZNwg8V FI0CWLcuX4nGz/OonBnvDYyHyGx2aZpE9Xm24sKWd5n2OcgxDZ2+vDIk/fTkj0s95t s6fzBIMsK1OJL/8bpMUZjdlqjuVBVI+9PD8uSMQanNrcGfVQLeMl5QGmzg41atE77G kw6RMI7mvOBL+TqC1HO8Z0iG5XP1cu6g1aaoN8DNcjTwbdA8W1/bBHcMZlsfWvy1Yy u5J7BZEDQfpwZoqG8guP/tXteaogV1q+sLDIhSKHydsSsm8A+bm7b5DBveB3xAYjyt 7a+FSDt+PmyLQ== Date: Tue, 14 Jan 2025 19:44:26 +0000 From: Mark Brown To: Conor Dooley Cc: linux-spi@vger.kernel.org, Conor Dooley , stable@vger.kernel.org, Daire McNamara , linux-kernel@vger.kernel.org Subject: Re: [PATCH] spi: microchip-core: prevent RX overflows when transmit size > FIFO size Message-ID: <33b35815-3575-490a-92de-4d1c2228257e@sirena.org.uk> References: <20250114-easiness-pregame-d1d2d4b57e7b@spud> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="x78JH1Kc4sb87YXt" Content-Disposition: inline In-Reply-To: <20250114-easiness-pregame-d1d2d4b57e7b@spud> X-Cookie: Sauron is alive in Argentina! --x78JH1Kc4sb87YXt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 14, 2025 at 05:13:49PM +0000, Conor Dooley wrote: > When the size of a transfer exceeds the size of the FIFO (32 bytes), RX > overflows will be generated and receive data will be corrupted and > warnings will be produced. For example, here's an error generated by a > transfer of 36 bytes: > spi_master spi0: mchp_corespi_interrupt: RX OVERFLOW: rxlen: 4, txlen: 0 > I am not entirely sure how this happens, as rxlen being 4 means that 32 > of 36 bytes have been read from the RX FIFO so there should be > sufficient room for 4 more bytes but timing is likely a factor as simply > adding a delay in the transmit path is enough to avoid the overflows. The reads from the FIFO happen prior to the check for overflow in the interrupt handler so if we overflow then take the interrupt we will read the full 32 byte FIFO before it sees that there was an overflow. > @@ -221,6 +221,13 @@ static inline void mchp_corespi_write_fifo(struct mc= hp_corespi *spi) > while ((i < fifo_max) && !(mchp_corespi_read(spi, REG_STATUS) & STATUS_= TXFIFO_FULL)) { > u32 word; > =20 > + /* > + * If the transfer is larger than FIFO_DEPTH, spin until space > + * is made in the RX FIFO to avoid losing data to RX overflows > + */ > + while (mchp_corespi_read(spi, REG_STATUS) & STATUS_RXFIFO_FULL) > + ; > + So, this is the transmit side but we're polling the RX FIFO and not doing anything to clear it? I see that the FIFO reads are driven from interrupt context. If I had to guess I'd say that there's latency in the interrupt being delivered (possibly to a different CPU) and when the transfer is being driven by the TX side it's stuffing data into the TX FIFO faster than interrupts are being delivered (the TX side just seems to busy wait on there being space in the FIFO which can't be good for slower speeds...) so the TX and RX sides of the transfer get out of sync. Given that AFAICT the controller has to RX all the time I suspect you want to move the RX processing out of interrupt context into the main _transfer_one() function and busy wait for that too, or push the TX side into the interrupt handler (which at first glance looks simpler). Either way the two directions will be driven from the same place and so not get out of sync. --x78JH1Kc4sb87YXt Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmeGvpkACgkQJNaLcl1U h9B7hQf/UIVH5UNFPT2b35lXAFt6B4oUtXc0b6v9C0YQKfHaCZ5VjDt0y98xeb7/ 0+/+R2H4nJ3+0QMKwg98rJsA+GBQKyaUz+hAxI0O/uAigphZauge7WgiV2DnZmOv 5uzDDkuKDVLyKZGuHObBvXkht6Oc1pObFzjkyCf4kiFLiMLUNA6CohKNZAH8CaIc OyyhD9y0o/YmArlNvkLOJGNll5BuLavTKDWqJyiHb6IQtSYZfk/MzZNFjYKu8QA3 kaMNzUbO42hyRO11Rli3VBAkOzb+yBnDIuI4tpM+JuMZ5W4lOF0lrZql/rVp1wrv kAaF10f0JVTEqm+IADzPUiV/47kdiA== =HVpn -----END PGP SIGNATURE----- --x78JH1Kc4sb87YXt--