From: Portia Stephens <portias@oss.tenstorrent.com>

The ssi_transfer function comments say that it takes a word varying
between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to transfer
but there is no means to indicate the number of bits that should
actually be transferred. All child classes of SSI_PERIPHERAL class have
transfer functions that, despite accepting a 32-bit tx, only transfer a
single byte; m25p80_transfer8(), ssi_sd_transfer(), ssd0323_transfer().

The current implementation depends on the SSI model to know
what peripheral model will be attached and what transfer size it
expects which is error prone. If a SSI_PERIPHERAL model was written that
accepted 32-bit transfers, it could not attach to any existing SSI
models.

This change updates the the naming of ssi_transfer to ssi_transfer8, as
well as changes the return value and transmit argument to be 8-bit.

Most ssi models handle this correctly already, sending a single byte at
a time. There are a few models that are written to support non 8-bit
transfers but there are no in-tree use cases that connect a peripheral
to the SSI device. These have been updated to use 8-bit transfers.

Portia Stephens (4):
  hw/ssi: Rename ssi_transfer to ssi_transfer8
  hw/ssi/pl022: Fix dropped upper bytes of ssi transfer
  hw/arm/strongarm: Fix dropped upper byte of ssi transfer
  hw/ssi/pnv_spi: Fix dropped upper bytes of ssi transfer

 hw/arm/strongarm.c         |  7 +++++--
 hw/ssi/allwinner-a10-spi.c |  2 +-
 hw/ssi/aspeed_smc.c        | 14 ++++++-------
 hw/ssi/bcm2835_spi.c       |  2 +-
 hw/ssi/ibex_spi_host.c     |  5 +++--
 hw/ssi/imx_spi.c           |  2 +-
 hw/ssi/mss-spi.c           |  2 +-
 hw/ssi/npcm7xx_fiu.c       | 42 +++++++++++++++++++-------------------
 hw/ssi/npcm_pspi.c         |  4 ++--
 hw/ssi/pl022.c             | 12 +++++++----
 hw/ssi/pnv_spi.c           | 28 ++++++++++---------------
 hw/ssi/sifive_spi.c        |  2 +-
 hw/ssi/ssi.c               |  4 ++--
 hw/ssi/stm32f2xx_spi.c     |  2 +-
 hw/ssi/xilinx_spi.c        | 10 ++++-----
 hw/ssi/xilinx_spips.c      |  4 ++--
 hw/ssi/xlnx-versal-ospi.c  |  4 ++--
 include/hw/ssi/ssi.h       | 17 ++++++++-------
 18 files changed, 82 insertions(+), 81 deletions(-)

-- 
2.43.0

On Tue, 25 Aug 2026 at 05:05, <stephensportia@gmail.com> wrote:
>
> From: Portia Stephens <portias@oss.tenstorrent.com>
>
> The ssi_transfer function comments say that it takes a word varying
> between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to transfer
> but there is no means to indicate the number of bits that should
> actually be transferred. All child classes of SSI_PERIPHERAL class have
> transfer functions that, despite accepting a 32-bit tx, only transfer a
> single byte; m25p80_transfer8(), ssi_sd_transfer(), ssd0323_transfer().
>
> The current implementation depends on the SSI model to know
> what peripheral model will be attached and what transfer size it
> expects which is error prone. If a SSI_PERIPHERAL model was written that
> accepted 32-bit transfers, it could not attach to any existing SSI
> models.

What is the motivation for this change?

As far as I know for the hardware SSI protocol, it is indeed
the case that the SSI controller (and/or the guest software) needs
to know what transfer size the attached device expects: the
controller just transmits (or expects to receive) however many
bits it is programmed for.

> This change updates the the naming of ssi_transfer to ssi_transfer8, as
> well as changes the return value and transmit argument to be 8-bit.

This means that the API will no longer work for a controller
and device that aren't 8-bit. We happen not to have any of those
devices today, but why specifically stop them working?

thanks
-- PMM

On Thu, Aug 27, 2026 at 7:19 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 25 Aug 2026 at 05:05, <stephensportia@gmail.com> wrote:
> >
> > From: Portia Stephens <portias@oss.tenstorrent.com>
> >
> > The ssi_transfer function comments say that it takes a word varying
> > between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to transfer
> > but there is no means to indicate the number of bits that should
> > actually be transferred. All child classes of SSI_PERIPHERAL class have
> > transfer functions that, despite accepting a 32-bit tx, only transfer a
> > single byte; m25p80_transfer8(), ssi_sd_transfer(), ssd0323_transfer().
> >
> > The current implementation depends on the SSI model to know
> > what peripheral model will be attached and what transfer size it
> > expects which is error prone. If a SSI_PERIPHERAL model was written that
> > accepted 32-bit transfers, it could not attach to any existing SSI
> > models.
>
> What is the motivation for this change?

