diff mbox

[RFC,1/2] spi: Add support for Zynq QSPI controller

Message ID 1404982207-4707-2-git-send-email-harinik@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Harini Katakam July 10, 2014, 8:50 a.m. UTC
This patch adds support for QSPI controller used by Zynq.

Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
 drivers/spi/Kconfig         |    6 +
 drivers/spi/Makefile        |    1 +
 drivers/spi/spi-zynq-qspi.c |  854 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 861 insertions(+)
 create mode 100644 drivers/spi/spi-zynq-qspi.c

Comments

Geert Uytterhoeven July 10, 2014, 9:18 a.m. UTC | #1
Hi Harini,

On Thu, Jul 10, 2014 at 10:50 AM, Harini Katakam <harinik@xilinx.com> wrote:
> +       master->flags = SPI_MASTER_QUAD_MODE;

SPI_MASTER_QUAD_MODE is not one of the SPI_MASTER_* defines
in include/linux/spi/spi.h?

> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
> +                           SPI_TX_DUAL | SPI_TX_QUAD;

Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 July 10, 2014, 9:31 a.m. UTC | #2
Hi Geert,

On Thu, Jul 10, 2014 at 2:48 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Harini,
>
> On Thu, Jul 10, 2014 at 10:50 AM, Harini Katakam <harinik@xilinx.com> wrote:
>> +       master->flags = SPI_MASTER_QUAD_MODE;
>
> SPI_MASTER_QUAD_MODE is not one of the SPI_MASTER_* defines
> in include/linux/spi/spi.h?
>

I'm sorry about that. That flag is unused - will remove this statement.

>> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>> +                           SPI_TX_DUAL | SPI_TX_QUAD;
>
> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>

Here the driver is just giving information that the controller support it.
The MTD layer enables dual/quad based on what the flash supports; quad
being the first priority
I understand that the spi core reads rx, tx-bus-width property and
master support flags and
performs the necessary checks.

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
Geert Uytterhoeven July 10, 2014, 9:42 a.m. UTC | #3
Hi Harini,

On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
<harinikatakamlinux@gmail.com> wrote:
>>> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>> +                           SPI_TX_DUAL | SPI_TX_QUAD;
>>
>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>
> Here the driver is just giving information that the controller support it.
> The MTD layer enables dual/quad based on what the flash supports; quad
> being the first priority
> I understand that the spi core reads rx, tx-bus-width property and
> master support flags and
> performs the necessary checks.

That's correct: as long as the rx, tx-bus-width  properties do not indicate a
Dual or Quad wiring, it won't be used.

However, based on schematics, someone may set the rx, tx-bus-width properties
to 4, which is correct, as DT describes the hardware. But this will fail to
work.
So I think it's safer not to announce Dual/Quad support in the driver until
the actual driver support is there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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
Punnaiah Choudary Kalluri July 10, 2014, 9:44 a.m. UTC | #4
SEkgR3JlZXQsDQoNCj4tLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPkZyb206IEhhcmluaSBL
YXRha2FtIFttYWlsdG86aGFyaW5pa2F0YWthbWxpbnV4QGdtYWlsLmNvbV0NCj5TZW50OiBUaHVy
c2RheSwgSnVseSAxMCwgMjAxNCAzOjAxIFBNDQo+VG86IEdlZXJ0IFV5dHRlcmhvZXZlbg0KPkNj
OiBNYXJrIEJyb3duOyBHcmFudCBMaWtlbHk7IFJvYiBIZXJyaW5nOyBQYXdlbCBNb2xsOyBNYXJr
IFJ1dGxhbmQ7IElhbg0KPkNhbXBiZWxsOyBLdW1hciBHYWxhOyBsaW51eC1zcGk7IGxpbnV4LWtl
cm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+ZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4
LWRvY0B2Z2VyLmtlcm5lbC5vcmc7IERhdmlkIFdvb2Rob3VzZTsNCj5CcmlhbiBOb3JyaXM7IE1h
cmVrIFZhxaF1dDsgQXJ0ZW0gQml0eXV0c2tpeTsgR2VlcnQgVXl0dGVyaG9ldmVuOyBTYXNjaGEN
Cj5IYXVlcjsgSmluZ29vIEhhbjsgU291cmF2IFBvZGRhcjsgTWljaGFsIFNpbWVrOyBQdW5uYWlh
aCBDaG91ZGFyeSBLYWxsdXJpDQo+U3ViamVjdDogUmU6IFtSRkMgUEFUQ0ggMS8yXSBzcGk6IEFk
ZCBzdXBwb3J0IGZvciBaeW5xIFFTUEkgY29udHJvbGxlcg0KPg0KPkhpIEdlZXJ0LA0KPg0KPk9u
IFRodSwgSnVsIDEwLCAyMDE0IGF0IDI6NDggUE0sIEdlZXJ0IFV5dHRlcmhvZXZlbg0KPjxnZWVy
dEBsaW51eC1tNjhrLm9yZz4gd3JvdGU6DQo+PiBIaSBIYXJpbmksDQo+Pg0KPj4gT24gVGh1LCBK
dWwgMTAsIDIwMTQgYXQgMTA6NTAgQU0sIEhhcmluaSBLYXRha2FtIDxoYXJpbmlrQHhpbGlueC5j
b20+DQo+d3JvdGU6DQo+Pj4gKyAgICAgICBtYXN0ZXItPmZsYWdzID0gU1BJX01BU1RFUl9RVUFE
X01PREU7DQo+Pg0KPj4gU1BJX01BU1RFUl9RVUFEX01PREUgaXMgbm90IG9uZSBvZiB0aGUgU1BJ
X01BU1RFUl8qIGRlZmluZXMNCj4+IGluIGluY2x1ZGUvbGludXgvc3BpL3NwaS5oPw0KPj4NCj4N
Cj5JJ20gc29ycnkgYWJvdXQgdGhhdC4gVGhhdCBmbGFnIGlzIHVudXNlZCAtIHdpbGwgcmVtb3Zl
IHRoaXMgc3RhdGVtZW50Lg0KPg0KPj4+ICsgICAgICAgbWFzdGVyLT5tb2RlX2JpdHMgPSBTUElf
Q1BPTCB8IFNQSV9DUEhBIHwgU1BJX1JYX0RVQUwgfA0KPlNQSV9SWF9RVUFEIHwNCj4+PiArICAg
ICAgICAgICAgICAgICAgICAgICAgICAgU1BJX1RYX0RVQUwgfCBTUElfVFhfUVVBRDsNCj4+DQo+
PiBZb3VyIGRyaXZlciBhZHZlcnRpc2VzIER1YWwvUXVhZCBTUEkgVHJhbnNmZXIgY2FwYWJpbGl0
aWVzLCBidXQgaXQgZG9lc24ndA0KPj4gY2hlY2sgc3BpX3RyYW5zZmVyLlt0cl14X25iaXRzPyBI
b3cgY2FuIGl0IGRldGVybWluZSB3aGVuIHRvIGVuYWJsZQ0KPkR1YWwvUXVhZD8NCj4+DQo+DQo+
SGVyZSB0aGUgZHJpdmVyIGlzIGp1c3QgZ2l2aW5nIGluZm9ybWF0aW9uIHRoYXQgdGhlIGNvbnRy
b2xsZXIgc3VwcG9ydCBpdC4NCj5UaGUgTVREIGxheWVyIGVuYWJsZXMgZHVhbC9xdWFkIGJhc2Vk
IG9uIHdoYXQgdGhlIGZsYXNoIHN1cHBvcnRzOyBxdWFkDQo+YmVpbmcgdGhlIGZpcnN0IHByaW9y
aXR5DQo+SSB1bmRlcnN0YW5kIHRoYXQgdGhlIHNwaSBjb3JlIHJlYWRzIHJ4LCB0eC1idXMtd2lk
dGggcHJvcGVydHkgYW5kDQo+bWFzdGVyIHN1cHBvcnQgZmxhZ3MgYW5kDQo+cGVyZm9ybXMgdGhl
IG5lY2Vzc2FyeSBjaGVja3MuDQoNCkp1c3QgdG8gYWRkLCB0aGUgenlucSBxc3BpIGNvbnRyb2xs
ZXIgd2lsbCBhdXRvbWF0aWNhbGx5IHNlbGVjdCB0aGUgSU8gbGluZXMgYmFzZWQNCk9uIHRoZSBm
bGFzaCBjb21tYW5kLiANCg0KUmVnYXJkcywNClB1bm5haWFoDQoNCj4NCj5SZWdhcmRzLA0KPkhh
cmluaQ0K
--
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 July 10, 2014, 10:33 a.m. UTC | #5
Hi Geert,

