From c6b4f031d96a4e1d59761b294829b058b098f3df Mon Sep 17 00:00:00 2001 From: Eugeniy Paltsev Date: Thu, 22 Mar 2018 13:50:43 +0300 Subject: [PATCH 1/6] DW SPI: fix tx data loss on FIFO flush In current implementation if some data still exists in Tx FIFO it can be silently flushed, i.e. dropped on disabling of the controller, which happens when writing 0 to DW_SPI_SSIENR (it happens in the beginning of new transfer) So add wait for current transmit operation to complete to be sure that current transmit operation is finished before new one. Signed-off-by: Eugeniy Paltsev Reviewed-by: Jagan Teki --- drivers/spi/designware_spi.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index c501aee..5e196b2 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -18,6 +18,7 @@ #include #include #include +#include #include DECLARE_GLOBAL_DATA_PTR; @@ -342,6 +343,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, u8 *rx = din; int ret = 0; u32 cr0 = 0; + u32 val; u32 cs; /* spi core configured to do 8 bit transfers */ @@ -394,6 +396,19 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, /* Start transfer in a polling loop */ ret = poll_transfer(priv); + /* + * Wait for current transmit operation to complete. + * Otherwise if some data still exists in Tx FIFO it can be + * silently flushed, i.e. dropped on disabling of the controller, + * which happens when writing 0 to DW_SPI_SSIENR which happens + * in the beginning of new transfer. + */ + if (readl_poll_timeout(priv->regs + DW_SPI_SR, val, + !(val & SR_TF_EMPT) || (val & SR_BUSY), + RX_TIMEOUT * 1000)) { + ret = -ETIMEDOUT; + } + return ret; } From fc282c7bcb6a622ce1a0cf82c55654dec5bcb0cd Mon Sep 17 00:00:00 2001 From: Eugeniy Paltsev Date: Thu, 22 Mar 2018 13:50:44 +0300 Subject: [PATCH 2/6] DW SPI: fix transmit only mode In current implementation we get -ETIMEDOUT error when we try to use transmit only mode (SPI_TMOD_TO) This happens because in transmit only mode input FIFO never gets any data which breaks our logic in dw_reader(): we are waiting until RX data will be ready in dw_reader, but this newer happens, so we return with error. Fix that by using SPI_TMOD_TR instead of SPI_TMOD_TO which allows to use RX FIFO. Signed-off-by: Eugeniy Paltsev Reviewed-by: Jagan Teki --- drivers/spi/designware_spi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 5e196b2..3296441 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -361,7 +361,11 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, else if (rx) priv->tmode = SPI_TMOD_RO; else - priv->tmode = SPI_TMOD_TO; + /* + * In transmit only mode (SPI_TMOD_TO) input FIFO never gets + * any data which breaks our logic in poll_transfer() above. + */ + priv->tmode = SPI_TMOD_TR; cr0 &= ~SPI_TMOD_MASK; cr0 |= (priv->tmode << SPI_TMOD_OFFSET); From d3d8aaec7401703079f296a6460fe649fb06581b Mon Sep 17 00:00:00 2001 From: Eugeniy Paltsev Date: Thu, 22 Mar 2018 13:50:45 +0300 Subject: [PATCH 3/6] DW SPI: refactor poll_transfer functions There is no sense in waiting for RX data in dw_reader function: there is no chance that RX data will appear in RX FIFO if RX FIFO is empty after previous TX write in dw_writer function. So get rid of this waiting. After that we can get rid of dw_reader return value and make it returning void. After that we can get rid of dw_reader return value check in poll_transfer function. With these changes we're getting closer to Linux DW SPI driver. Signed-off-by: Eugeniy Paltsev Reviewed-by: Jagan Teki --- drivers/spi/designware_spi.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 3296441..b51242c 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -286,28 +286,16 @@ static void dw_writer(struct dw_spi_priv *priv) } } -static int dw_reader(struct dw_spi_priv *priv) +static void dw_reader(struct dw_spi_priv *priv) { - unsigned start = get_timer(0); - u32 max; + u32 max = rx_max(priv); u16 rxw; - /* Wait for rx data to be ready */ - while (rx_max(priv) == 0) { - if (get_timer(start) > RX_TIMEOUT) - return -ETIMEDOUT; - } - - max = rx_max(priv); - while (max--) { rxw = dw_readw(priv, DW_SPI_DR); debug("%s: rx=0x%02x\n", __func__, rxw); - /* - * Care about rx only if the transfer's original "rx" is - * not null - */ + /* Care about rx if the transfer's original "rx" is not null */ if (priv->rx_end - priv->len) { if (priv->bits_per_word == 8) *(u8 *)(priv->rx) = rxw; @@ -316,19 +304,13 @@ static int dw_reader(struct dw_spi_priv *priv) } priv->rx += priv->bits_per_word >> 3; } - - return 0; } static int poll_transfer(struct dw_spi_priv *priv) { - int ret; - do { dw_writer(priv); - ret = dw_reader(priv); - if (ret < 0) - return ret; + dw_reader(priv); } while (priv->rx_end > priv->rx); return 0; From bcdcb3e61ebd0550355883aed3978028d0e7778b Mon Sep 17 00:00:00 2001 From: Eugeniy Paltsev Date: Thu, 22 Mar 2018 13:50:46 +0300 Subject: [PATCH 4/6] DW SPI: add option to use external gpio for chip select DW SPI internal chip select management has limitation: it hold CS line in active state only when the FIFO is not empty. If the FIFO freed before we add new data the SPI transaction will be broken. So add option to use external gpio for chip select. Gpio can be added via device tree using standard gpio bindings. Signed-off-by: Eugeniy Paltsev Reviewed-by: Jagan Teki --- drivers/spi/designware_spi.c | 51 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index b51242c..06f7774 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -10,6 +10,7 @@ * SPDX-License-Identifier: GPL-2.0 */ +#include #include #include #include @@ -98,6 +99,8 @@ struct dw_spi_priv { struct clk clk; unsigned long bus_clk_rate; + struct gpio_desc cs_gpio; /* External chip-select gpio */ + int bits_per_word; u8 cs; /* chip select pin */ u8 tmode; /* TR/TO/RO/EEPROM */ @@ -131,6 +134,32 @@ static inline void dw_writew(struct dw_spi_priv *priv, u32 offset, u16 val) __raw_writew(val, priv->regs + offset); } +static int request_gpio_cs(struct udevice *bus) +{ +#if defined(CONFIG_DM_GPIO) && !defined(CONFIG_SPL_BUILD) + struct dw_spi_priv *priv = dev_get_priv(bus); + int ret; + + /* External chip select gpio line is optional */ + ret = gpio_request_by_name(bus, "cs-gpio", 0, &priv->cs_gpio, 0); + if (ret == -ENOENT) + return 0; + + if (ret < 0) { + printf("Error: %d: Can't get %s gpio!\n", ret, bus->name); + return ret; + } + + if (dm_gpio_is_valid(&priv->cs_gpio)) { + dm_gpio_set_dir_flags(&priv->cs_gpio, + GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); + } + + debug("%s: used external gpio for CS management\n", __func__); +#endif + return 0; +} + static int dw_spi_ofdata_to_platdata(struct udevice *bus) { struct dw_spi_platdata *plat = bus->platdata; @@ -145,7 +174,7 @@ static int dw_spi_ofdata_to_platdata(struct udevice *bus) debug("%s: regs=%p max-frequency=%d\n", __func__, plat->regs, plat->frequency); - return 0; + return request_gpio_cs(bus); } static inline void spi_enable_chip(struct dw_spi_priv *priv, int enable) @@ -316,6 +345,18 @@ static int poll_transfer(struct dw_spi_priv *priv) return 0; } +static void external_cs_manage(struct udevice *dev, bool on) +{ +#if defined(CONFIG_DM_GPIO) && !defined(CONFIG_SPL_BUILD) + struct dw_spi_priv *priv = dev_get_priv(dev->parent); + + if (!dm_gpio_is_valid(&priv->cs_gpio)) + return; + + dm_gpio_set_value(&priv->cs_gpio, on ? 1 : 0); +#endif +} + static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, const void *dout, void *din, unsigned long flags) { @@ -334,6 +375,10 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, return -1; } + /* Start the transaction if necessary. */ + if (flags & SPI_XFER_BEGIN) + external_cs_manage(dev, false); + cr0 = (priv->bits_per_word - 1) | (priv->type << SPI_FRF_OFFSET) | (priv->mode << SPI_MODE_OFFSET) | (priv->tmode << SPI_TMOD_OFFSET); @@ -395,6 +440,10 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, ret = -ETIMEDOUT; } + /* Stop the transaction if necessary */ + if (flags & SPI_XFER_END) + external_cs_manage(dev, true); + return ret; } From 4b5f6c52e78d43710a0d062e31de741ec76ceea1 Mon Sep 17 00:00:00 2001 From: Eugeniy Paltsev Date: Thu, 22 Mar 2018 13:50:47 +0300 Subject: [PATCH 5/6] DW SPI: use 32 bit access instead of 16 and 32 bit mix Current DW SPI driver uses 32 bit access for some registers and 16 bit access for others. So if DW SPI IP is connected via bus which doesn't support 16 bit access we will get bus error. Fix that by switching to 32 bit access only instead of 16 and 32 bit mix Additional Documentation to Support this Change: The DW_apb_ssi databook states: "All registers in the DW_apb_ssi are addressed at 32-bit boundaries to remain consistent with the AHB bus. Where the physical size of any register is less than 32-bits wide, the upper unused bits of the 32-bit boundary are reserved. Writing to these bits has no effect; reading from these bits returns 0." [1] [1] Section 6.1 of dw_apb_ssi.pdf (version 3.22a) Signed-off-by: Eugeniy Paltsev Reviewed-by: Jagan Teki --- drivers/spi/designware_spi.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 06f7774..0e93b62 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -114,26 +114,16 @@ struct dw_spi_priv { void *rx_end; }; -static inline u32 dw_readl(struct dw_spi_priv *priv, u32 offset) +static inline u32 dw_read(struct dw_spi_priv *priv, u32 offset) { return __raw_readl(priv->regs + offset); } -static inline void dw_writel(struct dw_spi_priv *priv, u32 offset, u32 val) +static inline void dw_write(struct dw_spi_priv *priv, u32 offset, u32 val) { __raw_writel(val, priv->regs + offset); } -static inline u16 dw_readw(struct dw_spi_priv *priv, u32 offset) -{ - return __raw_readw(priv->regs + offset); -} - -static inline void dw_writew(struct dw_spi_priv *priv, u32 offset, u16 val) -{ - __raw_writew(val, priv->regs + offset); -} - static int request_gpio_cs(struct udevice *bus) { #if defined(CONFIG_DM_GPIO) && !defined(CONFIG_SPL_BUILD) @@ -179,14 +169,14 @@ static int dw_spi_ofdata_to_platdata(struct udevice *bus) static inline void spi_enable_chip(struct dw_spi_priv *priv, int enable) { - dw_writel(priv, DW_SPI_SSIENR, (enable ? 1 : 0)); + dw_write(priv, DW_SPI_SSIENR, (enable ? 1 : 0)); } /* Restart the controller, disable all interrupts, clean rx fifo */ static void spi_hw_init(struct dw_spi_priv *priv) { spi_enable_chip(priv, 0); - dw_writel(priv, DW_SPI_IMR, 0xff); + dw_write(priv, DW_SPI_IMR, 0xff); spi_enable_chip(priv, 1); /* @@ -197,13 +187,13 @@ static void spi_hw_init(struct dw_spi_priv *priv) u32 fifo; for (fifo = 1; fifo < 256; fifo++) { - dw_writew(priv, DW_SPI_TXFLTR, fifo); - if (fifo != dw_readw(priv, DW_SPI_TXFLTR)) + dw_write(priv, DW_SPI_TXFLTR, fifo); + if (fifo != dw_read(priv, DW_SPI_TXFLTR)) break; } priv->fifo_len = (fifo == 1) ? 0 : fifo; - dw_writew(priv, DW_SPI_TXFLTR, 0); + dw_write(priv, DW_SPI_TXFLTR, 0); } debug("%s: fifo_len=%d\n", __func__, priv->fifo_len); } @@ -272,7 +262,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv) u32 tx_left, tx_room, rxtx_gap; tx_left = (priv->tx_end - priv->tx) / (priv->bits_per_word >> 3); - tx_room = priv->fifo_len - dw_readw(priv, DW_SPI_TXFLR); + tx_room = priv->fifo_len - dw_read(priv, DW_SPI_TXFLR); /* * Another concern is about the tx/rx mismatch, we @@ -293,7 +283,7 @@ static inline u32 rx_max(struct dw_spi_priv *priv) { u32 rx_left = (priv->rx_end - priv->rx) / (priv->bits_per_word >> 3); - return min_t(u32, rx_left, dw_readw(priv, DW_SPI_RXFLR)); + return min_t(u32, rx_left, dw_read(priv, DW_SPI_RXFLR)); } static void dw_writer(struct dw_spi_priv *priv) @@ -309,7 +299,7 @@ static void dw_writer(struct dw_spi_priv *priv) else txw = *(u16 *)(priv->tx); } - dw_writew(priv, DW_SPI_DR, txw); + dw_write(priv, DW_SPI_DR, txw); debug("%s: tx=0x%02x\n", __func__, txw); priv->tx += priv->bits_per_word >> 3; } @@ -321,7 +311,7 @@ static void dw_reader(struct dw_spi_priv *priv) u16 rxw; while (max--) { - rxw = dw_readw(priv, DW_SPI_DR); + rxw = dw_read(priv, DW_SPI_DR); debug("%s: rx=0x%02x\n", __func__, rxw); /* Care about rx if the transfer's original "rx" is not null */ @@ -410,8 +400,8 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, debug("%s: cr0=%08x\n", __func__, cr0); /* Reprogram cr0 only if changed */ - if (dw_readw(priv, DW_SPI_CTRL0) != cr0) - dw_writew(priv, DW_SPI_CTRL0, cr0); + if (dw_read(priv, DW_SPI_CTRL0) != cr0) + dw_write(priv, DW_SPI_CTRL0, cr0); /* * Configure the desired SS (slave select 0...3) in the controller @@ -419,7 +409,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen, * automatically. So no cs_activate() etc is needed in this driver. */ cs = spi_chip_select(dev); - dw_writel(priv, DW_SPI_SER, 1 << cs); + dw_write(priv, DW_SPI_SER, 1 << cs); /* Enable controller after writing control registers */ spi_enable_chip(priv, 1); @@ -462,7 +452,7 @@ static int dw_spi_set_speed(struct udevice *bus, uint speed) /* clk_div doesn't support odd number */ clk_div = priv->bus_clk_rate / speed; clk_div = (clk_div + 1) & 0xfffe; - dw_writel(priv, DW_SPI_BAUDR, clk_div); + dw_write(priv, DW_SPI_BAUDR, clk_div); /* Enable controller after writing control registers */ spi_enable_chip(priv, 1); From 63018a3eddd5d9c64f7ee42615a63c6a67ab484d Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Thu, 22 Mar 2018 22:55:02 +0100 Subject: [PATCH 6/6] omap3: spi: Correct ti, pindir-d0-out-d1-in parsing The ti,pindir-d0-out-d1-in property is not expected to have a value according to the device-tree binding, so treat it as a boolean not a uint property. Signed-off-by: Sjoerd Simons Reviewed-by: Jagan Teki --- drivers/spi/omap3_spi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c index 053a67b..1ac691a 100644 --- a/drivers/spi/omap3_spi.c +++ b/drivers/spi/omap3_spi.c @@ -630,8 +630,10 @@ static int omap3_spi_probe(struct udevice *dev) (struct omap2_mcspi_platform_config*)dev_get_driver_data(dev); priv->regs = (struct mcspi *)(devfdt_get_addr(dev) + data->regs_offset); - priv->pin_dir = fdtdec_get_uint(blob, node, "ti,pindir-d0-out-d1-in", - MCSPI_PINDIR_D0_IN_D1_OUT); + if (fdtdec_get_bool(blob, node, "ti,pindir-d0-out-d1-in")) + priv->pin_dir = MCSPI_PINDIR_D0_OUT_D1_IN; + else + priv->pin_dir = MCSPI_PINDIR_D0_IN_D1_OUT; priv->wordlen = SPI_DEFAULT_WORDLEN; return 0; }