The motivation came from reviewing the designware ssi driver [1] and
realizing how error prone it was. The designware ssi supports varying
frame width but there is no way for the controller logic to know what
the transfer size an attached device expects. It seems like all the
code, that is actually connected to devices, is just written to expect
8-bit transfers so it should be made explicit.

https://www.mail-archive.com/qemu-devel@nongnu.org/msg1218153.html

>
> As far as I know for the hardware SSI protocol, it is indeed
> the case that the SSI controller (and/or the guest software) needs
> to know what transfer size the attached device expects: the
> controller just transmits (or expects to receive) however many
> bits it is programmed for.

This is true but do the devices currently model only support 8-bit
transfers in hardware, do they not support multiframe transfer? SPI
supports varying framewidth but the devices are not actually modeled
in this way.

A better solution may be to have multiple transfer functions,
ssi_transfer8, ssi_transfer32, etc. This would require the device to
explicitly set what it is expecting. It is challenging since the only
model's not using 8-bit transfer don't have any devices connected
upstream so testing is challenging.

>
> > This change updates the the naming of ssi_transfer to ssi_transfer8, as
> > well as changes the return value and transmit argument to be 8-bit.
>
> This means that the API will no longer work for a controller
> and device that aren't 8-bit. We happen not to have any of those
> devices today, but why specifically stop them working?
>
> thanks
> -- PMM

On Thu, 27 Aug 2026 at 11:37, Portia Stephens <stephensportia@gmail.com> wrote:
>
> On Thu, Aug 27, 2026 at 7:19 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 25 Aug 2026 at 05:05, <stephensportia@gmail.com> wrote:
> > >
> > > From: Portia Stephens <portias@oss.tenstorrent.com>
> > >
> > > The ssi_transfer function comments say that it takes a word varying
> > > between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to transfer
> > > but there is no means to indicate the number of bits that should
> > > actually be transferred. All child classes of SSI_PERIPHERAL class have
> > > transfer functions that, despite accepting a 32-bit tx, only transfer a
> > > single byte; m25p80_transfer8(), ssi_sd_transfer(), ssd0323_transfer().
> > >
> > > The current implementation depends on the SSI model to know
> > > what peripheral model will be attached and what transfer size it
> > > expects which is error prone. If a SSI_PERIPHERAL model was written that
> > > accepted 32-bit transfers, it could not attach to any existing SSI
> > > models.
> >
> > What is the motivation for this change?
>
> The motivation came from reviewing the designware ssi driver [1] and
> realizing how error prone it was. The designware ssi supports varying
> frame width but there is no way for the controller logic to know what
> the transfer size an attached device expects.

But isn't this just the way the SSI specification is? If we're
modelling a "you just have to get this right in software" bit
of hardware then we don't need to somehow try to add extra
checks in QEMU for whether the software hasn't actually done
things right.

> It seems like all the
> code, that is actually connected to devices, is just written to expect
> 8-bit transfers so it should be made explicit.
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg1218153.html
>
> >
> > As far as I know for the hardware SSI protocol, it is indeed
> > the case that the SSI controller (and/or the guest software) needs
> > to know what transfer size the attached device expects: the
> > controller just transmits (or expects to receive) however many
> > bits it is programmed for.
>
> This is true but do the devices currently model only support 8-bit
> transfers in hardware, do they not support multiframe transfer? SPI
> supports varying framewidth but the devices are not actually modeled
> in this way.

Do the actual devices we implement support varying framewidth,
or do they actually have exactly one frame width that the controller
needs to be programmed by the guest to use ?

> A better solution may be to have multiple transfer functions,
> ssi_transfer8, ssi_transfer32, etc. This would require the device to
> explicitly set what it is expecting. It is challenging since the only
> model's not using 8-bit transfer don't have any devices connected
> upstream so testing is challenging.

This doesn't allow for bit widths that aren't a multiple of 8.

thanks
-- PMM

