diff mbox

[RFC,2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller

Message ID 1432106871-27232-2-git-send-email-ranjit.waghmode@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ranjit Waghmode May 20, 2015, 7:27 a.m. UTC
This patch adds support for GQSPI controller driver used by
Zynq Ultrascale+ MPSoC

Signed-off-by: Ranjit Waghmode <ranjit.waghmode@xilinx.com>
---
 drivers/spi/Kconfig            |    6 +
 drivers/spi/Makefile           |    1 +
 drivers/spi/spi-zynqmp-gqspi.c | 1092 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1099 insertions(+)
 create mode 100644 drivers/spi/spi-zynqmp-gqspi.c

--
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Soren Brinkmann May 20, 2015, 2:55 p.m. UTC | #1
Hi Ranjit,

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
> This patch adds support for GQSPI controller driver used by
> Zynq Ultrascale+ MPSoC
> 
> Signed-off-by: Ranjit Waghmode <ranjit.waghmode@xilinx.com>
> ---
[...]
> +/**
> + * zynqmp_gqspi_read:	For GQSPI controller read operation
> + * @xqspi:	Pointer to the zynqmp_qspi structure
> + * @offset:	Offset from where to read
> + */
> +static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset)
> +{
> +	return readl_relaxed(xqspi->regs + offset);
> +}
> +
> +/**
> + * zynqmp_gqspi_write:	For GQSPI controller write operation
> + * @xqspi:	Pointer to the zynqmp_qspi structure
> + * @offset:	Offset where to write
> + * @val:	Value to be written
> + */
> +static inline void zynqmp_gqspi_write(struct zynqmp_qspi *xqspi, u32 offset,
> +				      u32 val)
> +{
> +	writel_relaxed(val, (xqspi->regs + offset));
> +}

why are you wrapping (readl|writel)_relaxed?

