Hi Dario, On Sat, 21 Feb 2026 at 02:43, Dario Binacchi <[email protected]> wrote: > > Allow dynamic configuration of the SPI word length. This is required > for controllers and slaves that need to operate with non-standard > word lengths, such as 9-bit wide transfers. > > Signed-off-by: Dario Binacchi <[email protected]> > --- > > (no changes since v1) > > drivers/spi/spi-uclass.c | 19 +++++++++++++++++++ > include/spi.h | 12 ++++++++++++ > 2 files changed, 31 insertions(+) >
Reviewed-by: Simon Glass <[email protected]> Thoughts below > diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c > index 49b584c648d6..a4a3b6c25b7d 100644 > --- a/drivers/spi/spi-uclass.c > +++ b/drivers/spi/spi-uclass.c > @@ -91,6 +91,20 @@ void dm_spi_release_bus(struct udevice *dev) > ops->release_bus(dev); > } > > +int dm_spi_set_wordlen(struct udevice *dev, unsigned int wordlen) > +{ > + struct udevice *bus = dev->parent; > + struct dm_spi_ops *ops = spi_get_ops(bus); > + > + if (bus->uclass->uc_drv->id != UCLASS_SPI) > + return -EOPNOTSUPP; This should not happen, right? We don't normally include these kinds of checks in U-Boot, to avoid code-size costs. I have long thought about adding 'CONFIG_SAFE' option behind which we could hide checks, which people could then enable if they with: if (CONFIG_IS_ENABLED(SAFE) && bus && bus->uclass && bus->uclass->uc_drv && bus->uclass->uc_drv->id != UCLASS_SPI) return -[error number to use for all of these checks]; (would need to check dev->parent and ops too) > + > + if (!ops->set_wordlen) > + return -ENOSYS; > + > + return ops->set_wordlen(dev, wordlen); > +} > + > int dm_spi_xfer(struct udevice *dev, unsigned int bitlen, > const void *dout, void *din, unsigned long flags) > { > @@ -144,6 +158,11 @@ int spi_set_speed(struct spi_slave *slave, uint hz) > return ret; > } > > +int spi_set_wordlen(struct spi_slave *slave, unsigned int wordlen) > +{ > + return dm_spi_set_wordlen(slave->dev, wordlen); > +} > + > int spi_xfer(struct spi_slave *slave, unsigned int bitlen, > const void *dout, void *din, unsigned long flags) > { > diff --git a/include/spi.h b/include/spi.h > index 2783200d663e..a92f54a7404b 100644 > --- a/include/spi.h > +++ b/include/spi.h > @@ -729,6 +729,18 @@ int dm_spi_claim_bus(struct udevice *dev); > */ > void dm_spi_release_bus(struct udevice *dev); > > +/** > + * Set the word length for SPI transactions > + * > + * Set the word length (number of bits per word) for SPI transactions. > + * > + * @slave: The SPI slave > + * @wordlen: The number of bits in a word > + * > + * Returns: 0 on success, -1 on failure. > + */ > +int dm_spi_set_wordlen(struct udevice *dev, unsigned int wordlen); > + > /** > * SPI transfer > * > -- > 2.43.0 > We should probably store the the wordlen in struct dm_spi_slave_plat We could even avoid a new operation, but then your driver would need to set the word on an xfer() call. Plis your new operation lines up with the existing ones. So this seems OK to me. Regards, Simon