On Thu, Aug 27, 2026 at 8:53 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 27 Aug 2026 at 11:37, Portia Stephens <stephensportia@gmail.com> wrote:
> >
> > On Thu, Aug 27, 2026 at 7:19 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Tue, 25 Aug 2026 at 05:05, <stephensportia@gmail.com> wrote:
> > > >
> > > > From: Portia Stephens <portias@oss.tenstorrent.com>
> > > >
> > > > The ssi_transfer function comments say that it takes a word varying
> > > > between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to transfer
> > > > but there is no means to indicate the number of bits that should
> > > > actually be transferred. All child classes of SSI_PERIPHERAL class have
> > > > transfer functions that, despite accepting a 32-bit tx, only transfer a
> > > > single byte; m25p80_transfer8(), ssi_sd_transfer(), ssd0323_transfer().
> > > >
> > > > The current implementation depends on the SSI model to know
> > > > what peripheral model will be attached and what transfer size it
> > > > expects which is error prone. If a SSI_PERIPHERAL model was written that
> > > > accepted 32-bit transfers, it could not attach to any existing SSI
> > > > models.
> > >
> > > What is the motivation for this change?
> >
> > The motivation came from reviewing the designware ssi driver [1] and
> > realizing how error prone it was. The designware ssi supports varying
> > frame width but there is no way for the controller logic to know what
> > the transfer size an attached device expects.
>
> But isn't this just the way the SSI specification is? If we're
> modelling a "you just have to get this right in software" bit
> of hardware then we don't need to somehow try to add extra
> checks in QEMU for whether the software hasn't actually done
> things right.
>
> > It seems like all the
> > code, that is actually connected to devices, is just written to expect
> > 8-bit transfers so it should be made explicit.
> >
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg1218153.html
> >
> > >
> > > As far as I know for the hardware SSI protocol, it is indeed
> > > the case that the SSI controller (and/or the guest software) needs
> > > to know what transfer size the attached device expects: the
> > > controller just transmits (or expects to receive) however many
> > > bits it is programmed for.
> >
> > This is true but do the devices currently model only support 8-bit
> > transfers in hardware, do they not support multiframe transfer? SPI
> > supports varying framewidth but the devices are not actually modeled
> > in this way.
>
> Do the actual devices we implement support varying framewidth,
> or do they actually have exactly one frame width that the controller
> needs to be programmed by the guest to use ?

From my reading of the m25p80 spec, it does not have a fixed frame
width. The frame would be determined by the CS being driven and the
clock running. A frame could be 1 byte cmd + 3 byte address for a
register read. For read mode, 1 byte cmd + 3 byte address + variable
length read data. I don't understand how the 8-bit transfer size
actually maps to what hardware does.

When writing a controller that supports varying framewidths, it
requires the controller model to know what transfer width will be
expected from the device connected. You could have guest software that
correctly sets the controller framewidth to 32-bits and connects a
device that provides a 32-bit transfer function. How can the
controller differentiate this from the case where the guest software
correctly sets a 32-bit framewidth on the controller for a m25p80 read
which has a 8-bit transfer size and the controller needs to call the
transfer function 4 times to clock out the requested read data.

Maybe I am misunderstanding what a transfer is or what a framewidth
actually is or what the realistic use cases are here.

>
> > A better solution may be to have multiple transfer functions,
> > ssi_transfer8, ssi_transfer32, etc. This would require the device to
> > explicitly set what it is expecting. It is challenging since the only
> > model's not using 8-bit transfer don't have any devices connected
> > upstream so testing is challenging.
>
> This doesn't allow for bit widths that aren't a multiple of 8.
>
> thanks
> -- PMM

On Thu, 2026-08-27 at 11:53 +0100, Peter Maydell wrote:
> On Thu, 27 Aug 2026 at 11:37, Portia Stephens
> <stephensportia@gmail.com> wrote:
> > 
> > On Thu, Aug 27, 2026 at 7:19 PM Peter Maydell
> > <peter.maydell@linaro.org> wrote:
> > > 
> > > On Tue, 25 Aug 2026 at 05:05, <stephensportia@gmail.com> wrote:
> > > > 
> > > > From: Portia Stephens <portias@oss.tenstorrent.com>
> > > > 
> > > > The ssi_transfer function comments say that it takes a word
> > > > varying
> > > > between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to
> > > > transfer
> > > > but there is no means to indicate the number of bits that
> > > > should
> > > > actually be transferred. All child classes of SSI_PERIPHERAL
> > > > class have
> > > > transfer functions that, despite accepting a 32-bit tx, only
> > > > transfer a
> > > > single byte; m25p80_transfer8(), ssi_sd_transfer(),
> > > > ssd0323_transfer().
> > > > 
> > > > The current implementation depends on the SSI model to know
> > > > what peripheral model will be attached and what transfer size
> > > > it
> > > > expects which is error prone. If a SSI_PERIPHERAL model was
> > > > written that
> > > > accepted 32-bit transfers, it could not attach to any existing
> > > > SSI
> > > > models.
> > > 
> > > What is the motivation for this change?
> > 
> > The motivation came from reviewing the designware ssi driver [1]
> > and
> > realizing how error prone it was. The designware ssi supports
> > varying
> > frame width but there is no way for the controller logic to know
> > what
> > the transfer size an attached device expects.
> 
> But isn't this just the way the SSI specification is? If we're
> modelling a "you just have to get this right in software" bit
> of hardware then we don't need to somehow try to add extra
> checks in QEMU for whether the software hasn't actually done
> things right.

