[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHi4H7FrnW0956t2BYrNNVFOEkJ4t2vfStMRCqKfkDkwD0vybA@mail.gmail.com>
Date: Fri, 19 Dec 2025 16:38:39 -0800
From: William Zhang <william.zhang@...adcom.com>
To: Jonas Gorski <jonas.gorski@...il.com>
Cc: Kursad Oney <kursad.oney@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, Mark Brown <broonie@...nel.org>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: bcm63xx-hsspi: add support for 1-2-2 read ops
Hi Jonas,
We are doing some testing on the SPI NAND chips and will update once
we have the result.
Thanks,
William
On Wed, Dec 17, 2025 at 1:10 PM Jonas Gorski <jonas.gorski@...il.com> wrote:
>
> Add support for 1-2-2 read ops by separately calculating the switch from
> single-bit to multi-bit, and then switching within the prepend data.
>
> This allows us to support single-bit write followed by multi-bit write
> followed by multi-bit read, and we do not need to reject 1-2-2 read
> operations anymore.
>
> Tested on BCM963268BU_P300 with custom fixup to allow 1-2-2 on the
> non-SDFP capable s25fl129p1 attached (which it actually supports):
>
> root@...nWrt:~# cat /sys/kernel/debug/spi-nor/spi1.0/params
> name s25fl129p1
> id 01 20 18 4d 01 01
> size 16.0 MiB
> write size 1
> page size 256
> address nbytes 3
> flags HAS_16BIT_SR | NO_READ_CR
>
> opcodes
> read 0xbb
> dummy cycles 4
> erase 0xd8
> program 0x02
> 8D extension none
>
> protocols
> read 1S-2S-2S
> write 1S-1S-1S
> register 1S-1S-1S
>
> Reading from flash is still working as expected:
>
> [ 1.070000] parser_imagetag: rootfs: CFE image tag found at 0x0 with version 6, board type 963168VX
> [ 1.080000] parser_imagetag: Partition 0 is rootfs offset 100 and length 665000
> [ 1.090000] parser_imagetag: Partition 1 is kernel offset 665100 and length 136fa1
> [ 1.100000] parser_imagetag: Spare partition is offset 7b0000 and length 30000
>
> Signed-off-by: Jonas Gorski <jonas.gorski@...il.com>
> ---
> drivers/spi/spi-bcm63xx-hsspi.c | 64 +++++++++++++++++++++------------
> 1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> index 18261cbd413b..d9e972ef2abd 100644
> --- a/drivers/spi/spi-bcm63xx-hsspi.c
> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> @@ -142,6 +142,7 @@ struct bcm63xx_hsspi {
> u32 wait_mode;
> u32 xfer_mode;
> u32 prepend_cnt;
> + u32 md_start;
> u8 *prepend_buf;
> };
>
> @@ -268,18 +269,20 @@ static bool bcm63xx_prepare_prepend_transfer(struct spi_controller *host,
> {
>
> struct bcm63xx_hsspi *bs = spi_controller_get_devdata(host);
> - bool tx_only = false;
> + bool tx_only = false, multidata = false;
> struct spi_transfer *t;
>
> /*
> * Multiple transfers within a message may be combined into one transfer
> * to the controller using its prepend feature. A SPI message is prependable
> * only if the following are all true:
> - * 1. One or more half duplex write transfer in single bit mode
> - * 2. Optional full duplex read/write at the end
> - * 3. No delay and cs_change between transfers
> + * 1. One or more half duplex write transfers at the start
> + * 2. Optional switch from single to dual bit within the write transfers
> + * 3. Optional full duplex read/write at the end if all single bit
> + * 4. No delay and cs_change between transfers
> */
> bs->prepend_cnt = 0;
> + bs->md_start = 0;
> list_for_each_entry(t, &msg->transfers, transfer_list) {
> if ((spi_delay_to_ns(&t->delay, t) > 0) || t->cs_change) {
> bcm63xx_prepend_printk_on_checkfail(bs,
> @@ -297,31 +300,44 @@ static bool bcm63xx_prepare_prepend_transfer(struct spi_controller *host,
> return false;
> }
>
> - if (t->tx_nbits > SPI_NBITS_SINGLE &&
> - !list_is_last(&t->transfer_list, &msg->transfers)) {
> + if (t->tx_nbits == SPI_NBITS_SINGLE &&
> + !list_is_last(&t->transfer_list, &msg->transfers) &&
> + multidata) {
> bcm63xx_prepend_printk_on_checkfail(bs,
> - "multi-bit prepend buf not supported!\n");
> + "single-bit after multi-bit not supported!\n");
> return false;
> }
>
> - if (t->tx_nbits == SPI_NBITS_SINGLE) {
> - memcpy(bs->prepend_buf + bs->prepend_cnt, t->tx_buf, t->len);
> - bs->prepend_cnt += t->len;
> - }
> + if (t->tx_nbits > SPI_NBITS_SINGLE)
> + multidata = true;
> +
> + memcpy(bs->prepend_buf + bs->prepend_cnt, t->tx_buf, t->len);
> + bs->prepend_cnt += t->len;
> +
> + if (t->tx_nbits == SPI_NBITS_SINGLE)
> + bs->md_start += t->len;
> +
> } else {
> if (!list_is_last(&t->transfer_list, &msg->transfers)) {
> bcm63xx_prepend_printk_on_checkfail(bs,
> "rx/tx_rx transfer not supported when it is not last one!\n");
> return false;
> }
> +
> + if (t->rx_buf && t->rx_nbits == SPI_NBITS_SINGLE &&
> + multidata) {
> + bcm63xx_prepend_printk_on_checkfail(bs,
> + "single-bit after multi-bit not supported!\n");
> + return false;
> + }
> }
>
> if (list_is_last(&t->transfer_list, &msg->transfers)) {
> memcpy(t_prepend, t, sizeof(struct spi_transfer));
>
> - if (tx_only && t->tx_nbits == SPI_NBITS_SINGLE) {
> + if (tx_only) {
> /*
> - * if the last one is also a single bit tx only transfer, merge
> + * if the last one is also a tx only transfer, merge
> * all of them into one single tx transfer
> */
> t_prepend->len = bs->prepend_cnt;
> @@ -329,7 +345,7 @@ static bool bcm63xx_prepare_prepend_transfer(struct spi_controller *host,
> bs->prepend_cnt = 0;
> } else {
> /*
> - * if the last one is not a tx only transfer or dual tx xfer, all
> + * if the last one is not a tx only transfer, all
> * the previous transfers are sent through prepend bytes and
> * make sure it does not exceed the max prepend len
> */
> @@ -339,6 +355,15 @@ static bool bcm63xx_prepare_prepend_transfer(struct spi_controller *host,
> return false;
> }
> }
> + /*
> + * If switching from single-bit to multi-bit, make sure
> + * the start offset does not exceed the maximum
> + */
> + if (multidata && bs->md_start > HSSPI_MAX_PREPEND_LEN) {
> + bcm63xx_prepend_printk_on_checkfail(bs,
> + "exceed max multi-bit offset, abort prepending transfers!\n");
> + return false;
> + }
> }
> }
>
> @@ -381,11 +406,11 @@ static int bcm63xx_hsspi_do_prepend_txrx(struct spi_device *spi,
>
> if (t->rx_nbits == SPI_NBITS_DUAL) {
> reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
> - reg |= bs->prepend_cnt << MODE_CTRL_MULTIDATA_RD_STRT_SHIFT;
> + reg |= bs->md_start << MODE_CTRL_MULTIDATA_RD_STRT_SHIFT;
> }
> if (t->tx_nbits == SPI_NBITS_DUAL) {
> reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
> - reg |= bs->prepend_cnt << MODE_CTRL_MULTIDATA_WR_STRT_SHIFT;
> + reg |= bs->md_start << MODE_CTRL_MULTIDATA_WR_STRT_SHIFT;
> }
> }
>
> @@ -692,13 +717,6 @@ static bool bcm63xx_hsspi_mem_supports_op(struct spi_mem *mem,
> if (!spi_mem_default_supports_op(mem, op))
> return false;
>
> - /* Controller doesn't support spi mem dual io mode */
> - if ((op->cmd.opcode == SPINOR_OP_READ_1_2_2) ||
> - (op->cmd.opcode == SPINOR_OP_READ_1_2_2_4B) ||
> - (op->cmd.opcode == SPINOR_OP_READ_1_2_2_DTR) ||
> - (op->cmd.opcode == SPINOR_OP_READ_1_2_2_DTR_4B))
> - return false;
> -
> return true;
> }
>
> --
> 2.43.0
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5473 bytes)
Powered by blists - more mailing lists