| Message ID | 1519736330-3985-1-git-send-email-jassisinghbrar@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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.
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, > +};
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.
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 --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");