From my reading of things this is a bug in QEMU, not a guest issue.

The fact that aspeed_smc_flash_setup() for example iterates over a
larger value to send a single byte at a time shows that currently
everyone *thinks* think function should send a single byte.

So it does seem broken today. Making the current function clear seems
like a good step. Supporting different or larger transfers in the
future is then possible.

> 
> > It seems like all the
> > code, that is actually connected to devices, is just written to
> > expect
> > 8-bit transfers so it should be made explicit.
> > 
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg1218153.html
> > 
> > > 
> > > As far as I know for the hardware SSI protocol, it is indeed
> > > the case that the SSI controller (and/or the guest software)
> > > needs
> > > to know what transfer size the attached device expects: the
> > > controller just transmits (or expects to receive) however many
> > > bits it is programmed for.
> > 
> > This is true but do the devices currently model only support 8-bit
> > transfers in hardware, do they not support multiframe transfer? SPI
> > supports varying framewidth but the devices are not actually
> > modeled
> > in this way.
> 
> Do the actual devices we implement support varying framewidth,
> or do they actually have exactly one frame width that the controller
> needs to be programmed by the guest to use ?
> 
> > A better solution may be to have multiple transfer functions,
> > ssi_transfer8, ssi_transfer32, etc. This would require the device
> > to
> > explicitly set what it is expecting. It is challenging since the
> > only
> > model's not using 8-bit transfer don't have any devices connected
> > upstream so testing is challenging.
> 
> This doesn't allow for bit widths that aren't a multiple of 8.

Wouldn't a ssi_transfer32() just be the same as today?

Alistair

> 
> thanks
> -- PMM

On Thu, 27 Aug 2026 at 12:03, Alistair <alistair@alistair23.me> wrote:
>
> On Thu, 2026-08-27 at 11:53 +0100, Peter Maydell wrote:
> > But isn't this just the way the SSI specification is? If we're
> > modelling a "you just have to get this right in software" bit
> > of hardware then we don't need to somehow try to add extra
> > checks in QEMU for whether the software hasn't actually done
> > things right.
>
> From my reading of things this is a bug in QEMU, not a guest issue.
>
> The fact that aspeed_smc_flash_setup() for example iterates over a
> larger value to send a single byte at a time shows that currently
> everyone *thinks* think function should send a single byte.

Doesn't that just show that the aspeed flash controller knows
it is always talking to an 8-bit SSI device ? It wouldn't
surprise me if the SPI-NOR standard in particular insisted on
8-bit transfers, though I can't find anything claiming to be
that standard.

> So it does seem broken today. Making the current function clear seems
> like a good step. Supporting different or larger transfers in the
> future is then possible.

I think the current interface already supports different or
larger transfers. We just happen to not be using that.

-- PMM

On Thu, 2026-08-27 at 12:17 +0100, Peter Maydell wrote:
> On Thu, 27 Aug 2026 at 12:03, Alistair <alistair@alistair23.me>
> wrote:
> > 
> > On Thu, 2026-08-27 at 11:53 +0100, Peter Maydell wrote:
> > > But isn't this just the way the SSI specification is? If we're
> > > modelling a "you just have to get this right in software" bit
> > > of hardware then we don't need to somehow try to add extra
> > > checks in QEMU for whether the software hasn't actually done
> > > things right.
> > 
> > From my reading of things this is a bug in QEMU, not a guest issue.
> > 
> > The fact that aspeed_smc_flash_setup() for example iterates over a
> > larger value to send a single byte at a time shows that currently
> > everyone *thinks* think function should send a single byte.
> 
> Doesn't that just show that the aspeed flash controller knows
> it is always talking to an 8-bit SSI device ? It wouldn't

But I don't think that's guaranteed to be true.