On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Harini,
>
> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
> <harinikatakamlinux@gmail.com> wrote:
>>>> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>>> +                           SPI_TX_DUAL | SPI_TX_QUAD;
>>>
>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>>
>> Here the driver is just giving information that the controller support it.
>> The MTD layer enables dual/quad based on what the flash supports; quad
>> being the first priority
>> I understand that the spi core reads rx, tx-bus-width property and
>> master support flags and
>> performs the necessary checks.
>
> That's correct: as long as the rx, tx-bus-width  properties do not indicate a
> Dual or Quad wiring, it won't be used.
>
> However, based on schematics, someone may set the rx, tx-bus-width properties
> to 4, which is correct, as DT describes the hardware. But this will fail to
> work.
> So I think it's safer not to announce Dual/Quad support in the driver until
> the actual driver support is there.
>

OK. Correct me if I'm wrong but announcing this support in master->flags is
just to say the controller supports it - Like Punnaiah mentioned in the other
mail, nothing specific needs to be done from the controller driver to enable
dual/quad support. This is at the SOC/IP level.
I agree it might or might not be supported at board-level.
But that's based on the user's hardware. Should master->flags
really take this into consideration?

BTW, I dint see master->mode_bits being used anywhere at the moment.

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
Geert Uytterhoeven July 10, 2014, 11:25 a.m. UTC | #6
Hi Harini,

On Thu, Jul 10, 2014 at 12:33 PM, Harini Katakam
<harinikatakamlinux@gmail.com> wrote:
> On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
>> <harinikatakamlinux@gmail.com> wrote:
>>>>> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>>>> +                           SPI_TX_DUAL | SPI_TX_QUAD;
>>>>
>>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>>>
>>> Here the driver is just giving information that the controller support it.
>>> The MTD layer enables dual/quad based on what the flash supports; quad
>>> being the first priority
>>> I understand that the spi core reads rx, tx-bus-width property and
>>> master support flags and
>>> performs the necessary checks.
>>
>> That's correct: as long as the rx, tx-bus-width  properties do not indicate a
>> Dual or Quad wiring, it won't be used.
>>
>> However, based on schematics, someone may set the rx, tx-bus-width properties
>> to 4, which is correct, as DT describes the hardware. But this will fail to
>> work.
>> So I think it's safer not to announce Dual/Quad support in the driver until
>> the actual driver support is there.
>
> OK. Correct me if I'm wrong but announcing this support in master->flags is
> just to say the controller supports it - Like Punnaiah mentioned in the other
> mail, nothing specific needs to be done from the controller driver to enable
> dual/quad support. This is at the SOC/IP level.
> I agree it might or might not be supported at board-level.

IC. So this is not a generic SPI controller, but a controller meant for QSPI
FLASHes? I.e. if you would connect a different device, the controller may
unexpectedly use Dual or Quad mode if it sees a byte fly by that looks like
a Quad SPI FLASH read command?

> But that's based on the user's hardware. Should master->flags
> really take this into consideration?

You mean master->mode_bits?

> BTW, I dint see master->mode_bits being used anywhere at the moment.

It is used to match SPI controller and slave features, cfr. spi_setup() in
drivers/spi/spi.c.

If Dual/Quad is supported, the bits should be set. Else spi_setup() will
clear the bits in the SPI slave's mode field, disabling Dual/Quad transfers.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 July 10, 2014, 11:55 a.m. UTC | #7
Hi Geert,

On Thu, Jul 10, 2014 at 4:55 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Harini,
>
> On Thu, Jul 10, 2014 at 12:33 PM, Harini Katakam
> <harinikatakamlinux@gmail.com> wrote:
>> On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam
>>> <harinikatakamlinux@gmail.com> wrote:
>>>>>> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD |
>>>>>> +                           SPI_TX_DUAL | SPI_TX_QUAD;
>>>>>
>>>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't
>>>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad?
>>>>
>>>> Here the driver is just giving information that the controller support it.
>>>> The MTD layer enables dual/quad based on what the flash supports; quad
>>>> being the first priority
>>>> I understand that the spi core reads rx, tx-bus-width property and
>>>> master support flags and
>>>> performs the necessary checks.
>>>
>>> That's correct: as long as the rx, tx-bus-width  properties do not indicate a
>>> Dual or Quad wiring, it won't be used.
>>>
>>> However, based on schematics, someone may set the rx, tx-bus-width properties
>>> to 4, which is correct, as DT describes the hardware. But this will fail to
>>> work.
>>> So I think it's safer not to announce Dual/Quad support in the driver until
>>> the actual driver support is there.
>>
>> OK. Correct me if I'm wrong but announcing this support in master->flags is
>> just to say the controller supports it - Like Punnaiah mentioned in the other
>> mail, nothing specific needs to be done from the controller driver to enable
>> dual/quad support. This is at the SOC/IP level.
>> I agree it might or might not be supported at board-level.
>
> IC. So this is not a generic SPI controller, but a controller meant for QSPI
> FLASHes? I.e. if you would connect a different device, the controller may
> unexpectedly use Dual or Quad mode if it sees a byte fly by that looks like
> a Quad SPI FLASH read command?
>

