spi: Add support for DMA transfers in sun4i SPI driver
Subsystems (2)
cover spi: Add support for DMA transfers in sun4i SPI driver
The following patchset provides corrections for PIO-mode and support for DMA transfers in sun4i SPI driver.
Sergey Suloev (6):
spi: core: handle timeout error from transfer_one()
spi: sun4i: restrict transfer length in PIO-mode
spi: sun4i: coding style/readability improvements
spi: sun4i: use completion provided by SPI core driver
spi: sun4i: introduce register set/unset helpers
spi: sun4i: add DMA transfers support
drivers/spi/spi-sun4i.c | 443 ++++++++++++++++++++++++++++++++++++------------ drivers/spi/spi.c | 5 +- 2 files changed, 341 insertions(+), 107 deletions(-)
Signature · 1 line
2.16.2
PATCH 1/6 spi: core: handle timeout error from transfer_one()
As long as sun4i/sun6i SPI drivers have overriden the default "wait for completion" procedure then we need to properly handle -ETIMEDOUT error from transfer_one().
--- drivers/spi/spi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/spi/spi.c +3 -2
@@ -1028,7 +1028,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,reinit_completion (&ctlr->xfer_completion); ret = ctlr->transfer_one (ctlr, msg->spi, xfer); -if (ret <0 ) { +if (ret <0 && ret != -ETIMEDOUT ) {SPI_STATISTICS_INCREMENT_FIELD (statm, errors); SPI_STATISTICS_INCREMENT_FIELD(stats,
@@ -1051,7 +1051,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr, msecs_to_jiffies(ms)); } -if (ms ==0 ) { +if (ms ==0 || ret == -ETIMEDOUT ) {SPI_STATISTICS_INCREMENT_FIELD (statm, timedout); SPI_STATISTICS_INCREMENT_FIELD(stats,
@@ -1059,6 +1059,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,dev_err (&msg->spi->dev,"SPI transfer timed out\n" ); msg->status = -ETIMEDOUT ; + ret =0 ; } }else {if (xfer->len)
Signature · 1 line
2.16.2
PATCH 2/6 spi: sun4i: restrict transfer length in PIO-mode
In reply to Sergey SuloevThere is no need to handle 3/4 empty/full interrupts as the maximum supported transfer length in PIO mode is 64 bytes for sun4i-family SoCs. As long as a problem was reported previously with filling FIFO on A10s then we stick with 63 bytes depth.
--- drivers/spi/spi-sun4i.c | 50 ++++++++++++------------------------------------- 1 file changed, 12 insertions(+), 38 deletions(-)
drivers/spi/spi-sun4i.c +12 -38
@@ -22,7 +22,12 @@#include <linux/spi/spi.h> -#define SUN4I_FIFO_DEPTH 64 +/* +* FIFO length is 64 bytes +* But filling the FIFO fully might cause a timeout +* on some devices, for example on spi2 on A10s +*/ +#define SUN4I_FIFO_DEPTH 63#define SUN4I_RXDATA_REG 0x00
@@ -202,7 +207,7 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)static size_t sun4i_spi_max_transfer_size (struct spi_device *spi) { -return SUN4I_FIFO_DEPTH -1 ; +return SUN4I_FIFO_DEPTH ; } static int sun4i_spi_transfer_one(struct spi_master *master,
@@ -216,11 +221,8 @@ static int sun4i_spi_transfer_one(struct spi_master *master,int ret =0 ;u32 reg; -/* We don't support transfer larger than the FIFO */ -if (tfr->len >SUN4I_MAX_XFER_SIZE ) -return -EMSGSIZE ; - -if (tfr->tx_buf && tfr->len >=SUN4I_MAX_XFER_SIZE ) +/* We don't support transfers larger than FIFO depth */ +if (tfr->len >SUN4I_FIFO_DEPTH )return -EMSGSIZE ;reinit_completion (&sspi->done);
@@ -313,17 +315,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,/* * Fill the TX FIFO -* Filling the FIFO fully causes timeout for some reason -* at least on spi2 on A10s */ -sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH -1 ); +sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH ); -/* Enable the interrupts */ -sun4i_spi_enable_interrupt (sspi,SUN4I_INT_CTL_TC | -SUN4I_INT_CTL_RF_F34 ); -/* Only enable Tx FIFO interrupt if we really need it */ -if (tx_len >SUN4I_FIFO_DEPTH ) -sun4i_spi_enable_interrupt (sspi,SUN4I_INT_CTL_TF_E34 ); +/* Enable the transfer complete interrupt */ +sun4i_spi_enable_interrupt (sspi,SUN4I_INT_CTL_TC );/* Start the transfer */ reg =sun4i_spi_read (sspi,SUN4I_CTL_REG );
@@ -363,28 +359,6 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)return IRQ_HANDLED ; } -/* Receive FIFO 3/4 full */ -if (status &SUN4I_INT_CTL_RF_F34 ) { -sun4i_spi_drain_fifo (sspi,SUN4I_FIFO_DEPTH ); -/* Only clear the interrupt _after_ draining the FIFO */ -sun4i_spi_write (sspi,SUN4I_INT_STA_REG ,SUN4I_INT_CTL_RF_F34 ); -return IRQ_HANDLED ; - } - -/* Transmit FIFO 3/4 empty */ -if (status &SUN4I_INT_CTL_TF_E34 ) { -sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH ); - -if (!sspi->len) -/* nothing left to transmit */ -sun4i_spi_disable_interrupt (sspi,SUN4I_INT_CTL_TF_E34 ); - -/* Only clear the interrupt _after_ re-seeding the FIFO */ -sun4i_spi_write (sspi,SUN4I_INT_STA_REG ,SUN4I_INT_CTL_TF_E34 ); - -return IRQ_HANDLED ; - } -return IRQ_NONE ; }
Signature · 1 line
2.16.2
Re: [PATCH 2/6] spi: sun4i: restrict transfer length in PIO-mode
Quoted 3 lines · Sergey Suloev
There is no need to handle 3/4 empty/full interrupts as the maximum supported transfer length in PIO mode is 64 bytes for sun4i-family SoCs.
That assumes that you'll be able to treat the FIFO full interrupt and drain the FIFO before we have the next byte coming in. This would require a real time system, and we're not in one of them.
Maxime
Signature · 10 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180403/31b106f5/attachment.sig>
Re: [PATCH 2/6] spi: sun4i: restrict transfer length in PIO-mode
Quoted 8 lines · Maxime Ripard
Quoted 3 lines · Sergey Suloev
There is no need to handle 3/4 empty/full interrupts as the maximum supported transfer length in PIO mode is 64 bytes for sun4i-family SoCs.
That assumes that you'll be able to treat the FIFO full interrupt and drain the FIFO before we have the next byte coming in. This would require a real time system, and we're not in one of them.
Quoted 8 lines
Maxime
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
so you think we should still handle 3/4 FIFO full ?
Re: [PATCH 2/6] spi: sun4i: restrict transfer length in PIO-mode
In reply to Maxime RipardQuoted 10 lines · Maxime Ripard
Quoted 3 lines · Sergey Suloev
There is no need to handle 3/4 empty/full interrupts as the maximum supported transfer length in PIO mode is 64 bytes for sun4i-family SoCs.
That assumes that you'll be able to treat the FIFO full interrupt and drain the FIFO before we have the next byte coming in. This would require a real time system, and we're not in one of them.
Maxime
AFAIK in SPI protocol we send and receive at the same time. As soon as the transfer length
is <= FIFO depth then it means that@the moment we get TC interrupt all data for this transfer
sent/received already.
Is your point here that draining FIFO might be a long operation and we can lose next portion of data ?
Re: [PATCH 2/6] spi: sun4i: restrict transfer length in PIO-mode
Quoted 10 lines · Sergey Suloev
Quoted 7 lines · Maxime Ripard
Quoted 3 lines · Sergey Suloev
There is no need to handle 3/4 empty/full interrupts as the maximum supported transfer length in PIO mode is 64 bytes for sun4i-family SoCs.
That assumes that you'll be able to treat the FIFO full interrupt and drain the FIFO before we have the next byte coming in. This would require a real time system, and we're not in one of them.
AFAIK in SPI protocol we send and receive at the same time.
It depends. The protocol allows it yes, but most devices I've seen can only operate in half duplex. But it's not really the point.
Quoted 6 lines
As soon as the transfer length is <= FIFO depth then it means that at the moment we get TC interrupt all data for this transfer sent/received already.
Is your point here that draining FIFO might be a long operation and we can lose next portion of data ?
My point is that, if you get another interrupt(s) right before the FIFO full interrupt, that interrupt is going to be masked for as long as it is needed for the previous handler(s) to execute.
If you're having another byte received while the interrupt is masked, you're losing data.
Maxime
Signature · 10 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180403/389746ed/attachment.sig>
Re: [PATCH 2/6] spi: sun4i: restrict transfer length in PIO-mode
Quoted 26 lines · Maxime Ripard
Quoted 9 lines · Sergey Suloev
Quoted 7 lines · Maxime Ripard
Quoted 3 lines · Sergey Suloev
There is no need to handle 3/4 empty/full interrupts as the maximum supported transfer length in PIO mode is 64 bytes for sun4i-family SoCs.
That assumes that you'll be able to treat the FIFO full interrupt and drain the FIFO before we have the next byte coming in. This would require a real time system, and we're not in one of them.
AFAIK in SPI protocol we send and receive at the same time.
It depends. The protocol allows it yes, but most devices I've seen can only operate in half duplex. But it's not really the point.
Quoted 6 lines
As soon as the transfer length is <= FIFO depth then it means that at the moment we get TC interrupt all data for this transfer sent/received already.
Is your point here that draining FIFO might be a long operation and we can lose next portion of data ?
My point is that, if you get another interrupt(s) right before the FIFO full interrupt, that interrupt is going to be masked for as long as it is needed for the previous handler(s) to execute.
If you're having another byte received while the interrupt is masked, you're losing data.
Quoted 8 lines
Maxime
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
ok, I am going to put back 3/4 full handler then.
PATCH 3/6 spi: sun4i: coding style/readability improvements
In reply to Sergey SuloevMinor changes to fulfill the coding style and
improve the readability.
--- drivers/spi/spi-sun4i.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
drivers/spi/spi-sun4i.c +17 -15
@@ -83,8 +83,11 @@#define SUN4I_FIFO_STA_TF_CNT_MASK 0x7f#define SUN4I_FIFO_STA_TF_CNT_BITS 16 +#define SUN4I_SPI_MAX_SPEED_HZ 100 * 1000 * 1000 +#define SUN4I_SPI_MIN_SPEED_HZ 3 * 1000 +#define SUN4I_SPI_MODE_BITS (SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST) +struct sun4i_spi { -struct spi_master *master;void __iomem *base_addr;struct clk *hclk;struct clk *mclk;
@@ -409,12 +412,23 @@ static int sun4i_spi_probe(struct platform_device *pdev)struct resource *res;int ret =0 , irq; - master =spi_alloc_master (&pdev->dev,sizeof (struct sun4i_spi )); + master =spi_alloc_master (&pdev->dev,sizeof (*sspi));if (!master) {dev_err (&pdev->dev,"Unable to allocate SPI Master\n" );return -ENOMEM ; } + master->max_speed_hz =SUN4I_SPI_MAX_SPEED_HZ ; + master->min_speed_hz =SUN4I_SPI_MIN_SPEED_HZ ; + master->num_chipselect =4 ; + master->mode_bits =SUN4I_SPI_MODE_BITS ; + master->bits_per_word_mask =SPI_BPW_MASK (8 ); + master->set_cs = sun4i_spi_set_cs; + master->transfer_one = sun4i_spi_transfer_one; + master->max_transfer_size = sun4i_spi_max_transfer_size; + master->dev.of_node = pdev->dev.of_node; + master->auto_runtime_pm = true; +platform_set_drvdata (pdev, master); sspi =spi_master_get_devdata (master);
@@ -433,24 +447,12 @@ static int sun4i_spi_probe(struct platform_device *pdev) } ret =devm_request_irq (&pdev->dev, irq, sun4i_spi_handler, -0 ,"sun4i-spi" , sspi); +0 ,dev_name (&pdev->dev), sspi);if (ret) {dev_err (&pdev->dev,"Cannot request IRQ\n" ); goto err_free_master; } - sspi->master = master; - master->max_speed_hz =100 *1000 *1000 ; - master->min_speed_hz =3 *1000 ; - master->set_cs = sun4i_spi_set_cs; - master->transfer_one = sun4i_spi_transfer_one; - master->num_chipselect =4 ; - master->mode_bits =SPI_CPOL |SPI_CPHA |SPI_CS_HIGH |SPI_LSB_FIRST ; - master->bits_per_word_mask =SPI_BPW_MASK (8 ); - master->dev.of_node = pdev->dev.of_node; - master->auto_runtime_pm = true; - master->max_transfer_size = sun4i_spi_max_transfer_size; - sspi->hclk =devm_clk_get (&pdev->dev,"ahb" );if (IS_ERR (sspi->hclk)) {dev_err (&pdev->dev,"Unable to acquire AHB clock\n" );
Signature · 1 line
2.16.2
PATCH 4/6 spi: sun4i: use completion provided by SPI core driver
In reply to Sergey SuloevAs long as the completion already provided by the SPI core then there is no need to waste extra-memory on this. Also a waiting function was added to avoid code duplication.
--- drivers/spi/spi-sun4i.c | 62 ++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 27 deletions(-)
drivers/spi/spi-sun4i.c +35 -27
@@ -92,8 +92,6 @@ struct sun4i_spi {struct clk *hclk;struct clk *mclk; -struct completion done; -const u8 *tx_buf;u8 *rx_buf;int len;
@@ -213,13 +211,36 @@ static size_t sun4i_spi_max_transfer_size(struct spi_device *spi)return SUN4I_FIFO_DEPTH ; } +static int sun4i_spi_wait_for_transfer (struct spi_device *spi, +struct spi_transfer *tfr) +{ +struct spi_master *master = spi->master; +unsigned int start, end, tx_time; +unsigned int timeout; + +/* calc required timeout from given speed & len values */ + tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); + start = jiffies; + timeout =wait_for_completion_timeout (&master->xfer_completion, +msecs_to_jiffies (tx_time)); + end = jiffies; +if (!timeout) { +dev_warn (&master->dev, +"%s: timeout transferring %u bytes@%iHz for %i(%i)ms" , +dev_name (&spi->dev), tfr->len, tfr->speed_hz, +jiffies_to_msecs (end - start), tx_time); +return -ETIMEDOUT ; + } + +return 0 ; +} +static int sun4i_spi_transfer_one (struct spi_master *master,struct spi_device *spi,struct spi_transfer *tfr) {struct sun4i_spi *sspi =spi_master_get_devdata (master); -unsigned int mclk_rate, div, timeout; -unsigned int start, end, tx_time; +unsigned int mclk_rate, div;unsigned int tx_len =0 ;int ret =0 ;u32 reg;
@@ -228,7 +249,6 @@ static int sun4i_spi_transfer_one(struct spi_master *master,if (tfr->len >SUN4I_FIFO_DEPTH )return -EMSGSIZE ; -reinit_completion (&sspi->done); sspi->tx_buf = tfr->tx_buf; sspi->rx_buf = tfr->rx_buf; sspi->len = tfr->len;
@@ -328,22 +348,8 @@ static int sun4i_spi_transfer_one(struct spi_master *master, reg =sun4i_spi_read (sspi,SUN4I_CTL_REG );sun4i_spi_write (sspi,SUN4I_CTL_REG , reg |SUN4I_CTL_XCH ); - tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); - start = jiffies; - timeout =wait_for_completion_timeout (&sspi->done, -msecs_to_jiffies (tx_time)); - end = jiffies; -if (!timeout) { -dev_warn (&master->dev, -"%s: timeout transferring %u bytes@%iHz for %i(%i)ms" , -dev_name (&spi->dev), tfr->len, tfr->speed_hz, -jiffies_to_msecs (end - start), tx_time); - ret = -ETIMEDOUT ; - goto out; - } - + ret =sun4i_spi_wait_for_transfer (spi, tfr); -out:sun4i_spi_write (sspi,SUN4I_INT_CTL_REG ,0 );return ret;
@@ -351,14 +357,18 @@ out:static irqreturn_t sun4i_spi_handler (int irq,void *dev_id) { -struct sun4i_spi *sspi = dev_id; -u32 status =sun4i_spi_read (sspi,SUN4I_INT_STA_REG ); +struct spi_master *master = dev_id; +struct sun4i_spi *sspi =spi_master_get_devdata (master); +u32 status; + + status =sun4i_spi_read (sspi,SUN4I_INT_STA_REG );/* Transfer complete */ if (status &SUN4I_INT_CTL_TC ) { -sun4i_spi_write (sspi,SUN4I_INT_STA_REG ,SUN4I_INT_CTL_TC ); +sun4i_spi_write (sspi,SUN4I_INT_STA_REG , +SUN4I_INT_CTL_TC );sun4i_spi_drain_fifo (sspi,SUN4I_FIFO_DEPTH ); -complete (&sspi->done); +spi_finalize_current_transfer (master);return IRQ_HANDLED ; }
@@ -447,7 +457,7 @@ static int sun4i_spi_probe(struct platform_device *pdev) } ret =devm_request_irq (&pdev->dev, irq, sun4i_spi_handler, -0 ,dev_name (&pdev->dev), sspi); +0 ,dev_name (&pdev->dev), master);if (ret) {dev_err (&pdev->dev,"Cannot request IRQ\n" ); goto err_free_master;
@@ -467,8 +477,6 @@ static int sun4i_spi_probe(struct platform_device *pdev) goto err_free_master; } -init_completion (&sspi->done); -/* * This wake-up/shutdown pattern is to be able to have the * device woken up, even if runtime_pm is disabled
Signature · 1 line
2.16.2
PATCH 5/6 spi: sun4i: introduce register set/unset helpers
In reply to Sergey SuloevTwo helper functions were added in order to update registers easily.
--- drivers/spi/spi-sun4i.c | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-)
drivers/spi/spi-sun4i.c +19 -21
@@ -107,29 +107,29 @@ static inline void sun4i_spi_write(struct sun4i_spi *sspi, u32 reg, u32 value)writel (value, sspi->base_addr + reg); } -static inline u32 sun4i_spi_get_tx_fifo_count (struct sun4i_spi *sspi) +static inline void sun4i_spi_set (struct sun4i_spi *sspi,u32 addr,u32 val) { -u32 reg =sun4i_spi_read (sspi,SUN4I_FIFO_STA_REG ); - - reg >>=SUN4I_FIFO_STA_TF_CNT_BITS ; +u32 reg =sun4i_spi_read (sspi, addr); -return reg &SUN4I_FIFO_STA_TF_CNT_MASK ; + reg |= val; +sun4i_spi_write (sspi, addr, reg); } -static inline void sun4i_spi_enable_interrupt (struct sun4i_spi *sspi,u32 mask) +static inline void sun4i_spi_unset (struct sun4i_spi *sspi,u32 addr,u32 val) { -u32 reg =sun4i_spi_read (sspi,SUN4I_INT_CTL_REG ); +u32 reg =sun4i_spi_read (sspi, addr); - reg |= mask; -sun4i_spi_write (sspi,SUN4I_INT_CTL_REG , reg); + reg &= ~val; +sun4i_spi_write (sspi, addr, reg); } -static inline void sun4i_spi_disable_interrupt (struct sun4i_spi *sspi,u32 mask) +static inline u32 sun4i_spi_get_tx_fifo_count (struct sun4i_spi *sspi) { -u32 reg =sun4i_spi_read (sspi,SUN4I_INT_CTL_REG ); +u32 reg =sun4i_spi_read (sspi,SUN4I_FIFO_STA_REG ); - reg &= ~mask; -sun4i_spi_write (sspi,SUN4I_INT_CTL_REG , reg); + reg >>=SUN4I_FIFO_STA_TF_CNT_BITS ; + +return reg &SUN4I_FIFO_STA_TF_CNT_MASK ; }static inline void sun4i_spi_drain_fifo (struct sun4i_spi *sspi,int len)
@@ -256,13 +256,12 @@ static int sun4i_spi_transfer_one(struct spi_master *master,/* Clear pending interrupts */ sun4i_spi_write (sspi,SUN4I_INT_STA_REG , ~0 ); +/* Reset FIFOs */ +sun4i_spi_set (sspi,SUN4I_CTL_REG , +SUN4I_CTL_RF_RST |SUN4I_CTL_TF_RST ); reg =sun4i_spi_read (sspi,SUN4I_CTL_REG ); -/* Reset FIFOs */ -sun4i_spi_write (sspi,SUN4I_CTL_REG , - reg |SUN4I_CTL_RF_RST |SUN4I_CTL_TF_RST ); -/* * Setup the transfer control register: Chip Select, * polarities, etc.
@@ -341,12 +340,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,*/ sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH ); -/* Enable the transfer complete interrupt */ -sun4i_spi_enable_interrupt (sspi,SUN4I_INT_CTL_TC ); +/* Enable transfer complete interrupt */ +sun4i_spi_set (sspi,SUN4I_INT_CTL_REG ,SUN4I_INT_CTL_TC );/* Start the transfer */ - reg =sun4i_spi_read (sspi,SUN4I_CTL_REG ); -sun4i_spi_write (sspi,SUN4I_CTL_REG , reg |SUN4I_CTL_XCH ); +sun4i_spi_set (sspi,SUN4I_CTL_REG ,SUN4I_CTL_XCH ); ret =sun4i_spi_wait_for_transfer (spi, tfr);
Signature · 1 line
2.16.2
Re: [PATCH 5/6] spi: sun4i: introduce register set/unset helpers
Quoted 4 lines · Sergey Suloev
Two helper functions were added in order to update registers easily.
Signed-off-by Sergey Suloev <[email protected]>
I'm not really sure what's easier about this one.
Maxime
Signature · 10 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180403/4e0f69f6/attachment-0001.sig>
Re: [PATCH 5/6] spi: sun4i: introduce register set/unset helpers
Quoted 7 lines · Maxime Ripard
Quoted 4 lines · Sergey Suloev
Two helper functions were added in order to update registers easily.
Signed-off-by Sergey Suloev <[email protected]>I'm not really sure what's easier about this one.
Quoted 8 lines
Maxime
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
well, just seems more readable vs doing "read, or, write" every time
PATCH 6/6 spi: sun4i: add DMA transfers support
In reply to Sergey SuloevDMA transfers are now available for sun4i-family SoCs. The DMA mode is used automatically as soon as requested transfer length is more than FIFO length.
--- drivers/spi/spi-sun4i.c | 291 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 271 insertions(+), 20 deletions(-)
drivers/spi/spi-sun4i.c +271 -20
@@ -14,6 +14,8 @@#include <linux/clk.h> #include <linux/delay.h> #include <linux/device.h> +#include <linux/dmaengine.h> +#include <linux/dma-mapping.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/module.h>
@@ -39,6 +41,7 @@#define SUN4I_CTL_CPHA BIT(2)#define SUN4I_CTL_CPOL BIT(3)#define SUN4I_CTL_CS_ACTIVE_LOW BIT(4) +#define SUN4I_CTL_DMA_DEDICATED BIT(5)#define SUN4I_CTL_LMTF BIT(6)#define SUN4I_CTL_TF_RST BIT(8)#define SUN4I_CTL_RF_RST BIT(9)
@@ -58,6 +61,8 @@#define SUN4I_INT_STA_REG 0x10#define SUN4I_DMA_CTL_REG 0x14 +#define SUN4I_CTL_DMA_RF_READY BIT(0) +#define SUN4I_CTL_DMA_TF_NOT_FULL BIT(10)#define SUN4I_WAIT_REG 0x18
@@ -169,6 +174,13 @@ static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len) } } +static bool sun4i_spi_can_dma (struct spi_master *master, +struct spi_device *spi, +struct spi_transfer *tfr) +{ +return tfr->len >SUN4I_FIFO_DEPTH ; +} +static void sun4i_spi_set_cs (struct spi_device *spi,bool enable) {struct sun4i_spi *sspi =spi_master_get_devdata (spi->master);
@@ -208,6 +220,11 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)static size_t sun4i_spi_max_transfer_size (struct spi_device *spi) { +struct spi_master *master = spi->master; + +if (master->can_dma) +return SUN4I_MAX_XFER_SIZE ; +return SUN4I_FIFO_DEPTH ; }
@@ -235,6 +252,160 @@ static int sun4i_spi_wait_for_transfer(struct spi_device *spi,return 0 ; } +static void sun4i_spi_dma_callback (void *param) +{ +struct spi_master *master = param; + +dev_dbg (&master->dev,"DMA transfer complete\n" ); +spi_finalize_current_transfer (master); +} + +static int sun4i_spi_dmap_prep_tx (struct spi_master *master, +struct spi_transfer *tfr, +dma_cookie_t *cookie) +{ +struct dma_async_tx_descriptor *chan_desc =NULL ; + + chan_desc =dmaengine_prep_slave_sg (master->dma_tx, + tfr->tx_sg.sgl, tfr->tx_sg.nents, +DMA_TO_DEVICE , +DMA_PREP_INTERRUPT |DMA_CTRL_ACK ); +if (!chan_desc) { +dev_err (&master->dev, +"Couldn't prepare TX DMA slave\n" ); +return -EIO ; + } + + chan_desc->callback = sun4i_spi_dma_callback; + chan_desc->callback_param = master; + + *cookie =dmaengine_submit (chan_desc); +dma_async_issue_pending (master->dma_tx); + +return 0 ; +} + +static int sun4i_spi_dmap_prep_rx (struct spi_master *master, +struct spi_transfer *tfr, +dma_cookie_t *cookie) +{ +struct dma_async_tx_descriptor *chan_desc =NULL ; + + chan_desc =dmaengine_prep_slave_sg (master->dma_rx, + tfr->rx_sg.sgl, tfr->rx_sg.nents, +DMA_FROM_DEVICE , +DMA_PREP_INTERRUPT |DMA_CTRL_ACK ); +if (!chan_desc) { +dev_err (&master->dev, +"Couldn't prepare RX DMA slave\n" ); +return -EIO ; + } + + chan_desc->callback = sun4i_spi_dma_callback; + chan_desc->callback_param = master; + + *cookie =dmaengine_submit (chan_desc); +dma_async_issue_pending (master->dma_rx); + +return 0 ; +} + +static int sun4i_spi_transfer_one_dma (struct spi_device *spi, +struct spi_transfer *tfr) +{ +struct spi_master *master = spi->master; +struct sun4i_spi *sspi =spi_master_get_devdata (master); +dma_cookie_t tx_cookie =0 , rx_cookie =0 ; +enum dma_status status; +int ret; +u32 reg =0 ; + +dev_dbg (&master->dev,"Using DMA mode for transfer\n" ); + +if (sspi->tx_buf) { + ret =sun4i_spi_dmap_prep_tx (master, tfr, &tx_cookie); +if (ret) + goto out; + + reg |=SUN4I_CTL_DMA_TF_NOT_FULL ; + } + +if (sspi->rx_buf) { + ret =sun4i_spi_dmap_prep_rx (master, tfr, &rx_cookie); +if (ret) + goto out; + + reg |=SUN4I_CTL_DMA_RF_READY ; + } + +sun4i_spi_write (sspi,SUN4I_DMA_CTL_REG , reg); + +/* Dedicated DMA requests */ +sun4i_spi_set (sspi,SUN4I_CTL_REG ,SUN4I_CTL_DMA_DEDICATED ); + +/* Start transfer */ +sun4i_spi_set (sspi,SUN4I_CTL_REG ,SUN4I_CTL_XCH ); + +/* Wait for completion */ + ret =sun4i_spi_wait_for_transfer (spi, tfr); +if (ret) + goto out; + +if (sspi->tx_buf && (status =dma_async_is_tx_complete (master->dma_tx, + tx_cookie,NULL ,NULL ))) { +dev_warn (&master->dev, +"DMA returned completion status of: %s\n" , + status ==DMA_ERROR ?"error" :"in progress" ); + } +if (sspi->rx_buf && (status =dma_async_is_tx_complete (master->dma_rx, + rx_cookie,NULL ,NULL ))) { +dev_warn (&master->dev, +"DMA returned completion status of: %s\n" , + status ==DMA_ERROR ?"error" :"in progress" ); + } + +out: +if (ret) { +dev_dbg (&master->dev,"DMA channel teardown\n" ); + +if (sspi->tx_buf) +dmaengine_terminate_sync (master->dma_tx); +if (sspi->rx_buf) +dmaengine_terminate_sync (master->dma_rx); + } + +sun4i_spi_drain_fifo (sspi,SUN4I_FIFO_DEPTH ); + +return ret; +} + +static int sun4i_spi_transfer_one_pio (struct spi_device *spi, +struct spi_transfer *tfr) +{ +struct spi_master *master = spi->master; +struct sun4i_spi *sspi =spi_master_get_devdata (master); +int ret; + +/* Explicit disable DMA requests */ +sun4i_spi_write (sspi,SUN4I_DMA_CTL_REG ,0 ); +sun4i_spi_unset (sspi,SUN4I_CTL_REG ,SUN4I_CTL_DMA_DEDICATED ); + +/* Fill the TX FIFO */ +sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH ); + +/* Enable transfer complete irq */ +sun4i_spi_set (sspi,SUN4I_INT_CTL_REG ,SUN4I_INT_CTL_TC ); + +/* Start transfer */ +sun4i_spi_set (sspi,SUN4I_CTL_REG ,SUN4I_CTL_XCH ); + + ret =sun4i_spi_wait_for_transfer (spi, tfr); + +sun4i_spi_write (sspi,SUN4I_INT_CTL_REG ,0 ); + +return ret; +} +static int sun4i_spi_transfer_one (struct spi_master *master,struct spi_device *spi,struct spi_transfer *tfr)
@@ -242,13 +413,22 @@ static int sun4i_spi_transfer_one(struct spi_master *master,struct sun4i_spi *sspi =spi_master_get_devdata (master);unsigned int mclk_rate, div;unsigned int tx_len =0 ; -int ret =0 ;u32 reg; -/* We don't support transfers larger than FIFO depth */ -if (tfr->len >SUN4I_FIFO_DEPTH ) +/* A zero length transfer never finishes if programmed +in the hardware */ +if (!tfr->len) +return 0 ; + +if (tfr->len >SUN4I_MAX_XFER_SIZE )return -EMSGSIZE ; +if (!master->can_dma) { +/* Don't support transfer larger than the FIFO */ +if (tfr->len >SUN4I_FIFO_DEPTH ) +return -EMSGSIZE ; + } + sspi->tx_buf = tfr->tx_buf; sspi->rx_buf = tfr->rx_buf; sspi->len = tfr->len;
@@ -335,22 +515,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,sun4i_spi_write (sspi,SUN4I_BURST_CNT_REG ,SUN4I_BURST_CNT (tfr->len));sun4i_spi_write (sspi,SUN4I_XMIT_CNT_REG ,SUN4I_XMIT_CNT (tx_len)); -/* -* Fill the TX FIFO -*/ -sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH ); - -/* Enable transfer complete interrupt */ -sun4i_spi_set (sspi,SUN4I_INT_CTL_REG ,SUN4I_INT_CTL_TC ); - -/* Start the transfer */ -sun4i_spi_set (sspi,SUN4I_CTL_REG ,SUN4I_CTL_XCH ); - - ret =sun4i_spi_wait_for_transfer (spi, tfr); - -sun4i_spi_write (sspi,SUN4I_INT_CTL_REG ,0 ); +if (sun4i_spi_can_dma (master, spi, tfr)) +return sun4i_spi_transfer_one_dma (spi, tfr); -return ret; +return sun4i_spi_transfer_one_pio (spi, tfr); }static irqreturn_t sun4i_spi_handler (int irq,void *dev_id)
@@ -363,8 +531,7 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)/* Transfer complete */ if (status &SUN4I_INT_CTL_TC ) { -sun4i_spi_write (sspi,SUN4I_INT_STA_REG , -SUN4I_INT_CTL_TC ); +sun4i_spi_write (sspi,SUN4I_INT_STA_REG ,SUN4I_INT_CTL_TC );sun4i_spi_drain_fifo (sspi,SUN4I_FIFO_DEPTH );spi_finalize_current_transfer (master);return IRQ_HANDLED ;
@@ -413,6 +580,76 @@ static int sun4i_spi_runtime_suspend(struct device *dev)return 0 ; } +static int sun4i_spi_dma_setup (struct device *dev, +struct resource *res) +{ +struct spi_master *master =dev_get_drvdata (dev); +struct dma_slave_config dma_sconf; +int ret; + + master->dma_tx =dma_request_slave_channel_reason (dev,"tx" ); +if (IS_ERR (master->dma_tx)) { +dev_err (dev,"Unable to acquire DMA TX channel\n" ); + ret =PTR_ERR (master->dma_tx); + goto out; + } + + dma_sconf.direction =DMA_MEM_TO_DEV ; + dma_sconf.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.dst_addr = res->start +SUN4I_TXDATA_REG ; + dma_sconf.dst_maxburst =1 ; + dma_sconf.src_maxburst =1 ; + + ret =dmaengine_slave_config (master->dma_tx, &dma_sconf); +if (ret) { +dev_err (dev,"Unable to configure DMA TX slave\n" ); + goto err_rel_tx; + } + + master->dma_rx =dma_request_slave_channel_reason (dev,"rx" ); +if (IS_ERR (master->dma_rx)) { +dev_err (dev,"Unable to acquire DMA RX channel\n" ); + ret =PTR_ERR (master->dma_rx); + goto err_rel_tx; + } + + dma_sconf.direction =DMA_DEV_TO_MEM ; + dma_sconf.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.src_addr = res->start +SUN4I_RXDATA_REG ; + dma_sconf.src_maxburst =1 ; + dma_sconf.dst_maxburst =1 ; + + ret =dmaengine_slave_config (master->dma_rx, &dma_sconf); +if (ret) { +dev_err (dev,"Unable to configure DMA RX slave\n" ); + goto err_rel_rx; + } + +/* don't set can_dma unless both channels are valid*/ + master->can_dma = sun4i_spi_can_dma; + +return 0 ; + +err_rel_rx: +dma_release_channel (master->dma_rx); +err_rel_tx: +dma_release_channel (master->dma_tx); +out: + master->dma_tx =NULL ; + master->dma_rx =NULL ; +return ret; +} + +static void sun4i_spi_dma_release (struct spi_master *master) +{ +if (master->can_dma) { +dma_release_channel (master->dma_rx); +dma_release_channel (master->dma_tx); + } +} +static int sun4i_spi_probe (struct platform_device *pdev) {struct spi_master *master;
@@ -475,6 +712,15 @@ static int sun4i_spi_probe(struct platform_device *pdev) goto err_free_master; } + ret =sun4i_spi_dma_setup (&pdev->dev, res); +if (ret) { +if (ret == -EPROBE_DEFER ) { +/* wait for the dma driver to load */ + goto err_free_master; + } +dev_warn (&pdev->dev,"DMA transfer not supported\n" ); + } +/* * This wake-up/shutdown pattern is to be able to have the * device woken up, even if runtime_pm is disabled
@@ -501,14 +747,19 @@ err_pm_disable:pm_runtime_disable (&pdev->dev);sun4i_spi_runtime_suspend (&pdev->dev); err_free_master: +sun4i_spi_dma_release (master);spi_master_put (master);return ret; }static int sun4i_spi_remove (struct platform_device *pdev) { +struct spi_master *master =platform_get_drvdata (pdev); +pm_runtime_force_suspend (&pdev->dev); +sun4i_spi_dma_release (master); +return 0 ; }
Signature · 1 line
2.16.2
Re: [PATCH 6/6] spi: sun4i: add DMA transfers support
Quoted 7 lines · Sergey Suloev
drivers/spi/spi-sun4i.c +7
-0+static int sun4i_spi_dma_setup (struct device *dev, +struct resource *res) +{ +struct spi_master *master =dev_get_drvdata (dev); +struct dma_slave_config dma_sconf; +int ret; +
drivers/spi/spi-sun4i.c static int sun4i_spi_runtime_suspend(struct device *dev) · line 599
Quoted 10 lines
drivers/spi/spi-sun4i.c +10
-0+ master->dma_tx =dma_request_slave_channel_reason (dev,"tx" ); +if (IS_ERR (master->dma_tx)) { +dev_err (dev,"Unable to acquire DMA TX channel\n" ); + ret =PTR_ERR (master->dma_tx); + goto out; + } + + dma_sconf.direction =DMA_MEM_TO_DEV ; + dma_sconf.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ;
I guess that would depend on the size of the transfer, right?
drivers/spi/spi-sun4i.c static int sun4i_spi_runtime_suspend(struct device *dev) · line 602
Quoted 3 lines
drivers/spi/spi-sun4i.c +3
-0+ dma_sconf.dst_addr = res->start +SUN4I_TXDATA_REG ; + dma_sconf.dst_maxburst =1 ; + dma_sconf.src_maxburst =1 ;
And a burst of 1 seems sub-optimal here.
drivers/spi/spi-sun4i.c static int sun4i_spi_probe(struct platform_device *pdev) · line 721
Quoted 7 lines
drivers/spi/spi-sun4i.c +7
-0+ ret =sun4i_spi_dma_setup (&pdev->dev, res); +if (ret) { +if (ret == -EPROBE_DEFER ) { +/* wait for the dma driver to load */ + goto err_free_master; + } +dev_warn (&pdev->dev,"DMA transfer not supported\n" );
Saying why it's not supported would be great.
Maxime
Signature · 10 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180403/e3cb651f/attachment.sig>
Re: [PATCH 6/6] spi: sun4i: add DMA transfers support
Quoted 11 lines · Maxime Ripard
Quoted 10 lines · Sergey Suloev
drivers/spi/spi-sun4i.c +10
-0+static int sun4i_spi_dma_setup (struct device *dev, +struct resource *res) +{ +struct spi_master *master =dev_get_drvdata (dev); +struct dma_slave_config dma_sconf; +int ret; + + master->dma_tx =dma_request_slave_channel_reason (dev,"tx" ); +if (IS_ERR (master->dma_tx)) { +dev_err (dev,"Unable to acquire DMA TX channel\n" );
Quoted 8 lines
drivers/spi/spi-sun4i.c static int sun4i_spi_runtime_suspend(struct device *dev) · line 599
Quoted 7 lines
drivers/spi/spi-sun4i.c +7
-0+ ret =PTR_ERR (master->dma_tx); + goto out; + } + + dma_sconf.direction =DMA_MEM_TO_DEV ; + dma_sconf.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ;I guess that would depend on the size of the transfer, right?
no
"this is the width in bytes of the source (RX)register where DMA data shall be read. If the sourceis memory this may be ignored depending on architecture."
AFAIK is should be 1 byte for SPI side and seems to be ignored for memory side, but as soon as I don't know what should be correct value for memory side I just put 1 there too.
Quoted 5 lines
drivers/spi/spi-sun4i.c static int sun4i_spi_runtime_suspend(struct device *dev) · line 602
Quoted 4 lines
drivers/spi/spi-sun4i.c +3
-0+ dma_sconf.dst_addr = res->start +SUN4I_TXDATA_REG ; + dma_sconf.dst_maxburst =1 ; + dma_sconf.src_maxburst =1 ;And a burst of 1 seems sub-optimal here.
I did some tests before with 3/4 FIFO size but it didn't work and I got stuck with 1 byte.
It seems like 1 byte is the correct value because in SPI protocol we can only send 1 byte in 1 burst.
Quoted 9 lines
drivers/spi/spi-sun4i.c static int sun4i_spi_probe(struct platform_device *pdev) · line 721
Quoted 8 lines
drivers/spi/spi-sun4i.c +7
-0+ ret =sun4i_spi_dma_setup (&pdev->dev, res); +if (ret) { +if (ret == -EPROBE_DEFER ) { +/* wait for the dma driver to load */ + goto err_free_master; + } +dev_warn (&pdev->dev,"DMA transfer not supported\n" );Saying why it's not supported would be great.
I can put more info in this log
but there is already a message printed from sun4_spi_dma_setup() if any error occurs
Quoted 3 lines
Maxime
Re: [PATCH 6/6] spi: sun4i: add DMA transfers support
Quoted 21 lines · Sergey Suloev
Quoted 11 lines · Maxime Ripard
Quoted 10 lines · Sergey Suloev
drivers/spi/spi-sun4i.c +10
-0+static int sun4i_spi_dma_setup (struct device *dev, +struct resource *res) +{ +struct spi_master *master =dev_get_drvdata (dev); +struct dma_slave_config dma_sconf; +int ret; + + master->dma_tx =dma_request_slave_channel_reason (dev,"tx" ); +if (IS_ERR (master->dma_tx)) { +dev_err (dev,"Unable to acquire DMA TX channel\n" );Quoted 8 lines
drivers/spi/spi-sun4i.c static int sun4i_spi_runtime_suspend(struct device *dev) · line 599
Quoted 7 lines
drivers/spi/spi-sun4i.c +7
-0+ ret =PTR_ERR (master->dma_tx); + goto out; + } + + dma_sconf.direction =DMA_MEM_TO_DEV ; + dma_sconf.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconf.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ;I guess that would depend on the size of the transfer, right?
Quoted 7 lines
no
"this is the width in bytes of the source (RX)register where DMA data shall be read. If the sourceis memory this may be ignored depending on architecture."
AFAIK is should be 1 byte for SPI side and seems to be ignored for memory side, but as soon as I don't know what should be correct value for memory side I just put 1 there too.
I meant the number of bits per word, sorry. That width would only apply if you have 8 bits per word, but that seems to always be the case. So nevermind.
Quoted 10 lines
Quoted 5 lines
drivers/spi/spi-sun4i.c static int sun4i_spi_runtime_suspend(struct device *dev) · line 602
Quoted 3 lines
drivers/spi/spi-sun4i.c +3
-0+ dma_sconf.dst_addr = res->start +SUN4I_TXDATA_REG ; + dma_sconf.dst_maxburst =1 ; + dma_sconf.src_maxburst =1 ;And a burst of 1 seems sub-optimal here.
I did some tests before with 3/4 FIFO size but it didn't work and I got stuck with 1 byte.
It seems like 1 byte is the correct value because in SPI protocol we can only send 1 byte in 1 burst.
That's not about the SPI burst, it's about the DMA burst.
Quoted 10 lines
Quoted 9 lines
drivers/spi/spi-sun4i.c static int sun4i_spi_probe(struct platform_device *pdev) · line 721
Quoted 8 lines
drivers/spi/spi-sun4i.c +7
-0+ ret =sun4i_spi_dma_setup (&pdev->dev, res); +if (ret) { +if (ret == -EPROBE_DEFER ) { +/* wait for the dma driver to load */ + goto err_free_master; + } +dev_warn (&pdev->dev,"DMA transfer not supported\n" );Saying why it's not supported would be great.
Quoted 3 lines
I can put more info in this log
but there is already a message printed fromsun4_spi_dma_setup()if any error occurs
Then you don't need both.
Maxime
Signature · 10 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180404/12ce3204/attachment.sig>