On real hardware the software could know that it's only ever connected
to an 8-bit SSI device, and set the driver accordingly. That seems
entirely possible.

But the hardware seems to support larger sizes reading [1].

In this case the QEMU code is the hardware, and I don't see how we know
we are only ever accessing an 8-bit device.

1:
https://patchew.org/linux/20220304083643.1079142-1-clg@kaod.org/20220304083643.1079142-5-clg@kaod.org/

> surprise me if the SPI-NOR standard in particular insisted on
> 8-bit transfers, though I can't find anything claiming to be
> that standard.
> 
> > So it does seem broken today. Making the current function clear
> > seems
> > like a good step. Supporting different or larger transfers in the
> > future is then possible.
> 
> I think the current interface already supports different or
> larger transfers. We just happen to not be using that.

Kind of. It does support larger transfers, but there is no size
argument. So how can the SSI device know how much was sent? Everything
seems to just assume a single byte, but it has no way to actually know
that.

Alistair

> 
> -- PMM

On 8/28/26 02:07, Alistair wrote:
> On Thu, 2026-08-27 at 12:17 +0100, Peter Maydell wrote:
>> On Thu, 27 Aug 2026 at 12:03, Alistair <alistair@alistair23.me>
>> wrote:
>>>
>>> On Thu, 2026-08-27 at 11:53 +0100, Peter Maydell wrote:
>>>> But isn't this just the way the SSI specification is? If we're
>>>> modelling a "you just have to get this right in software" bit
>>>> of hardware then we don't need to somehow try to add extra
>>>> checks in QEMU for whether the software hasn't actually done
>>>> things right.
>>>
>>>  From my reading of things this is a bug in QEMU, not a guest issue.
>>>
>>> The fact that aspeed_smc_flash_setup() for example iterates over a
>>> larger value to send a single byte at a time shows that currently
>>> everyone *thinks* think function should send a single byte.
>>
>> Doesn't that just show that the aspeed flash controller knows
>> it is always talking to an 8-bit SSI device ? It wouldn't
> 
> But I don't think that's guaranteed to be true.
> 
> On real hardware the software could know that it's only ever connected
> to an 8-bit SSI device, and set the driver accordingly. That seems
> entirely possible.
> 
> But the hardware seems to support larger sizes reading [1].
> 
> In this case the QEMU code is the hardware, and I don't see how we know
> we are only ever accessing an 8-bit device.
> 
> 1:
> https://patchew.org/linux/20220304083643.1079142-1-clg@kaod.org/20220304083643.1079142-5-clg@kaod.org/

On the Aspeed SoC, MMIO accesses (1, 2, 4 bytes) to the SPI controller
are converted by the hardware into SPI bus transfers. The exact
serialization is hardware-specific. QEMU's choice is to generate
byte-oriented SPI transfers.

In any case, SPI is generally byte-oriented, with bits shifted over
1/2/4/8 I/O lines. So byte transfer is sufficient.

C.

> 
>> surprise me if the SPI-NOR standard in particular insisted on
>> 8-bit transfers, though I can't find anything claiming to be
>> that standard.
>>
>>> So it does seem broken today. Making the current function clear
>>> seems
>>> like a good step. Supporting different or larger transfers in the
>>> future is then possible.
>>
>> I think the current interface already supports different or
>> larger transfers. We just happen to not be using that.
> 
> Kind of. It does support larger transfers, but there is no size
> argument. So how can the SSI device know how much was sent? Everything
> seems to just assume a single byte, but it has no way to actually know
> that.
> 
> Alistair
> 
>>
>> -- PMM

