[PATCH v2] spi: bcm63xx: fix premature CS deassertion on RX-only transactions

[PATCH v2] spi: bcm63xx: fix premature CS deassertion on RX-only transactions

[Hang Zhou]

On BCM6358 (and also observed on BCM6368) the controller appears to
only generate as many SPI clocks as bytes that have been written into
the TX FIFO. For RX-only transfers the driver programs the transfer
length in SPI_MSG_CTL but does not write anything into the FIFO, so
chip select is deasserted early and the RX transfer segment is never
fully clocked in.

A concrete failing case is a three-transfer MAC address read from
SPI-NOR:
  - TX 0x03 (read command)
  - TX 3-byte address
  - RX 6 bytes (MAC)

In contrast, a two-transfer JEDEC-ID read (0x9f + 6-byte RX) works
because the driver uses prepend_len and writes dummy bytes into the
TX FIFO for the RX part.

Fix this by writing 0xff dummy bytes into the TX FIFO for RX-only
segments so that the number of bytes written to the FIFO matches the
total message length seen by the controller.

Fixes: b17de076062a ("spi/bcm63xx: work around inability to keep CS up")

Signed-off-by: Hang Zhou <929513338@qq.com>
---
 drivers/spi/spi-bcm63xx.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index 8510400e7867..76ccd0f62c2e 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -154,6 +154,20 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi, struct spi_transfer *first,
 
 		if (t->rx_buf) {
 			do_rx = true;
+
+			/*
+			 * In certain hardware implementations, there appears to be a
+			 * hidden accumulator that tracks the number of bytes written into
+			 * the hardware FIFO, and this accumulator overrides the length in
+			 * the SPI_MSG_CTL register.
+			 *
+			 * Therefore, for read-only transfers, we need to write some dummy
+			 * value into the FIFO to keep the accumulator tracking the correct
+			 * length.
+			 */
+			if (!t->tx_buf)
+				memset_io(bs->tx_io + len, 0xFF, t->len);
+
 			/* prepend is half-duplex write only */
 			if (t == first)
 				prepend_len = 0;
-- 
2.34.1

Re: [PATCH v2] spi: bcm63xx: fix premature CS deassertion on RX-only transactions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On Mon, Nov 17, 2025 at 01:08:35AM +1100, Hang Zhou wrote:

> Fix this by writing 0xff dummy bytes into the TX FIFO for RX-only
> segments so that the number of bytes written to the FIFO matches the
> total message length seen by the controller.

Why 0xff?  The core's _MUST_TX handling writes 0x00 which feels a bit
more natural.  Given that this is PIO rather than DMA the memset_toio()
is going to be better than using the core (which is more for DMA).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] spi: bcm63xx: fix premature CS deassertion on RX-only transactions

[Hang Zhou]

Hi Mark,

Thank you for the review.

I initially picked 0xFF because in most case, the SPI bus is high when it's idle, but in practice 0x00 works just as well here.

If you prefer, I'm happy to switch the dummy value to 0x00 for consistency and respin this as v3.

Best regards,
Hang Zhou

Re: [PATCH v2] spi: bcm63xx: fix premature CS deassertion on RX-only transactions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On Tue, Nov 18, 2025 at 03:53:38AM +1100, Hang Zhou wrote:

> I initially picked 0xFF because in most case, the SPI bus is high when it's idle, but in practice 0x00 works just as well here.

> If you prefer, I'm happy to switch the dummy value to 0x00 for consistency and respin this as v3.

No, it's OK - if this hardware defaults to keeing the signal high then
0xff is a better choice.  Most hardware defaults to keeping the line
low.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] spi: bcm63xx: fix premature CS deassertion on RX-only transactions

[Mark Brown]

On Mon, 17 Nov 2025 01:08:35 +1100, Hang Zhou wrote:
> On BCM6358 (and also observed on BCM6368) the controller appears to
> only generate as many SPI clocks as bytes that have been written into
> the TX FIFO. For RX-only transfers the driver programs the transfer
> length in SPI_MSG_CTL but does not write anything into the FIFO, so
> chip select is deasserted early and the RX transfer segment is never
> fully clocked in.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: bcm63xx: fix premature CS deassertion on RX-only transactions
      commit: fd9862f726aedbc2f29a29916cabed7bcf5cadb6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark