[PATCH 6/7] spi: atmel-quadspi: Add support for classic SPI mode

[Eugen Hristev]

Hello Alex,

On 1/23/25 14:12, Alexander Dahl wrote:
> The qspi controller on sama5d2 and sam9x60 supports "classic" SPI mode
> without spi-mem enhancements and accelerations, very similar to the old
> SPI controller on sam9g20 or the modern flexcom controllers of the same
> SoC family.
> 
> Register interface differs somewhat, especially because only one
> hardware controlled CS line is supported.  Some fields are missing, some
> are in different registers, but in principal it works similar.  So code
> is very much inspired by the old atmel-spi driver.
> 
> Tested on sam9x60 with a non-mainline driver to configure an FPGA.
> 
> Signed-off-by: Alexander Dahl <ada at thorsis.com>
> ---
>  drivers/spi/atmel-quadspi.c | 155 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 154 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index d29b5ab96e9..a2faac5a505 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -10,11 +10,13 @@
>   */
>  
>  #include <malloc.h>
> +#include <asm/gpio.h>
>  #include <asm/io.h>
>  #include <clk.h>
>  #include <dm.h>
>  #include <errno.h>
>  #include <fdtdec.h>
> +#include <log.h>
>  #include <dm/device_compat.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
> @@ -258,6 +260,7 @@ struct atmel_qspi_caps {
>  
>  struct atmel_qspi_priv_ops;
>  
> +#define MAX_CS_COUNT	2
>  struct atmel_qspi {
>  	void __iomem *regs;
>  	void __iomem *mem;
> @@ -267,6 +270,7 @@ struct atmel_qspi {
>  	struct udevice *dev;
>  	ulong bus_clk_rate;
>  	u32 mr;
> +	struct gpio_desc cs_gpios[MAX_CS_COUNT];
>  };
>  
>  struct atmel_qspi_priv_ops {
> @@ -940,6 +944,135 @@ static int atmel_qspi_sama7g5_set_speed(struct udevice *bus, uint hz)
>  	return ret;
>  }
>  
> +static int atmel_qspi_claim_bus(struct udevice *dev)
> +{
> +	struct udevice *bus = dev_get_parent(dev);
> +	struct atmel_qspi *aq = dev_get_priv(bus);
> +	int ret;
> +
> +	aq->mr &= ~QSPI_MR_CSMODE_MASK;
> +	aq->mr |= QSPI_MR_CSMODE_LASTXFER | QSPI_MR_WDRBT;
> +	atmel_qspi_write(aq->mr, aq, QSPI_MR);
> +
> +	ret = atmel_qspi_set_serial_memory_mode(aq, false);
> +	if (ret)
> +		return log_ret(ret);
> +
> +	/* de-assert all chip selects */
> +	if (IS_ENABLED(CONFIG_DM_GPIO)) {
> +		for (int i = 0; i < ARRAY_SIZE(aq->cs_gpios); i++) {
> +			if (dm_gpio_is_valid(&aq->cs_gpios[i]))
> +				dm_gpio_set_value(&aq->cs_gpios[i], 0);
> +		}
> +	}
> +
> +	atmel_qspi_write(QSPI_CR_QSPIEN, aq, QSPI_CR);
> +
> +	return 0;
> +}
> +
> +static int atmel_qspi_release_bus(struct udevice *dev)
> +{
> +	struct udevice *bus = dev_get_parent(dev);
> +	struct atmel_qspi *aq = dev_get_priv(bus);
> +
> +	/* de-assert all chip selects */
> +	if (IS_ENABLED(CONFIG_DM_GPIO)) {
> +		for (int i = 0; i < ARRAY_SIZE(aq->cs_gpios); i++) {
> +			if (dm_gpio_is_valid(&aq->cs_gpios[i]))
> +				dm_gpio_set_value(&aq->cs_gpios[i], 0);
> +		}
> +	}
> +
> +	atmel_qspi_write(QSPI_CR_QSPIDIS, aq, QSPI_CR);
> +
> +	return 0;
> +}
> +
> +static int atmel_qspi_set_cs(struct udevice *dev, int value)
> +{
> +	struct udevice *bus = dev_get_parent(dev);
> +	struct atmel_qspi *aq = dev_get_priv(bus);
> +	int cs = spi_chip_select(dev);
> +
> +	if (IS_ENABLED(CONFIG_DM_GPIO)) {
> +		if (!dm_gpio_is_valid(&aq->cs_gpios[cs]))
> +			return log_ret(-ENOENT);
> +
> +		return dm_gpio_set_value(&aq->cs_gpios[cs], value);
> +	} else {
> +		return -ENOENT;
> +	}
> +}
> +
> +static int atmel_qspi_xfer(struct udevice *dev, unsigned int bitlen,
> +			   const void *dout, void *din, unsigned long flags)
> +{
> +	struct udevice *bus = dev_get_parent(dev);
> +	struct atmel_qspi *aq = dev_get_priv(bus);
> +	unsigned int len, len_rx, len_tx;
> +	const u8 *txp = dout;
> +	u8 *rxp = din;
> +	u32 reg;
> +	int ret;
> +
> +	if (bitlen == 0)
> +		goto out;
> +
> +	if (bitlen % 8) {
> +		flags |= SPI_XFER_END;
> +		goto out;
> +	}
> +
> +	len = bitlen / 8;
> +
> +	if (flags & SPI_XFER_BEGIN) {
> +		ret = atmel_qspi_set_cs(dev, 1);
> +		if (ret)
> +			return log_ret(ret);
> +		reg = atmel_qspi_read(aq, QSPI_RD);
> +	}
> +
> +	for (len_tx = 0, len_rx = 0; len_rx < len; ) {
> +		u32 status = atmel_qspi_read(aq, QSPI_SR);
> +		u8 value;
> +
> +		if (status & QSPI_SR_OVRES)
> +			return log_ret(-1);

Any specific meaning for '-1' ?

> +
> +		if (len_tx < len && (status & QSPI_SR_TDRE)) {
> +			if (txp)
> +				value = *txp++;
> +			else
> +				value = 0;
> +			atmel_qspi_write(value, aq, QSPI_TD);
> +			len_tx++;
> +		}
> +
> +		if (status & QSPI_SR_RDRF) {
> +			value = atmel_qspi_read(aq, QSPI_RD);
> +			if (rxp)
> +				*rxp++ = value;
> +			len_rx++;

As this loop is finished only when len_rx == len, can it happen that
this increment is always skipped thus forever looping ?

> +		}
> +	}
> +
> +out:
> +	if (flags & SPI_XFER_END) {
> +		readl_poll_timeout(aq->regs + QSPI_SR, reg,
> +				   reg & QSPI_SR_TXEMPTY,
> +				   ATMEL_QSPI_TIMEOUT);
> +
> +		atmel_qspi_write(QSPI_CR_LASTXFER, aq, QSPI_CR);
> +
> +		ret = atmel_qspi_set_cs(dev, 0);
> +		if (ret)
> +			return log_ret(ret);
> +	}
> +
> +	return 0;
> +}
> +
>  static int atmel_qspi_set_speed(struct udevice *bus, uint hz)
>  {
>  	struct atmel_qspi *aq = dev_get_priv(bus);
> @@ -1089,6 +1222,23 @@ static int atmel_qspi_probe(struct udevice *dev)
>  	else
>  		aq->ops = &atmel_qspi_priv_ops;
>  
> +	if (IS_ENABLED(CONFIG_DM_GPIO)) {
> +		ret = gpio_request_list_by_name(dev, "cs-gpios", aq->cs_gpios,
> +						ARRAY_SIZE(aq->cs_gpios), 0);
> +		if (ret < 0) {
> +			pr_err("Can't get %s gpios! Error: %d", dev->name, ret);

Is cs-gpios mandatory ? Apparently binding does not enforce it

> +			return ret;
> +		}
> +
> +		for (int i = 0; i < ARRAY_SIZE(aq->cs_gpios); i++) {
> +			if (!dm_gpio_is_valid(&aq->cs_gpios[i]))
> +				continue;
in here you just skip if gpio isn't valid apparently.
> +
> +			dm_gpio_set_dir_flags(&aq->cs_gpios[i],
> +					      GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
> +		}
> +	}
> +
>  	/* Map the registers */
>  	ret = dev_read_resource_byname(dev, "qspi_base", &res);
>  	if (ret) {
> @@ -1127,9 +1277,12 @@ static const struct spi_controller_mem_ops atmel_qspi_mem_ops = {
>  };
>  
>  static const struct dm_spi_ops atmel_qspi_ops = {
> +	.claim_bus = atmel_qspi_claim_bus,
> +	.release_bus = atmel_qspi_release_bus,
> +	.xfer = atmel_qspi_xfer,
> +	.mem_ops = &atmel_qspi_mem_ops,
>  	.set_speed = atmel_qspi_set_speed,
>  	.set_mode = atmel_qspi_set_mode,
> -	.mem_ops = &atmel_qspi_mem_ops,
>  };
>  
>  static const struct atmel_qspi_caps atmel_sama5d2_qspi_caps = {};