On Mon, Aug 31, 2026 at 9:03 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 8/28/26 02:07, Alistair wrote:
> > On Thu, 2026-08-27 at 12:17 +0100, Peter Maydell wrote:
> >> On Thu, 27 Aug 2026 at 12:03, Alistair <alistair@alistair23.me>
> >> wrote:
> >>>
> >>> On Thu, 2026-08-27 at 11:53 +0100, Peter Maydell wrote:
> >>>> But isn't this just the way the SSI specification is? If we're
> >>>> modelling a "you just have to get this right in software" bit
> >>>> of hardware then we don't need to somehow try to add extra
> >>>> checks in QEMU for whether the software hasn't actually done
> >>>> things right.
> >>>
> >>>  From my reading of things this is a bug in QEMU, not a guest issue.
> >>>
> >>> The fact that aspeed_smc_flash_setup() for example iterates over a
> >>> larger value to send a single byte at a time shows that currently
> >>> everyone *thinks* think function should send a single byte.
> >>
> >> Doesn't that just show that the aspeed flash controller knows
> >> it is always talking to an 8-bit SSI device ? It wouldn't
> >
> > But I don't think that's guaranteed to be true.
> >
> > On real hardware the software could know that it's only ever connected
> > to an 8-bit SSI device, and set the driver accordingly. That seems
> > entirely possible.
> >
> > But the hardware seems to support larger sizes reading [1].
> >
> > In this case the QEMU code is the hardware, and I don't see how we know
> > we are only ever accessing an 8-bit device.
> >
> > 1:
> > https://patchew.org/linux/20220304083643.1079142-1-clg@kaod.org/20220304083643.1079142-5-clg@kaod.org/
>
> On the Aspeed SoC, MMIO accesses (1, 2, 4 bytes) to the SPI controller
> are converted by the hardware into SPI bus transfers. The exact
> serialization is hardware-specific. QEMU's choice is to generate
> byte-oriented SPI transfers.
>
> In any case, SPI is generally byte-oriented, with bits shifted over
> 1/2/4/8 I/O lines. So byte transfer is sufficient.

Correct, one evidence is the spi-mem driver interface in the Linux
kernel for the spi-nor dummy cycles is converted to bytes. Although
some spi-nor devices do support different bit widths other than
8-bit/16-bit/24-bit/32-bit, other bit widths that are not multiple of
8 are currently not supported in the kernel, neither does QEMU.

>
> C.
>
> >
> >> surprise me if the SPI-NOR standard in particular insisted on
> >> 8-bit transfers, though I can't find anything claiming to be
> >> that standard.
> >>
> >>> So it does seem broken today. Making the current function clear
> >>> seems
> >>> like a good step. Supporting different or larger transfers in the
> >>> future is then possible.
> >>
> >> I think the current interface already supports different or
> >> larger transfers. We just happen to not be using that.
> >
> > Kind of. It does support larger transfers, but there is no size
> > argument. So how can the SSI device know how much was sent? Everything
> > seems to just assume a single byte, but it has no way to actually know
> > that.
> >

Regards,
Bin

On 27/8/26 11:19, Peter Maydell wrote:
> On Tue, 25 Aug 2026 at 05:05, <stephensportia@gmail.com> wrote:
>>
>> From: Portia Stephens <portias@oss.tenstorrent.com>
>>
>> The ssi_transfer function comments say that it takes a word varying
>> between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to transfer
>> but there is no means to indicate the number of bits that should
>> actually be transferred. All child classes of SSI_PERIPHERAL class have
>> transfer functions that, despite accepting a 32-bit tx, only transfer a
>> single byte; m25p80_transfer8(), ssi_sd_transfer(), ssd0323_transfer().
>>
>> The current implementation depends on the SSI model to know
>> what peripheral model will be attached and what transfer size it
>> expects which is error prone. If a SSI_PERIPHERAL model was written that
>> accepted 32-bit transfers, it could not attach to any existing SSI
>> models.
> 
> What is the motivation for this change?
> 
> As far as I know for the hardware SSI protocol, it is indeed
> the case that the SSI controller (and/or the guest software) needs
> to know what transfer size the attached device expects: the
> controller just transmits (or expects to receive) however many
> bits it is programmed for.
> 
>> This change updates the the naming of ssi_transfer to ssi_transfer8, as
>> well as changes the return value and transmit argument to be 8-bit.
> 
> This means that the API will no longer work for a controller
> and device that aren't 8-bit.

Yes exactly. Clearer than the mail I just wrote, thanks hehe.
> We happen not to have any of those
> devices today, but why specifically stop them working?

I guess remembering having see forks with 16-bit devices, but
that was few years ago (I know forks don't have they voice here,
but just to mention the current API is working for them).

