[0/2] spi: fsi: Reduce max transfer size to 8 bytes

Message

Eddie James July 16, 2021, 1:39 p.m. UTC

The security restrictions on the FSI-attached SPI controllers have
been applied universally to all controllers, so the controller can no
longer transfer more than 8 bytes for one transfer. Refactor the driver
to remove the looping and support for larger transfers, and remove the
"restricted" compatible string, as all the controllers are now
considered restricted.

Eddie James (2):
  spi: fsi: Reduce max transfer size to 8 bytes
  dt-bindings: fsi: Remove ibm,fsi2spi-restricted compatible

 .../devicetree/bindings/fsi/ibm,fsi2spi.yaml  |   1 -
 drivers/spi/spi-fsi.c                         | 125 +++---------------
 2 files changed, 22 insertions(+), 104 deletions(-)

Comments

David Laight July 17, 2021, 1:46 p.m. UTC | #1

From: Eddie James
> Sent: 16 July 2021 14:39
> 
> The security restrictions on the FSI-attached SPI controllers have
> been applied universally to all controllers, so the controller can no
> longer transfer more than 8 bytes for one transfer. Refactor the driver
> to remove the looping and support for larger transfers, and remove the
> "restricted" compatible string, as all the controllers are now
> considered restricted.

Aren't there significant performance (and device wear?) penalties
for doing short SPI eeprom writes?

IIRC (and I'm not getting my serial busses confused) a write request
can request an aligned transfer of up to (typically) 32 bytes.
At which point you need to wait for the status to indicate 'complete'.

So restricting writes to 8 bytes increases block write times
by a factor of 4.

Increasing the numbers of writes by a factor or 4 may also have
an effect on device wear - but that is more likely only affected
by erase cycles.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Eddie James July 19, 2021, 3:57 p.m. UTC | #2

On Sat, 2021-07-17 at 13:46 +0000, David Laight wrote:
> From: Eddie James
> > Sent: 16 July 2021 14:39
> > 
> > The security restrictions on the FSI-attached SPI controllers have
> > been applied universally to all controllers, so the controller can
> > no
> > longer transfer more than 8 bytes for one transfer. Refactor the
> > driver
> > to remove the looping and support for larger transfers, and remove
> > the
> > "restricted" compatible string, as all the controllers are now
> > considered restricted.
> 
> Aren't there significant performance (and device wear?) penalties
> for doing short SPI eeprom writes?

Probably so. However there is no choice in the matter, as the SPI
controller can't process larger reads/writes.
(I should note that the controller/driver CAN process up to 40 byte
writes. However, the driver must report the minimum of read and write
sizes in the max_transfer_size callback, so no client driver should
request larger than the max read size - 8 bytes).

Thanks,
Eddie

> 
> IIRC (and I'm not getting my serial busses confused) a write request
> can request an aligned transfer of up to (typically) 32 bytes.
> At which point you need to wait for the status to indicate
> 'complete'.
> 
> So restricting writes to 8 bytes increases block write times
> by a factor of 4.
> 
> Increasing the numbers of writes by a factor or 4 may also have
> an effect on device wear - but that is more likely only affected
> by erase cycles.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
> MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

Mark Brown July 19, 2021, 6 p.m. UTC | #3

On Fri, 16 Jul 2021 08:39:13 -0500, Eddie James wrote:
> The security restrictions on the FSI-attached SPI controllers have
> been applied universally to all controllers, so the controller can no
> longer transfer more than 8 bytes for one transfer. Refactor the driver
> to remove the looping and support for larger transfers, and remove the
> "restricted" compatible string, as all the controllers are now
> considered restricted.
> 
> [...]

Applied to

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

Thanks!

[1/2] spi: fsi: Reduce max transfer size to 8 bytes
      commit: 34d34a56a5ea1e54a5af4f34c6ac9df724129351
[2/2] dt-bindings: fsi: Remove ibm,fsi2spi-restricted compatible
      commit: 2b2d4dfca4e7cb6de70985b1579a6c08c027b8c9

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