[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAyq3Sb-dmmF8YsOdTYxZqX8Wn4d=WiAXkS4bHp5FLn3S+ouXg@mail.gmail.com>
Date: Tue, 12 Nov 2024 10:42:48 +0800
From: Cheng Ming Lin <linchengming884@...il.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>
Cc: pratyush@...nel.org, mwalle@...nel.org, miquel.raynal@...tlin.com,
richard@....at, vigneshr@...com, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org, alvinzhou@...c.com.tw, leoyu@...c.com.tw,
Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH] mtd: spi-nor: core: replace dummy buswidth from addr to data
Hi Tudor,
Tudor Ambarus <tudor.ambarus@...aro.org> 於 2024年11月11日 週一 下午6:18寫道:
>
>
>
> On 11/7/24 9:30 AM, Cheng Ming Lin wrote:
> > From: Cheng Ming Lin <chengminglin@...c.com.tw>
> >
> > The default dummy cycle for Macronix SPI NOR flash in Octal Output
> > Read Mode(1-1-8) is 20.
> >
> > Currently, the dummy buswidth is set according to the address bus width.
> > In the 1-1-8 mode, this means the dummy buswidth is 1. When converting
> > dummy cycles to bytes, this results in 20 x 1 / 8 = 2 bytes, causing the
> > host to read data 4 cycles too early.
> >
> > Since the protocol data buswidth is always greater than or equal to the
> > address buswidth. Setting the dummy buswidth to match the data buswidth
> > increases the likelihood that the dummy cycle-to-byte conversion will be
> > divisible, preventing the host from reading data prematurely.
>
> This is still very wrong and the `fix` is working just by chance.
> Consider what happens when one requires 10 dummy cycles. BTW, does this
> fix a real problem, or it's just a theoretical fix?
In 1-1-8 mode, setting the dummy buswidth to match the data
buswidth ensures a dummy buswidth of 8, which can accommodate
all types of dummy cycles.
This patch resolves a significant issue in 1-1-8 mode, as described above.
>
> I don't like that we're chasing our tails here. The fix is to pass
> directly cycles instead of bytes and update MTD and SPI to use cycles. I
> had an attempt to fix this in the past, but I can't allocate time to
> respin. Are you interested in taking over and fixing MTD and SPI?
>
I understand your concerns and agree with your approach. However,
this fix may require some time to implement, and we're currently facing
this issue with a pressing need for resolution. If I can allocate time in
the near feature, I'd be interested in taking on the work to address this
for both MTD and SPI.
> > Signed-off-by: Cheng Ming Lin <chengminglin@...c.com.tw>
> > ---
> > drivers/mtd/spi-nor/core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index f9c189ed7353..c7aceaa8a43f 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -89,7 +89,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
> > op->addr.buswidth = spi_nor_get_protocol_addr_nbits(proto);
> >
> > if (op->dummy.nbytes)
> > - op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
> > + op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);
> >
> > if (op->data.nbytes)
> > op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
Powered by blists - more mailing lists