On Tue, 2026-08-25 at 14:04 +1000, stephensportia@gmail.com wrote:
> From: Portia Stephens <portias@oss.tenstorrent.com>
> 
> The ssi_transfer function comments say that it takes a word varying
> between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to
> transfer
> but there is no means to indicate the number of bits that should
> actually be transferred. All child classes of SSI_PERIPHERAL class
> have
> transfer functions that, despite accepting a 32-bit tx, only transfer
> a
> single byte; m25p80_transfer8(), ssi_sd_transfer(),
> ssd0323_transfer().
> 
> The current implementation depends on the SSI model to know
> what peripheral model will be attached and what transfer size it
> expects which is error prone. If a SSI_PERIPHERAL model was written
> that
> accepted 32-bit transfers, it could not attach to any existing SSI
> models.
> 
> This change updates the the naming of ssi_transfer to ssi_transfer8,
> as
> well as changes the return value and transmit argument to be 8-bit.
> 
> Most ssi models handle this correctly already, sending a single byte
> at
> a time. There are a few models that are written to support non 8-bit
> transfers but there are no in-tree use cases that connect a
> peripheral
> to the SSI device. These have been updated to use 8-bit transfers.
> 
> Portia Stephens (4):
>   hw/ssi: Rename ssi_transfer to ssi_transfer8
>   hw/ssi/pl022: Fix dropped upper bytes of ssi transfer
>   hw/arm/strongarm: Fix dropped upper byte of ssi transfer
>   hw/ssi/pnv_spi: Fix dropped upper bytes of ssi transfer

Thanks!

Applied to riscv-to-apply.next

Alistair

> 
>  hw/arm/strongarm.c         |  7 +++++--
>  hw/ssi/allwinner-a10-spi.c |  2 +-
>  hw/ssi/aspeed_smc.c        | 14 ++++++-------
>  hw/ssi/bcm2835_spi.c       |  2 +-
>  hw/ssi/ibex_spi_host.c     |  5 +++--
>  hw/ssi/imx_spi.c           |  2 +-
>  hw/ssi/mss-spi.c           |  2 +-
>  hw/ssi/npcm7xx_fiu.c       | 42 +++++++++++++++++++-----------------
> --
>  hw/ssi/npcm_pspi.c         |  4 ++--
>  hw/ssi/pl022.c             | 12 +++++++----
>  hw/ssi/pnv_spi.c           | 28 ++++++++++---------------
>  hw/ssi/sifive_spi.c        |  2 +-
>  hw/ssi/ssi.c               |  4 ++--
>  hw/ssi/stm32f2xx_spi.c     |  2 +-
>  hw/ssi/xilinx_spi.c        | 10 ++++-----
>  hw/ssi/xilinx_spips.c      |  4 ++--
>  hw/ssi/xlnx-versal-ospi.c  |  4 ++--
>  include/hw/ssi/ssi.h       | 17 ++++++++-------
>  18 files changed, 82 insertions(+), 81 deletions(-)

Hi Alistair,

On 27/8/26 07:37, Alistair wrote:
> On Tue, 2026-08-25 at 14:04 +1000, stephensportia@gmail.com wrote:
>> From: Portia Stephens <portias@oss.tenstorrent.com>
>>
>> The ssi_transfer function comments say that it takes a word varying
>> between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to
>> transfer
>> but there is no means to indicate the number of bits that should
>> actually be transferred. All child classes of SSI_PERIPHERAL class
>> have
>> transfer functions that, despite accepting a 32-bit tx, only transfer
>> a
>> single byte; m25p80_transfer8(), ssi_sd_transfer(),
>> ssd0323_transfer().
>>
>> The current implementation depends on the SSI model to know
>> what peripheral model will be attached and what transfer size it
>> expects which is error prone. If a SSI_PERIPHERAL model was written
>> that
>> accepted 32-bit transfers, it could not attach to any existing SSI
>> models.
>>
>> This change updates the the naming of ssi_transfer to ssi_transfer8,
>> as
>> well as changes the return value and transmit argument to be 8-bit.
>>
>> Most ssi models handle this correctly already, sending a single byte
>> at
>> a time. There are a few models that are written to support non 8-bit
>> transfers but there are no in-tree use cases that connect a
>> peripheral
>> to the SSI device. These have been updated to use 8-bit transfers.
>>
>> Portia Stephens (4):
>>    hw/ssi: Rename ssi_transfer to ssi_transfer8
>>    hw/ssi/pl022: Fix dropped upper bytes of ssi transfer
>>    hw/arm/strongarm: Fix dropped upper byte of ssi transfer
>>    hw/ssi/pnv_spi: Fix dropped upper bytes of ssi transfer
> 
> Thanks!
> 
> Applied to riscv-to-apply.next

Can you hold on before merging this please (or drop it from
your riscv queue)? I tagged this series to review but didn't
got a sufficient large enough slot to look at it.

In short, the reason I think this isn't the correct way to go
is SPI "words" can be any number of bits. While QEMU only
models 8-bit word devices, I have be working with 16-bit and
even 24-bit words ones, so I believe the current implementation
is right. I have be thinking of a better way to model this
granularity in our class hooks, but haven't find a good one yet.
Restricting some devices to 8-bit to simplify them move part
of the complexity to the host controller so I'm not sure it
is useful. I'll return with clearer comments when I get more
time.

