Re: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes

From: Boris Brezillon
Date: Tue Jun 12 2018 - 03:22:55 EST

Hi Neil,

On Tue, 12 Jun 2018 08:05:13 +1000
NeilBrown <neil@xxxxxxxxxx> wrote:

> On Mon, Jun 11 2018, Boris Brezillon wrote:
> >
> > Also, I'd prefer to have this patch split in 2:
> > 1/ one patch removing the check in spi_nor_write()
> > 2/ and the second patch removing the while() loop in m25p80_write()
> >
> > How about the following commit messages for those 2 patches:
> >
> > 1:
> > "
> > mtd: spi-nor: Support controllers with limited TX FIFO size
> >
> > Some SPI controllers can't write nor->page_size bytes in a single step
> > because their TX FIFO is too small.
> I no longer think this is good justification for changes to m25p80 or to
> anything outside the low-level SPI driver. The size of the FIFO is not
> related to the maximum message size.
> An SPI transaction involves:
> - asserting chip-select
> - sending/receiving a bunch of bits
> - deasserting chip-select
> That middle part of sending/receiving bits can be done:
> - one bit at a time by bit-banging GPIOs
> - 32 bytes at a time by filling a buffer, running the SPI engine, then
> reading results out of the buffer (assuming a 32-byte buffer)
> - with one single DMA operation if the SPI engine can DMA to/from
> main memory
> and there are probably other options.
> If your SPI engine only handles (say) 32 bytes at a time, then call it
> repeatedly, while holding chip-select asserted the whole time.

Except you're not necessarily in control of the CS signal, and that's
most of the time the case with high-level (Q)SPI mem controllers like
the NXP/FSL QSPI controller (see the datasheet here if you want to check

> The SPI
> clock is controlled by the host and in this case it will not have a
> stable frequency for the whole transaction (occasionally pauses), but
> that doesn't matter. The slave just watches for the clock transitions
> and don't care when they come as long as they don't come too fast.
> I recently modified the mt7621 spi driver (in drivers/staging) to work
> like this and got much better throughput when reading from spi-nor flash
> memory. It handles writes of full pages (36 bytes at at time!) without
> problems.

Just because you managed to solve the problem in one driver does not
mean the problem does not exist for others. I read this datasheet [1]
several times and couldn't find a way to say 'I want to keep the CS
asserted between 2 transactions', so I think we really need this patch.