Yes. It would.

>> But that's based on the user's hardware. Should master->flags
>> really take this into consideration?
>
> You mean master->mode_bits?

Yes, i mean mode_bits. That was typo.

>
>> BTW, I dint see master->mode_bits being used anywhere at the moment.
>
> It is used to match SPI controller and slave features, cfr. spi_setup() in
> drivers/spi/spi.c.
>
> If Dual/Quad is supported, the bits should be set. Else spi_setup() will
> clear the bits in the SPI slave's mode field, disabling Dual/Quad transfers.
>

OK.

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
Mark Brown July 10, 2014, 12:01 p.m. UTC | #8
On Thu, Jul 10, 2014 at 04:03:19PM +0530, Harini Katakam wrote:

> OK. Correct me if I'm wrong but announcing this support in master->flags is
> just to say the controller supports it - Like Punnaiah mentioned in the other

No, it's broken to set this if there is no ability to use it.

> mail, nothing specific needs to be done from the controller driver to enable
> dual/quad support. This is at the SOC/IP level.

How does the client driver select the width to use for a transfer?
Harini Katakam July 10, 2014, 12:39 p.m. UTC | #9
Hi Mark,

On Thu, Jul 10, 2014 at 5:31 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jul 10, 2014 at 04:03:19PM +0530, Harini Katakam wrote:
>
>> OK. Correct me if I'm wrong but announcing this support in master->flags is
>> just to say the controller supports it - Like Punnaiah mentioned in the other
>
> No, it's broken to set this if there is no ability to use it.
>
>> mail, nothing specific needs to be done from the controller driver to enable
>> dual/quad support. This is at the SOC/IP level.
>
> How does the client driver select the width to use for a transfer?

This controller is meant to be used only with flash devices.
The flash devices' supported width will be reflected in a table in MTD layer.
When selecting, priority is given to quad over dual and single in the MTD and
it will send commands using the supported tx/rx bus width accordingly.
About the supported bus width on board, tx-bus-width and rx-bus-width
properties in dts will have the info; which I believe spi core uses.

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
Mark Brown July 11, 2014, 1:38 p.m. UTC | #10
On Thu, Jul 10, 2014 at 02:20:06PM +0530, Harini Katakam wrote:

> This patch adds support for QSPI controller used by Zynq.

The driver looks pretty clean but there are a couple of issues below,
including a little bit more of the flash specifics.

