linux-kernel - Re: [PATCH] spi: bcm63xx: fix premature CS deassertion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAOiHx=k7ULBgdq+B=5D7Zon5R8vwPuEc0t9H8QA66x0Zhvan8g@mail.gmail.com>
Date: Sun, 9 Nov 2025 21:32:11 +0100
From: Jonas Gorski <jonas.gorski@...il.com>
To: 串亖の六花 <929513338@...com>
Cc: broonie <broonie@...nel.org>, linux-spi <linux-spi@...r.kernel.org>, 
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: bcm63xx: fix premature CS deassertion

Hi,

On Sun, Nov 9, 2025 at 2:02 PM 串亖の六花 <929513338@...com> wrote:
>
> The bcm63xx SPI controller is programmed to use full-duplex mode
> when a spi_message contains multiple TX and RX transfers and there is no
> prepend (i.e. do_rx && do_tx && prepend_len == 0). In this case the
> hardware shifts out the contents of the TX FIFO while clocking in RX
> data.
>
> On a BCM6358 device, this causes the chip-select signal to drop too early
> before actually read anything.
>
> A concrete failing case is a 3-transfer message used to read the MAC
> address from SPI-NOR:
>   - TX 0x03 (read command)
>   - TX 3-byte address (any)
>   - RX 6 bytes (MAC)
>
> In contrast, a 2-transfer JEDEC-ID read (0x9F + 6-byte RX) works because the
> driver uses "prepend_len".
>
> To keep CS asserted and generate proper dummy clocks for RX transfers,
> fill the TX FIFO with 0xff bytes for RX segments.

Thank you for the patch! I could reproduce the behavior on master with
a BCM6368 with SPI flash (getting to netboot OpenWrt main on a 32 MiB
RAM device was an adventure lol).

I played around a bit, and it looks like there is a hidden accumulator
for how many bytes were written into the fifo buffer, and this
overrides the bytes count in the control message for duplex messages.

It does not matter where you write those extra bytes for the dummy tx
to do during rx - it doesn't even have to be within the message
length, I could just write them in a loop to the end of the fifo
buffer, and it still worked. It just matters that the amount of bytes
written to the fifo buffer match the total message length. It also
does not matter what you write for those read-only parts, I tried
0x00, 0xaa, both worked as well. Though 0xff is probably the safest.

That being said ...

>
> This patch resolves the issue on BCM6358 (tested with OpenWrt) and in
> the v6.12.57 kernel:

This patch should be against spi-next (?) from
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git. Won't
probably not matter much though, as the driver doesn't get many
changes anymore.

>

Here should be a Fixes: tag, e.g.:

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

> Signed-off-by: Hang Zhou <929513338@...com>

Your name here does not match the From from this email.

> ---
>  drivers/spi/spi-bcm63xx.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
> index 000000000000..000000000000 100644
> --- a/drivers/spi/spi-bcm63xx.c
> +++ b/drivers/spi/spi-bcm63xx.c
> @@ -247,6 +247,7 @@ static int bcm63xx_txrx_bufs(struct spi_device *spi, struct spi_transfer *first,
>
>             if (t->rx_buf) {
>                   do_rx = true;
> +                 memset_io(bs->tx_io + len, 0xFF, t->len);

You need to check that there is no t->tx_buf, else you will overwrite
the already written tx_buf for actual duplex messages with 0xff. And
the fifo counter will be off.

>                   /* prepend is half-duplex write only */
>                   if (t == first)
>                         prepend_len = 0;
> --

Best regards,
Jonas