From 172289df4004cccf798316b19cc3e6284dbe901f Mon Sep 17 00:00:00 2001 From: Chris Blair Date: Sat, 4 Jun 2011 07:57:47 +0100 Subject: [PATCH 1/7] spi/pl022: only enable RX interrupts when TX is complete For interrupt mode transfers, start with only TX interrupts enabled to reduce the overall number of interrupts received. Once TX is complete, enable RX interrupts to complete the transfer. Reviewed-by: Viresh Kumar Signed-off-by: Chris Blair Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 5559b2299198..5b7209f9f100 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -1244,9 +1244,9 @@ static irqreturn_t pl022_interrupt_handler(int irq, void *dev_id) if ((pl022->tx == pl022->tx_end) && (flag == 0)) { flag = 1; - /* Disable Transmit interrupt */ - writew(readw(SSP_IMSC(pl022->virtbase)) & - (~SSP_IMSC_MASK_TXIM), + /* Disable Transmit interrupt, enable receive interrupt */ + writew((readw(SSP_IMSC(pl022->virtbase)) & + ~SSP_IMSC_MASK_TXIM) | SSP_IMSC_MASK_RXIM, SSP_IMSC(pl022->virtbase)); } @@ -1379,12 +1379,17 @@ static void pump_transfers(unsigned long data) } err_config_dma: - writew(ENABLE_ALL_INTERRUPTS, SSP_IMSC(pl022->virtbase)); + /* enable all interrupts except RX */ + writew(ENABLE_ALL_INTERRUPTS & ~SSP_IMSC_MASK_RXIM, SSP_IMSC(pl022->virtbase)); } static void do_interrupt_dma_transfer(struct pl022 *pl022) { - u32 irqflags = ENABLE_ALL_INTERRUPTS; + /* + * Default is to enable all interrupts except RX - + * this will be enabled once TX is complete + */ + u32 irqflags = ENABLE_ALL_INTERRUPTS & ~SSP_IMSC_MASK_RXIM; /* Enable target chip */ pl022->cur_chip->cs_control(SSP_CHIP_SELECT); From c4a4784308f5cb86ef3c6c1288524ecb63c1fc87 Mon Sep 17 00:00:00 2001 From: Jonas Aaberg Date: Mon, 28 Feb 2011 16:42:41 +0100 Subject: [PATCH 2/7] spi/pl022: fix build warnings The driver build complains with newer compilers unless you initialize this struct properly. Reviewed-by: Viresh Kumar Signed-off-by: Jonas Aaberg Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 5b7209f9f100..0d0b16541d94 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -1886,7 +1886,7 @@ static int pl022_setup(struct spi_device *spi) { struct pl022_config_chip const *chip_info; struct chip_data *chip; - struct ssp_clock_params clk_freq = {0, }; + struct ssp_clock_params clk_freq = { .cpsdvsr = 0, .scr = 0}; int status = 0; struct pl022 *pl022 = spi_master_get_devdata(spi->master); unsigned int bits = spi->bits_per_word; From 4ca9fb46d8045b2473d608cd9e4580c6e0f7d5e0 Mon Sep 17 00:00:00 2001 From: Virupax Sadashivpetimath Date: Mon, 17 Oct 2011 14:54:08 +0200 Subject: [PATCH 3/7] spi/pl022: skip default configuration before suspending The loading of the default configuration before suspending has been in the driver since its inception, but it is not really needed. Especially so since we take to all the trouble of enabling and disabling power and clock just to do this. Let's scrap this now. Reviewed-by: Viresh Kumar Signed-off-by: Virupax Sadashivpetimath Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 0d0b16541d94..b4038f97f6b0 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -2310,11 +2310,6 @@ static int pl022_suspend(struct device *dev) return status; } - amba_vcore_enable(pl022->adev); - amba_pclk_enable(pl022->adev); - load_ssp_default_config(pl022); - amba_pclk_disable(pl022->adev); - amba_vcore_disable(pl022->adev); dev_dbg(dev, "suspended\n"); return 0; } From d4b6af2e0e29278f2353c7c8721ddf2f2446414f Mon Sep 17 00:00:00 2001 From: Chris Blair Date: Fri, 4 Nov 2011 07:43:41 +0000 Subject: [PATCH 4/7] spi/pl022: move device disable to workqueue thread Moves the disabling of the device and clocks to the same thread in which the device and clocks are enabled. This avoids SMP issues where the device can be enabled for a transfer by one thread and then disabled by the completion of the previous transfer in another thread. Reviewed-by: Viresh Kumar Signed-off-by: Chris Blair Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index b4038f97f6b0..d165c0bfcab5 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -512,8 +512,6 @@ static void giveback(struct pl022 *pl022) msg->state = NULL; if (msg->complete) msg->complete(msg->context); - /* This message is completed, so let's turn off the clocks & power */ - pm_runtime_put(&pl022->adev->dev); } /** @@ -1509,14 +1507,18 @@ static void pump_messages(struct work_struct *work) struct pl022 *pl022 = container_of(work, struct pl022, pump_messages); unsigned long flags; + bool was_busy = false; /* Lock queue and check for queue work */ spin_lock_irqsave(&pl022->queue_lock, flags); if (list_empty(&pl022->queue) || !pl022->running) { + if (pl022->busy) + pm_runtime_put(&pl022->adev->dev); pl022->busy = false; spin_unlock_irqrestore(&pl022->queue_lock, flags); return; } + /* Make sure we are not already running a message */ if (pl022->cur_msg) { spin_unlock_irqrestore(&pl022->queue_lock, flags); @@ -1527,7 +1529,10 @@ static void pump_messages(struct work_struct *work) list_entry(pl022->queue.next, struct spi_message, queue); list_del_init(&pl022->cur_msg->queue); - pl022->busy = true; + if (pl022->busy) + was_busy = true; + else + pl022->busy = true; spin_unlock_irqrestore(&pl022->queue_lock, flags); /* Initial message state */ @@ -1537,12 +1542,14 @@ static void pump_messages(struct work_struct *work) /* Setup the SPI using the per chip configuration */ pl022->cur_chip = spi_get_ctldata(pl022->cur_msg->spi); - /* - * We enable the core voltage and clocks here, then the clocks - * and core will be disabled when giveback() is called in each method - * (poll/interrupt/DMA) - */ - pm_runtime_get_sync(&pl022->adev->dev); + if (!was_busy) + /* + * We enable the core voltage and clocks here, then the clocks + * and core will be disabled when this workqueue is run again + * and there is no more work to be done. + */ + pm_runtime_get_sync(&pl022->adev->dev); + restore_state(pl022); flush(pl022); From 0ad2deeab5d3fc80fc7cd85638f805830254ef1d Mon Sep 17 00:00:00 2001 From: Virupax Sadashivpetimath Date: Mon, 17 Oct 2011 14:52:47 +0200 Subject: [PATCH 5/7] spi/pl022: disable the PL022 block when unused Make sure we clear the enable bit when the block is not used. This will save some energy in certain hardware versions. Reviewed-by: Viresh Kumar Signed-off-by: Virupax Sadashivpetimath Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index d165c0bfcab5..82a929f916fd 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -1512,8 +1512,12 @@ static void pump_messages(struct work_struct *work) /* Lock queue and check for queue work */ spin_lock_irqsave(&pl022->queue_lock, flags); if (list_empty(&pl022->queue) || !pl022->running) { - if (pl022->busy) + if (pl022->busy) { + /* nothing more to do - disable spi/ssp and power off */ + writew((readw(SSP_CR1(pl022->virtbase)) & + (~SSP_CR1_MASK_SSE)), SSP_CR1(pl022->virtbase)); pm_runtime_put(&pl022->adev->dev); + } pl022->busy = false; spin_unlock_irqrestore(&pl022->queue_lock, flags); return; From 53e4acea0e819a6a8513e10a0773f2259ede0481 Mon Sep 17 00:00:00 2001 From: Chris Blair Date: Tue, 8 Nov 2011 08:54:46 +0000 Subject: [PATCH 6/7] spi/pl022: add support for pm_runtime autosuspend Adds support for configuring the spi bus to use autosuspend for runtime power management. This can reduce the latency in starting an spi transfer by not suspending the device immediately following completion of a transfer. If another transfer then takes place before the autosuspend timeout, the call to resume the device can return immediately rather than needing to risk sleeping in order to resume the device. Reviewed-by: Viresh Kumar Signed-off-by: Chris Blair Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 20 ++++++++++++++++++-- include/linux/amba/pl022.h | 4 ++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 82a929f916fd..13988a3024bb 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -1516,7 +1516,13 @@ static void pump_messages(struct work_struct *work) /* nothing more to do - disable spi/ssp and power off */ writew((readw(SSP_CR1(pl022->virtbase)) & (~SSP_CR1_MASK_SSE)), SSP_CR1(pl022->virtbase)); - pm_runtime_put(&pl022->adev->dev); + + if (pl022->master_info->autosuspend_delay > 0) { + pm_runtime_mark_last_busy(&pl022->adev->dev); + pm_runtime_put_autosuspend(&pl022->adev->dev); + } else { + pm_runtime_put(&pl022->adev->dev); + } } pl022->busy = false; spin_unlock_irqrestore(&pl022->queue_lock, flags); @@ -2247,7 +2253,17 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id) dev_dbg(dev, "probe succeeded\n"); /* let runtime pm put suspend */ - pm_runtime_put(dev); + if (platform_info->autosuspend_delay > 0) { + dev_info(&adev->dev, + "will use autosuspend for runtime pm, delay %dms\n", + platform_info->autosuspend_delay); + pm_runtime_set_autosuspend_delay(dev, + platform_info->autosuspend_delay); + pm_runtime_use_autosuspend(dev); + pm_runtime_put_autosuspend(dev); + } else { + pm_runtime_put(dev); + } return 0; err_spi_register: diff --git a/include/linux/amba/pl022.h b/include/linux/amba/pl022.h index 4ce98f54186b..572f637299c9 100644 --- a/include/linux/amba/pl022.h +++ b/include/linux/amba/pl022.h @@ -238,6 +238,9 @@ struct dma_chan; * @enable_dma: if true enables DMA driven transfers. * @dma_rx_param: parameter to locate an RX DMA channel. * @dma_tx_param: parameter to locate a TX DMA channel. + * @autosuspend_delay: delay in ms following transfer completion before the + * runtime power management system suspends the device. A setting of 0 + * indicates no delay and the device will be suspended immediately. */ struct pl022_ssp_controller { u16 bus_id; @@ -246,6 +249,7 @@ struct pl022_ssp_controller { bool (*dma_filter)(struct dma_chan *chan, void *filter_param); void *dma_rx_param; void *dma_tx_param; + int autosuspend_delay; }; /** From 8b8d719161c386668161daf29b2632f094e326c8 Mon Sep 17 00:00:00 2001 From: Virupax Sadashivpetimath Date: Thu, 10 Nov 2011 12:43:24 +0530 Subject: [PATCH 7/7] spi/pl022: make the chip deselect handling thread safe There is a possibility that the pump_message and giveback run in parallel on a SMP system. Both the pump_message and giveback threads work on the same SPI message queue. Results will be in correct if the pump_message gets to work on the queue first. when the pump_message works with the queue, it reads the head of the queue and removes it from the queue. pump_message activates the chip select depending on this message read. This leads to giveback working on the modified queue or a emptied queue. If the queue is empty or if the next message on the queue (which is not the actual next message, as the pump message has removed the next message from the queue) is not for the same SPI device as that Of the previous one, giveback will de-activate the chip select activated by pump_message(), which is wrong. To solve this problem pump_message is made not to run and access the queue until the giveback is done handling the queue. I.e. by making the cur_msg NULL after the giveback has read the queue. Also a state variable has been introduced to keep track of when the CS for next message is already activated and avoid to double-activate it. Reviewed-by: Viresh Kumar Signed-off-by: Virupax Sadashivpetimath Signed-off-by: Linus Walleij --- drivers/spi/spi-pl022.c | 72 +++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 13988a3024bb..1cff8ad470d1 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -340,6 +340,10 @@ struct vendor_data { * @cur_msg: Pointer to current spi_message being processed * @cur_transfer: Pointer to current spi_transfer * @cur_chip: pointer to current clients chip(assigned from controller_state) + * @next_msg_cs_active: the next message in the queue has been examined + * and it was found that it uses the same chip select as the previous + * message, so we left it active after the previous transfer, and it's + * active already. * @tx: current position in TX buffer to be read * @tx_end: end position in TX buffer to be read * @rx: current position in RX buffer to be written @@ -373,6 +377,7 @@ struct pl022 { struct spi_message *cur_msg; struct spi_transfer *cur_transfer; struct chip_data *cur_chip; + bool next_msg_cs_active; void *tx; void *tx_end; void *rx; @@ -445,23 +450,9 @@ static void giveback(struct pl022 *pl022) struct spi_transfer *last_transfer; unsigned long flags; struct spi_message *msg; - void (*curr_cs_control) (u32 command); + pl022->next_msg_cs_active = false; - /* - * This local reference to the chip select function - * is needed because we set curr_chip to NULL - * as a step toward termininating the message. - */ - curr_cs_control = pl022->cur_chip->cs_control; - spin_lock_irqsave(&pl022->queue_lock, flags); - msg = pl022->cur_msg; - pl022->cur_msg = NULL; - pl022->cur_transfer = NULL; - pl022->cur_chip = NULL; - queue_work(pl022->workqueue, &pl022->pump_messages); - spin_unlock_irqrestore(&pl022->queue_lock, flags); - - last_transfer = list_entry(msg->transfers.prev, + last_transfer = list_entry(pl022->cur_msg->transfers.prev, struct spi_transfer, transfer_list); @@ -473,18 +464,13 @@ static void giveback(struct pl022 *pl022) */ udelay(last_transfer->delay_usecs); - /* - * Drop chip select UNLESS cs_change is true or we are returning - * a message with an error, or next message is for another chip - */ - if (!last_transfer->cs_change) - curr_cs_control(SSP_CHIP_DESELECT); - else { + if (!last_transfer->cs_change) { struct spi_message *next_msg; - /* Holding of cs was hinted, but we need to make sure - * the next message is for the same chip. Don't waste - * time with the following tests unless this was hinted. + /* + * cs_change was not set. We can keep the chip select + * enabled if there is message in the queue and it is + * for the same spi device. * * We cannot postpone this until pump_messages, because * after calling msg->complete (below) the driver that @@ -501,14 +487,26 @@ static void giveback(struct pl022 *pl022) struct spi_message, queue); spin_unlock_irqrestore(&pl022->queue_lock, flags); - /* see if the next and current messages point - * to the same chip + /* + * see if the next and current messages point + * to the same spi device. */ - if (next_msg && next_msg->spi != msg->spi) + if (next_msg && next_msg->spi != pl022->cur_msg->spi) next_msg = NULL; - if (!next_msg || msg->state == STATE_ERROR) - curr_cs_control(SSP_CHIP_DESELECT); + if (!next_msg || pl022->cur_msg->state == STATE_ERROR) + pl022->cur_chip->cs_control(SSP_CHIP_DESELECT); + else + pl022->next_msg_cs_active = true; } + + spin_lock_irqsave(&pl022->queue_lock, flags); + msg = pl022->cur_msg; + pl022->cur_msg = NULL; + pl022->cur_transfer = NULL; + pl022->cur_chip = NULL; + queue_work(pl022->workqueue, &pl022->pump_messages); + spin_unlock_irqrestore(&pl022->queue_lock, flags); + msg->state = NULL; if (msg->complete) msg->complete(msg->context); @@ -1350,7 +1348,7 @@ static void pump_transfers(unsigned long data) */ udelay(previous->delay_usecs); - /* Drop chip select only if cs_change is requested */ + /* Reselect chip select only if cs_change was requested */ if (previous->cs_change) pl022->cur_chip->cs_control(SSP_CHIP_SELECT); } else { @@ -1389,8 +1387,10 @@ static void do_interrupt_dma_transfer(struct pl022 *pl022) */ u32 irqflags = ENABLE_ALL_INTERRUPTS & ~SSP_IMSC_MASK_RXIM; - /* Enable target chip */ - pl022->cur_chip->cs_control(SSP_CHIP_SELECT); + /* Enable target chip, if not already active */ + if (!pl022->next_msg_cs_active) + pl022->cur_chip->cs_control(SSP_CHIP_SELECT); + if (set_up_next_transfer(pl022, pl022->cur_transfer)) { /* Error path */ pl022->cur_msg->state = STATE_ERROR; @@ -1445,7 +1445,8 @@ static void do_polling_transfer(struct pl022 *pl022) } else { /* STATE_START */ message->state = STATE_RUNNING; - pl022->cur_chip->cs_control(SSP_CHIP_SELECT); + if (!pl022->next_msg_cs_active) + pl022->cur_chip->cs_control(SSP_CHIP_SELECT); } /* Configuration Changing Per Transfer */ @@ -1604,6 +1605,7 @@ static int start_queue(struct pl022 *pl022) pl022->cur_msg = NULL; pl022->cur_transfer = NULL; pl022->cur_chip = NULL; + pl022->next_msg_cs_active = false; spin_unlock_irqrestore(&pl022->queue_lock, flags); queue_work(pl022->workqueue, &pl022->pump_messages);