> +static void zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
> +{
> +	struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
> +	u32 config_reg;
> +
> +	config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
> +
> +	/* Select upper/lower page before asserting CS */
> +	if (xqspi->is_stacked) {
> +		u32 lqspi_cfg_reg;

Like with the dual and quad mode stuff this looks very much like it's
specific to flash rather than something that applies to a generic SPI
driver.  However it does look like it's a generic SPI device which could
be used in other applications which makes things a bit tricky.  We don't
have a really good answer for this right now unfortunately, probably we
need some sort of special interface between the SPI and flash subsystems
to allow flash to use the flash specific stuff.

For use as a generic SPI device what I'd suggest is stripping out the
flash specifics, merging the rest of the support and then considering
the flash specifics separately.

> +/**
> + * zynq_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 zynq_prepare_transfer_hardware(struct spi_master *master)
> +{
> +	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
> +
> +	zynq_qspi_config_clock_mode(master->cur_msg->spi);

The clock mode needs to be (and is) configured per transfer so I'd
expect it's possible to remove this call.

> +	ret = clk_prepare_enable(xqspi->refclk);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable device clock.\n");

It's better to display the error code.

> +		clk_disable(xqspi->pclk);

This needs to be disable_unprepare().

> +static SIMPLE_DEV_PM_OPS(zynq_qspi_dev_pm_ops, zynq_qspi_suspend,
> +			 zynq_qspi_resume);

It would be better to also implement runtime PM support to disable the
clocks while the device is idle as well, that will save a small amount
of power while the device isn't doing anything.
Harini Katakam July 14, 2014, 7:22 a.m. UTC | #11
Hi Mark,

My mail seems to have missed the below reply.
Anyway, please find my answer below:

> -----Original Message-----
> From: Punnaiah Choudary Kalluri
> Sent: Monday, July 14, 2014 12:03 PM
> To: Harini Katakam
> Subject: FW: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
> 
> 
> 
> >-----Original Message-----
> >From: Mark Brown [mailto:broonie@kernel.org]
> >Sent: Thursday, July 10, 2014 8:37 PM
> >To: Harini Katakam
> >Cc: Geert Uytterhoeven; Grant Likely; Rob Herring; Pawel Moll; Mark
> Rutland;
> >Ian Campbell; Kumar Gala; linux-spi; linux-kernel@vger.kernel.org;
> >devicetree@vger.kernel.org; linux-doc@vger.kernel.org; David
> Woodhouse;
> >Brian Norris; Marek VaĊĦut; Artem Bityutskiy; Geert Uytterhoeven; Sascha
> >Hauer; Jingoo Han; Sourav Poddar; Michal Simek; Punnaiah Choudary
> Kalluri
> >Subject: Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
> >
> >On Thu, Jul 10, 2014 at 06:09:55PM +0530, Harini Katakam wrote:
> >> On Thu, Jul 10, 2014 at 5:31 PM, Mark Brown <broonie@kernel.org>
> wrote:
> >
> >> > How does the client driver select the width to use for a transfer?
> >
> >> This controller is meant to be used only with flash devices.
> >> The flash devices' supported width will be reflected in a table in MTD
> layer.
> >> When selecting, priority is given to quad over dual and single in the MTD
> and
> >> it will send commands using the supported tx/rx bus width accordingly.
> >> About the supported bus width on board, tx-bus-width and rx-bus-width
> >> properties in dts will have the info; which I believe spi core uses.
> >
> >If it's only intended to be used as a flash controller (and might
> >misbehave if used as such, if the command detection false triggers) then
> >it is probably better to support it as a flash controller rather than as
> >a SPI controller.  Or can the flash-specific features be disabled?

There is provision to switch to legacy (generic spi) mode but it is not usually used.
As you can can see, the flash related specifics come into play only when two flash devices
are used. For a single slave, it will be generic.
As per your suggestions, I could send this driver in multiple stages -
Initially, qspi driver without flash specifics (this will work straight-away for a single flash)
In the next set, flash specifics changes for dual flash configurations (parallel/stacked)
Is that acceptable?

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
Harini Katakam July 14, 2014, 7:27 a.m. UTC | #12
Hi Mark,

On Fri, Jul 11, 2014 at 7:08 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jul 10, 2014 at 02:20:06PM +0530, Harini Katakam wrote:
>
>> This patch adds support for QSPI controller used by Zynq.
>
> The driver looks pretty clean but there are a couple of issues below,
> including a little bit more of the flash specifics.
>
>> +static void zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
>> +{
>> +     struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
>> +     u32 config_reg;
>> +
>> +     config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
>> +
>> +     /* Select upper/lower page before asserting CS */
>> +     if (xqspi->is_stacked) {
>> +             u32 lqspi_cfg_reg;
>
> Like with the dual and quad mode stuff this looks very much like it's
> specific to flash rather than something that applies to a generic SPI
> driver.  However it does look like it's a generic SPI device which could
> be used in other applications which makes things a bit tricky.  We don't
> have a really good answer for this right now unfortunately, probably we
> need some sort of special interface between the SPI and flash subsystems
> to allow flash to use the flash specific stuff.
>
> For use as a generic SPI device what I'd suggest is stripping out the
> flash specifics, merging the rest of the support and then considering
> the flash specifics separately.

Reply in the other thread.

>
>> +/**
>> + * zynq_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 zynq_prepare_transfer_hardware(struct spi_master *master)
>> +{
>> +     struct zynq_qspi *xqspi = spi_master_get_devdata(master);
>> +
>> +     zynq_qspi_config_clock_mode(master->cur_msg->spi);
>
> The clock mode needs to be (and is) configured per transfer so I'd
> expect it's possible to remove this call.
>

Yeah I understand. It can go away from here and there should be a
prepare_message as per Lars-Peter's patches on spi-cadence.
I'll reflect the same in this driver.

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
Mark Brown July 14, 2014, 6:07 p.m. UTC | #13
On Mon, Jul 14, 2014 at 07:22:24AM +0000, Harini Katakam wrote:

> As per your suggestions, I could send this driver in multiple stages -
> Initially, qspi driver without flash specifics (this will work straight-away for a single flash)
> In the next set, flash specifics changes for dual flash configurations (parallel/stacked)
> Is that acceptable?

Yes, that should be fine.
diff mbox

Patch

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 213b5cb..9642148 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -558,6 +558,12 @@  config SPI_XTENSA_XTFPGA
 	  start and deasserting on end.
 
 
+config SPI_ZYNQ_QSPI
+	tristate "Xilinx Zynq QSPI controller"
+	depends on ARCH_ZYNQ
+	help
+	  This selects the Xilinx Zynq Quad SPI controller master driver.
+
 config SPI_NUC900
 	tristate "Nuvoton NUC900 series SPI"
 	depends on ARCH_W90X900
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 929c9f5..0bfe75e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -83,3 +83,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_ZYNQ_QSPI)		+= spi-zynq-qspi.o
diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
new file mode 100644
index 0000000..2a352a9
--- /dev/null
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -0,0 +1,854 @@ 
+/*
+ * Xilinx Zynq Quad-SPI (QSPI) controller driver (master mode only)
+ *
+ * Copyright (C) 2009 - 2014 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/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/workqueue.h>
+
+/* Name of this driver */
+#define DRIVER_NAME			"zynq-qspi"
+
+/* Register offset definitions */
+#define ZYNQ_QSPI_CONFIG_OFFSET		0x00 /* Configuration  Register, RW */
+#define ZYNQ_QSPI_STATUS_OFFSET		0x04 /* Interrupt Status Register, RO */
+#define ZYNQ_QSPI_IEN_OFFSET		0x08 /* Interrupt Enable Register, WO */
+#define ZYNQ_QSPI_IDIS_OFFSET		0x0C /* Interrupt Disable Reg, WO */
+#define ZYNQ_QSPI_IMASK_OFFSET		0x10 /* Interrupt Enabled Mask Reg,RO */
+#define ZYNQ_QSPI_ENABLE_OFFSET		0x14 /* Enable/Disable Register, RW */
+#define ZYNQ_QSPI_DELAY_OFFSET		0x18 /* Delay Register, RW */
+#define ZYNQ_QSPI_TXD_00_00_OFFSET	0x1C /* Transmit 4-byte inst, WO */
+#define ZYNQ_QSPI_TXD_00_01_OFFSET	0x80 /* Transmit 1-byte inst, WO */
+#define ZYNQ_QSPI_TXD_00_10_OFFSET	0x84 /* Transmit 2-byte inst, WO */
+#define ZYNQ_QSPI_TXD_00_11_OFFSET	0x88 /* Transmit 3-byte inst, WO */
+#define ZYNQ_QSPI_RXD_OFFSET		0x20 /* Data Receive Register, RO */
+#define ZYNQ_QSPI_SIC_OFFSET		0x24 /* Slave Idle Count Register, RW */
+#define ZYNQ_QSPI_TX_THRESH_OFFSET	0x28 /* TX FIFO Watermark Reg, RW */
+#define ZYNQ_QSPI_RX_THRESH_OFFSET	0x2C /* RX FIFO Watermark Reg, RW */
+#define ZYNQ_QSPI_GPIO_OFFSET		0x30 /* GPIO Register, RW */
+#define ZYNQ_QSPI_LINEAR_CFG_OFFSET	0xA0 /* Linear Adapter Config Ref, RW */
+#define ZYNQ_QSPI_MOD_ID_OFFSET		0xFC /* Module ID Register, RO */
+
+/*
+ * QSPI Configuration Register bit Masks
+ *
+ * This register contains various control bits that effect the operation
+ * of the QSPI controller
+ */
+#define ZYNQ_QSPI_CONFIG_IFMODE_MASK	0x80000000 /* Flash Memory Interface */
+#define ZYNQ_QSPI_CONFIG_MANSRT_MASK	0x00010000 /* Manual TX Start */
+#define ZYNQ_QSPI_CONFIG_MANSRTEN_MASK	0x00008000 /* Enable Manual TX Mode */
+#define ZYNQ_QSPI_CONFIG_SSFORCE_MASK	0x00004000 /* Manual Chip Select */
+#define ZYNQ_QSPI_CONFIG_BDRATE_MASK	0x00000038 /* Baud Rate Divisor Mask */
+#define ZYNQ_QSPI_CONFIG_CPHA_MASK	0x00000004 /* Clock Phase Control */
+#define ZYNQ_QSPI_CONFIG_CPOL_MASK	0x00000002 /* Clock Polarity Control */
+#define ZYNQ_QSPI_CONFIG_SSCTRL_MASK	0x00003C00 /* Slave Select Mask */
+#define ZYNQ_QSPI_CONFIG_FWIDTH_MASK	0x000000C0 /* FIFO width */
+#define ZYNQ_QSPI_CONFIG_MSTREN_MASK	0x00000001 /* Master Mode */
+
+/*
+ * QSPI Configuration Register - Baud rate and slave select
+ *
+ * These are the values used in the calculation of baud rate divisor and
+ * setting the slave select.
+ */
+#define ZYNQ_QSPI_BAUD_DIV_MAX		7 /* Baud rate divisor maximum */
+#define ZYNQ_QSPI_BAUD_DIV_SHIFT	3 /* Baud rate divisor shift in CR */
+#define ZYNQ_QSPI_SS_SHIFT		10 /* Slave Select field shift in CR */
+
+/*
+ * QSPI Interrupt Registers bit Masks
+ *
+ * All the four interrupt registers (Status/Mask/Enable/Disable) have the same
+ * bit definitions.
+ */
+#define ZYNQ_QSPI_IXR_TXNFULL_MASK	0x00000004 /* QSPI TX FIFO Overflow */
+#define ZYNQ_QSPI_IXR_TXFULL_MASK	0x00000008 /* QSPI TX FIFO is full */
+#define ZYNQ_QSPI_IXR_RXNEMTY_MASK	0x00000010 /* QSPI RX FIFO Not Empty */
+#define ZYNQ_QSPI_IXR_ALL_MASK		(ZYNQ_QSPI_IXR_TXNFULL_MASK | \
+					ZYNQ_QSPI_IXR_RXNEMTY_MASK)
+
+/*
+ * QSPI Enable Register bit Masks
+ *
+ * This register is used to enable or disable the QSPI controller
+ */
+#define ZYNQ_QSPI_ENABLE_ENABLE_MASK	0x00000001 /* QSPI Enable Bit Mask */
+
+/*
+ * QSPI Linear Configuration Register
+ *
+ * It is named Linear Configuration but it controls other modes when not in
+ * linear mode also.
+ */
+#define ZYNQ_QSPI_LCFG_TWO_MEM_MASK	0x40000000 /* LQSPI Two memories Mask */
+#define ZYNQ_QSPI_LCFG_SEP_BUS_MASK	0x20000000 /* LQSPI Separate bus Mask */
+#define ZYNQ_QSPI_LCFG_U_PAGE_MASK	0x10000000 /* LQSPI Upper Page Mask */
+
+#define ZYNQ_QSPI_LCFG_DUMMY_SHIFT	8
+
+#define ZYNQ_QSPI_FAST_READ_QOUT_CODE	0x6B /* read instruction code */
+#define ZYNQ_QSPI_FIFO_DEPTH		63 /* FIFO depth in words */
+#define ZYNQ_QSPI_RX_THRESHOLD		32 /* Rx FIFO threshold level */
+#define ZYNQ_QSPI_TX_THRESHOLD		1 /* Tx FIFO threshold level */
+
+/* Default number of chip selects */
+#define ZYNQ_QSPI_DEFAULT_NUM_CS	1
+
+/**
+ * struct zynq_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
+ * @config_reg_lock:	Lock used for accessing configuration register
+ * @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
+ * @is_parallel:	Flag to indicate two flash devices are in parallel
+ * @is_stacked:		Flag to indicate two flash devices are stacked
+ */
+struct zynq_qspi {
+	void __iomem *regs;
+	struct clk *refclk;
+	struct clk *pclk;
+	int irq;
+	const void *txbuf;
+	void *rxbuf;
+	int bytes_to_transfer;
+	int bytes_to_receive;
+	u32 is_parallel;
+	u32 is_stacked;
+	u8 is_instr;
+};
+
+/*
+ * Inline functions for the QSPI controller read/write
+ */
+static inline u32 zynq_qspi_read(struct zynq_qspi *xqspi, u32 offset)
+{
+	return readl_relaxed(xqspi->regs + offset);
+}
+
+static inline void zynq_qspi_write(struct zynq_qspi *xqspi, u32 offset,
+				   u32 val)
+{
+	writel_relaxed(val, xqspi->regs + offset);
+}
+
+/**
+ * zynq_qspi_init_hw - Initialize the hardware
+ * @xqspi:	Pointer to the zynq_qspi structure
+ *
+ * The default settings of the QSPI controller's configurable parameters on
+ * reset are
+ *	- Master mode
+ *	- Baud rate divisor is set to 2
+ *	- Tx thresold set to 1l Rx threshold set to 32
+ *	- Flash memory interface mode enabled
+ *	- Size of the word to be transferred as 8 bit
+ * 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 size of the word to be transferred as 32 bit
+ *	- Set the little endian mode of TX FIFO and
+ *	- Enable the QSPI controller
+ */
+static void zynq_qspi_init_hw(struct zynq_qspi *xqspi)
+{
+	u32 config_reg;
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_IDIS_OFFSET, 0x7F);
+
+	/* Disable linear mode as the boot loader may have used it */
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET, 0);
+
+	/* Clear the RX FIFO */
+	while (zynq_qspi_read(xqspi, ZYNQ_QSPI_STATUS_OFFSET) &
+			      ZYNQ_QSPI_IXR_RXNEMTY_MASK)
+		zynq_qspi_read(xqspi, ZYNQ_QSPI_RXD_OFFSET);
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_STATUS_OFFSET, 0x7F);
+	config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+	config_reg &= ~(ZYNQ_QSPI_CONFIG_MSTREN_MASK |
+			ZYNQ_QSPI_CONFIG_CPOL_MASK |
+			ZYNQ_QSPI_CONFIG_CPHA_MASK |
+			ZYNQ_QSPI_CONFIG_BDRATE_MASK |
+			ZYNQ_QSPI_CONFIG_SSFORCE_MASK |
+			ZYNQ_QSPI_CONFIG_MANSRTEN_MASK |
+			ZYNQ_QSPI_CONFIG_MANSRT_MASK);
+	config_reg |= (ZYNQ_QSPI_CONFIG_MSTREN_MASK |
+		       ZYNQ_QSPI_CONFIG_SSFORCE_MASK |
+		       ZYNQ_QSPI_CONFIG_FWIDTH_MASK |
+		       ZYNQ_QSPI_CONFIG_IFMODE_MASK);
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_RX_THRESH_OFFSET,
+			ZYNQ_QSPI_RX_THRESHOLD);
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_TX_THRESH_OFFSET,
+			ZYNQ_QSPI_TX_THRESHOLD);
+
+	if (xqspi->is_parallel)
+		/* Enable two memories on seperate buses */
+		zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET,
+				(ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
+				ZYNQ_QSPI_LCFG_SEP_BUS_MASK |
+				(1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
+				ZYNQ_QSPI_FAST_READ_QOUT_CODE));
+	if (xqspi->is_stacked)
+		/* Enable two memories on shared bus */
+		zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET,
+				(ZYNQ_QSPI_LCFG_TWO_MEM_MASK |
+				(1 << ZYNQ_QSPI_LCFG_DUMMY_SHIFT) |
+				ZYNQ_QSPI_FAST_READ_QOUT_CODE));
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET,
+			ZYNQ_QSPI_ENABLE_ENABLE_MASK);
+}
+
+/**
+ * zynq_qspi_copy_read_data - Copy data to RX buffer
+ * @xqspi:	Pointer to the zynq_qspi structure
+ * @data:	The 32 bit variable where data is stored
+ * @size:	Number of bytes to be copied from data to RX buffer
+ *
+ * Note: In case of dual parallel connection, even number of bytes are read
+ * when odd bytes are requested to avoid transfer of a nibble to each flash.
+ * The receive buffer though, is populated with the number of bytes requested.
+ */
+static void zynq_qspi_copy_read_data(struct zynq_qspi *xqspi, u32 data, u8 size)
+{
+	if (xqspi->rxbuf) {
+		if (!xqspi->is_parallel || xqspi->is_instr) {
+			memcpy(xqspi->rxbuf, ((u8 *) &data) + 4 - size, size);
+			xqspi->rxbuf += size;
+		} else {
+			u8 buff[4], len;
+
+			len = size;
+			size = size % 2 ? size + 1 : size;
+			memcpy(buff, ((u8 *) &data) + 4 - size, size);
+			memcpy(xqspi->rxbuf, buff, len);
+			xqspi->rxbuf += len;
+		}
+	}
+	xqspi->bytes_to_receive -= size;
+	if (xqspi->bytes_to_receive < 0)
+		xqspi->bytes_to_receive = 0;
+}
+
+/**
+ * zynq_qspi_copy_write_data - Copy data from TX buffer
+ * @xqspi:	Pointer to the zynq_qspi structure
+ * @data:	Pointer to the 32 bit variable where data is to be copied
+ * @size:	Number of bytes to be copied from TX buffer to data
+ */
+static void zynq_qspi_copy_write_data(struct zynq_qspi *xqspi, u32 *data,
+				      u8 size)
+{
+	if (xqspi->txbuf) {
+		memcpy(data, xqspi->txbuf, size);
+		xqspi->txbuf += size;
+	} else {
+		*data = 0;
+	}
+
+	xqspi->bytes_to_transfer -= size;
+}
+
+/**
+ * zynq_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 zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
+{
+	struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
+	u32 config_reg;
+
+	config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+
+	/* Select upper/lower page before asserting CS */
+	if (xqspi->is_stacked) {
+		u32 lqspi_cfg_reg;
+
+		lqspi_cfg_reg = zynq_qspi_read(xqspi,
+					       ZYNQ_QSPI_LINEAR_CFG_OFFSET);
+		if (qspi->master->flags & SPI_MASTER_U_PAGE)
+			lqspi_cfg_reg |= ZYNQ_QSPI_LCFG_U_PAGE_MASK;
+		else
+			lqspi_cfg_reg &= ~ZYNQ_QSPI_LCFG_U_PAGE_MASK;
+		zynq_qspi_write(xqspi, ZYNQ_QSPI_LINEAR_CFG_OFFSET,
+				lqspi_cfg_reg);
+	}
+
+	if (is_high) {
+		/* Deselect the slave */
+		config_reg |= ZYNQ_QSPI_CONFIG_SSCTRL_MASK;
+	} else {
+		/* Select the slave */
+		config_reg &= ~ZYNQ_QSPI_CONFIG_SSCTRL_MASK;
+		config_reg |= (((~(BIT(qspi->chip_select))) <<
+				 ZYNQ_QSPI_SS_SHIFT) &
+				 ZYNQ_QSPI_CONFIG_SSCTRL_MASK);
+		xqspi->is_instr = 1;
+	}
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+}
+
+/**
+ * zynq_qspi_config_clock_mode - Sets clock frequency
+ * @qspi:	Pointer to the spi_device structure
+ *
+ * Sets the requested clock polarity and phase.
+ */
+static void zynq_qspi_config_clock_mode(struct spi_device *qspi)
+{
+	struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
+	u32 config_reg;
+
+	config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+
+	/* Set the QSPI clock phase and clock polarity */
+	config_reg &= (~ZYNQ_QSPI_CONFIG_CPHA_MASK) &
+		      (~ZYNQ_QSPI_CONFIG_CPOL_MASK);
+	if (qspi->mode & SPI_CPHA)
+		config_reg |= ZYNQ_QSPI_CONFIG_CPHA_MASK;
+	if (qspi->mode & SPI_CPOL)
+		config_reg |= ZYNQ_QSPI_CONFIG_CPOL_MASK;
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+}
+
+/**
+ * zynq_qspi_config_clock_freq - Sets clock frequency
+ * @qspi:	Pointer to the spi_device structure
+ * @transfer:	Pointer to the spi_transfer structure which provides
+ *		information about next transfer setup parameters
+ *
+ * Sets the requested clock frequency.
+ * Note: If the requested frequency is not an exact match with what can be
+ * obtained using the prescalar 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 SPI
+ * controller the driver will set the highest or lowest frequency supported by
+ * controller.
+ */
+static void zynq_qspi_config_clock_freq(struct spi_device *qspi,
+				       struct spi_transfer *transfer)
+{
+	struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
+	u32 config_reg, baud_rate_val = 0;
+
+	/* Set the clock frequency */
+	/* If requested frequency is zero, default to lowest speed */
+	while ((baud_rate_val < ZYNQ_QSPI_BAUD_DIV_MAX)  &&
+	       (clk_get_rate(xqspi->refclk) / (2 << baud_rate_val)) >
+	       transfer->speed_hz)
+		baud_rate_val++;
+
+	config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
+	config_reg &= ~ZYNQ_QSPI_CONFIG_BDRATE_MASK;
+	config_reg |= (baud_rate_val << ZYNQ_QSPI_BAUD_DIV_SHIFT);
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_CONFIG_OFFSET, config_reg);
+}
+
+/**
+ * zynq_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
+ *
+ */
+static int zynq_qspi_setup_transfer(struct spi_device *qspi,
+				    struct spi_transfer *transfer)
+{
+	zynq_qspi_config_clock_freq(qspi, transfer);
+
+	return 0;
+}
+
+/**
+ * zynq_qspi_fill_tx_fifo - Fills the TX FIFO with as many bytes as possible
+ * @xqspi:	Pointer to the zynq_qspi structure
+ * @size:	Size of the fifo to be filled
+ */
+static void zynq_qspi_fill_tx_fifo(struct zynq_qspi *xqspi, u32 size)
+{
+	u32 fifocount;
+
+	for (fifocount = 0; (fifocount < size) &&
+	     (xqspi->bytes_to_transfer >= 4); fifocount++) {
+		if (xqspi->txbuf) {
+			zynq_qspi_write(xqspi,
+					ZYNQ_QSPI_TXD_00_00_OFFSET,
+					*((u32 *)xqspi->txbuf));
+			xqspi->txbuf += 4;
+		} else {
+			zynq_qspi_write(xqspi,
+					ZYNQ_QSPI_TXD_00_00_OFFSET, 0x00);
+		}
+		xqspi->bytes_to_transfer -= 4;
+	}
+}
+
+/**
+ * zynq_qspi_tx_dual_parallel - Handles odd byte tx for dual parallel
+ *
+ * In dual parallel configuration, when read/write data operations
+ * are performed, odd data bytes have to be converted to even to
+ * avoid a nibble (of data when programming / dummy when reading)
+ * going to individual flash devices, where a byte is expected.
+ * This check is only for data and will not apply for commands.
+ *
+ * @xqspi:	Pointer to the zynq_qspi structure
+ * @data:	Data to be transmitted
+ * @len:	No. of bytes to be transmitted
+ */
+static inline void zynq_qspi_tx_dual_parallel(struct zynq_qspi *xqspi,
+					      u32 data, u32 len)
+{
+	len = len % 2 ? len + 1 : len;
+	if (len == 4)
+		zynq_qspi_write(xqspi, ZYNQ_QSPI_TXD_00_00_OFFSET, data);
+	else
+		zynq_qspi_write(xqspi, ZYNQ_QSPI_TXD_00_01_OFFSET +
+				((len - 1) * 4), data);
+}
+
+/**
+ * zynq_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 zynq_qspi_irq(int irq, void *dev_id)
+{
+	struct spi_master *master = dev_id;
+	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+	u32 intr_status, rxcount, rxindex = 0;
+	u8 offset[3] = {ZYNQ_QSPI_TXD_00_01_OFFSET, ZYNQ_QSPI_TXD_00_10_OFFSET,
+			ZYNQ_QSPI_TXD_00_11_OFFSET};
+
+	intr_status = zynq_qspi_read(xqspi, ZYNQ_QSPI_STATUS_OFFSET);
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_STATUS_OFFSET, intr_status);
+
+	if ((intr_status & ZYNQ_QSPI_IXR_TXNFULL_MASK) ||
+	    (intr_status & ZYNQ_QSPI_IXR_RXNEMTY_MASK)) {
+		/*
+		 * This bit is set when Tx FIFO has < THRESHOLD entries.
+		 * We have the THRESHOLD value set to 1,
+		 * so this bit indicates Tx FIFO is empty.
+		 */
+		u32 data;
+
+		rxcount = xqspi->bytes_to_receive - xqspi->bytes_to_transfer;
+		rxcount = (rxcount % 4) ? ((rxcount/4) + 1) : (rxcount/4);
+
+		/* Read out the data from the RX FIFO */
+		while ((rxindex < rxcount) &&
+		       (rxindex < ZYNQ_QSPI_RX_THRESHOLD)) {
+			if (xqspi->bytes_to_receive >= 4) {
+				if (xqspi->rxbuf) {
+					(*(u32 *)xqspi->rxbuf) =
+					zynq_qspi_read(xqspi,
+						       ZYNQ_QSPI_RXD_OFFSET);
+					xqspi->rxbuf += 4;
+				} else {
+					data = zynq_qspi_read(xqspi,
+							ZYNQ_QSPI_RXD_OFFSET);
+				}
+				xqspi->bytes_to_receive -= 4;
+			} else {
+				data = zynq_qspi_read(xqspi,
+						      ZYNQ_QSPI_RXD_OFFSET);
+				zynq_qspi_copy_read_data(xqspi, data,
+						xqspi->bytes_to_receive);
+			}
+			rxindex++;
+		}
+
+		if (xqspi->bytes_to_transfer) {
+			if (xqspi->bytes_to_transfer >= 4) {
+				/* There is more data to send */
+				zynq_qspi_fill_tx_fifo(xqspi,
+						       ZYNQ_QSPI_RX_THRESHOLD);
+			} else if (intr_status & ZYNQ_QSPI_IXR_TXNFULL_MASK) {
+				int tmp;
+
+				tmp = xqspi->bytes_to_transfer;
+				zynq_qspi_copy_write_data(xqspi, &data,
+					xqspi->bytes_to_transfer);
+
+				if (!xqspi->is_parallel || xqspi->is_instr)
+					zynq_qspi_write(xqspi,
+							offset[tmp - 1], data);
+				else {
+					zynq_qspi_tx_dual_parallel(xqspi, data,
+								   tmp);
+				}
+			}
+		} else {
+			/*
+			 * If transfer and receive is completed then only send
+			 * complete signal.
+			 */
+			if (!xqspi->bytes_to_receive) {
+				zynq_qspi_write(xqspi,
+						ZYNQ_QSPI_IDIS_OFFSET,
+						ZYNQ_QSPI_IXR_ALL_MASK);
+				spi_finalize_current_transfer(master);
+				xqspi->is_instr = 0;
+			}
+		}
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+/**
+ * zynq_qspi_transfer_one - Initiates the QSPI transfer
+ * @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 zynq_qspi_transfer_one(struct spi_master *master,
+				  struct spi_device *qspi,
+				  struct spi_transfer *transfer)
+{
+	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+	u32 data;
+
+	xqspi->txbuf = transfer->tx_buf;
+	xqspi->rxbuf = transfer->rx_buf;
+	xqspi->bytes_to_transfer = transfer->len;
+	xqspi->bytes_to_receive = transfer->len;
+
+	zynq_qspi_setup_transfer(qspi, transfer);
+
+	if (transfer->len >= 4) {
+		zynq_qspi_fill_tx_fifo(xqspi, ZYNQ_QSPI_FIFO_DEPTH);
+	} else {
+		zynq_qspi_copy_write_data(xqspi, &data, transfer->len);
+
+		if (!xqspi->is_parallel || xqspi->is_instr)
+			zynq_qspi_write(xqspi, ZYNQ_QSPI_TXD_00_01_OFFSET +
+					((transfer->len - 1) * 4), data);
+		else {
+			zynq_qspi_tx_dual_parallel(xqspi, data, transfer->len);
+		}
+	}
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_IEN_OFFSET,
+			ZYNQ_QSPI_IXR_ALL_MASK);
+
+	return transfer->len;
+}
+
+/**
+ * zynq_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 zynq_prepare_transfer_hardware(struct spi_master *master)
+{
+	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+	zynq_qspi_config_clock_mode(master->cur_msg->spi);
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET,
+			ZYNQ_QSPI_ENABLE_ENABLE_MASK);
+
+	return 0;
+}
+
+/**
+ * zynq_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 zynq_unprepare_transfer_hardware(struct spi_master *master)
+{
+	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
+
+	return 0;
+}
+
+/**
+ * zynq_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 zynq_qspi_suspend(struct device *_dev)
+{
+	struct platform_device *pdev = container_of(_dev,
+			struct platform_device, dev);
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+	spi_master_suspend(master);
+
+	clk_disable_unprepare(xqspi->refclk);
+	clk_disable_unprepare(xqspi->pclk);
+
+	return 0;
+}
+
+/**
+ * zynq_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 zynq_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 zynq_qspi *xqspi = spi_master_get_devdata(master);
+	int ret = 0;
+
+	ret = clk_prepare_enable(xqspi->pclk);
+	if (ret) {
+		dev_err(dev, "Cannot enable APB clock.\n");
+		return ret;
+	}
+
+	ret = clk_prepare_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(zynq_qspi_dev_pm_ops, zynq_qspi_suspend,
+			 zynq_qspi_resume);
+
+/**
+ * zynq_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 zynq_qspi_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct spi_master *master;
+	struct zynq_qspi *xqspi;
+	struct resource *res;
+	u32 num_cs, qspi_mode;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(*xqspi));
+	if (master == NULL)
+		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;
+	}
+
+	if (of_property_read_u32(pdev->dev.of_node, "xlnx,qspi-mode",
+				 &qspi_mode) < 0)
+		dev_warn(&pdev->dev,
+			 "qspi-mode not found; defaulting to single\n");
+
+	/* Default single mode */
+	xqspi->is_parallel = 0;
+	xqspi->is_stacked = 0;
+	if (qspi_mode == 1)
+		xqspi->is_stacked = 1;
+	else if (qspi_mode == 2)
+		xqspi->is_parallel = 1;
+
+	xqspi->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(xqspi->pclk)) {
+		dev_err(&pdev->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(&pdev->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(&pdev->dev, "Unable to enable APB clock.\n");
+		goto remove_master;
+	}
+
+	ret = clk_prepare_enable(xqspi->refclk);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to enable device clock.\n");
+		goto clk_dis_pclk;
+	}
+
+	/* QSPI controller initializations */
+	zynq_qspi_init_hw(xqspi);
+
+	xqspi->irq = platform_get_irq(pdev, 0);
+	if (xqspi->irq <= 0) {
+		ret = -ENXIO;
+		dev_err(&pdev->dev, "irq resource not found\n");
+		goto remove_master;
+	}
+	ret = devm_request_irq(&pdev->dev, xqspi->irq, zynq_qspi_irq,
+			       0, pdev->name, master);
+	if (ret != 0) {
+		ret = -ENXIO;
+		dev_err(&pdev->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 = ZYNQ_QSPI_DEFAULT_NUM_CS;
+	else
+		master->num_chipselect = num_cs;
+
+	master->set_cs = zynq_qspi_chipselect;
+	master->transfer_one = zynq_qspi_transfer_one;
+	master->prepare_transfer_hardware = zynq_prepare_transfer_hardware;
+	master->unprepare_transfer_hardware = zynq_unprepare_transfer_hardware;
+	master->flags = SPI_MASTER_QUAD_MODE;
+
+	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;
+
+	ret = spi_register_master(master);
+	if (ret) {
+		dev_err(&pdev->dev, "spi_register_master failed\n");
+		goto clk_dis_all;
+	}
+
+	return ret;
+
+clk_dis_all:
+	clk_disable_unprepare(xqspi->refclk);
+clk_dis_pclk:
+	clk_disable_unprepare(xqspi->pclk);
+remove_master:
+	spi_master_put(master);
+	return ret;
+}
+
+/**
+ * zynq_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 on success and error value on failure
+ */
+static int zynq_qspi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct zynq_qspi *xqspi = spi_master_get_devdata(master);
+
+	zynq_qspi_write(xqspi, ZYNQ_QSPI_ENABLE_OFFSET, 0);
+
+	clk_disable_unprepare(xqspi->refclk);
+	clk_disable_unprepare(xqspi->pclk);
+
+	spi_unregister_master(master);
+
+	return 0;
+}
+
+static const struct of_device_id zynq_qspi_of_match[] = {
+	{ .compatible = "xlnx,zynq-qspi-1.0", },
+	{ /* end of table */ }
+};
+MODULE_DEVICE_TABLE(of, zynq_qspi_of_match);
+
+/*
+ * zynq_qspi_driver - This structure defines the QSPI platform driver
+ */
+static struct platform_driver zynq_qspi_driver = {
+	.probe = zynq_qspi_probe,
+	.remove = zynq_qspi_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = zynq_qspi_of_match,
+		.pm = &zynq_qspi_dev_pm_ops,
+	},
+};
+
+module_platform_driver(zynq_qspi_driver);
+
+MODULE_AUTHOR("Xilinx, Inc.");
+MODULE_DESCRIPTION("Xilinx Zynq QSPI driver");
+MODULE_LICENSE("GPL");