[...]
> +/**
> + * zynqmp_qspi_copy_read_data:	Copy data to RX buffer
> + * @xqspi:	Pointer to the zynqmp_qspi structure
> + * @data:	The 32 bit variable where data is stored
> + * @size:	Number of bytes to be copied from data to RX buffer
> + */
> +static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi *xqspi,
> +				       u32 data, u8 size)
> +{
> +	memcpy(xqspi->rxbuf, ((u8 *) &data), size);

Is this cast really needed? IIRC, memcpy takes (void *). The outer
parentheses for that argument are not needed.

> +	xqspi->rxbuf += size;
> +	xqspi->bytes_to_receive -= size;
> +}
> +
> +/**
> + * zynqmp_prepare_transfer_hardware:	Prepares hardware for transfer.
> + * @master:	Pointer to the spi_master structure which provides
> + *		information about the controller.
> + *
> + * This function enables SPI master controller.
> + *
> + * Return:	Always 0
> + */
> +static int zynqmp_prepare_transfer_hardware(struct spi_master *master)
> +{
> +	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
> +
> +	clk_enable(xqspi->refclk);
> +	clk_enable(xqspi->pclk);

Did you consider using runtime_pm? Then this would just bit
pm_runtime_get_sync(...)

> +	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
> +	return 0;
> +}
> +
> +/**
> + * zynqmp_unprepare_transfer_hardware:	Relaxes hardware after transfer
> + * @master:	Pointer to the spi_master structure which provides
> + *		information about the controller.
> + *
> + * This function disables the SPI master controller.
> + *
> + * Return:	Always 0
> + */
> +static int zynqmp_unprepare_transfer_hardware(struct spi_master *master)
> +{
> +	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
> +
> +	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
> +	clk_disable(xqspi->refclk);
> +	clk_disable(xqspi->pclk);

and this would become pm_runtime_put()

[...]
> +/**
> + * zynqmp_qspi_filltxfifo:	Fills the TX FIFO as long as there is room in
> + *				the FIFO or the bytes required to be
> + *				transmitted.
> + * @xqspi:	Pointer to the zynqmp_qspi structure
> + * @size:	Number of bytes to be copied from TX buffer to TX FIFO
> + */
> +static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int size)
> +{
> +	u32 count = 0;
> +
> +	while ((xqspi->bytes_to_transfer > 0) && (count < size)) {
> +		writel(*((u32 *) xqspi->txbuf), xqspi->regs + GQSPI_TXD_OFST);

Here the writel wrapper is not used. Is there a reason, then it would
probably  deserve a comment.

[...]
> +/**
> + * zynqmp_process_dma_irq:	Handler for DMA done interrupt of QSPI
> + *				controller
> + * @xqspi:	zynqmp_qspi instance pointer
> + *
> + * This function handles DMA interrupt only.
> + */
> +static void zynqmp_process_dma_irq(struct zynqmp_qspi *xqspi)
> +{
> +	u32 config_reg, genfifoentry;
> +
> +	dma_unmap_single(xqspi->dev, xqspi->dma_addr,
> +				xqspi->dma_rx_bytes, DMA_FROM_DEVICE);
> +	xqspi->rxbuf += xqspi->dma_rx_bytes;
> +	xqspi->bytes_to_receive -= xqspi->dma_rx_bytes;
> +	xqspi->dma_rx_bytes = 0;
> +
> +	/* Disabling the DMA interrupts */
> +	writel(GQSPI_QSPIDMA_DST_I_EN_DONE_MASK,
> +			xqspi->regs + GQSPI_QSPIDMA_DST_I_DIS_OFST);
> +
> +	if (xqspi->bytes_to_receive > 0) {
> +		/* Switch to IO mode,for remaining bytes to receive */
> +		config_reg = readl(xqspi->regs + GQSPI_CONFIG_OFST);
> +		config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
> +		writel(config_reg, xqspi->regs + GQSPI_CONFIG_OFST);
> +
> +		/* Initiate the transfer of remaining bytes */
> +		genfifoentry = xqspi->genfifoentry;
> +		genfifoentry |= xqspi->bytes_to_receive;
> +		writel(genfifoentry,
> +				xqspi->regs + GQSPI_GEN_FIFO_OFST);
> +
> +		/* Dummy generic FIFO entry */
> +		writel(0x0, xqspi->regs + GQSPI_GEN_FIFO_OFST);

not using wrapper?

> +
> +		/* Manual start */
> +		zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
> +			(readl(xqspi->regs + GQSPI_CONFIG_OFST) |

not using wrapper?

Overall:
The usage of those readl/writel wrappers seems inconsistent to me. I
usually prefer not wrapping things like that at all but at least it
should be consistent.

And I believe the handling of clocks would benefit from using of the
runtime_pm framework.

	Thanks,
	Sören
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown May 20, 2015, 3:25 p.m. UTC | #2
On Wed, May 20, 2015 at 07:55:53AM -0700, Sören Brinkmann wrote:
> On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:

> > +static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset)

> > +static inline void zynqmp_gqspi_write(struct zynqmp_qspi *xqspi, u32 offset,
> > +				      u32 val)

> why are you wrapping (readl|writel)_relaxed?

It's very common to wrap the lookup from driver data to the MMIO
address.
Mark Brown May 22, 2015, 11:58 a.m. UTC | #3
On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote:

This looks pretty good overall but there are a few issues below from a
first read through.

> +static void zynqmp_gqspi_selectflash(struct zynqmp_qspi *instanceptr,
> +				     u8 flashcs, u8 flashbus)

Is this a SPI controller or a flash controller?  It looks like it is
actually a SPI controller but the above is a bit worrying.

> +{
> +	/*
> +	 * Bus and CS lines selected here will be updated in the instance and
> +	 * used for subsequent GENFIFO entries during transfer.
> +	 */
> +	/* Choose slave select line */

Coding style - at least a blank linne between the two comment blocks.

> +	switch (flashcs) {
> +	case GQSPI_SELECT_FLASH_CS_BOTH:
> +		instanceptr->genfifocs = GQSPI_GENFIFO_CS_LOWER |
> +		    GQSPI_GENFIFO_CS_UPPER;
> +		break;
> +	case GQSPI_SELECT_FLASH_CS_UPPER:
> +		instanceptr->genfifocs = GQSPI_GENFIFO_CS_UPPER;
> +		break;
> +	case GQSPI_SELECT_FLASH_CS_LOWER:
> +	default:
> +		instanceptr->genfifocs = GQSPI_GENFIFO_CS_LOWER;
> +	}

Why is there a default case here?  That's going to men we try to handle
any random chip select that gets passed in as pointing to this lower
device which doesn't seem right.  The fact that this is trying to handle
mirroring of the chip select to two devices is also raising alarm bells
here...

> +/**
> + * zynqmp_qspi_copy_read_data:	Copy data to RX buffer
> + * @xqspi:	Pointer to the zynqmp_qspi structure
> + * @data:	The 32 bit variable where data is stored
> + * @size:	Number of bytes to be copied from data to RX buffer
> + */
> +static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi *xqspi,
> +				       u32 data, u8 size)
> +{
> +	memcpy(xqspi->rxbuf, ((u8 *) &data), size);
> +	xqspi->rxbuf += size;
> +	xqspi->bytes_to_receive -= size;
> +}

This looks like there's some type abuse going on and is going to be
broken for 64 bit systems - why are we passing pointers around as 32 bit
unsigned values?

> +static int zynqmp_prepare_transfer_hardware(struct spi_master *master)
> +{
> +	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
> +
> +	clk_enable(xqspi->refclk);
> +	clk_enable(xqspi->pclk);

Should be checking return codes here.

> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool is_high)
> +{

> +	if (is_high) {
> +		/* Manually start the generic FIFO command */
> +		zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
> +				zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
> +				GQSPI_CFG_START_GEN_FIFO_MASK);

No, this is broken - setting the chip select should set the chip select,
it shouldn't have any impact on transfers.  Transfers should be started
in the transfer operations.

> +	/* If req_hz == 0, default to lowest speed */
> +	while ((baud_rate_val < GQSPI_BAUD_DIV_MAX) &&
> +	       (clk_get_rate(xqspi->refclk) /
> +		(GQSPI_BAUD_DIV_SHIFT << baud_rate_val)) > req_hz)
> +		baud_rate_val++;

It'd be better to factor the clk_get_rate() out of the loop rather than
repeatedly calling into the clock API.

> + * Sets the operational mode of QSPI controller for the next QSPI transfer,
> + * baud rate and divisor value to setup the requested qspi clock.
> + *
> + * Return:	0 Always
> + */
> +static int zynqmp_qspi_setup(struct spi_device *qspi)
> +{
> +	if (qspi->master->busy)
> +		return -EBUSY;
> +	return zynqmp_qspi_setup_transfer(qspi, NULL);
> +}

The setup() function shouldn't affect the hardwaer but _setup_transfer()
does.  See spi-summary.

> +static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int size)
> +{
> +	u32 count = 0;
> +
> +	while ((xqspi->bytes_to_transfer > 0) && (count < size)) {
> +		writel(*((u32 *) xqspi->txbuf), xqspi->regs + GQSPI_TXD_OFST);
> +		if (xqspi->bytes_to_transfer >= 4) {
> +			xqspi->txbuf += 4;
> +			xqspi->bytes_to_transfer -= 4;
> +		} else {
> +			xqspi->txbuf += xqspi->bytes_to_transfer;
> +			xqspi->bytes_to_transfer = 0;
> +		}
> +		count++;
> +	}
> +}

This doesn't appear to take any account of endianness which is
especially alarming in the case where we're dealing with the final word
and the casting here looks unsafe.  I'd expect to be using endanness
annotated pointers with no need for casting.

> + */
> +static void zynqmp_process_dma_irq(struct zynqmp_qspi *xqspi)
> +{
> +	u32 config_reg, genfifoentry;
> +
> +	dma_unmap_single(xqspi->dev, xqspi->dma_addr,
> +				xqspi->dma_rx_bytes, DMA_FROM_DEVICE);

Don't manually do DMA mapping, let the core do it.  It looks like we
need to enhance the core DMA mapping to support partial mapping of
transfers here, this case where you're doing the tail of the transfer as
PIO is a reasonable one (I'm a bit surprised we haven't run into it
before to be honest).

> +static inline u32 zynqmp_qspi_selectspimode(u8 spimode)
> +{
> +	u32 mask;
> +
> +	switch (spimode) {
> +	case GQSPI_SELECT_MODE_DUALSPI:
> +		mask = GQSPI_GENFIFO_MODE_DUALSPI;
> +		break;
> +	case GQSPI_SELECT_MODE_QUADSPI:
> +		mask = GQSPI_GENFIFO_MODE_QUADSPI;
> +		break;
> +	case GQSPI_SELECT_MODE_SPI:
> +	default:
> +		mask = GQSPI_GENFIFO_MODE_SPI;
> +	}

Again this default case looks buggy.

> +static int __maybe_unused zynqmp_qspi_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = container_of(dev,
> +						    struct platform_device,
> +						    dev);
> +	struct spi_master *master = platform_get_drvdata(pdev);
> +
> +	spi_master_suspend(master);
> +
> +	zynqmp_unprepare_transfer_hardware(master);

Why are you manually unpreparing the hardware here?  Obviously if the
core has suspended it ought to have done this...

> +	ret = of_property_read_u32(pdev->dev.of_node, "num-cs", &num_cs);
> +	if (ret < 0)
> +		master->num_chipselect = GQSPI_DEFAULT_NUM_CS;
> +	else
> +		master->num_chipselect = num_cs;

Why is this selectable from DT?  I'm not seeing any code here which
really validates chip select numbers or handles arbatrary chip select
numbers...
Harini Katakam May 22, 2015, 3:13 p.m. UTC | #4
Hi Mark,

On Fri, May 22, 2015 at 5:28 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote:
>
> This looks pretty good overall but there are a few issues below from a
> first read through.
>
>> +static void zynqmp_gqspi_selectflash(struct zynqmp_qspi *instanceptr,
>> +                                  u8 flashcs, u8 flashbus)
>
> Is this a SPI controller or a flash controller?  It looks like it is
> actually a SPI controller but the above is a bit worrying.

It is a Quad SPI controller. SPI NOR flash devices are the most common
interface. But we hope to keep this driver generic.
The above function name is probably misleading; it can be renamed.

>
>> +{
>> +     /*
>> +      * Bus and CS lines selected here will be updated in the instance and
>> +      * used for subsequent GENFIFO entries during transfer.
>> +      */
>> +     /* Choose slave select line */
>
> Coding style - at least a blank linne between the two comment blocks.
>
>> +     switch (flashcs) {
>> +     case GQSPI_SELECT_FLASH_CS_BOTH:
>> +             instanceptr->genfifocs = GQSPI_GENFIFO_CS_LOWER |
>> +                 GQSPI_GENFIFO_CS_UPPER;
>> +             break;
>> +     case GQSPI_SELECT_FLASH_CS_UPPER:
>> +             instanceptr->genfifocs = GQSPI_GENFIFO_CS_UPPER;
>> +             break;
>> +     case GQSPI_SELECT_FLASH_CS_LOWER:
>> +     default:
>> +             instanceptr->genfifocs = GQSPI_GENFIFO_CS_LOWER;
>> +     }
>
> Why is there a default case here?  That's going to men we try to handle
> any random chip select that gets passed in as pointing to this lower
> device which doesn't seem right.  The fact that this is trying to handle
> mirroring of the chip select to two devices is also raising alarm bells
> here...

This SPI controller has two CS lines and two data bus.
Two devices can be connected to these and either the upper or the
lower or both (Explained below) can be selected.

When two flash devices are used, one of the HW configurations in
which they can be connected is called "parallel" mode where they
share the same CS but use separate 4 bit data bus each
(essentially making it 8 bit bus width).
Another configuration is "stacked" mode where the
same 4 bit data bus is shared and lower or upper CS lines are
used to select the flash.

Nevertheless, we will deal with acceptable ways to add such features
incrementally or maybe send an RFC separately.
For the current patch (which is intended to support a single device),
the above selection of "both" CS wont be necessary.
@Ranjit, please remove it.

<snip>
>> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool is_high)
>> +{
>
>> +     if (is_high) {
>> +             /* Manually start the generic FIFO command */
>> +             zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
>> +                             zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
>> +                             GQSPI_CFG_START_GEN_FIFO_MASK);
>
> No, this is broken - setting the chip select should set the chip select,
> it shouldn't have any impact on transfers.  Transfers should be started
> in the transfer operations.
>

This is the only way to assert the CS. It doesn't start transferring any data.
The controller has a Generic FIFO. All operations to be performed on the
bus have to be given in the form of any entry in this FIFO.
The controller fetches each entry from the FIFO and performs the operations.
And that includes asserting and de-asserting CS. There is no dedicated
register bit to assert or de-assert the CS.

The GEN FIFO entry has fields such as:
- TX or RX
- CS - lower/upper
- data bus - lower/upper
- bytecount
- bus width - x1 or x2 or x4
etc.

@Ranjit It might be useful to describe the GENFIFO format in
the driver at some appropriate place.

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ranjit Waghmode May 28, 2015, 11:51 a.m. UTC | #5
SGkgU29yZW4sDQoNClNvcnJ5IGZvciBiZWluZyBsYXRlIGZvciB0aGlzIHJlcGx5IGFzIHRoaXMg
dGhyZWFkIGlzIG1vdmVkIGEgc3RlcCBhaGVhZC4NCg0KT24gV2VkLCAyMDE1LTA1LTIwIGF0IDEy
OjU3UE0gKzA1MzAsIFJhbmppdCBXYWdobW9kZSB3cm90ZToNCj4gVGhpcyBwYXRjaCBhZGRzIHN1
cHBvcnQgZm9yIEdRU1BJIGNvbnRyb2xsZXIgZHJpdmVyIHVzZWQgYnkgWnlucSANCj4gVWx0cmFz
Y2FsZSsgTVBTb0MNCj4gDQo+IFNpZ25lZC1vZmYtYnk6IFJhbmppdCBXYWdobW9kZSA8cmFuaml0
LndhZ2htb2RlQHhpbGlueC5jb20+DQo+IC0tLQ0KWy4uLl0NCj4gKy8qKg0KPiArICogenlucW1w
X2dxc3BpX3JlYWQ6CUZvciBHUVNQSSBjb250cm9sbGVyIHJlYWQgb3BlcmF0aW9uDQo+ICsgKiBA
eHFzcGk6CVBvaW50ZXIgdG8gdGhlIHp5bnFtcF9xc3BpIHN0cnVjdHVyZQ0KPiArICogQG9mZnNl
dDoJT2Zmc2V0IGZyb20gd2hlcmUgdG8gcmVhZA0KPiArICovDQo+ICtzdGF0aWMgdTMyIHp5bnFt
cF9ncXNwaV9yZWFkKHN0cnVjdCB6eW5xbXBfcXNwaSAqeHFzcGksIHUzMiBvZmZzZXQpIHsNCj4g
KwlyZXR1cm4gcmVhZGxfcmVsYXhlZCh4cXNwaS0+cmVncyArIG9mZnNldCk7IH0NCj4gKw0KPiAr
LyoqDQo+ICsgKiB6eW5xbXBfZ3FzcGlfd3JpdGU6CUZvciBHUVNQSSBjb250cm9sbGVyIHdyaXRl
IG9wZXJhdGlvbg0KPiArICogQHhxc3BpOglQb2ludGVyIHRvIHRoZSB6eW5xbXBfcXNwaSBzdHJ1
Y3R1cmUNCj4gKyAqIEBvZmZzZXQ6CU9mZnNldCB3aGVyZSB0byB3cml0ZQ0KPiArICogQHZhbDoJ
VmFsdWUgdG8gYmUgd3JpdHRlbg0KPiArICovDQo+ICtzdGF0aWMgaW5saW5lIHZvaWQgenlucW1w
X2dxc3BpX3dyaXRlKHN0cnVjdCB6eW5xbXBfcXNwaSAqeHFzcGksIHUzMiBvZmZzZXQsDQo+ICsJ
CQkJICAgICAgdTMyIHZhbCkNCj4gK3sNCj4gKwl3cml0ZWxfcmVsYXhlZCh2YWwsICh4cXNwaS0+
cmVncyArIG9mZnNldCkpOyB9DQoNCndoeSBhcmUgeW91IHdyYXBwaW5nIChyZWFkbHx3cml0ZWwp
X3JlbGF4ZWQ/DQoNClsuLi5dDQo+ICsvKioNCj4gKyAqIHp5bnFtcF9xc3BpX2NvcHlfcmVhZF9k
YXRhOglDb3B5IGRhdGEgdG8gUlggYnVmZmVyDQo+ICsgKiBAeHFzcGk6CVBvaW50ZXIgdG8gdGhl
IHp5bnFtcF9xc3BpIHN0cnVjdHVyZQ0KPiArICogQGRhdGE6CVRoZSAzMiBiaXQgdmFyaWFibGUg
d2hlcmUgZGF0YSBpcyBzdG9yZWQNCj4gKyAqIEBzaXplOglOdW1iZXIgb2YgYnl0ZXMgdG8gYmUg
Y29waWVkIGZyb20gZGF0YSB0byBSWCBidWZmZXINCj4gKyAqLw0KPiArc3RhdGljIHZvaWQgenlu
cW1wX3FzcGlfY29weV9yZWFkX2RhdGEoc3RydWN0IHp5bnFtcF9xc3BpICp4cXNwaSwNCj4gKwkJ
CQkgICAgICAgdTMyIGRhdGEsIHU4IHNpemUpDQo+ICt7DQo+ICsJbWVtY3B5KHhxc3BpLT5yeGJ1
ZiwgKCh1OCAqKSAmZGF0YSksIHNpemUpOw0KDQpJcyB0aGlzIGNhc3QgcmVhbGx5IG5lZWRlZD8g
SUlSQywgbWVtY3B5IHRha2VzICh2b2lkICopLiBUaGUgb3V0ZXIgcGFyZW50aGVzZXMgZm9yIHRo
YXQgYXJndW1lbnQgYXJlIG5vdCBuZWVkZWQuDQpbUmFuaml0XSA6IHRha2VuIGNhcmUNCg0KPiAr
CXhxc3BpLT5yeGJ1ZiArPSBzaXplOw0KPiArCXhxc3BpLT5ieXRlc190b19yZWNlaXZlIC09IHNp
emU7DQo+ICt9DQo+ICsNCj4gKy8qKg0KPiArICogenlucW1wX3ByZXBhcmVfdHJhbnNmZXJfaGFy
ZHdhcmU6CVByZXBhcmVzIGhhcmR3YXJlIGZvciB0cmFuc2Zlci4NCj4gKyAqIEBtYXN0ZXI6CVBv
aW50ZXIgdG8gdGhlIHNwaV9tYXN0ZXIgc3RydWN0dXJlIHdoaWNoIHByb3ZpZGVzDQo+ICsgKgkJ
aW5mb3JtYXRpb24gYWJvdXQgdGhlIGNvbnRyb2xsZXIuDQo+ICsgKg0KPiArICogVGhpcyBmdW5j
dGlvbiBlbmFibGVzIFNQSSBtYXN0ZXIgY29udHJvbGxlci4NCj4gKyAqDQo+ICsgKiBSZXR1cm46
CUFsd2F5cyAwDQo+ICsgKi8NCj4gK3N0YXRpYyBpbnQgenlucW1wX3ByZXBhcmVfdHJhbnNmZXJf
aGFyZHdhcmUoc3RydWN0IHNwaV9tYXN0ZXIgDQo+ICsqbWFzdGVyKSB7DQo+ICsJc3RydWN0IHp5
bnFtcF9xc3BpICp4cXNwaSA9IHNwaV9tYXN0ZXJfZ2V0X2RldmRhdGEobWFzdGVyKTsNCj4gKw0K
PiArCWNsa19lbmFibGUoeHFzcGktPnJlZmNsayk7DQo+ICsJY2xrX2VuYWJsZSh4cXNwaS0+cGNs
ayk7DQoNCkRpZCB5b3UgY29uc2lkZXIgdXNpbmcgcnVudGltZV9wbT8gVGhlbiB0aGlzIHdvdWxk
IGp1c3QgYml0DQpwbV9ydW50aW1lX2dldF9zeW5jKC4uLikNCg0KPiArCXp5bnFtcF9ncXNwaV93
cml0ZSh4cXNwaSwgR1FTUElfRU5fT0ZTVCwgR1FTUElfRU5fTUFTSyk7DQo+ICsJcmV0dXJuIDA7
DQo+ICt9DQo+ICsNCj4gKy8qKg0KPiArICogenlucW1wX3VucHJlcGFyZV90cmFuc2Zlcl9oYXJk
d2FyZToJUmVsYXhlcyBoYXJkd2FyZSBhZnRlciB0cmFuc2Zlcg0KPiArICogQG1hc3RlcjoJUG9p
bnRlciB0byB0aGUgc3BpX21hc3RlciBzdHJ1Y3R1cmUgd2hpY2ggcHJvdmlkZXMNCj4gKyAqCQlp
bmZvcm1hdGlvbiBhYm91dCB0aGUgY29udHJvbGxlci4NCj4gKyAqDQo+ICsgKiBUaGlzIGZ1bmN0
aW9uIGRpc2FibGVzIHRoZSBTUEkgbWFzdGVyIGNvbnRyb2xsZXIuDQo+ICsgKg0KPiArICogUmV0
dXJuOglBbHdheXMgMA0KPiArICovDQo+ICtzdGF0aWMgaW50IHp5bnFtcF91bnByZXBhcmVfdHJh
bnNmZXJfaGFyZHdhcmUoc3RydWN0IHNwaV9tYXN0ZXIgDQo+ICsqbWFzdGVyKSB7DQo+ICsJc3Ry
dWN0IHp5bnFtcF9xc3BpICp4cXNwaSA9IHNwaV9tYXN0ZXJfZ2V0X2RldmRhdGEobWFzdGVyKTsN
Cj4gKw0KPiArCXp5bnFtcF9ncXNwaV93cml0ZSh4cXNwaSwgR1FTUElfRU5fT0ZTVCwgMHgwKTsN
Cj4gKwljbGtfZGlzYWJsZSh4cXNwaS0+cmVmY2xrKTsNCj4gKwljbGtfZGlzYWJsZSh4cXNwaS0+
cGNsayk7DQoNCmFuZCB0aGlzIHdvdWxkIGJlY29tZSBwbV9ydW50aW1lX3B1dCgpDQoNClsuLi5d
DQo+ICsvKioNCj4gKyAqIHp5bnFtcF9xc3BpX2ZpbGx0eGZpZm86CUZpbGxzIHRoZSBUWCBGSUZP
IGFzIGxvbmcgYXMgdGhlcmUgaXMgcm9vbSBpbg0KPiArICoJCQkJdGhlIEZJRk8gb3IgdGhlIGJ5
dGVzIHJlcXVpcmVkIHRvIGJlDQo+ICsgKgkJCQl0cmFuc21pdHRlZC4NCj4gKyAqIEB4cXNwaToJ
UG9pbnRlciB0byB0aGUgenlucW1wX3FzcGkgc3RydWN0dXJlDQo+ICsgKiBAc2l6ZToJTnVtYmVy
IG9mIGJ5dGVzIHRvIGJlIGNvcGllZCBmcm9tIFRYIGJ1ZmZlciB0byBUWCBGSUZPDQo+ICsgKi8N
Cj4gK3N0YXRpYyB2b2lkIHp5bnFtcF9xc3BpX2ZpbGx0eGZpZm8oc3RydWN0IHp5bnFtcF9xc3Bp
ICp4cXNwaSwgaW50IA0KPiArc2l6ZSkgew0KPiArCXUzMiBjb3VudCA9IDA7DQo+ICsNCj4gKwl3
aGlsZSAoKHhxc3BpLT5ieXRlc190b190cmFuc2ZlciA+IDApICYmIChjb3VudCA8IHNpemUpKSB7
DQo+ICsJCXdyaXRlbCgqKCh1MzIgKikgeHFzcGktPnR4YnVmKSwgeHFzcGktPnJlZ3MgKyBHUVNQ
SV9UWERfT0ZTVCk7DQoNCkhlcmUgdGhlIHdyaXRlbCB3cmFwcGVyIGlzIG5vdCB1c2VkLiBJcyB0
aGVyZSBhIHJlYXNvbiwgdGhlbiBpdCB3b3VsZCBwcm9iYWJseSAgZGVzZXJ2ZSBhIGNvbW1lbnQu
DQoNClsuLi5dDQo+ICsvKioNCj4gKyAqIHp5bnFtcF9wcm9jZXNzX2RtYV9pcnE6CUhhbmRsZXIg
Zm9yIERNQSBkb25lIGludGVycnVwdCBvZiBRU1BJDQo+ICsgKgkJCQljb250cm9sbGVyDQo+ICsg
KiBAeHFzcGk6CXp5bnFtcF9xc3BpIGluc3RhbmNlIHBvaW50ZXINCj4gKyAqDQo+ICsgKiBUaGlz
IGZ1bmN0aW9uIGhhbmRsZXMgRE1BIGludGVycnVwdCBvbmx5Lg0KPiArICovDQo+ICtzdGF0aWMg
dm9pZCB6eW5xbXBfcHJvY2Vzc19kbWFfaXJxKHN0cnVjdCB6eW5xbXBfcXNwaSAqeHFzcGkpIHsN
Cj4gKwl1MzIgY29uZmlnX3JlZywgZ2VuZmlmb2VudHJ5Ow0KPiArDQo+ICsJZG1hX3VubWFwX3Np
bmdsZSh4cXNwaS0+ZGV2LCB4cXNwaS0+ZG1hX2FkZHIsDQo+ICsJCQkJeHFzcGktPmRtYV9yeF9i
eXRlcywgRE1BX0ZST01fREVWSUNFKTsNCj4gKwl4cXNwaS0+cnhidWYgKz0geHFzcGktPmRtYV9y
eF9ieXRlczsNCj4gKwl4cXNwaS0+Ynl0ZXNfdG9fcmVjZWl2ZSAtPSB4cXNwaS0+ZG1hX3J4X2J5
dGVzOw0KPiArCXhxc3BpLT5kbWFfcnhfYnl0ZXMgPSAwOw0KPiArDQo+ICsJLyogRGlzYWJsaW5n
IHRoZSBETUEgaW50ZXJydXB0cyAqLw0KPiArCXdyaXRlbChHUVNQSV9RU1BJRE1BX0RTVF9JX0VO
X0RPTkVfTUFTSywNCj4gKwkJCXhxc3BpLT5yZWdzICsgR1FTUElfUVNQSURNQV9EU1RfSV9ESVNf
T0ZTVCk7DQo+ICsNCj4gKwlpZiAoeHFzcGktPmJ5dGVzX3RvX3JlY2VpdmUgPiAwKSB7DQo+ICsJ
CS8qIFN3aXRjaCB0byBJTyBtb2RlLGZvciByZW1haW5pbmcgYnl0ZXMgdG8gcmVjZWl2ZSAqLw0K
PiArCQljb25maWdfcmVnID0gcmVhZGwoeHFzcGktPnJlZ3MgKyBHUVNQSV9DT05GSUdfT0ZTVCk7
DQo+ICsJCWNvbmZpZ19yZWcgJj0gfkdRU1BJX0NGR19NT0RFX0VOX01BU0s7DQo+ICsJCXdyaXRl
bChjb25maWdfcmVnLCB4cXNwaS0+cmVncyArIEdRU1BJX0NPTkZJR19PRlNUKTsNCj4gKw0KPiAr
CQkvKiBJbml0aWF0ZSB0aGUgdHJhbnNmZXIgb2YgcmVtYWluaW5nIGJ5dGVzICovDQo+ICsJCWdl
bmZpZm9lbnRyeSA9IHhxc3BpLT5nZW5maWZvZW50cnk7DQo+ICsJCWdlbmZpZm9lbnRyeSB8PSB4
cXNwaS0+Ynl0ZXNfdG9fcmVjZWl2ZTsNCj4gKwkJd3JpdGVsKGdlbmZpZm9lbnRyeSwNCj4gKwkJ
CQl4cXNwaS0+cmVncyArIEdRU1BJX0dFTl9GSUZPX09GU1QpOw0KPiArDQo+ICsJCS8qIER1bW15
IGdlbmVyaWMgRklGTyBlbnRyeSAqLw0KPiArCQl3cml0ZWwoMHgwLCB4cXNwaS0+cmVncyArIEdR
U1BJX0dFTl9GSUZPX09GU1QpOw0KDQpub3QgdXNpbmcgd3JhcHBlcj8NCg0KPiArDQo+ICsJCS8q
IE1hbnVhbCBzdGFydCAqLw0KPiArCQl6eW5xbXBfZ3FzcGlfd3JpdGUoeHFzcGksIEdRU1BJX0NP
TkZJR19PRlNULA0KPiArCQkJKHJlYWRsKHhxc3BpLT5yZWdzICsgR1FTUElfQ09ORklHX09GU1Qp
IHwNCg0Kbm90IHVzaW5nIHdyYXBwZXI/DQoNCk92ZXJhbGw6DQpUaGUgdXNhZ2Ugb2YgdGhvc2Ug
cmVhZGwvd3JpdGVsIHdyYXBwZXJzIHNlZW1zIGluY29uc2lzdGVudCB0byBtZS4gSSB1c3VhbGx5
IHByZWZlciBub3Qgd3JhcHBpbmcgdGhpbmdzIGxpa2UgdGhhdCBhdCBhbGwgYnV0IGF0IGxlYXN0
IGl0IHNob3VsZCBiZSBjb25zaXN0ZW50Lg0KW1JhbmppdF06IFJlbW92aW5nIGFsbCBpbmNvbnNp
c3RlbmNpZXMgYnkgdXNpbmcgd3JhcHBlciBmdW5jdGlvbiBldmVyeXdoZXJlLg0KDQpBbmQgSSBi
ZWxpZXZlIHRoZSBoYW5kbGluZyBvZiBjbG9ja3Mgd291bGQgYmVuZWZpdCBmcm9tIHVzaW5nIG9m
IHRoZSBydW50aW1lX3BtIGZyYW1ld29yay4NCltSYW5qaXRdOiBXaWxsIGFkZCBhYm92ZSBzdWdn
ZXN0aW9uIGFuZCBzZW5kIG5leHQgdmVyc2lvbi4NCg0KVGhhbmtzICYgUmVnYXJkcywNClJhbmpp
dCBXYWdobW9kZQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown May 28, 2015, 3:03 p.m. UTC | #6
On Fri, May 22, 2015 at 08:43:54PM +0530, Harini Katakam wrote:
> On Fri, May 22, 2015 at 5:28 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote:

> > Why is there a default case here?  That's going to men we try to handle
> > any random chip select that gets passed in as pointing to this lower
> > device which doesn't seem right.  The fact that this is trying to handle
> > mirroring of the chip select to two devices is also raising alarm bells
> > here...

> This SPI controller has two CS lines and two data bus.
> Two devices can be connected to these and either the upper or the
> lower or both (Explained below) can be selected.

> When two flash devices are used, one of the HW configurations in
> which they can be connected is called "parallel" mode where they

I know what wiring chip selects in parallel is but that's not the
question - the question is about the handling of the default case.

> >> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool is_high)
> >> +{

> >> +     if (is_high) {
> >> +             /* Manually start the generic FIFO command */
> >> +             zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
> >> +                             zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
> >> +                             GQSPI_CFG_START_GEN_FIFO_MASK);

> > No, this is broken - setting the chip select should set the chip select,
> > it shouldn't have any impact on transfers.  Transfers should be started
> > in the transfer operations.

> This is the only way to assert the CS. It doesn't start transferring any data.

OK, then you can't implement a separate set_cs() operation and shouldn't
be trying to do so.  This will break in multiple ways when the framework
tries to use the operations separately.  You probably need to implement
a single flat transfer() operation.
Punnaiah Choudary Kalluri May 28, 2015, 3:41 p.m. UTC | #7
Hi Mark,

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Thursday, May 28, 2015 8:34 PM
> To: Harini Katakam
> Cc: Ranjit Abhimanyu Waghmode; Rob Herring; Pawel Moll; Mark Rutland;
> ijc+devicetree@hellion.org.uk; Kumar Gala; Michal Simek; Soren Brinkmann;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-spi; Punnaiah Choudary Kalluri;
> ran27jit@gmail.com
> Subject: Re: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC
> GQSPI controller
> 
> On Fri, May 22, 2015 at 08:43:54PM +0530, Harini Katakam wrote:
> > On Fri, May 22, 2015 at 5:28 PM, Mark Brown <broonie@kernel.org> wrote:
> > > On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote:
> 
> > > Why is there a default case here?  That's going to men we try to handle
> > > any random chip select that gets passed in as pointing to this lower
> > > device which doesn't seem right.  The fact that this is trying to handle
> > > mirroring of the chip select to two devices is also raising alarm bells
> > > here...
> 
> > This SPI controller has two CS lines and two data bus.
> > Two devices can be connected to these and either the upper or the
> > lower or both (Explained below) can be selected.
> 
> > When two flash devices are used, one of the HW configurations in
> > which they can be connected is called "parallel" mode where they
> 
> I know what wiring chip selects in parallel is but that's not the
> question - the question is about the handling of the default case.

Yes, we should not handle default case here. We will change this. 

> 
> > >> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool
> is_high)
> > >> +{
> 
> > >> +     if (is_high) {
> > >> +             /* Manually start the generic FIFO command */
> > >> +             zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
> > >> +                             zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
> > >> +                             GQSPI_CFG_START_GEN_FIFO_MASK);
> 
> > > No, this is broken - setting the chip select should set the chip select,
> > > it shouldn't have any impact on transfers.  Transfers should be started
> > > in the transfer operations.
> 
> > This is the only way to assert the CS. It doesn't start transferring any data.
> 
> OK, then you can't implement a separate set_cs() operation and shouldn't
> be trying to do so.  This will break in multiple ways when the framework
> tries to use the operations separately.  You probably need to implement
> a single flat transfer() operation.

As we said it will not start any data transfer. In order to set chip select (chip select only) we need to
1. Frame a control word with following parameters like the chip select that we would like to set and hold time
 2. Update the control word to fifo 

To have a performance advantage (may be not trivial) we are executing this fifo entry along with the first
 Data transfer entry in transfer function so that we can avoid waiting for fifo empty in set_cs function.

 We will ensure CS assert by waiting till the fifo empty in set_cs function and justify the what the 
Function supposed do.

Regards,
Punnaiah
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Harini Katakam May 29, 2015, 10 a.m. UTC | #8
Hi Mark,

On Thu, May 28, 2015 at 9:11 PM, Punnaiah Choudary Kalluri
<punnaiah.choudary.kalluri@xilinx.com> wrote:
> Hi Mark,
>
>> -----Original Message-----
>> From: Mark Brown [mailto:broonie@kernel.org]
>> Sent: Thursday, May 28, 2015 8:34 PM
>> To: Harini Katakam
>> Cc: Ranjit Abhimanyu Waghmode; Rob Herring; Pawel Moll; Mark Rutland;
>> ijc+devicetree@hellion.org.uk; Kumar Gala; Michal Simek; Soren Brinkmann;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; linux-spi; Punnaiah Choudary Kalluri;
>> ran27jit@gmail.com
>> Subject: Re: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC
>> GQSPI controller
>>
>> On Fri, May 22, 2015 at 08:43:54PM +0530, Harini Katakam wrote:
>> > On Fri, May 22, 2015 at 5:28 PM, Mark Brown <broonie@kernel.org> wrote:
>> > > On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote:
>>
>> > > Why is there a default case here?  That's going to men we try to handle
>> > > any random chip select that gets passed in as pointing to this lower
>> > > device which doesn't seem right.  The fact that this is trying to handle
>> > > mirroring of the chip select to two devices is also raising alarm bells
>> > > here...
>>
>> > This SPI controller has two CS lines and two data bus.
>> > Two devices can be connected to these and either the upper or the
>> > lower or both (Explained below) can be selected.
>>
>> > When two flash devices are used, one of the HW configurations in
>> > which they can be connected is called "parallel" mode where they
>>
>> I know what wiring chip selects in parallel is but that's not the
>> question - the question is about the handling of the default case.
>
> Yes, we should not handle default case here. We will change this.
>

Yes, that's right. I was just elaborating the mirroring part.

>>
>> > >> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool
>> is_high)
>> > >> +{
>>
>> > >> +     if (is_high) {
>> > >> +             /* Manually start the generic FIFO command */
>> > >> +             zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
>> > >> +                             zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
>> > >> +                             GQSPI_CFG_START_GEN_FIFO_MASK);
>>
>> > > No, this is broken - setting the chip select should set the chip select,
>> > > it shouldn't have any impact on transfers.  Transfers should be started
>> > > in the transfer operations.
>>
>> > This is the only way to assert the CS. It doesn't start transferring any data.
>>
>> OK, then you can't implement a separate set_cs() operation and shouldn't
>> be trying to do so.  This will break in multiple ways when the framework
>> tries to use the operations separately.  You probably need to implement
>> a single flat transfer() operation.
>
> As we said it will not start any data transfer. In order to set chip select (chip select only) we need to
> 1. Frame a control word with following parameters like the chip select that we would like to set and hold time
>  2. Update the control word to fifo
>
> To have a performance advantage (may be not trivial) we are executing this fifo entry along with the first
>  Data transfer entry in transfer function so that we can avoid waiting for fifo empty in set_cs function.
>
>  We will ensure CS assert by waiting till the fifo empty in set_cs function and justify the what the
> Function supposed do.
>

Yes, if we wait for cs assert to be executed (which is just the time
controller takes to fetch this command
and execute), this set_cs will be independent as expected. The
framework can use it anytime.

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 72b0590..4406a85 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -610,6 +610,12 @@  config SPI_XTENSA_XTFPGA
 	  16 bit words in SPI mode 0, automatically asserting CS on transfer
 	  start and deasserting on end.

+config SPI_ZYNQMP_GQSPI
+	tristate "Xilinx ZynqMP GQSPI controller"
+	depends on SPI_MASTER
+	help
+	  Enables Xilinx GQSPI controller driver for Zynq UltraScale+ MPSoC.
+
 config SPI_NUC900
 	tristate "Nuvoton NUC900 series SPI"
 	depends on ARCH_W90X900
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d8cbf65..52db832 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -89,3 +89,4 @@  obj-$(CONFIG_SPI_TXX9)			+= spi-txx9.o
 obj-$(CONFIG_SPI_XCOMM)		+= spi-xcomm.o
 obj-$(CONFIG_SPI_XILINX)		+= spi-xilinx.o
 obj-$(CONFIG_SPI_XTENSA_XTFPGA)		+= spi-xtensa-xtfpga.o
+obj-$(CONFIG_SPI_ZYNQMP_GQSPI)		+= spi-zynqmp-gqspi.o
diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
new file mode 100644
index 0000000..12ab885
--- /dev/null
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -0,0 +1,1092 @@ 
+/*
+ * Xilinx Zynq UltraScale+ MPSoC Quad-SPI (QSPI) controller driver
+ * (master mode only)
+ *
+ * Copyright (C) 2009 - 2015 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+/* Generic QSPI register offsets */
+#define GQSPI_CONFIG_OFST		0x00000100
+#define GQSPI_ISR_OFST			0x00000104
+#define GQSPI_IDR_OFST			0x0000010C
+#define GQSPI_IER_OFST			0x00000108
+#define GQSPI_IMASK_OFST		0x00000110
+#define GQSPI_EN_OFST			0x00000114
+#define GQSPI_TXD_OFST			0x0000011C
+#define GQSPI_RXD_OFST			0x00000120
+#define GQSPI_TX_THRESHOLD_OFST		0x00000128
+#define GQSPI_RX_THRESHOLD_OFST		0x0000012C
+#define GQSPI_LPBK_DLY_ADJ_OFST		0x00000138
+#define GQSPI_GEN_FIFO_OFST		0x00000140
+#define GQSPI_SEL_OFST			0x00000144
+#define GQSPI_GF_THRESHOLD_OFST		0x00000150
+#define GQSPI_FIFO_CTRL_OFST		0x0000014C
+#define GQSPI_QSPIDMA_DST_CTRL_OFST	0x0000080C
+#define GQSPI_QSPIDMA_DST_SIZE_OFST	0x00000804
+#define GQSPI_QSPIDMA_DST_STS_OFST	0x00000808
+#define GQSPI_QSPIDMA_DST_I_STS_OFST	0x00000814
+#define GQSPI_QSPIDMA_DST_I_EN_OFST	0x00000818
+#define GQSPI_QSPIDMA_DST_I_DIS_OFST	0x0000081C
+#define GQSPI_QSPIDMA_DST_I_MASK_OFST	0x00000820
+#define GQSPI_QSPIDMA_DST_ADDR_OFST	0x00000800
+#define GQSPI_QSPIDMA_DST_ADDR_MSB_OFST 0x00000828
+
+/* GQSPI register bit masks */
+#define GQSPI_SEL_MASK				0x00000001
+#define GQSPI_EN_MASK				0x00000001
+#define GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK	0x00000020
+#define GQSPI_ISR_WR_TO_CLR_MASK		0x00000002
+#define GQSPI_IDR_ALL_MASK			0x00000FBE
+#define GQSPI_CFG_MODE_EN_MASK			0xC0000000
+#define GQSPI_CFG_GEN_FIFO_START_MODE_MASK	0x20000000
+#define GQSPI_CFG_ENDIAN_MASK			0x04000000
+#define GQSPI_CFG_EN_POLL_TO_MASK		0x00100000
+#define GQSPI_CFG_WP_HOLD_MASK			0x00080000
+#define GQSPI_CFG_BAUD_RATE_DIV_MASK		0x00000038
+#define GQSPI_CFG_CLK_PHA_MASK			0x00000004
+#define GQSPI_CFG_CLK_POL_MASK			0x00000002
+#define GQSPI_CFG_START_GEN_FIFO_MASK		0x10000000
+#define GQSPI_GENFIFO_IMM_DATA_MASK		0x000000FF
+#define GQSPI_GENFIFO_DATA_XFER			0x00000100
+#define GQSPI_GENFIFO_EXP			0x00000200
+#define GQSPI_GENFIFO_MODE_SPI			0x00000400
+#define GQSPI_GENFIFO_MODE_DUALSPI		0x00000800
+#define GQSPI_GENFIFO_MODE_QUADSPI		0x00000C00
+#define GQSPI_GENFIFO_MODE_MASK			0x00000C00
+#define GQSPI_GENFIFO_CS_LOWER			0x00001000
+#define GQSPI_GENFIFO_CS_UPPER			0x00002000
+#define GQSPI_GENFIFO_BUS_LOWER			0x00004000
+#define GQSPI_GENFIFO_BUS_UPPER			0x00008000
+#define GQSPI_GENFIFO_BUS_BOTH			0x0000C000
+#define GQSPI_GENFIFO_BUS_MASK			0x0000C000
+#define GQSPI_GENFIFO_TX			0x00010000
+#define GQSPI_GENFIFO_RX			0x00020000
+#define GQSPI_GENFIFO_STRIPE			0x00040000
+#define GQSPI_GENFIFO_POLL			0x00080000
+#define GQSPI_GENFIFO_EXP_START			0x00000100
+#define GQSPI_FIFO_CTRL_RST_RX_FIFO_MASK	0x00000004
+#define GQSPI_FIFO_CTRL_RST_TX_FIFO_MASK	0x00000002
+#define GQSPI_FIFO_CTRL_RST_GEN_FIFO_MASK	0x00000001
+#define GQSPI_ISR_RXEMPTY_MASK			0x00000800
+#define GQSPI_ISR_GENFIFOFULL_MASK		0x00000400
+#define GQSPI_ISR_GENFIFONOT_FULL_MASK		0x00000200
+#define GQSPI_ISR_TXEMPTY_MASK			0x00000100
+#define GQSPI_ISR_GENFIFOEMPTY_MASK		0x00000080
+#define GQSPI_ISR_RXFULL_MASK			0x00000020
+#define GQSPI_ISR_RXNEMPTY_MASK			0x00000010
+#define GQSPI_ISR_TXFULL_MASK			0x00000008
+#define GQSPI_ISR_TXNOT_FULL_MASK		0x00000004
+#define GQSPI_ISR_POLL_TIME_EXPIRE_MASK		0x00000002
+#define GQSPI_IER_TXNOT_FULL_MASK		0x00000004
+#define GQSPI_IER_RXEMPTY_MASK			0x00000800
+#define GQSPI_IER_POLL_TIME_EXPIRE_MASK		0x00000002
+#define GQSPI_IER_RXNEMPTY_MASK			0x00000010
+#define GQSPI_IER_GENFIFOEMPTY_MASK		0x00000080
+#define GQSPI_IER_TXEMPTY_MASK			0x00000100
+#define GQSPI_QSPIDMA_DST_INTR_ALL_MASK		0x000000FE
+#define GQSPI_QSPIDMA_DST_STS_WTC		0x0000E000
+#define GQSPI_CFG_MODE_EN_DMA_MASK		0x80000000
+#define GQSPI_ISR_IDR_MASK			0x00000994
+#define GQSPI_QSPIDMA_DST_I_EN_DONE_MASK	0x00000002
+#define GQSPI_QSPIDMA_DST_I_STS_DONE_MASK	0x00000002
+#define GQSPI_IRQ_MASK				0x00000980
+
+#define GQSPI_CFG_BAUD_RATE_DIV_SHIFT		3
+#define GQSPI_GENFIFO_CS_SETUP			0x4
+#define GQSPI_GENFIFO_CS_HOLD			0x3
+#define GQSPI_TXD_DEPTH				64
+#define GQSPI_RX_FIFO_THRESHOLD			32
+#define GQSPI_RX_FIFO_FILL	(GQSPI_RX_FIFO_THRESHOLD * 4)
+#define GQSPI_TX_FIFO_THRESHOLD_RESET_VAL	32
+#define GQSPI_TX_FIFO_FILL	(GQSPI_TXD_DEPTH -\
+				GQSPI_TX_FIFO_THRESHOLD_RESET_VAL)
+#define GQSPI_GEN_FIFO_THRESHOLD_RESET_VAL	0X10
+#define GQSPI_QSPIDMA_DST_CTRL_RESET_VAL	0x803FFA00
+#define GQSPI_SELECT_FLASH_CS_LOWER		0x1
+#define GQSPI_SELECT_FLASH_CS_UPPER		0x2
+#define GQSPI_SELECT_FLASH_CS_BOTH		0x3
+#define GQSPI_SELECT_FLASH_BUS_LOWER		0x1
+#define GQSPI_SELECT_FLASH_BUS_UPPER		0x2
+#define GQSPI_SELECT_FLASH_BUS_BOTH		0x3
+#define GQSPI_BAUD_DIV_MAX	7	/* Baud rate divisor maximum */
+#define GQSPI_BAUD_DIV_SHIFT	2	/* Baud rate divisor shift */
+#define GQSPI_SELECT_MODE_SPI		0x1
+#define GQSPI_SELECT_MODE_DUALSPI	0x2
+#define GQSPI_SELECT_MODE_QUADSPI	0x4
+#define GQSPI_DMA_UNALIGN		0x3
+#define GQSPI_DEFAULT_NUM_CS	1	/* Default number of chip selects */
+
+enum mode_type {GQSPI_MODE_IO, GQSPI_MODE_DMA};
+
+/**
+ * struct zynqmp_qspi - Defines qspi driver instance
+ * @regs:		Virtual address of the QSPI controller registers
+ * @refclk:		Pointer to the peripheral clock
+ * @pclk:		Pointer to the APB clock
+ * @irq:		IRQ number
+ * @dev:		Pointer to struct device
+ * @txbuf:		Pointer to the TX buffer
+ * @rxbuf:		Pointer to the RX buffer
+ * @bytes_to_transfer:	Number of bytes left to transfer
+ * @bytes_to_receive:	Number of bytes left to receive
+ * @genfifocs:		Used for chip select
+ * @genfifobus:		Used to select the upper or lower bus
+ * @dma_rx_bytes:	Remaining bytes to receive by DMA mode
+ * @dma_addr:		DMA address after mapping the kernel buffer
+ * @genfifoentry	Used for storing the genfifoentry instruction.
+ * @mode:		Defines the mode in which QSPI is operating
+ */
+struct zynqmp_qspi {
+	void __iomem *regs;
+	struct clk *refclk;
+	struct clk *pclk;
+	int irq;
+	struct device *dev;
+	const void *txbuf;
+	void *rxbuf;
+	int bytes_to_transfer;
+	int bytes_to_receive;
+	u32 genfifocs;
+	u32 genfifobus;
+	u32 dma_rx_bytes;
+	dma_addr_t dma_addr;
+	u32 genfifoentry;
+	enum mode_type mode;
+};
+
+/**
+ * zynqmp_gqspi_read:	For GQSPI controller read operation
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ * @offset:	Offset from where to read
+ */
+static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset)
+{
+	return readl_relaxed(xqspi->regs + offset);
+}
+
+/**
+ * zynqmp_gqspi_write:	For GQSPI controller write operation
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ * @offset:	Offset where to write
+ * @val:	Value to be written
+ */
+static inline void zynqmp_gqspi_write(struct zynqmp_qspi *xqspi, u32 offset,
+				      u32 val)
+{
+	writel_relaxed(val, (xqspi->regs + offset));
+}
+
+/**
+ * zynqmp_gqspi_selectflash:	For selection of flash
+ * @instanceptr:	Pointer to the zynqmp_qspi structure
+ * @flashcs:	For chip select
+ * @flashbus:	To check which bus is selected- upper or lower
+ */
+static void zynqmp_gqspi_selectflash(struct zynqmp_qspi *instanceptr,
+				     u8 flashcs, u8 flashbus)
+{
+	/*
+	 * Bus and CS lines selected here will be updated in the instance and
+	 * used for subsequent GENFIFO entries during transfer.
+	 */
+	/* Choose slave select line */
+	switch (flashcs) {
+	case GQSPI_SELECT_FLASH_CS_BOTH:
+		instanceptr->genfifocs = GQSPI_GENFIFO_CS_LOWER |
+		    GQSPI_GENFIFO_CS_UPPER;
+		break;
+	case GQSPI_SELECT_FLASH_CS_UPPER:
+		instanceptr->genfifocs = GQSPI_GENFIFO_CS_UPPER;
+		break;
+	case GQSPI_SELECT_FLASH_CS_LOWER:
+	default:
+		instanceptr->genfifocs = GQSPI_GENFIFO_CS_LOWER;
+	}
+
+	/* Choose bus */
+	switch (flashbus) {
+	case GQSPI_SELECT_FLASH_BUS_BOTH:
+		instanceptr->genfifobus = GQSPI_GENFIFO_BUS_LOWER |
+		    GQSPI_GENFIFO_BUS_UPPER;
+		break;
+	case GQSPI_SELECT_FLASH_BUS_UPPER:
+		instanceptr->genfifobus = GQSPI_GENFIFO_BUS_UPPER;
+		break;
+	case GQSPI_SELECT_FLASH_BUS_LOWER:
+	default:
+		instanceptr->genfifobus = GQSPI_GENFIFO_BUS_LOWER;
+	}
+}
+
+/**
+ * zynqmp_qspi_init_hw:	Initialize the hardware
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ *
+ * The default settings of the QSPI controller's configurable parameters on
+ * reset are
+ *	- Master mode
+ *	- TX threshold set to 1
+ *	- RX threshold set to 1
+ *	- Flash memory interface mode enabled
+ * This function performs the following actions
+ *	- Disable and clear all the interrupts
+ *	- Enable manual slave select
+ *	- Enable manual start
+ *	- Deselect all the chip select lines
+ *	- Set the little endian mode of TX FIFO and
+ *	- Enable the QSPI controller
+ */
+static void zynqmp_qspi_init_hw(struct zynqmp_qspi *xqspi)
+{
+	u32 config_reg;
+
+	/* Select the GQSPI mode */
+	zynqmp_gqspi_write(xqspi, GQSPI_SEL_OFST, GQSPI_SEL_MASK);
+	/* Clear and disable interrupts */
+	zynqmp_gqspi_write(xqspi, GQSPI_ISR_OFST,
+			   zynqmp_gqspi_read(xqspi, GQSPI_ISR_OFST) |
+			   GQSPI_ISR_WR_TO_CLR_MASK);
+	/* Clear the DMA STS */
+	zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_I_STS_OFST,
+			   zynqmp_gqspi_read(xqspi,
+					     GQSPI_QSPIDMA_DST_I_STS_OFST));
+	zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_STS_OFST,
+			   zynqmp_gqspi_read(xqspi,
+					     GQSPI_QSPIDMA_DST_STS_OFST) |
+					     GQSPI_QSPIDMA_DST_STS_WTC);
+	zynqmp_gqspi_write(xqspi, GQSPI_IDR_OFST, GQSPI_IDR_ALL_MASK);
+	zynqmp_gqspi_write(xqspi,
+			   GQSPI_QSPIDMA_DST_I_DIS_OFST,
+			   GQSPI_QSPIDMA_DST_INTR_ALL_MASK);
+	/* Disable the GQSPI */
+	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
+	config_reg = zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST);
+	config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
+	/* Manual start */
+	config_reg |= GQSPI_CFG_GEN_FIFO_START_MODE_MASK;
+	/* Little endian by default */
+	config_reg &= ~GQSPI_CFG_ENDIAN_MASK;
+	/* Disable poll time out */
+	config_reg &= ~GQSPI_CFG_EN_POLL_TO_MASK;
+	/* Set hold bit */
+	config_reg |= GQSPI_CFG_WP_HOLD_MASK;
+	/* Clear pre-scalar by default */
+	config_reg &= ~GQSPI_CFG_BAUD_RATE_DIV_MASK;
+	/* CPHA 0 */
+	config_reg &= ~GQSPI_CFG_CLK_PHA_MASK;
+	/* CPOL 0 */
+	config_reg &= ~GQSPI_CFG_CLK_POL_MASK;
+	zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST, config_reg);
+
+	/* Clear the TX and RX FIFO */
+	zynqmp_gqspi_write(xqspi, GQSPI_FIFO_CTRL_OFST,
+			   GQSPI_FIFO_CTRL_RST_RX_FIFO_MASK |
+			   GQSPI_FIFO_CTRL_RST_TX_FIFO_MASK |
+			   GQSPI_FIFO_CTRL_RST_GEN_FIFO_MASK);
+	/* Set by default to allow for high frequencies */
+	zynqmp_gqspi_write(xqspi, GQSPI_LPBK_DLY_ADJ_OFST,
+			   zynqmp_gqspi_read(xqspi, GQSPI_LPBK_DLY_ADJ_OFST) |
+			   GQSPI_LPBK_DLY_ADJ_USE_LPBK_MASK);
+	/* Reset thresholds */
+	zynqmp_gqspi_write(xqspi, GQSPI_TX_THRESHOLD_OFST,
+			   GQSPI_TX_FIFO_THRESHOLD_RESET_VAL);
+	zynqmp_gqspi_write(xqspi, GQSPI_RX_THRESHOLD_OFST,
+			   GQSPI_RX_FIFO_THRESHOLD);
+	zynqmp_gqspi_write(xqspi, GQSPI_GF_THRESHOLD_OFST,
+			   GQSPI_GEN_FIFO_THRESHOLD_RESET_VAL);
+	zynqmp_gqspi_selectflash(xqspi,
+				 GQSPI_SELECT_FLASH_CS_LOWER,
+				 GQSPI_SELECT_FLASH_BUS_LOWER);
+	/* Initialize DMA */
+	zynqmp_gqspi_write(xqspi,
+			GQSPI_QSPIDMA_DST_CTRL_OFST,
+			GQSPI_QSPIDMA_DST_CTRL_RESET_VAL);
+
+	/* Enable the GQSPI */
+	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
+}
+
+/**
+ * zynqmp_qspi_copy_read_data:	Copy data to RX buffer
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ * @data:	The 32 bit variable where data is stored
+ * @size:	Number of bytes to be copied from data to RX buffer
+ */
+static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi *xqspi,
+				       u32 data, u8 size)
+{
+	memcpy(xqspi->rxbuf, ((u8 *) &data), size);
+	xqspi->rxbuf += size;
+	xqspi->bytes_to_receive -= size;
+}
+
+/**
+ * zynqmp_prepare_transfer_hardware:	Prepares hardware for transfer.
+ * @master:	Pointer to the spi_master structure which provides
+ *		information about the controller.
+ *
+ * This function enables SPI master controller.
+ *
+ * Return:	Always 0
+ */
+static int zynqmp_prepare_transfer_hardware(struct spi_master *master)
+{
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
+
+	clk_enable(xqspi->refclk);
+	clk_enable(xqspi->pclk);
+	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
+	return 0;
+}
+
+/**
+ * zynqmp_unprepare_transfer_hardware:	Relaxes hardware after transfer
+ * @master:	Pointer to the spi_master structure which provides
+ *		information about the controller.
+ *
+ * This function disables the SPI master controller.
+ *
+ * Return:	Always 0
+ */
+static int zynqmp_unprepare_transfer_hardware(struct spi_master *master)
+{
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
+
+	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
+	clk_disable(xqspi->refclk);
+	clk_disable(xqspi->pclk);
+	return 0;
+}
+
+/**
+ * zynqmp_qspi_chipselect:	Select or deselect the chip select line
+ * @qspi:	Pointer to the spi_device structure
+ * @is_high:	Select(0) or deselect (1) the chip select line
+ */
+static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool is_high)
+{
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(qspi->master);
+	u32 genfifoentry = 0x0, statusreg, timeout;
+
+	genfifoentry |= GQSPI_GENFIFO_MODE_SPI;
+	genfifoentry |= xqspi->genfifobus;
+
+	if (!is_high) {
+		genfifoentry |= xqspi->genfifocs;
+		genfifoentry |= GQSPI_GENFIFO_CS_SETUP;
+	} else {
+		genfifoentry |= GQSPI_GENFIFO_CS_HOLD;
+	}
+
+	zynqmp_gqspi_write(xqspi, GQSPI_GEN_FIFO_OFST, genfifoentry);
+
+	if (is_high) {
+		/* Manually start the generic FIFO command */
+		zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
+				zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
+				GQSPI_CFG_START_GEN_FIFO_MASK);
+		timeout = 10000;
+		/* Wait until the generic FIFO command is empty */
+		do {
+			statusreg = zynqmp_gqspi_read(xqspi, GQSPI_ISR_OFST);
+			timeout--;
+		} while (!(statusreg &
+			   GQSPI_ISR_GENFIFOEMPTY_MASK) &&
+			 (statusreg & GQSPI_ISR_TXEMPTY_MASK) && timeout);
+		if (!timeout)
+			dev_err(xqspi->dev, "Chip select timed out\n");
+	}
+}
+
+/**
+ * zynqmp_qspi_setup_transfer:	Configure QSPI controller for specified
+ *				transfer
+ * @qspi:	Pointer to the spi_device structure
+ * @transfer:	Pointer to the spi_transfer structure which provides
+ *		information about next transfer setup parameters
+ *
+ * Sets the operational mode of QSPI controller for the next QSPI transfer and
+ * sets the requested clock frequency.
+ *
+ * Return:	Always 0
+ *
+ * Note:
+ *	If the requested frequency is not an exact match with what can be
+ *	obtained using the pre-scalar value, the driver sets the clock
+ *	frequency which is lower than the requested frequency (maximum lower)
+ *	for the transfer.
+ *
+ *	If the requested frequency is higher or lower than that is supported
+ *	by the QSPI controller the driver will set the highest or lowest
+ *	frequency supported by controller.
+ */
+static int zynqmp_qspi_setup_transfer(struct spi_device *qspi,
+				      struct spi_transfer *transfer)
+{
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(qspi->master);
+	u32 config_reg, req_hz, baud_rate_val = 0;
+
+	if (transfer)
+		req_hz = transfer->speed_hz;
+	else
+		req_hz = qspi->max_speed_hz;
+
+	/* Set the clock frequency */
+	/* If req_hz == 0, default to lowest speed */
+	while ((baud_rate_val < GQSPI_BAUD_DIV_MAX) &&
+	       (clk_get_rate(xqspi->refclk) /
+		(GQSPI_BAUD_DIV_SHIFT << baud_rate_val)) > req_hz)
+		baud_rate_val++;
+
+	config_reg = zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST);
+
+	/* Set the QSPI clock phase and clock polarity */
+	config_reg &= (~GQSPI_CFG_CLK_PHA_MASK) & (~GQSPI_CFG_CLK_POL_MASK);
+
+	if (qspi->mode & SPI_CPHA)
+		config_reg |= GQSPI_CFG_CLK_PHA_MASK;
+	if (qspi->mode & SPI_CPOL)
+		config_reg |= GQSPI_CFG_CLK_POL_MASK;
+
+	config_reg &= ~GQSPI_CFG_BAUD_RATE_DIV_MASK;
+	config_reg |= (baud_rate_val << GQSPI_CFG_BAUD_RATE_DIV_SHIFT);
+	zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST, config_reg);
+	return 0;
+}
+
+/**
+ * zynqmp_qspi_setup:	Configure the QSPI controller
+ * @qspi:	Pointer to the spi_device structure
+ *
+ * Sets the operational mode of QSPI controller for the next QSPI transfer,
+ * baud rate and divisor value to setup the requested qspi clock.
+ *
+ * Return:	0 Always
+ */
+static int zynqmp_qspi_setup(struct spi_device *qspi)
+{
+	if (qspi->master->busy)
+		return -EBUSY;
+	return zynqmp_qspi_setup_transfer(qspi, NULL);
+}
+
+/**
+ * zynqmp_qspi_filltxfifo:	Fills the TX FIFO as long as there is room in
+ *				the FIFO or the bytes required to be
+ *				transmitted.
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ * @size:	Number of bytes to be copied from TX buffer to TX FIFO
+ */
+static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int size)
+{
+	u32 count = 0;
+
+	while ((xqspi->bytes_to_transfer > 0) && (count < size)) {
+		writel(*((u32 *) xqspi->txbuf), xqspi->regs + GQSPI_TXD_OFST);
+		if (xqspi->bytes_to_transfer >= 4) {
+			xqspi->txbuf += 4;
+			xqspi->bytes_to_transfer -= 4;
+		} else {
+			xqspi->txbuf += xqspi->bytes_to_transfer;
+			xqspi->bytes_to_transfer = 0;
+		}
+		count++;
+	}
+}
+
+/**
+ * zynqmp_qspi_readrxfifo:	Fills the RX FIFO as long as there is room in
+ *				the FIFO.
+ * @xqspi:	Pointer to the zynqmp_qspi structure
+ * @size:	Number of bytes to be copied from RX buffer to RX FIFO
+ */
+static void zynqmp_qspi_readrxfifo(struct zynqmp_qspi *xqspi, u32 size)
+{
+	int count = 0;
+	u32 data;
+
+	while ((count < size) && (xqspi->bytes_to_receive > 0)) {
+		if (xqspi->bytes_to_receive >= 4) {
+			(*(u32 *) xqspi->rxbuf) =
+			    readl(xqspi->regs + GQSPI_RXD_OFST);
+			xqspi->rxbuf += 4;
+			xqspi->bytes_to_receive -= 4;
+			count += 4;
+		} else {
+			data = readl(xqspi->regs + GQSPI_RXD_OFST);
+			count += xqspi->bytes_to_receive;
+			zynqmp_qspi_copy_read_data(xqspi, data,
+						   xqspi->bytes_to_receive);
+			xqspi->bytes_to_receive = 0;
+		}
+	}
+}
+
+/**
+ * zynqmp_process_dma_irq:	Handler for DMA done interrupt of QSPI
+ *				controller
+ * @xqspi:	zynqmp_qspi instance pointer
+ *
+ * This function handles DMA interrupt only.
+ */
+static void zynqmp_process_dma_irq(struct zynqmp_qspi *xqspi)
+{
+	u32 config_reg, genfifoentry;
+
+	dma_unmap_single(xqspi->dev, xqspi->dma_addr,
+				xqspi->dma_rx_bytes, DMA_FROM_DEVICE);
+	xqspi->rxbuf += xqspi->dma_rx_bytes;
+	xqspi->bytes_to_receive -= xqspi->dma_rx_bytes;
+	xqspi->dma_rx_bytes = 0;
+
+	/* Disabling the DMA interrupts */
+	writel(GQSPI_QSPIDMA_DST_I_EN_DONE_MASK,
+			xqspi->regs + GQSPI_QSPIDMA_DST_I_DIS_OFST);
+
+	if (xqspi->bytes_to_receive > 0) {
+		/* Switch to IO mode,for remaining bytes to receive */
+		config_reg = readl(xqspi->regs + GQSPI_CONFIG_OFST);
+		config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
+		writel(config_reg, xqspi->regs + GQSPI_CONFIG_OFST);
+
+		/* Initiate the transfer of remaining bytes */
+		genfifoentry = xqspi->genfifoentry;
+		genfifoentry |= xqspi->bytes_to_receive;
+		writel(genfifoentry,
+				xqspi->regs + GQSPI_GEN_FIFO_OFST);
+
+		/* Dummy generic FIFO entry */
+		writel(0x0, xqspi->regs + GQSPI_GEN_FIFO_OFST);
+
+		/* Manual start */
+		zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
+			(readl(xqspi->regs + GQSPI_CONFIG_OFST) |
+			GQSPI_CFG_START_GEN_FIFO_MASK));
+
+		/* Enable the RX interrupts for IO mode */
+		zynqmp_gqspi_write(xqspi, GQSPI_IER_OFST,
+				GQSPI_IER_GENFIFOEMPTY_MASK |
+				GQSPI_IER_RXNEMPTY_MASK |
+				GQSPI_IER_RXEMPTY_MASK);
+	}
+}
+
+/**
+ * zynqmp_qspi_irq:	Interrupt service routine of the QSPI controller
+ * @irq:	IRQ number
+ * @dev_id:	Pointer to the xqspi structure
+ *
+ * This function handles TX empty only.
+ * On TX empty interrupt this function reads the received data from RX FIFO
+ * and fills the TX FIFO if there is any data remaining to be transferred.
+ *
+ * Return:	IRQ_HANDLED when interrupt is handled
+ *		IRQ_NONE otherwise.
+ */
+static irqreturn_t zynqmp_qspi_irq(int irq, void *dev_id)
+{
+	struct spi_master *master = dev_id;
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
+	int ret = IRQ_NONE;
+	u32 status, mask, dma_status = 0;
+
+	status = readl(xqspi->regs + GQSPI_ISR_OFST);
+	writel(status, xqspi->regs + GQSPI_ISR_OFST);
+	mask = (status & ~(readl(xqspi->regs + GQSPI_IMASK_OFST)));
+
+	/* Read and clear DMA status */
+	if (xqspi->mode == GQSPI_MODE_DMA) {
+		dma_status = readl(xqspi->regs + GQSPI_QSPIDMA_DST_I_STS_OFST);
+		writel(dma_status, xqspi->regs + GQSPI_QSPIDMA_DST_I_STS_OFST);
+	}
+
+	if (mask & GQSPI_ISR_TXNOT_FULL_MASK) {
+		zynqmp_qspi_filltxfifo(xqspi, GQSPI_TX_FIFO_FILL);
+		ret = IRQ_HANDLED;
+	}
+
+	if (dma_status & GQSPI_QSPIDMA_DST_I_STS_DONE_MASK) {
+		zynqmp_process_dma_irq(xqspi);
+		ret = IRQ_HANDLED;
+	} else if (!(mask & GQSPI_IER_RXEMPTY_MASK) &&
+			(mask & GQSPI_IER_GENFIFOEMPTY_MASK)) {
+		zynqmp_qspi_readrxfifo(xqspi, GQSPI_RX_FIFO_FILL);
+		ret = IRQ_HANDLED;
+	}
+
+	if ((xqspi->bytes_to_receive == 0) && (xqspi->bytes_to_transfer == 0)
+			&& ((status & GQSPI_IRQ_MASK) == GQSPI_IRQ_MASK)) {
+		writel(GQSPI_ISR_IDR_MASK, xqspi->regs + GQSPI_IDR_OFST);
+		spi_finalize_current_transfer(master);
+		ret = IRQ_HANDLED;
+	}
+	return ret;
+}
+
+/**
+ * zynqmp_qspi_selectspimode:	Selects SPI mode - x1 or x2 or x4.
+ * @spimode:	spimode - SPI or DUAL or QUAD.
+ * Return:	Mask to set desired SPI mode in GENFIFO entry.
+ */
+static inline u32 zynqmp_qspi_selectspimode(u8 spimode)
+{
+	u32 mask;
+
+	switch (spimode) {
+	case GQSPI_SELECT_MODE_DUALSPI:
+		mask = GQSPI_GENFIFO_MODE_DUALSPI;
+		break;
+	case GQSPI_SELECT_MODE_QUADSPI:
+		mask = GQSPI_GENFIFO_MODE_QUADSPI;
+		break;
+	case GQSPI_SELECT_MODE_SPI:
+	default:
+		mask = GQSPI_GENFIFO_MODE_SPI;
+	}
+
+	return mask;
+}
+
+/**
+ * zynq_qspi_setuprxdma:	This function sets up the RX DMA operation
+ * @xqspi:	xqspi is a pointer to the GQSPI instance.
+ */
+static void zynq_qspi_setuprxdma(struct zynqmp_qspi *xqspi)
+{
+	u32 rx_bytes, rx_rem, config_reg;
+	dma_addr_t addr;
+	u64 dma_align =  (u64)(uintptr_t)xqspi->rxbuf;
+
+	if ((xqspi->bytes_to_receive < 8) ||
+		((dma_align & GQSPI_DMA_UNALIGN) != 0x0)) {
+		/* Setting to IO mode */
+		config_reg = zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST);
+		config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
+		zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST, config_reg);
+		xqspi->mode = GQSPI_MODE_IO;
+		xqspi->dma_rx_bytes = 0;
+		return;
+	}
+
+	rx_rem = xqspi->bytes_to_receive % 4;
+	rx_bytes = (xqspi->bytes_to_receive - rx_rem);
+
+	addr = dma_map_single(xqspi->dev, (void *)xqspi->rxbuf,
+						rx_bytes, DMA_FROM_DEVICE);
+	if (dma_mapping_error(xqspi->dev, addr))
+		dev_err(xqspi->dev, "ERR:rxdma:memory not mapped\n");
+
+	xqspi->dma_rx_bytes = rx_bytes;
+	xqspi->dma_addr = addr;
+	zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_ADDR_OFST,
+				(u32)(addr & 0xffffffff));
+	addr = ((addr >> 16) >> 16);
+	zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_ADDR_MSB_OFST,
+				((u32)addr) & 0xfff);
+
+	/* Enabling the DMA mode */
+	config_reg = zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST);
+	config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
+	config_reg |= GQSPI_CFG_MODE_EN_DMA_MASK;
+	zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST, config_reg);
+
+	/* Switch to DMA mode */
+	xqspi->mode = GQSPI_MODE_DMA;
+
+	/* Write the number of bytes to transfer */
+	zynqmp_gqspi_write(xqspi, GQSPI_QSPIDMA_DST_SIZE_OFST, rx_bytes);
+}
+
+/**
+ * zynqmp_qspi_txrxsetup:	This function checks the TX/RX buffers in
+ *				the transfer and sets up the GENFIFO entries,
+ *				TX FIFO as required.
+ * @xqspi:	xqspi is a pointer to the GQSPI instance.
+ * @transfer:	It is a pointer to the structure containing transfer data.
+ * @genfifoentry:	genfifoentry is pointer to the variable in which
+ *			GENFIFO	mask is returned to calling function
+ */
+static void zynqmp_qspi_txrxsetup(struct zynqmp_qspi *xqspi,
+				  struct spi_transfer *transfer,
+				  u32 *genfifoentry)
+{
+	u32 config_reg;
+
+	/* Transmit */
+	if ((xqspi->txbuf != NULL) && (xqspi->rxbuf == NULL)) {
+		/* Setup data to be TXed */
+		*genfifoentry &= ~GQSPI_GENFIFO_RX;
+		*genfifoentry |= GQSPI_GENFIFO_DATA_XFER;
+		*genfifoentry |= GQSPI_GENFIFO_TX;
+		*genfifoentry |= zynqmp_qspi_selectspimode(transfer->tx_nbits);
+		xqspi->bytes_to_transfer = transfer->len;
+		if (xqspi->mode == GQSPI_MODE_DMA) {
+			config_reg = zynqmp_gqspi_read(xqspi,
+							GQSPI_CONFIG_OFST);
+			config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
+			zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
+								config_reg);
+			xqspi->mode = GQSPI_MODE_IO;
+		}
+		zynqmp_qspi_filltxfifo(xqspi, GQSPI_TXD_DEPTH);
+		/* Discard RX data */
+		xqspi->bytes_to_receive = 0;
+	} else if ((xqspi->txbuf == NULL) && (xqspi->rxbuf != NULL)) {
+		/* Receive */
+
+		/* TX auto fill */
+		*genfifoentry &= ~GQSPI_GENFIFO_TX;
+		/* Setup RX */
+		*genfifoentry |= GQSPI_GENFIFO_DATA_XFER;
+		*genfifoentry |= GQSPI_GENFIFO_RX;
+		*genfifoentry |= zynqmp_qspi_selectspimode(transfer->rx_nbits);
+		xqspi->bytes_to_transfer = 0;
+		xqspi->bytes_to_receive = transfer->len;
+		zynq_qspi_setuprxdma(xqspi);
+	}
+}
+
+/**
+ * zynqmp_qspi_start_transfer:	Initiates the QSPI transfer
+ * @master:	Pointer to the spi_master structure which provides
+ *		information about the controller.
+ * @qspi:	Pointer to the spi_device structure
+ * @transfer:	Pointer to the spi_transfer structure which provide information
+ *		about next transfer parameters
+ *
+ * This function fills the TX FIFO, starts the QSPI transfer, and waits for the
+ * transfer to be completed.
+ *
+ * Return:	Number of bytes transferred in the last transfer
+ */
+static int zynqmp_qspi_start_transfer(struct spi_master *master,
+				      struct spi_device *qspi,
+				      struct spi_transfer *transfer)
+{
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
+	u32 genfifoentry = 0x0, transfer_len;
+
+	xqspi->txbuf = transfer->tx_buf;
+	xqspi->rxbuf = transfer->rx_buf;
+
+	genfifoentry |= xqspi->genfifocs;
+	genfifoentry |= xqspi->genfifobus;
+
+	zynqmp_qspi_txrxsetup(xqspi, transfer, &genfifoentry);
+
+	if (xqspi->mode == GQSPI_MODE_DMA)
+		transfer_len = xqspi->dma_rx_bytes;
+	else
+		transfer_len = transfer->len;
+
+	xqspi->genfifoentry = genfifoentry;
+	if ((transfer_len) < GQSPI_GENFIFO_IMM_DATA_MASK) {
+		genfifoentry &= ~GQSPI_GENFIFO_IMM_DATA_MASK;
+		genfifoentry |= transfer_len;
+		zynqmp_gqspi_write(xqspi, GQSPI_GEN_FIFO_OFST, genfifoentry);
+	} else {
+		int tempcount = transfer_len;
+		u32 exponent = 8;	/* 2^8 = 256 */
+		u8 imm_data = tempcount & 0xFF;
+
+		tempcount &= ~(tempcount & 0xFF);
+		/* Immediate entry */
+		if (tempcount != 0) {
+			/* Exponent entries */
+			genfifoentry |= GQSPI_GENFIFO_EXP;
+			while (tempcount != 0) {
+				if (tempcount & GQSPI_GENFIFO_EXP_START) {
+					genfifoentry &=
+					    ~GQSPI_GENFIFO_IMM_DATA_MASK;
+					genfifoentry |= exponent;
+					zynqmp_gqspi_write(xqspi,
+							   GQSPI_GEN_FIFO_OFST,
+							   genfifoentry);
+				}
+				tempcount = tempcount >> 1;
+				exponent++;
+			}
+		}
+		if (imm_data != 0) {
+			genfifoentry &= ~GQSPI_GENFIFO_EXP;
+			genfifoentry &= ~GQSPI_GENFIFO_IMM_DATA_MASK;
+			genfifoentry |= (u8) (imm_data & 0xFF);
+			zynqmp_gqspi_write(xqspi,
+					   GQSPI_GEN_FIFO_OFST, genfifoentry);
+		}
+	}
+
+	if ((xqspi->mode == GQSPI_MODE_IO) &&
+			(xqspi->rxbuf != NULL)) {
+		/* Dummy generic FIFO entry */
+		writel(0x0, xqspi->regs + GQSPI_GEN_FIFO_OFST);
+	}
+
+	/* Since we are using manual mode */
+	zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
+			   zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) |
+			   GQSPI_CFG_START_GEN_FIFO_MASK);
+
+	if (xqspi->txbuf != NULL)
+		/* Enable interrupts for TX */
+		zynqmp_gqspi_write(xqspi, GQSPI_IER_OFST,
+				   GQSPI_IER_TXEMPTY_MASK |
+					GQSPI_IER_GENFIFOEMPTY_MASK |
+					GQSPI_IER_TXNOT_FULL_MASK);
+
+	if (xqspi->rxbuf != NULL) {
+		/* Enable interrupts for RX */
+		if (xqspi->mode == GQSPI_MODE_DMA) {
+			/* Enable DMA interrupts */
+			zynqmp_gqspi_write(xqspi,
+					GQSPI_QSPIDMA_DST_I_EN_OFST,
+					GQSPI_QSPIDMA_DST_I_EN_DONE_MASK);
+		} else {
+			zynqmp_gqspi_write(xqspi, GQSPI_IER_OFST,
+					GQSPI_IER_GENFIFOEMPTY_MASK |
+					GQSPI_IER_RXNEMPTY_MASK |
+					GQSPI_IER_RXEMPTY_MASK);
+		}
+	}
+
+	return transfer->len;
+}
+
+/**
+ * zynqmp_qspi_suspend:	Suspend method for the QSPI driver
+ * @_dev:	Address of the platform_device structure
+ *
+ * This function stops the QSPI driver queue and disables the QSPI controller
+ *
+ * Return:	Always 0
+ */
+static int __maybe_unused zynqmp_qspi_suspend(struct device *dev)
+{
+	struct platform_device *pdev = container_of(dev,
+						    struct platform_device,
+						    dev);
+	struct spi_master *master = platform_get_drvdata(pdev);
+
+	spi_master_suspend(master);
+
+	zynqmp_unprepare_transfer_hardware(master);
+
+	return 0;
+}
+
+/**
+ * zynqmp_qspi_resume:	Resume method for the QSPI driver
+ * @dev:	Address of the platform_device structure
+ *
+ * The function starts the QSPI driver queue and initializes the QSPI
+ * controller
+ *
+ * Return:	0 on success and error value on error
+ */
+static int __maybe_unused zynqmp_qspi_resume(struct device *dev)
+{
+	struct platform_device *pdev = container_of(dev,
+						    struct platform_device,
+						    dev);
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
+	int ret = 0;
+
+	ret = clk_enable(xqspi->pclk);
+	if (ret) {
+		dev_err(dev, "Cannot enable APB clock.\n");
+		return ret;
+	}
+
+	ret = clk_enable(xqspi->refclk);
+	if (ret) {
+		dev_err(dev, "Cannot enable device clock.\n");
+		clk_disable(xqspi->pclk);
+		return ret;
+	}
+
+	spi_master_resume(master);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(zynqmp_qspi_dev_pm_ops, zynqmp_qspi_suspend,
+			 zynqmp_qspi_resume);
+
+/**
+ * zynqmp_qspi_probe:	Probe method for the QSPI driver
+ * @pdev:	Pointer to the platform_device structure
+ *
+ * This function initializes the driver data structures and the hardware.
+ *
+ * Return:	0 on success and error value on failure
+ */
+static int zynqmp_qspi_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct spi_master *master;
+	struct zynqmp_qspi *xqspi;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	u32 num_cs;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(*xqspi));
+	if (!master)
+		return -ENOMEM;
+
+	xqspi = spi_master_get_devdata(master);
+	master->dev.of_node = pdev->dev.of_node;
+	platform_set_drvdata(pdev, master);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	xqspi->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(xqspi->regs)) {
+		ret = PTR_ERR(xqspi->regs);
+		goto remove_master;
+	}
+
+	xqspi->dev = dev;
+	xqspi->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(xqspi->pclk)) {
+		dev_err(dev, "pclk clock not found.\n");
+		ret = PTR_ERR(xqspi->pclk);
+		goto remove_master;
+	}
+
+	xqspi->refclk = devm_clk_get(&pdev->dev, "ref_clk");
+	if (IS_ERR(xqspi->refclk)) {
+		dev_err(dev, "ref_clk clock not found.\n");
+		ret = PTR_ERR(xqspi->refclk);
+		goto remove_master;
+	}
+
+	ret = clk_prepare_enable(xqspi->pclk);
+	if (ret) {
+		dev_err(dev, "Unable to enable APB clock.\n");
+		goto remove_master;
+	}
+
+	ret = clk_prepare_enable(xqspi->refclk);
+	if (ret) {
+		dev_err(dev, "Unable to enable device clock.\n");
+		goto clk_dis_pclk;
+	}
+
+	/* QSPI controller initializations */
+	zynqmp_qspi_init_hw(xqspi);
+
+	xqspi->irq = platform_get_irq(pdev, 0);
+	if (xqspi->irq <= 0) {
+		ret = -ENXIO;
+		dev_err(dev, "irq resource not found\n");
+		goto remove_master;
+	}
+	ret = devm_request_irq(&pdev->dev, xqspi->irq, zynqmp_qspi_irq,
+			       0, pdev->name, master);
+	if (ret != 0) {
+		ret = -ENXIO;
+		dev_err(dev, "request_irq failed\n");
+		goto remove_master;
+	}
+
+	ret = of_property_read_u32(pdev->dev.of_node, "num-cs", &num_cs);
+	if (ret < 0)
+		master->num_chipselect = GQSPI_DEFAULT_NUM_CS;
+	else
+		master->num_chipselect = num_cs;
+
+	master->setup = zynqmp_qspi_setup;
+	master->set_cs = zynqmp_qspi_chipselect;
+	master->transfer_one = zynqmp_qspi_start_transfer;
+	master->prepare_transfer_hardware = zynqmp_prepare_transfer_hardware;
+	master->unprepare_transfer_hardware =
+					zynqmp_unprepare_transfer_hardware;
+	master->max_speed_hz = clk_get_rate(xqspi->refclk) / 2;
+	master->bits_per_word_mask = SPI_BPW_MASK(8);
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
+			    SPI_TX_DUAL | SPI_TX_QUAD;
+
+	if (master->dev.parent == NULL)
+		master->dev.parent = &master->dev;
+
+	ret = spi_register_master(master);
+	if (ret)
+		goto clk_dis_all;
+
+	return 0;
+
+clk_dis_all:
+	clk_disable_unprepare(xqspi->refclk);
+clk_dis_pclk:
+	clk_disable_unprepare(xqspi->pclk);
+remove_master:
+	spi_master_put(master);
+
+	return ret;
+}
+
+/**
+ * zynqmp_qspi_remove:	Remove method for the QSPI driver
+ * @pdev:	Pointer to the platform_device structure
+ *
+ * This function is called if a device is physically removed from the system or
+ * if the driver module is being unloaded. It frees all resources allocated to
+ * the device.
+ *
+ * Return:	0 Always
+ */
+static int zynqmp_qspi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
+
+	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
+	clk_disable_unprepare(xqspi->refclk);
+	clk_disable_unprepare(xqspi->pclk);
+
+	spi_unregister_master(master);
+
+	return 0;
+}
+
+static const struct of_device_id zynqmp_qspi_of_match[] = {
+	{ .compatible = "xlnx,zynqmp-qspi-1.0", },
+	{ /* End of table */ }
+};
+
+MODULE_DEVICE_TABLE(of, zynqmp_qspi_of_match);
+
+static struct platform_driver zynqmp_qspi_driver = {
+	.probe = zynqmp_qspi_probe,
+	.remove = zynqmp_qspi_remove,
+	.driver = {
+		.name = "zynqmp-qspi",
+		.of_match_table = zynqmp_qspi_of_match,
+		.pm = &zynqmp_qspi_dev_pm_ops,
+	},
+};
+
+module_platform_driver(zynqmp_qspi_driver);
+
+MODULE_AUTHOR("Xilinx, Inc.");
+MODULE_DESCRIPTION("Xilinx Zynqmp QSPI driver");
+MODULE_LICENSE("GPL");