From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8CC8230F547 for ; Wed, 12 Aug 2026 08:20:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786522817; cv=none; b=Zqd32WVX+IORXemFbltHOJMnhny8uNJTGslTC5pdjEETuw0pqTdOPiVRoroXK8kIFJo12Q0ePsn9sgPJI6027luJb0Oalw8sOhewYeT+49Cl0W33DDHqZXHBZGfa64Y+zjSOGSJQaeSW4f/5j3/ErxTlBotqGzplXMmOGCVvaC8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786522817; c=relaxed/simple; bh=19QElKc8Z4AqLbONO1diyd00Et4pA8DxWHmwjN27wXw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Q2fX40M2vC+0p4BmDgoXHouWjNIx1rFaD8YDoLkCOFDnD+jTSFSw5lbzMGHljTrcFS33rN3aW0mOj0YHsiTz5JqmXZrWr/FNhISWEbXcIklUShYV0flTyxKvPIyOxsrwup+hq1ALhft7oexFQnf10RhsqRBpjPM4lablZHW/atk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=RZPc4Eur; arc=none smtp.client-ip=209.85.128.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="RZPc4Eur" Received: by mail-wm1-f47.google.com with SMTP id 5b1f17b1804b1-4953e04ef16so6268855e9.2 for ; Wed, 12 Aug 2026 01:20:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1786522813; x=1787127613; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=Yr1Y7iy9IPSmNqTQd+CZP9OkFr9hJGR5T9MqLlZLr88=; b=RZPc4EurzQYkphc1z9Ibp0bBjvw/ffS2U/t5rZdrOehS9CqMzbaQ5zwvKIq5XEQ4DZ W5KNGXDWP0Q8igT1RdGN4Kc8RzqFxzdw//9aSNFNgdfw9uGQdEZjgapFRLcnKWECoFvV rq1mW+jlCFNmVtNkKtjWGdJ7vG3LZ6BEgsz9L1DJzxSNrsd6qGCzSncSOC05XiIIqGx/ duKTjdYWm35dvVnjF1cPlu3g0NrMfUP7Vs8H78b4oGWJxu7Zm3LHRXdLRHIcP7+U7uV/ pujjsj0yCzoRydqITzUzGHBG3H6pMtIvpN1FIKZ6uumZQ46JhY4bNqDO7t50oQdUzMzB Jz8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786522813; x=1787127613; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=Yr1Y7iy9IPSmNqTQd+CZP9OkFr9hJGR5T9MqLlZLr88=; b=JeS//lV3s57b31EgnKffZjccawGTCaklz6sqA/8QkTevD3AufJQpnHN7TPEXJeMI2h +t7H4e/QmaaLhG6RemSEUUKNJVoVyJwzRTjiv0JK0b5V6PMZN/JWHSjJyypdHg4XBVrl 6esPe1EbjsvOAz4TXOew4xPvzrz/QE0kuXV0Mg+Frno9wj2Awy0fgHKB30Men9iivV0z R24f/Y2X2Nz+1malLfDde4vNPstvDvCt6vFUH4a7QJeY8g9WgA5g5eyIjp8WMLxKR7zA TdZoX1zyPd9EPPG10qJQAVJrLYJ7UsIQLhY3bycmTCVghEefUEurCum08S6wC921v5cr FALg== X-Forwarded-Encrypted: i=1; AHgh+RpGDk7PPEhULROTcneLHEOZ9Ep8DX3ow9umjoYnHZJ0VF+8ZBZsq8gVKcE5FGQmReZpFtiVg3QhQHIthHk=@vger.kernel.org X-Gm-Message-State: AOJu0YxaZZOOGtJnPAgqX56rA98CEj7o5NlUlpLIkmVj7PHP5xmkN1Gv 8KbBKdB8C7w1Zm9WI7sNJDn/KiJGso5MgyYkyTzNV07UJse1Fn5mkXNL/agsdSUqSCZUvIoaaDr /uVLl6CxQhg== X-Gm-Gg: AR+sD11u343vF7EcLaRmusyV10VkzoUdQhHNbzkeaYzwjqLul7OR4U3wJ8egiUzhJ3K 0lGBtLSPmpQhpiQg0HIvFaYIuQFyOfWYj0WsN7GCOKuVNqys8dHhIaJAJ4xqhCgE1MUXRJuGUCp Fn530GsqDSyAJumjTWByYzWemE9IPdn9a00C7S+vOinnduDNoDEKYQfdvbGUHFiqxJkb7BfSsBf GNfyLwhdpJvYR03OB7uHQ/tUWHmUhjaMbX/KngKa3DMJU/a9FtiVPK0k4JmCLBo9TLXNkM5Z7AS n+EOsxzje1+aB9IS/4zvGrZVRqZchHShcVAIQ+9Lywu4AjduEMCtAh3BsjKnKwxbXH4lsuyxmLT xOTZlg+IgdYXBnA7ilQFdTvpvN81dxmecJXr2Hh2l4TTm7WafmUbmDHRivt9LM9HzCaKPB3LbtJ OogrGDxMuXy5ZEhIcMl8/+UuoPFOayE5syJA2vboowzWzcDE837U2k6/ftBwmpFk5ktA== X-Received: by 2002:a05:600d:6446:10b0:499:593c:3384 with SMTP id 5b1f17b1804b1-4997c1070f3mr28058535e9.6.1786522812683; Wed, 12 Aug 2026 01:20:12 -0700 (PDT) Received: from [192.168.1.3] ([37.18.141.193]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4997aaa5d04sm51386525e9.2.2026.08.12.01.20.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 12 Aug 2026 01:20:12 -0700 (PDT) Message-ID: Date: Wed, 12 Aug 2026 09:20:11 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 6.12.y] spi: spi-fsl-dspi: Avoid setup_accel logic for DMA transfers To: Vladimir Oltean , Sasha Levin Cc: stable@vger.kernel.org, Larisa Grigore , Greg Kroah-Hartman , Mark Brown , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, Mehmet Fide References: <20260811091322.3592403-1-mehmet.fide@gmail.com> <20260811184500.stable-0013-sashal@kernel.org> <20260811212856.4wp56ryqgtgzahhc@skbuf> Content-Language: en-US From: James Clark In-Reply-To: <20260811212856.4wp56ryqgtgzahhc@skbuf> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/08/2026 22:28, Vladimir Oltean wrote: > On Tue, Aug 11, 2026 at 02:55:56PM -0400, Sasha Levin wrote: >>> Either way the behaviour is the same. On vf610 in DMA mode the accel path drops >>> the tail of odd length transfers and byte swaps under SPI_LSB_FIRST, and this >>> commit removes both. That is what makes it worth having in 6.12.y, whatever the >>> original intent was. >> >> cac7e5054115 ("spi: spi-fsl-dspi: Avoid setup_accel logic for DMA transfers") >> applies cleanly to 6.12 with no dependencies, so this is not a mechanical >> question - it is whether it qualifies. It has no Fixes: tag and no stable tag, >> and James reads it as a refactor. >> >> Larisa, Mark, Vladimir - was this a fix? If so, a Fixes: tag would let me take >> it here and on the older trees, where it applies just as cleanly. >> >> -- >> Thanks, >> Sasha > > It wasn't understood as a correctness change until now, but yes, it is a fix. > > Fixes: a957499bd437 ("spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode") > Acked-by: Vladimir Oltean > > Explanation: > As part of the original introduction of dspi_setup_accel() in commit > 6c1c26ecd9a3 ("spi: spi-fsl-dspi: Accelerate transfers using larger word > size if possible"), it was well understood that this is not applicable > to DMA transfers. > > The reason is that the correct clustering of 8 bit frames into 16 bit PUSHR > transfers ultimately depends on the ability to modify the SPI_CTAR_FMSZ > (frame size) on the go. In the case of a 3 byte SPI transfer using > 8-on-16 acceleration, the logic of this clustering is to first transfer > the first 2 bytes using a 16-bit PUSHR transfer (with SPI_CTAR_FMSZ=15), > then to update SPI_CTAR_FMSZ=7 in order to be able to push the last byte > using a single 8-bit PUSHR write. > > The difference between FIFO mode and DMA mode is that in DMA mode, there > is no software hook to update SPI_CTAR_FMSZ in between PUSHR FIFO > updates. The DMA engine handles them. > > This was well understood and was the basis of this code path, which > explicitly excluded DMA from dspi_setup_accel() with its dynamic frame > size updating: > > /* > * Static CTAR setup for modes that don't dynamically adjust it > * via dspi_setup_accel (aka for DMA) > */ > regmap_write(dspi->regmap, SPI_CTAR(0), > dspi->cur_chip->ctar_val | > SPI_FRAME_BITS(transfer->bits_per_word)); > > However, this truth was forgotten soon after, because as soon as a bug > report came in - the trigger behind commit a957499bd437 ("spi: > spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode") - it became > broken. > > Namely, the separate code path for static SPI_CTAR_FMSZ settings for DMA > mode got deleted, and dspi_dma_xfer() started calling dspi_setup_accel(). > This had two effects: > - dspi_setup_accel() correctly updates dspi->oper_word_size, necessary > in common code: intended, fixes the bug reported by Michael Walle > - dspi_setup_accel() enables 8-on-16 acceleration for DMA mode now, > which will transfer 1 byte too few if the buffer size is odd (it > incorrectly assumes that the caller can dynamically alter > SPI_CTAR_FMSZ and then send the trailing word separately): > unintended, causes the bug reported by Mehmet Fide > > The breakage probably went largely unnoticed because Michael Walle's > peripheral only used even-sized buffers (a flash, IIRC), and the silicon > on which I regularly test the DSPI driver doesn't use DMA. > > The commit under question here - cac7e5054115 ("spi: spi-fsl-dspi: Avoid > setup_accel logic for DMA transfers") - fixes the unintended side effect > while maintaining the intention of previous bug fix a957499bd437 ("spi: > spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode"). By having > the "goto no_accel", we bypass the 8-on-16 acceleration on DMA, while > still assigning dspi->oper_word_size - which was the reason for calling > dspi_setup_accel() in the first place. > > Note that 8-on-16 acceleration is not intrinsically broken for DMA mode > (it can yield a DMA buffer more densely packed with PUSHR data), it just My commit message on cac7e5054115 was probably a bit misleading then, because there is some benefit. I was only thinking from the point of the FIFO, not the memory backing a DMA transfer. > needs more work to skip it for odd-sized transfers. However, that work > may or may not be justified from a performance standpoint, so the > approach taken here is reasonable. > The commit could have mentioned that it wastes 1 byte per entry in favor of simplicity and correctness. But DMA isn't limited in size like the FIFO, so waste isn't an issue. > > Regarding the SPI_LSB_FIRST issue - from the description it seems to be > a completely distinct problem not intrinsically limited to DMA mode > (should also be visible in XSPI mode), so disabling dspi_setup_accel() > on Vyber and Coldfire only partially addresses it. > > I don't have a use case for SPI_LSB_FIRST peripherals, so I don't > personally mind another "goto no_accel" follow-up patch rather than > fixing the underlying byte packing mechanism, BUT this should be done > by the issue reporter with a proper explanation in the commit message > now that the issue is more clearly understood, rather than just be > happy that backporting commit cac7e5054115 sidesteps the problem on his > platform. > > I am currently on vacation, and I am unable to do much testing on actual > hardware. I also haven't completely evaluated the SPI_LSB_FIRST behaviour > with 8-on-16 acceleration, it just *seems* plausible that there is an issue.