diff mbox

[PATCHv4,2/3] spi: Add spi driver for Socionext Synquacer platform

Message ID 1519736330-3985-1-git-send-email-jassisinghbrar@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jassi Brar Feb. 27, 2018, 12:58 p.m. UTC
From: Jassi Brar <jaswinder.singh@linaro.org>

This patch adds support for controller found on synquacer platforms.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 drivers/spi/Kconfig         |  11 +
 drivers/spi/Makefile        |   1 +
 drivers/spi/spi-synquacer.c | 663 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 675 insertions(+)
 create mode 100644 drivers/spi/spi-synquacer.c

Comments

Geert Uytterhoeven Feb. 28, 2018, 11:17 a.m. UTC | #1
Hi Jassi,

On Tue, Feb 27, 2018 at 1:58 PM,  <jassisinghbrar@gmail.com> wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> This patch adds support for controller found on synquacer platforms.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/spi/spi-synquacer.c

> +static void read_fifo(struct synquacer_spi *sspi)
> +{
> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
> +       int i;

unsigned int, as len is unsigned.

> +static void write_fifo(struct synquacer_spi *sspi)
> +{
> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
> +       int i;

unsigned int, as len is unsigned.

> +static int synquacer_spi_config(struct spi_master *master,
> +                               struct spi_device *spi,
> +                               struct spi_transfer *xfer)
> +{
> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
> +       unsigned int speed, mode, bpw, cs, bus_width;
> +       unsigned long rate;

unsigned int, as max_speed_hz is u32, else you'll do a 64/32-bit division later.

> +static int synquacer_spi_transfer_one(struct spi_master *master,
> +                                     struct spi_device *spi,
> +                                     struct spi_transfer *xfer)
> +{
> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
> +       int ret, words, busy = 0;

unsigned int words, as xfr->len is unsigned.

> +       unsigned long bpw;

unsigned int is plenty (bits_per_word is even u8).

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
Trent Piepho Feb. 28, 2018, 5:55 p.m. UTC | #2
T24gVHVlLCAyMDE4LTAyLTI3IGF0IDE4OjI4ICswNTMwLCBqYXNzaXNpbmdoYnJhckBnbWFpbC5j
b20gd3JvdGU6DQo+IEZyb206IEphc3NpIEJyYXIgPGphc3dpbmRlci5zaW5naEBsaW5hcm8ub3Jn
Pg0KPiANCj4gVGhpcyBwYXRjaCBhZGRzIHN1cHBvcnQgZm9yIGNvbnRyb2xsZXIgZm91bmQgb24g
c3lucXVhY2VyIHBsYXRmb3Jtcy4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IEphc3NpIEJyYXIgPGph
c3dpbmRlci5zaW5naEBsaW5hcm8ub3JnPg0KPiAtLS0NCj4gIGRyaXZlcnMvc3BpL0tjb25maWcg
ICAgICAgICB8ICAxMSArDQo+ICBkcml2ZXJzL3NwaS9NYWtlZmlsZSAgICAgICAgfCAgIDEgKw0K
PiAgZHJpdmVycy9zcGkvc3BpLXN5bnF1YWNlci5jIHwgNjYzICsrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrDQo+ICAzIGZpbGVzIGNoYW5nZWQsIDY3NSBpbnNlcnRp
b25zKCspDQo+ICBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9zcGkvc3BpLXN5bnF1YWNlci5j
DQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zcGkvS2NvbmZpZyBiL2RyaXZlcnMvc3BpL0tj
b25maWcNCj4gaW5kZXggNjAzNzgzOS4uOWUwNGJiZSAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9z
cGkvS2NvbmZpZw0KPiArKysgYi9kcml2ZXJzL3NwaS9LY29uZmlnDQo+IEBAIC02NTksNiArNjU5
LDE3IEBAIGNvbmZpZyBTUElfU1VONkkNCj4gIAloZWxwDQo+ICAJICBUaGlzIGVuYWJsZXMgdXNp
bmcgdGhlIFNQSSBjb250cm9sbGVyIG9uIHRoZSBBbGx3aW5uZXIgQTMxIFNvQ3MuDQo+ICANCj4g
K2NvbmZpZyBTUElfU1lOUVVBQ0VSDQo+ICsJdHJpc3RhdGUgIlNvY2lvbmV4dCdzIFN5bnF1YWNl
ciBIaWdoU3BlZWQgU1BJIGNvbnRyb2xsZXIiDQo+ICsJZGVwZW5kcyBvbiBBUkNIX1NZTlFVQUNF
UiB8fCBDT01QSUxFX1RFU1QNCj4gKwlzZWxlY3QgU1BJX0JJVEJBTkcNCj4gKwloZWxwDQo+ICsJ
ICBTUEkgZHJpdmVyIGZvciBTb2Npb25leHQncyBIaWdoIHNwZWVkIFNQSSBjb250cm9sbGVyIHdo
aWNoIHByb3ZpZGVzDQo+ICsJICB2YXJpb3VzIG9wZXJhdGluZyBtb2RlcyBmb3IgaW50ZXJmYWNp
bmcgdG8gc2VyaWFsIHBlcmlwaGVyYWwgZGV2aWNlcw0KPiArCSAgdGhhdCB1c2UgdGhlIGRlLWZh
Y3RvIHN0YW5kYXJkIFNQSSBwcm90b2NvbC4NCj4gKw0KPiArCSAgSXQgYWxzbyBzdXBwb3J0cyB0
aGUgbmV3IGR1YWwtYml0IGFuZCBxdWFkLWJpdCBTUEkgcHJvdG9jb2wuDQo+ICsNCg0KQXMgc29t
ZW9uZSB3aG8gZGVzaWducyBhbmQgZGV2ZWxvcHMgZW1iZWRkZWQgTGludXggZGV2aWNlcywgaXQg
d291bGQgYmUNCnJlYWxseSB1c2VmdWwgaWYgdGhlcmUgd2FzIGEgYml0IG1vcmUgZG9jdW1lbnRh
dGlvbiBhYm91dCB0aGUgZHJpdmVyLg0KDQpUaGUgaGFyZHdhcmUgZG9lc24ndCBzdXBwb3J0IERN
QSBvciBldmVuIGludGVycnVwdHMsIGFuZCBtdXN0IGJ1c3kgd2FpdA0KZm9yIHRoZSBkdXJhdGlv
biBvZiB0aGUgZW50aXJlIFNQSSB0cmFuc2ZlcnMuICBUaGF0J3Mga2luZCBvZiBhIEJpZw0KRGVh
bC4gIEEgZGVzaWduIHRoYXQgaW52b2x2ZXMgbG90cyBvZiBTUEkgdHJhbnNmZXJzIG9uIGEgc2xv
dyBjbG9jaw0Kd2lsbCB3b3JrIGZpbmUgb24gbW9zdCBTUEkgaGFyZHdhcmUsIGJ1dCB3aWxsIGJl
IHV0dGVybHkgdW5mZWFzaWJsZQ0KaGVyZS4gIEl0IHdvdWxkIGJlIG5pY2UgdG8gc2F5IHNvbWV0
aGluZyBhYm91dCB0aGF0LiAgSSdkIHByb2JhYmx5IHB1dA0KaXQgaW4gZHJpdmVyIGNvZGUu
--
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
Jassi Brar Feb. 28, 2018, 6:29 p.m. UTC | #3
On Wed, Feb 28, 2018 at 11:25 PM, Trent Piepho <tpiepho@impinj.com> wrote:
> On Tue, 2018-02-27 at 18:28 +0530, jassisinghbrar@gmail.com wrote:
>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> This patch adds support for controller found on synquacer platforms.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>> ---
>>  drivers/spi/Kconfig         |  11 +
>>  drivers/spi/Makefile        |   1 +
>>  drivers/spi/spi-synquacer.c | 663 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 675 insertions(+)
>>  create mode 100644 drivers/spi/spi-synquacer.c
>>
>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> index 6037839..9e04bbe 100644
>> --- a/drivers/spi/Kconfig
>> +++ b/drivers/spi/Kconfig
>> @@ -659,6 +659,17 @@ config SPI_SUN6I
>>       help
>>         This enables using the SPI controller on the Allwinner A31 SoCs.
>>
>> +config SPI_SYNQUACER
>> +     tristate "Socionext's Synquacer HighSpeed SPI controller"
>> +     depends on ARCH_SYNQUACER || COMPILE_TEST
>> +     select SPI_BITBANG
>> +     help
>> +       SPI driver for Socionext's High speed SPI controller which provides
>> +       various operating modes for interfacing to serial peripheral devices
>> +       that use the de-facto standard SPI protocol.
>> +
>> +       It also supports the new dual-bit and quad-bit SPI protocol.
>> +
>
> As someone who designs and develops embedded Linux devices, it would be
> really useful if there was a bit more documentation about the driver.
>
> The hardware doesn't support DMA or even interrupts, and must busy wait
> for the duration of the entire SPI transfers.  That's kind of a Big
> Deal.  A design that involves lots of SPI transfers on a slow clock
> will work fine on most SPI hardware, but will be utterly unfeasible
> here.  It would be nice to say something about that.  I'd probably put
> it in driver code.
>
From marketing point of view, why would I want to advertise my limitations? :)

Its not like any random platform might want to use this driver, nor
the enduser could do anything about it (its not a s/w issue). The
decision to integrate the ip in a SoC is already taken before the
driver is written. Obviously the users that need high spi performance
are not its intended target.

BTW, the IP is actually not that bad. It is meant for interfacing
flash devices and can operate in two modes - "direct" mode where it
maps flash memory as i/o region and the other last resort "spi" mode
where we could manually drive the flash. This driver implements that
limited "spi" mode.

Cheers!
--
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
Jassi Brar Feb. 28, 2018, 6:35 p.m. UTC | #4
On Wed, Feb 28, 2018 at 4:47 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Jassi,
>
> On Tue, Feb 27, 2018 at 1:58 PM,  <jassisinghbrar@gmail.com> wrote:
>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> This patch adds support for controller found on synquacer platforms.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> Thanks for your patch!
>
>> --- /dev/null
>> +++ b/drivers/spi/spi-synquacer.c
>
>> +static void read_fifo(struct synquacer_spi *sspi)
>> +{
>> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
>> +       int i;
>
> unsigned int, as len is unsigned.
>
>> +static void write_fifo(struct synquacer_spi *sspi)
>> +{
>> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
>> +       int i;
>
> unsigned int, as len is unsigned.
>
>> +static int synquacer_spi_config(struct spi_master *master,
>> +                               struct spi_device *spi,
>> +                               struct spi_transfer *xfer)
>> +{
>> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
>> +       unsigned int speed, mode, bpw, cs, bus_width;
>> +       unsigned long rate;
>
> unsigned int, as max_speed_hz is u32, else you'll do a 64/32-bit division later.
>
>> +static int synquacer_spi_transfer_one(struct spi_master *master,
>> +                                     struct spi_device *spi,
>> +                                     struct spi_transfer *xfer)
>> +{
>> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
>> +       int ret, words, busy = 0;
>
> unsigned int words, as xfr->len is unsigned.
>
>> +       unsigned long bpw;
>
> unsigned int is plenty (bits_per_word is even u8).
>
Will fix all.

Thank you.
--
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
Ard Biesheuvel Feb. 28, 2018, 6:36 p.m. UTC | #5
On 28 February 2018 at 18:29, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Wed, Feb 28, 2018 at 11:25 PM, Trent Piepho <tpiepho@impinj.com> wrote:
>> On Tue, 2018-02-27 at 18:28 +0530, jassisinghbrar@gmail.com wrote:
>>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>>
>>> This patch adds support for controller found on synquacer platforms.
>>>
>>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>>> ---
>>>  drivers/spi/Kconfig         |  11 +
>>>  drivers/spi/Makefile        |   1 +
>>>  drivers/spi/spi-synquacer.c | 663 ++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 675 insertions(+)
>>>  create mode 100644 drivers/spi/spi-synquacer.c
>>>
>>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>>> index 6037839..9e04bbe 100644
>>> --- a/drivers/spi/Kconfig
>>> +++ b/drivers/spi/Kconfig
>>> @@ -659,6 +659,17 @@ config SPI_SUN6I
>>>       help
>>>         This enables using the SPI controller on the Allwinner A31 SoCs.
>>>
>>> +config SPI_SYNQUACER
>>> +     tristate "Socionext's Synquacer HighSpeed SPI controller"
>>> +     depends on ARCH_SYNQUACER || COMPILE_TEST
>>> +     select SPI_BITBANG
>>> +     help
>>> +       SPI driver for Socionext's High speed SPI controller which provides
>>> +       various operating modes for interfacing to serial peripheral devices
>>> +       that use the de-facto standard SPI protocol.
>>> +
>>> +       It also supports the new dual-bit and quad-bit SPI protocol.
>>> +
>>
>> As someone who designs and develops embedded Linux devices, it would be
>> really useful if there was a bit more documentation about the driver.
>>
>> The hardware doesn't support DMA or even interrupts, and must busy wait
>> for the duration of the entire SPI transfers.  That's kind of a Big
>> Deal.  A design that involves lots of SPI transfers on a slow clock
>> will work fine on most SPI hardware, but will be utterly unfeasible
>> here.  It would be nice to say something about that.  I'd probably put
>> it in driver code.
>>
> From marketing point of view, why would I want to advertise my limitations? :)
>
> Its not like any random platform might want to use this driver, nor
> the enduser could do anything about it (its not a s/w issue). The
> decision to integrate the ip in a SoC is already taken before the
> driver is written. Obviously the users that need high spi performance
> are not its intended target.
>
> BTW, the IP is actually not that bad. It is meant for interfacing
> flash devices and can operate in two modes - "direct" mode where it
> maps flash memory as i/o region and the other last resort "spi" mode
> where we could manually drive the flash. This driver implements that
> limited "spi" mode.
>

My SoC manual does list interrupts for this IP block. Are you sure
SynQuacer uses the same version of the IP as before?
--
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
Trent Piepho Feb. 28, 2018, 6:57 p.m. UTC | #6
On Wed, 2018-02-28 at 23:59 +0530, Jassi Brar wrote:
> On Wed, Feb 28, 2018 at 11:25 PM, Trent Piepho <tpiepho@impinj.com> wrote:

> > On Tue, 2018-02-27 at 18:28 +0530, jassisinghbrar@gmail.com wrote:

> > > 

> > > +config SPI_SYNQUACER

> > > +     tristate "Socionext's Synquacer HighSpeed SPI controller"

> > > +     depends on ARCH_SYNQUACER || COMPILE_TEST

> > > +     select SPI_BITBANG

> > > +     help

> > > +       SPI driver for Socionext's High speed SPI controller which provides

> > > +       various operating modes for interfacing to serial peripheral devices

> > > +       that use the de-facto standard SPI protocol.

> > > +

> > > +       It also supports the new dual-bit and quad-bit SPI protocol.

> > > +

> > 

> > As someone who designs and develops embedded Linux devices, it would be

> > really useful if there was a bit more documentation about the driver.

> > 

> > The hardware doesn't support DMA or even interrupts, and must busy wait

> > for the duration of the entire SPI transfers.  That's kind of a Big

> > Deal.  A design that involves lots of SPI transfers on a slow clock

> > will work fine on most SPI hardware, but will be utterly unfeasible

> > here.  It would be nice to say something about that.  I'd probably put

> > it in driver code.

> > 

> 

> From marketing point of view, why would I want to advertise my limitations? :)

> 

> Its not like any random platform might want to use this driver, nor

> the enduser could do anything about it (its not a s/w issue). The

> decision to integrate the ip in a SoC is already taken before the

> driver is written. Obviously the users that need high spi performance

> are not its intended target.


It helps when choosing hardware, designing products, and also with
software design.  Example:

I really need another GPIO pin on this SoC.  Here's this interrupt line
from a spi slave.  I don't need that and can take the pin.  I can poll
the slave at a low rate and it'll be ok as there is no tight latency
requirement.

That might use a lot more CPU that one would think.

> BTW, the IP is actually not that bad. It is meant for interfacing

> flash devices and can operate in two modes - "direct" mode where it

> maps flash memory as i/o region and the other last resort "spi" mode

> where we could manually drive the flash. This driver implements that

> limited "spi" mode.


Here's another example, someone has a "direct" flash on their hardware,
probably just copied from the reference design.  In doing cost
engineering for the next rev, they determine that they don't need much
performance from this flash and should be able to use a cheaper non-vol 
storage part (eeprom..).  The new part has a Linux driver.  No one
realizes the significance of switching from "direct" to "spi" mode.
Andy Shevchenko March 1, 2018, 10:02 a.m. UTC | #7
On Tue, Feb 27, 2018 at 2:58 PM,  <jassisinghbrar@gmail.com> wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> This patch adds support for controller found on synquacer platforms.

Thanks for the patch!

Unfortunately it has a set of typical mistakes.
Perhaps you guys eventually do follow some common style?

See my comments below.

> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/spinlock.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>

> +#define PCC0           0x4
> +#define PCC(n)         (PCC0 + (n) * 4)
> +#define RTM    BIT(3)
> +#define ACES   BIT(2)
> +#define SAFESYNC       BIT(16)
> +#define CPHA   BIT(0)
> +#define CPOL   BIT(1)
> +#define SSPOL  BIT(4)
> +#define SDIR   BIT(7)

> +#define SS2CD  5

What is this?

> +#define SENDIAN        BIT(8)

Why above list of bits is unordered?

> +#define CDRS_SHIFT     9

> +#define CDRS_MASK      0x7f

GENMASK() ?

> +#define        DMSTART         0x38
> +#define TRIGGER                BIT(0)
> +#define DMSTOP         BIT(8)

> +#define CS_MASK                3

GENMASK() ?

> +#define CS_SHIFT       16
> +#define DATA_TXRX      0
> +#define DATA_RX                1
> +#define DATA_TX                2

> +#define DATA_MASK      3

GENMASK() ?

> +#define DATA_SHIFT     26
> +#define BUS_WIDTH      24
> +
> +#define        DMBCC           0x3c
> +#define DMSTATUS       0x40

> +#define RX_DATA_MASK   0x1f

GENMASK() ?

> +#define RX_DATA_SHIFT  8

> +#define TX_DATA_MASK   0x1f

GENMASK() ?

> +#define TX_DATA_SHIFT  16
> +
> +#define TXBITCNT       0x44
> +
> +#define FIFOCFG                0x4c

> +#define BPW_MASK       0x3

GENMASK() ?

> +#define BPW_SHIFT      8
> +#define RX_FLUSH       BIT(11)
> +#define TX_FLUSH       BIT(12)

> +#define RX_TRSHLD_MASK         0xf

GENMASK() ?

> +#define RX_TRSHLD_SHIFT                0

> +#define TX_TRSHLD_MASK         0xf

GENMASK() ?

> +#define TX_TRSHLD_SHIFT                4
> +
> +#define TXFIFO         0x50
> +#define RXFIFO         0x90
> +#define MID            0xfc
> +
> +#define FIFO_DEPTH     16
> +#define TX_TRSHLD      4
> +#define RX_TRSHLD      (FIFO_DEPTH - TX_TRSHLD)
> +
> +#define TXBIT  BIT(1)
> +#define RXBIT  BIT(2)
> +
> +#define IHCLK  0
> +#define IPCLK  1

To all above. Can you do all definitions under a dedicated namespace?

> +struct synquacer_spi {

> +       struct device *dev;
> +       struct spi_master *master;

Isn't one refers to another?

> +
> +       unsigned int cs;
> +       unsigned int bpw;
> +       unsigned int mode;
> +       unsigned int speed;
> +       bool aces, rtm;
> +       void *rx_buf;
> +       const void *tx_buf;

> +       struct clk *clk[2];

Ouch. Please, split with a proper names.

> +       void __iomem *regs;
> +       unsigned int tx_words, rx_words;
> +       unsigned int bus_width;
> +};
> +
> +static void read_fifo(struct synquacer_spi *sspi)
> +{
> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
> +       int i;
> +
> +       len = (len >> RX_DATA_SHIFT) & RX_DATA_MASK;
> +       len = min_t(unsigned int, len, sspi->rx_words);
> +

> +       switch (sspi->bpw) {
> +       case 8:
> +               {
> +               u8 *buf = sspi->rx_buf;
> +
> +               for (i = 0; i < len; i++)
> +                       *buf++ = readb_relaxed(sspi->regs + RXFIFO);

readsb() ?

> +               sspi->rx_buf = buf;
> +               break;
> +               }
> +       case 16:
> +               {
> +               u16 *buf = sspi->rx_buf;
> +
> +               for (i = 0; i < len; i++)
> +                       *buf++ = readw_relaxed(sspi->regs + RXFIFO);
> +               sspi->rx_buf = buf;
> +               break;
> +               }

readsw() ?

> +       default:
> +               {
> +               u32 *buf = sspi->rx_buf;
> +
> +               for (i = 0; i < len; i++)
> +                       *buf++ = readl_relaxed(sspi->regs + RXFIFO);
> +               sspi->rx_buf = buf;
> +               break;
> +               }

readsl() ?

> +       }
> +
> +       sspi->rx_words -= len;
> +}
> +
> +static void write_fifo(struct synquacer_spi *sspi)
> +{
> +       u32 len = readl_relaxed(sspi->regs + DMSTATUS);
> +       int i;
> +
> +       len = (len >> TX_DATA_SHIFT) & TX_DATA_MASK;
> +       len = min_t(unsigned int, FIFO_DEPTH - len, sspi->tx_words);
> +
> +       switch (sspi->bpw) {
> +       case 8:
> +               {
> +               const u8 *buf = sspi->tx_buf;
> +
> +               for (i = 0; i < len; i++)
> +                       writeb_relaxed(*buf++, sspi->regs + TXFIFO);
> +               sspi->tx_buf = buf;
> +               break;
> +               }

writesb() ?

> +       case 16:
> +               {
> +               const u16 *buf = sspi->tx_buf;
> +
> +               for (i = 0; i < len; i++)
> +                       writew_relaxed(*buf++, sspi->regs + TXFIFO);
> +               sspi->tx_buf = buf;
> +               break;
> +               }

writesw() ?

> +       default:
> +               {
> +               const u32 *buf = sspi->tx_buf;
> +
> +               for (i = 0; i < len; i++)
> +                       writel_relaxed(*buf++, sspi->regs + TXFIFO);
> +               sspi->tx_buf = buf;
> +               break;
> +               }

writesl() ?

> +       }
> +       sspi->tx_words -= len;
> +}
> +
> +static int synquacer_spi_config(struct spi_master *master,
> +                               struct spi_device *spi,
> +                               struct spi_transfer *xfer)
> +{
> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);
> +       unsigned int speed, mode, bpw, cs, bus_width;
> +       unsigned long rate;
> +       u32 val, div;
> +
> +       /* Full Duplex only on 1bit wide bus */

1-bit

> +}

> +static int synquacer_spi_transfer_one(struct spi_master *master,
> +                                     struct spi_device *spi,
> +                                     struct spi_transfer *xfer)
> +{

> +       /* See if we can transfer 4-bytes as 1 word even if not asked */

Why?

> +       bpw = xfer->bits_per_word;
> +       if (bpw == 8 && !(xfer->len % 4) && !(spi->mode & SPI_LSB_FIRST))
> +               xfer->bits_per_word = 32;

> +}

> +static void synquacer_spi_set_cs(struct spi_device *spi, bool enable)
> +{
> +       struct synquacer_spi *sspi = spi_master_get_devdata(spi->master);
> +       u32 val;

> +       if (!enable) {

Why not to use positive condition?

> +       } else {

> +       }
> +}

> +static int synquacer_spi_enable(struct spi_master *master)
> +{
> +       struct synquacer_spi *sspi = spi_master_get_devdata(master);

> +       u32 val;

unsigned int retries = 65535;

> +
> +       /* Disable module */
> +       writel_relaxed(0, sspi->regs + MCTRL);

> +       val = 0xfffff;
> +       while (--val && (readl_relaxed(sspi->regs + MCTRL) & MES))
> +               cpu_relax();
> +       if (!val)
> +               return -EBUSY;

while (readl...(...) && --retries)
 cpu_relax();
if (!retries)
 return -EBUSY;


> +static int synquacer_spi_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct spi_master *master;
> +       struct synquacer_spi *sspi;
> +       struct resource *res;
> +       int ret;
> +


> +       master = spi_alloc_master(&pdev->dev, sizeof(*sspi));

s/master/controller/ ?

I guess it might make sense to do for entire driver.

> +       if (!master)
> +               return -ENOMEM;

+ empty line ?

> +       platform_set_drvdata(pdev, master);

> +       clk_prepare_enable(sspi->clk[IPCLK]);

Also can fail.

> +       ret = clk_prepare_enable(sspi->clk[IHCLK]);
> +       if (ret)
> +               goto put_spi;

> +#ifdef CONFIG_PM_SLEEP

__maybe_unused

> +static int synquacer_spi_suspend(struct device *dev)

> +static int synquacer_spi_resume(struct device *dev)

> +               clk_prepare_enable(sspi->clk[IPCLK]);

It can fail.

> +               ret = clk_prepare_enable(sspi->clk[IHCLK]);
> +               if (ret < 0) {
> +                       dev_err(dev, "failed to enable clk (%d)\n", ret);
> +                       return ret;
> +               }

> +}
> +#endif /* CONFIG_PM_SLEEP */

> +static const struct dev_pm_ops synquacer_spi_pm_ops = {
> +       SET_SYSTEM_SLEEP_PM_OPS(synquacer_spi_suspend, synquacer_spi_resume)
> +};

static SIMPLE_DEV_PM_OPS() ?

> +static const struct of_device_id synquacer_spi_of_match[] = {
> +       {.compatible = "socionext,synquacer-spi",},

> +       {},

No comma needed.

> +};
> +MODULE_DEVICE_TABLE(of, synquacer_spi_of_match);

> +static struct platform_driver synquacer_spi_driver = {
> +       .driver = {
> +               .name = "synquacer-spi",
> +               .pm = &synquacer_spi_pm_ops,

> +               .of_match_table = of_match_ptr(synquacer_spi_of_match),

of_match_ptr() should be dropped.

> +       },
> +       .probe = synquacer_spi_probe,
> +       .remove = synquacer_spi_remove,
> +};
Andy Shevchenko March 1, 2018, 10:04 a.m. UTC | #8
On Thu, Mar 1, 2018 at 12:02 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:

>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/scatterlist.h>
>> +#include <linux/slab.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/spinlock.h>

>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>

Forgot to add.
Is there any clock provider? I perhaps missed it.
Jassi Brar March 1, 2018, 2:11 p.m. UTC | #9
On Thu, Mar 1, 2018 at 12:06 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 28 February 2018 at 18:29, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Wed, Feb 28, 2018 at 11:25 PM, Trent Piepho <tpiepho@impinj.com> wrote:
>>> On Tue, 2018-02-27 at 18:28 +0530, jassisinghbrar@gmail.com wrote:
>>>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>>>
>>>> This patch adds support for controller found on synquacer platforms.
>>>>
>>>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>>>> ---
>>>>  drivers/spi/Kconfig         |  11 +
>>>>  drivers/spi/Makefile        |   1 +
>>>>  drivers/spi/spi-synquacer.c | 663 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 675 insertions(+)
>>>>  create mode 100644 drivers/spi/spi-synquacer.c
>>>>
>>>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>>>> index 6037839..9e04bbe 100644
>>>> --- a/drivers/spi/Kconfig
>>>> +++ b/drivers/spi/Kconfig
>>>> @@ -659,6 +659,17 @@ config SPI_SUN6I
>>>>       help
>>>>         This enables using the SPI controller on the Allwinner A31 SoCs.
>>>>
>>>> +config SPI_SYNQUACER
>>>> +     tristate "Socionext's Synquacer HighSpeed SPI controller"
>>>> +     depends on ARCH_SYNQUACER || COMPILE_TEST
>>>> +     select SPI_BITBANG
>>>> +     help
>>>> +       SPI driver for Socionext's High speed SPI controller which provides
>>>> +       various operating modes for interfacing to serial peripheral devices
>>>> +       that use the de-facto standard SPI protocol.
>>>> +
>>>> +       It also supports the new dual-bit and quad-bit SPI protocol.
>>>> +
>>>
>>> As someone who designs and develops embedded Linux devices, it would be
>>> really useful if there was a bit more documentation about the driver.
>>>
>>> The hardware doesn't support DMA or even interrupts, and must busy wait
>>> for the duration of the entire SPI transfers.  That's kind of a Big
>>> Deal.  A design that involves lots of SPI transfers on a slow clock
>>> will work fine on most SPI hardware, but will be utterly unfeasible
>>> here.  It would be nice to say something about that.  I'd probably put
>>> it in driver code.
>>>
>> From marketing point of view, why would I want to advertise my limitations? :)
>>
>> Its not like any random platform might want to use this driver, nor
>> the enduser could do anything about it (its not a s/w issue). The
>> decision to integrate the ip in a SoC is already taken before the
>> driver is written. Obviously the users that need high spi performance
>> are not its intended target.
>>
>> BTW, the IP is actually not that bad. It is meant for interfacing
>> flash devices and can operate in two modes - "direct" mode where it
>> maps flash memory as i/o region and the other last resort "spi" mode
>> where we could manually drive the flash. This driver implements that
>> limited "spi" mode.
>>
>
> My SoC manual does list interrupts for this IP block. Are you sure
> SynQuacer uses the same version of the IP as before?
>
Having got hold of english specs, the newer version does seem to
support IRQ despite being named same "FIP006" as before.
Let me investigate deeper.

Thanks.
--
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 6037839..9e04bbe 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -659,6 +659,17 @@  config SPI_SUN6I
 	help
 	  This enables using the SPI controller on the Allwinner A31 SoCs.
 
+config SPI_SYNQUACER
+	tristate "Socionext's Synquacer HighSpeed SPI controller"
+	depends on ARCH_SYNQUACER || COMPILE_TEST
+	select SPI_BITBANG
+	help
+	  SPI driver for Socionext's High speed SPI controller which provides
+	  various operating modes for interfacing to serial peripheral devices
+	  that use the de-facto standard SPI protocol.
+
+	  It also supports the new dual-bit and quad-bit SPI protocol.
+
 config SPI_MXS
 	tristate "Freescale MXS SPI controller"
 	depends on ARCH_MXS
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 34c5f28..7c222f2 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -96,6 +96,7 @@  obj-$(CONFIG_SPI_STM32) 		+= spi-stm32.o
 obj-$(CONFIG_SPI_ST_SSC4)		+= spi-st-ssc4.o
 obj-$(CONFIG_SPI_SUN4I)			+= spi-sun4i.o
 obj-$(CONFIG_SPI_SUN6I)			+= spi-sun6i.o
+obj-$(CONFIG_SPI_SYNQUACER)		+= spi-synquacer.o
 obj-$(CONFIG_SPI_TEGRA114)		+= spi-tegra114.o
 obj-$(CONFIG_SPI_TEGRA20_SFLASH)	+= spi-tegra20-sflash.o
 obj-$(CONFIG_SPI_TEGRA20_SLINK)		+= spi-tegra20-slink.o
diff --git a/drivers/spi/spi-synquacer.c b/drivers/spi/spi-synquacer.c
new file mode 100644
index 0000000..45c6c6c
--- /dev/null
+++ b/drivers/spi/spi-synquacer.c
@@ -0,0 +1,663 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// Synquacer HSSPI controller driver
+//
+// Copyright (c) 2015-2018 Socionext Inc.
+// Copyright (c) 2018 Linaro Ltd.
+//
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/spinlock.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+
+#define MCTRL		0x0
+#define MEN	BIT(0)
+#define CSEN	BIT(1)
+#define BPCLK	BIT(3)
+#define MES	BIT(4)
+#define SYNCON	BIT(5)
+
+#define PCC0		0x4
+#define PCC(n)		(PCC0 + (n) * 4)
+#define RTM	BIT(3)
+#define ACES	BIT(2)
+#define SAFESYNC	BIT(16)
+#define CPHA	BIT(0)
+#define CPOL	BIT(1)
+#define SSPOL	BIT(4)
+#define SDIR	BIT(7)
+#define SS2CD	5
+#define SENDIAN	BIT(8)
+#define CDRS_SHIFT	9
+#define CDRS_MASK	0x7f
+
+#define TXF		0x14
+#define TXE		0x18
+#define TXC		0x1c
+#define RXF		0x20
+#define RXE		0x24
+#define RXC		0x28
+
+#define FAULTF		0x2c
+#define FAULTC		0x30
+
+#define DMCFG		0x34
+#define SSDC		BIT(1)
+#define MSTARTEN	BIT(2)
+
+#define	DMSTART		0x38
+#define TRIGGER		BIT(0)
+#define DMSTOP		BIT(8)
+#define CS_MASK		3
+#define CS_SHIFT	16
+#define DATA_TXRX	0
+#define DATA_RX		1
+#define DATA_TX		2
+#define DATA_MASK	3
+#define DATA_SHIFT	26
+#define BUS_WIDTH	24
+
+#define	DMBCC		0x3c
+#define DMSTATUS	0x40
+#define RX_DATA_MASK	0x1f
+#define RX_DATA_SHIFT	8
+#define TX_DATA_MASK	0x1f
+#define TX_DATA_SHIFT	16
+
+#define TXBITCNT	0x44
+
+#define FIFOCFG		0x4c
+#define BPW_MASK	0x3
+#define BPW_SHIFT	8
+#define RX_FLUSH	BIT(11)
+#define TX_FLUSH	BIT(12)
+#define RX_TRSHLD_MASK		0xf
+#define RX_TRSHLD_SHIFT		0
+#define TX_TRSHLD_MASK		0xf
+#define TX_TRSHLD_SHIFT		4
+
+#define TXFIFO		0x50
+#define RXFIFO		0x90
+#define MID		0xfc
+
+#define FIFO_DEPTH	16
+#define TX_TRSHLD	4
+#define RX_TRSHLD	(FIFO_DEPTH - TX_TRSHLD)
+
+#define TXBIT	BIT(1)
+#define RXBIT	BIT(2)
+
+#define IHCLK	0
+#define IPCLK	1
+
+struct synquacer_spi {
+	struct device *dev;
+	struct spi_master *master;
+
+	unsigned int cs;
+	unsigned int bpw;
+	unsigned int mode;
+	unsigned int speed;
+	bool aces, rtm;
+	void *rx_buf;
+	const void *tx_buf;
+	struct clk *clk[2];
+	void __iomem *regs;
+	unsigned int tx_words, rx_words;
+	unsigned int bus_width;
+};
+
+static void read_fifo(struct synquacer_spi *sspi)
+{
+	u32 len = readl_relaxed(sspi->regs + DMSTATUS);
+	int i;
+
+	len = (len >> RX_DATA_SHIFT) & RX_DATA_MASK;
+	len = min_t(unsigned int, len, sspi->rx_words);
+
+	switch (sspi->bpw) {
+	case 8:
+		{
+		u8 *buf = sspi->rx_buf;
+
+		for (i = 0; i < len; i++)
+			*buf++ = readb_relaxed(sspi->regs + RXFIFO);
+		sspi->rx_buf = buf;
+		break;
+		}
+	case 16:
+		{
+		u16 *buf = sspi->rx_buf;
+
+		for (i = 0; i < len; i++)
+			*buf++ = readw_relaxed(sspi->regs + RXFIFO);
+		sspi->rx_buf = buf;
+		break;
+		}
+	default:
+		{
+		u32 *buf = sspi->rx_buf;
+
+		for (i = 0; i < len; i++)
+			*buf++ = readl_relaxed(sspi->regs + RXFIFO);
+		sspi->rx_buf = buf;
+		break;
+		}
+	}
+
+	sspi->rx_words -= len;
+}
+
+static void write_fifo(struct synquacer_spi *sspi)
+{
+	u32 len = readl_relaxed(sspi->regs + DMSTATUS);
+	int i;
+
+	len = (len >> TX_DATA_SHIFT) & TX_DATA_MASK;
+	len = min_t(unsigned int, FIFO_DEPTH - len, sspi->tx_words);
+
+	switch (sspi->bpw) {
+	case 8:
+		{
+		const u8 *buf = sspi->tx_buf;
+
+		for (i = 0; i < len; i++)
+			writeb_relaxed(*buf++, sspi->regs + TXFIFO);
+		sspi->tx_buf = buf;
+		break;
+		}
+	case 16:
+		{
+		const u16 *buf = sspi->tx_buf;
+
+		for (i = 0; i < len; i++)
+			writew_relaxed(*buf++, sspi->regs + TXFIFO);
+		sspi->tx_buf = buf;
+		break;
+		}
+	default:
+		{
+		const u32 *buf = sspi->tx_buf;
+
+		for (i = 0; i < len; i++)
+			writel_relaxed(*buf++, sspi->regs + TXFIFO);
+		sspi->tx_buf = buf;
+		break;
+		}
+	}
+	sspi->tx_words -= len;
+}
+
+static int synquacer_spi_config(struct spi_master *master,
+				struct spi_device *spi,
+				struct spi_transfer *xfer)
+{
+	struct synquacer_spi *sspi = spi_master_get_devdata(master);
+	unsigned int speed, mode, bpw, cs, bus_width;
+	unsigned long rate;
+	u32 val, div;
+
+	/* Full Duplex only on 1bit wide bus */
+	if (xfer->rx_buf && xfer->tx_buf &&
+	    (xfer->rx_nbits != 1 || xfer->tx_nbits != 1)) {
+		dev_err(sspi->dev,
+			"RX and TX bus widths must match for Full-Duplex!\n");
+		return -EINVAL;
+	}
+
+	if (xfer->tx_buf)
+		bus_width = xfer->tx_nbits;
+	else
+		bus_width = xfer->rx_nbits;
+
+	mode = spi->mode;
+	cs = spi->chip_select;
+	speed = xfer->speed_hz;
+	bpw = xfer->bits_per_word;
+
+	/* return if nothing to change */
+	if (speed == sspi->speed &&
+	    bus_width == sspi->bus_width && bpw == sspi->bpw &&
+	    mode == sspi->mode && cs == sspi->cs) {
+		return 0;
+	}
+
+	rate = master->max_speed_hz;
+
+	div = DIV_ROUND_UP(rate, speed);
+	if (div > 254) {
+		dev_err(sspi->dev, "Requested rate too low (%u)\n",
+			sspi->speed);
+		return -EINVAL;
+	}
+
+	val = readl_relaxed(sspi->regs + PCC(cs));
+	val &= ~SAFESYNC;
+	if (bpw == 8 &&	(mode & (SPI_TX_DUAL | SPI_RX_DUAL)) && div < 3)
+		val |= SAFESYNC;
+	if (bpw == 8 &&	(mode & (SPI_TX_QUAD | SPI_RX_QUAD)) && div < 6)
+		val |= SAFESYNC;
+	if (bpw == 16 && (mode & (SPI_TX_QUAD | SPI_RX_QUAD)) && div < 3)
+		val |= SAFESYNC;
+
+	if (mode & SPI_CPHA)
+		val |= CPHA;
+	else
+		val &= ~CPHA;
+
+	if (mode & SPI_CPOL)
+		val |= CPOL;
+	else
+		val &= ~CPOL;
+
+	if (mode & SPI_CS_HIGH)
+		val |= SSPOL;
+	else
+		val &= ~SSPOL;
+
+	if (mode & SPI_LSB_FIRST)
+		val |= SDIR;
+	else
+		val &= ~SDIR;
+
+	if (sspi->aces)
+		val |= ACES;
+	else
+		val &= ~ACES;
+
+	if (sspi->rtm)
+		val |= RTM;
+	else
+		val &= ~RTM;
+
+	val |= (3 << SS2CD);
+	val |= SENDIAN;
+
+	val &= ~(CDRS_MASK << CDRS_SHIFT);
+	val |= ((div >> 1) << CDRS_SHIFT);
+
+	writel_relaxed(val, sspi->regs + PCC(cs));
+
+	val = readl_relaxed(sspi->regs + FIFOCFG);
+	val &= ~(BPW_MASK << BPW_SHIFT);
+	val |= ((bpw / 8 - 1) << BPW_SHIFT);
+	writel_relaxed(val, sspi->regs + FIFOCFG);
+
+	val = readl_relaxed(sspi->regs + DMSTART);
+	val &= ~(DATA_MASK << DATA_SHIFT);
+
+	if (xfer->tx_buf && xfer->rx_buf)
+		val |= (DATA_TXRX << DATA_SHIFT);
+	else if (xfer->rx_buf)
+		val |= (DATA_RX << DATA_SHIFT);
+	else
+		val |= (DATA_TX << DATA_SHIFT);
+
+	val &= ~(3 << BUS_WIDTH);
+	val |= ((bus_width >> 1) << BUS_WIDTH);
+	writel_relaxed(val, sspi->regs + DMSTART);
+
+	sspi->bpw = bpw;
+	sspi->mode = mode;
+	sspi->speed = speed;
+	sspi->cs = spi->chip_select;
+	sspi->bus_width = bus_width;
+
+	return 0;
+}
+
+static int synquacer_spi_transfer_one(struct spi_master *master,
+				      struct spi_device *spi,
+				      struct spi_transfer *xfer)
+{
+	struct synquacer_spi *sspi = spi_master_get_devdata(master);
+	int ret, words, busy = 0;
+	unsigned long bpw;
+	u32 val;
+
+	val = readl_relaxed(sspi->regs + FIFOCFG);
+	val |= RX_FLUSH;
+	val |= TX_FLUSH;
+	writel_relaxed(val, sspi->regs + FIFOCFG);
+
+	/* See if we can transfer 4-bytes as 1 word even if not asked */
+	bpw = xfer->bits_per_word;
+	if (bpw == 8 && !(xfer->len % 4) && !(spi->mode & SPI_LSB_FIRST))
+		xfer->bits_per_word = 32;
+
+	ret = synquacer_spi_config(master, spi, xfer);
+
+	/* restore */
+	xfer->bits_per_word = bpw;
+
+	if (ret)
+		return ret;
+
+	sspi->tx_buf = xfer->tx_buf;
+	sspi->rx_buf = xfer->rx_buf;
+
+	switch (sspi->bpw) {
+	case 8:
+		words = xfer->len;
+		break;
+	case 16:
+		words = xfer->len / 2;
+		break;
+	default:
+		words = xfer->len / 4;
+		break;
+	}
+
+	if (xfer->tx_buf) {
+		busy |= TXBIT;
+		sspi->tx_words = words;
+	} else {
+		sspi->tx_words = 0;
+	}
+
+	if (xfer->rx_buf) {
+		busy |= RXBIT;
+		sspi->rx_words = words;
+	} else {
+		sspi->rx_words = 0;
+	}
+
+	if (xfer->tx_buf)
+		write_fifo(sspi);
+
+	if (xfer->rx_buf) {
+		val = readl_relaxed(sspi->regs + FIFOCFG);
+		val &= ~(RX_TRSHLD_MASK << RX_TRSHLD_SHIFT);
+		val |= ((sspi->rx_words > FIFO_DEPTH ?
+			RX_TRSHLD : sspi->rx_words) << RX_TRSHLD_SHIFT);
+		writel_relaxed(val, sspi->regs + FIFOCFG);
+	}
+
+	writel_relaxed(~0, sspi->regs + TXC);
+	writel_relaxed(~0, sspi->regs + RXC);
+
+	/* Trigger */
+	val = readl_relaxed(sspi->regs + DMSTART);
+	val |= TRIGGER;
+	writel_relaxed(val, sspi->regs + DMSTART);
+
+	while (busy & (RXBIT | TXBIT)) {
+		if (sspi->rx_words)
+			read_fifo(sspi);
+		else
+			busy &= ~RXBIT;
+
+		if (sspi->tx_words) {
+			write_fifo(sspi);
+		} else {
+			u32 len;
+
+			do { /* wait for shifter to empty out */
+				cpu_relax();
+				len = readl_relaxed(sspi->regs + DMSTATUS);
+				len = (len >> TX_DATA_SHIFT) & TX_DATA_MASK;
+			} while (xfer->tx_buf && len);
+			busy &= ~TXBIT;
+		}
+	}
+
+	return 0;
+}
+
+static void synquacer_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct synquacer_spi *sspi = spi_master_get_devdata(spi->master);
+	u32 val;
+
+	val = readl_relaxed(sspi->regs + DMSTART);
+	val &= ~(CS_MASK << CS_SHIFT);
+	val |= spi->chip_select << CS_SHIFT;
+
+	if (!enable) {
+		writel_relaxed(val, sspi->regs + DMSTART);
+
+		val = readl_relaxed(sspi->regs + DMSTART);
+		val &= ~DMSTOP;
+		writel_relaxed(val, sspi->regs + DMSTART);
+	} else {
+		val |= DMSTOP;
+		writel_relaxed(val, sspi->regs + DMSTART);
+
+		if (sspi->rx_buf) {
+			u32 buf[16];
+
+			sspi->rx_buf = buf;
+			sspi->rx_words = 16;
+			read_fifo(sspi);
+		}
+	}
+}
+
+static int synquacer_spi_enable(struct spi_master *master)
+{
+	struct synquacer_spi *sspi = spi_master_get_devdata(master);
+	u32 val;
+
+	/* Disable module */
+	writel_relaxed(0, sspi->regs + MCTRL);
+	val = 0xfffff;
+	while (--val && (readl_relaxed(sspi->regs + MCTRL) & MES))
+		cpu_relax();
+	if (!val)
+		return -EBUSY;
+
+	writel_relaxed(0, sspi->regs + TXE);
+	writel_relaxed(0, sspi->regs + RXE);
+	val = readl_relaxed(sspi->regs + TXF);
+	writel_relaxed(val, sspi->regs + TXC);
+	val = readl_relaxed(sspi->regs + RXF);
+	writel_relaxed(val, sspi->regs + RXC);
+	val = readl_relaxed(sspi->regs + FAULTF);
+	writel_relaxed(val, sspi->regs + FAULTC);
+
+	val = readl_relaxed(sspi->regs + DMCFG);
+	val &= ~SSDC;
+	val &= ~MSTARTEN;
+	writel_relaxed(val, sspi->regs + DMCFG);
+
+	val = readl_relaxed(sspi->regs + MCTRL);
+	if (sspi->clk[IPCLK])
+		val |= BPCLK;
+	else
+		val &= ~BPCLK;
+
+	val &= ~CSEN;
+	val |= MEN;
+	val |= SYNCON;
+	writel_relaxed(val, sspi->regs + MCTRL);
+
+	return 0;
+}
+
+static int synquacer_spi_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct spi_master *master;
+	struct synquacer_spi *sspi;
+	struct resource *res;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(*sspi));
+	if (!master)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, master);
+
+	sspi = spi_master_get_devdata(master);
+	sspi->dev = &pdev->dev;
+	sspi->master = master;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	sspi->regs = devm_ioremap_resource(sspi->dev, res);
+	if (IS_ERR(sspi->regs)) {
+		ret = PTR_ERR(sspi->regs);
+		goto put_spi;
+	}
+
+	sspi->clk[IHCLK] = devm_clk_get(sspi->dev, "iHCLK");
+	if (IS_ERR(sspi->clk[IHCLK])) {
+		dev_err(&pdev->dev, "iHCLK not found\n");
+		ret = PTR_ERR(sspi->clk[IHCLK]);
+		goto put_spi;
+	}
+
+	sspi->clk[IPCLK] = devm_clk_get(sspi->dev, "iPCLK");
+	if (IS_ERR(sspi->clk[IPCLK]))
+		sspi->clk[IPCLK] = NULL;
+
+	sspi->aces = of_property_read_bool(np, "socionext,set-aces");
+	sspi->rtm = of_property_read_bool(np, "socionext,use-rtm");
+
+	master->num_chipselect = 4; /* max 4 supported */
+
+	clk_prepare_enable(sspi->clk[IPCLK]);
+	ret = clk_prepare_enable(sspi->clk[IHCLK]);
+	if (ret)
+		goto put_spi;
+
+	master->dev.of_node = np;
+	master->auto_runtime_pm = true;
+	master->bus_num = pdev->id;
+
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_TX_DUAL | SPI_RX_DUAL |
+				SPI_TX_QUAD | SPI_RX_QUAD;
+	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(24)
+					 | SPI_BPW_MASK(16) | SPI_BPW_MASK(8);
+
+	if (sspi->clk[IPCLK])
+		master->max_speed_hz = clk_get_rate(sspi->clk[IPCLK]);
+	else
+		master->max_speed_hz = clk_get_rate(sspi->clk[IHCLK]);
+	master->min_speed_hz = master->max_speed_hz / 254;
+
+	master->set_cs = synquacer_spi_set_cs;
+	master->transfer_one = synquacer_spi_transfer_one;
+
+	ret = synquacer_spi_enable(master);
+	if (ret)
+		goto fail_enable;
+
+	pm_runtime_set_active(sspi->dev);
+	pm_runtime_enable(sspi->dev);
+
+	ret = devm_spi_register_master(sspi->dev, master);
+	if (ret)
+		goto disable_pm;
+
+	return 0;
+
+disable_pm:
+	pm_runtime_disable(sspi->dev);
+fail_enable:
+	clk_disable_unprepare(sspi->clk[IHCLK]);
+	clk_disable_unprepare(sspi->clk[IPCLK]);
+put_spi:
+	spi_master_put(master);
+
+	return ret;
+}
+
+static int synquacer_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct synquacer_spi *sspi = spi_master_get_devdata(master);
+
+	pm_runtime_disable(sspi->dev);
+	clk_disable_unprepare(sspi->clk[IHCLK]);
+	clk_disable_unprepare(sspi->clk[IPCLK]);
+	spi_master_put(master);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int synquacer_spi_suspend(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct synquacer_spi *sspi = spi_master_get_devdata(master);
+	int ret;
+
+	ret = spi_master_suspend(master);
+	if (ret)
+		return ret;
+
+	if (!pm_runtime_suspended(dev)) {
+		clk_disable_unprepare(sspi->clk[IPCLK]);
+		clk_disable_unprepare(sspi->clk[IHCLK]);
+	}
+
+	return ret;
+}
+
+static int synquacer_spi_resume(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct synquacer_spi *sspi = spi_master_get_devdata(master);
+	int ret;
+
+	if (!pm_runtime_suspended(dev)) {
+		/* Ensure reconfigure during next xfer */
+		sspi->speed = 0;
+
+		clk_prepare_enable(sspi->clk[IPCLK]);
+		ret = clk_prepare_enable(sspi->clk[IHCLK]);
+		if (ret < 0) {
+			dev_err(dev, "failed to enable clk (%d)\n", ret);
+			return ret;
+		}
+
+		ret = synquacer_spi_enable(master);
+		if (ret) {
+			dev_err(dev, "failed to enable spi (%d)\n", ret);
+			return ret;
+		}
+	}
+
+	ret = spi_master_resume(master);
+	if (ret < 0) {
+		clk_disable_unprepare(sspi->clk[IHCLK]);
+		clk_disable_unprepare(sspi->clk[IPCLK]);
+	}
+
+	return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops synquacer_spi_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(synquacer_spi_suspend, synquacer_spi_resume)
+};
+
+static const struct of_device_id synquacer_spi_of_match[] = {
+	{.compatible = "socionext,synquacer-spi",},
+	{},
+};
+MODULE_DEVICE_TABLE(of, synquacer_spi_of_match);
+
+static struct platform_driver synquacer_spi_driver = {
+	.driver = {
+		.name = "synquacer-spi",
+		.pm = &synquacer_spi_pm_ops,
+		.of_match_table = of_match_ptr(synquacer_spi_of_match),
+	},
+	.probe = synquacer_spi_probe,
+	.remove = synquacer_spi_remove,
+};
+module_platform_driver(synquacer_spi_driver);
+
+MODULE_DESCRIPTION("Socionext Synquacer HS-SPI controller driver");
+MODULE_AUTHOR("Jassi Brar <jaswinder.singh@linaro.org>");
+MODULE_LICENSE("GPL v2");