Thanks,

Phil.

On Thu, 27 Aug 2026 at 06:37, Alistair <alistair@alistair23.me> wrote:
>
> On Tue, 2026-08-25 at 14:04 +1000, stephensportia@gmail.com wrote:
> > From: Portia Stephens <portias@oss.tenstorrent.com>
> >
> > The ssi_transfer function comments say that it takes a word varying
> > between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to
> > transfer
> > but there is no means to indicate the number of bits that should
> > actually be transferred. All child classes of SSI_PERIPHERAL class
> > have
> > transfer functions that, despite accepting a 32-bit tx, only transfer
> > a
> > single byte; m25p80_transfer8(), ssi_sd_transfer(),
> > ssd0323_transfer().
> >
> > The current implementation depends on the SSI model to know
> > what peripheral model will be attached and what transfer size it
> > expects which is error prone. If a SSI_PERIPHERAL model was written
> > that
> > accepted 32-bit transfers, it could not attach to any existing SSI
> > models.
> >
> > This change updates the the naming of ssi_transfer to ssi_transfer8,
> > as
> > well as changes the return value and transmit argument to be 8-bit.
> >
> > Most ssi models handle this correctly already, sending a single byte
> > at
> > a time. There are a few models that are written to support non 8-bit
> > transfers but there are no in-tree use cases that connect a
> > peripheral
> > to the SSI device. These have been updated to use 8-bit transfers.
> >
> > Portia Stephens (4):
> >   hw/ssi: Rename ssi_transfer to ssi_transfer8
> >   hw/ssi/pl022: Fix dropped upper bytes of ssi transfer
> >   hw/arm/strongarm: Fix dropped upper byte of ssi transfer
> >   hw/ssi/pnv_spi: Fix dropped upper bytes of ssi transfer
>
> Thanks!
>
> Applied to riscv-to-apply.next

Would you mind holding off on that until we figure out whether
this is a correct change and why we need it, please?

thanks
-- PMM

On Thu, 2026-08-27 at 10:20 +0100, Peter Maydell wrote:
> On Thu, 27 Aug 2026 at 06:37, Alistair <alistair@alistair23.me>
> wrote:
> > 
> > On Tue, 2026-08-25 at 14:04 +1000, stephensportia@gmail.com wrote:
> > > From: Portia Stephens <portias@oss.tenstorrent.com>
> > > 
> > > The ssi_transfer function comments say that it takes a word
> > > varying
> > > between 8-bits and 32-bits. ssi_transfer takes a 32-bit arg to
> > > transfer
> > > but there is no means to indicate the number of bits that should
> > > actually be transferred. All child classes of SSI_PERIPHERAL
> > > class
> > > have
> > > transfer functions that, despite accepting a 32-bit tx, only
> > > transfer
> > > a
> > > single byte; m25p80_transfer8(), ssi_sd_transfer(),
> > > ssd0323_transfer().
> > > 
> > > The current implementation depends on the SSI model to know
> > > what peripheral model will be attached and what transfer size it
> > > expects which is error prone. If a SSI_PERIPHERAL model was
> > > written
> > > that
> > > accepted 32-bit transfers, it could not attach to any existing
> > > SSI
> > > models.
> > > 
> > > This change updates the the naming of ssi_transfer to
> > > ssi_transfer8,
> > > as
> > > well as changes the return value and transmit argument to be 8-
> > > bit.
> > > 
> > > Most ssi models handle this correctly already, sending a single
> > > byte
> > > at
> > > a time. There are a few models that are written to support non 8-
> > > bit
> > > transfers but there are no in-tree use cases that connect a
> > > peripheral
> > > to the SSI device. These have been updated to use 8-bit
> > > transfers.
> > > 
> > > Portia Stephens (4):
> > >   hw/ssi: Rename ssi_transfer to ssi_transfer8
> > >   hw/ssi/pl022: Fix dropped upper bytes of ssi transfer
> > >   hw/arm/strongarm: Fix dropped upper byte of ssi transfer
> > >   hw/ssi/pnv_spi: Fix dropped upper bytes of ssi transfer
> > 
> > Thanks!
> > 
> > Applied to riscv-to-apply.next
> 
> Would you mind holding off on that until we figure out whether
> this is a correct change and why we need it, please?

Sure. Sorry I thought no one else was interested in reading it. Will
drop it until discussions are sorted

Alistair

> 
> thanks
> -- PMM