Skip to content

Commit f2d4f10

Browse files
brooniebroonie
committed
Merge series "spi: bcm2835: Interrupt-handling optimisations" from Robin Murphy <robin.murphy@arm.com>:
Hi all, Although Florian was concerned about a trivial inline check to deal with shared IRQs adding overhead, the reality is that it would be so small as to not be worth even thinking about unless the driver was already tuned to squeeze out every last cycle. And a brief look over the code shows that that clearly isn't the case. This is an example of some of the easy low-hanging fruit that jumps out just from code inspection. Based on disassembly and ARM1176 cycle timings, patch #2 should save the equivalent of 2-3 shared interrupt checks off the critical path in all cases, and patch #3 possibly up to about 100x more. I don't have any means to test these patches, let alone measure performance, so they're only backed by the principle that less code - and in particular fewer memory accesses - is almost always better. There is almost certainly a *lot* more to be had from careful use of relaxed I/O accessors, not doing a read-modify-write of CS at every reset, tweaking the loops further to avoid unnecessary writebacks to variables, and so on. However since I'm not invested in this personally I'm not going to pursue it any further; I'm throwing these patches out as more of a demonstration to back up my original drive-by review comments, so if anyone want to pick them up and run with them then please do so. Robin. Robin Murphy (3): spi: bcm3835: Tidy up bcm2835_spi_reset_hw() spi: bcm2835: Micro-optimise IRQ handler spi: bcm2835: Micro-optimise FIFO loops drivers/spi/spi-bcm2835.c | 45 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 23 deletions(-) -- 2.23.0.dirty _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
2 parents 95f2fd2 + 26751de commit f2d4f10

File tree

1 file changed

+22
-23
lines changed

1 file changed

+22
-23
lines changed

‎drivers/spi/spi-bcm2835.c‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ MODULE_PARM_DESC(polling_limit_us,
8686
* @clk: core clock, divided to calculate serial clock
8787
* @irq: interrupt, signals TX FIFO empty or RX FIFO ¾ full
8888
* @tfr: SPI transfer currently processed
89+
* @ctlr: SPI controller reverse lookup
8990
* @tx_buf: pointer whence next transmitted byte is read
9091
* @rx_buf: pointer where next received byte is written
9192
* @tx_len: remaining bytes to transmit
@@ -125,6 +126,7 @@ struct bcm2835_spi {
125126
struct clk *clk;
126127
int irq;
127128
struct spi_transfer *tfr;
129+
struct spi_controller *ctlr;
128130
const u8 *tx_buf;
129131
u8 *rx_buf;
130132
int tx_len;
@@ -243,13 +245,13 @@ static inline void bcm2835_rd_fifo_count(struct bcm2835_spi *bs, int count)
243245

244246
bs->rx_len -= count;
245247

246-
while (count > 0) {
248+
do {
247249
val = bcm2835_rd(bs, BCM2835_SPI_FIFO);
248250
len = min(count, 4);
249251
memcpy(bs->rx_buf, &val, len);
250252
bs->rx_buf += len;
251253
count -= 4;
252-
}
254+
} while (count > 0);
253255
}
254256

255257
/**
@@ -269,7 +271,7 @@ static inline void bcm2835_wr_fifo_count(struct bcm2835_spi *bs, int count)
269271

270272
bs->tx_len -= count;
271273

272-
while (count > 0) {
274+
do {
273275
if (bs->tx_buf) {
274276
len = min(count, 4);
275277
memcpy(&val, bs->tx_buf, len);
@@ -279,7 +281,7 @@ static inline void bcm2835_wr_fifo_count(struct bcm2835_spi *bs, int count)
279281
}
280282
bcm2835_wr(bs, BCM2835_SPI_FIFO, val);
281283
count -= 4;
282-
}
284+
} while (count > 0);
283285
}
284286

285287
/**
@@ -308,12 +310,11 @@ static inline void bcm2835_rd_fifo_blind(struct bcm2835_spi *bs, int count)
308310
count = min(count, bs->rx_len);
309311
bs->rx_len -= count;
310312

311-
while (count) {
313+
do {
312314
val = bcm2835_rd(bs, BCM2835_SPI_FIFO);
313315
if (bs->rx_buf)
314316
*bs->rx_buf++ = val;
315-
count--;
316-
}
317+
} while (--count);
317318
}
318319

319320
/**
@@ -328,16 +329,14 @@ static inline void bcm2835_wr_fifo_blind(struct bcm2835_spi *bs, int count)
328329
count = min(count, bs->tx_len);
329330
bs->tx_len -= count;
330331

331-
while (count) {
332+
do {
332333
val = bs->tx_buf ? *bs->tx_buf++ : 0;
333334
bcm2835_wr(bs, BCM2835_SPI_FIFO, val);
334-
count--;
335-
}
335+
} while (--count);
336336
}
337337

338-
static void bcm2835_spi_reset_hw(struct spi_controller *ctlr)
338+
static void bcm2835_spi_reset_hw(struct bcm2835_spi *bs)
339339
{
340-
struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
341340
u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
342341

343342
/* Disable SPI interrupts and transfer */
@@ -363,8 +362,7 @@ static void bcm2835_spi_reset_hw(struct spi_controller *ctlr)
363362

364363
static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
365364
{
366-
struct spi_controller *ctlr = dev_id;
367-
struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
365+
struct bcm2835_spi *bs = dev_id;
368366
u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
369367

370368
/*
@@ -386,9 +384,9 @@ static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
386384

387385
if (!bs->rx_len) {
388386
/* Transfer complete - reset SPI HW */
389-
bcm2835_spi_reset_hw(ctlr);
387+
bcm2835_spi_reset_hw(bs);
390388
/* wake up the framework */
391-
complete(&ctlr->xfer_completion);
389+
complete(&bs->ctlr->xfer_completion);
392390
}
393391

394392
return IRQ_HANDLED;
@@ -607,7 +605,7 @@ static void bcm2835_spi_dma_rx_done(void *data)
607605
bcm2835_spi_undo_prologue(bs);
608606

609607
/* reset fifo and HW */
610-
bcm2835_spi_reset_hw(ctlr);
608+
bcm2835_spi_reset_hw(bs);
611609

612610
/* and mark as completed */;
613611
complete(&ctlr->xfer_completion);
@@ -641,7 +639,7 @@ static void bcm2835_spi_dma_tx_done(void *data)
641639
dmaengine_terminate_async(ctlr->dma_rx);
642640

643641
bcm2835_spi_undo_prologue(bs);
644-
bcm2835_spi_reset_hw(ctlr);
642+
bcm2835_spi_reset_hw(bs);
645643
complete(&ctlr->xfer_completion);
646644
}
647645

@@ -825,14 +823,14 @@ static int bcm2835_spi_transfer_one_dma(struct spi_controller *ctlr,
825823
if (!bs->rx_buf && !bs->tx_dma_active &&
826824
cmpxchg(&bs->rx_dma_active, true, false)) {
827825
dmaengine_terminate_async(ctlr->dma_rx);
828-
bcm2835_spi_reset_hw(ctlr);
826+
bcm2835_spi_reset_hw(bs);
829827
}
830828

831829
/* wait for wakeup in framework */
832830
return 1;
833831

834832
err_reset_hw:
835-
bcm2835_spi_reset_hw(ctlr);
833+
bcm2835_spi_reset_hw(bs);
836834
bcm2835_spi_undo_prologue(bs);
837835
return ret;
838836
}
@@ -1074,7 +1072,7 @@ static int bcm2835_spi_transfer_one_poll(struct spi_controller *ctlr,
10741072
}
10751073

10761074
/* Transfer complete - reset SPI HW */
1077-
bcm2835_spi_reset_hw(ctlr);
1075+
bcm2835_spi_reset_hw(bs);
10781076
/* and return without waiting for completion */
10791077
return 0;
10801078
}
@@ -1182,7 +1180,7 @@ static void bcm2835_spi_handle_err(struct spi_controller *ctlr,
11821180
bcm2835_spi_undo_prologue(bs);
11831181

11841182
/* and reset */
1185-
bcm2835_spi_reset_hw(ctlr);
1183+
bcm2835_spi_reset_hw(bs);
11861184
}
11871185

11881186
static int chip_match_name(struct gpio_chip *chip, void *data)
@@ -1311,6 +1309,7 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
13111309
ctlr->dev.of_node = pdev->dev.of_node;
13121310

13131311
bs = spi_controller_get_devdata(ctlr);
1312+
bs->ctlr = ctlr;
13141313

13151314
bs->regs = devm_platform_ioremap_resource(pdev, 0);
13161315
if (IS_ERR(bs->regs)) {
@@ -1345,7 +1344,7 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
13451344
BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX);
13461345

13471346
err = devm_request_irq(&pdev->dev, bs->irq, bcm2835_spi_interrupt, 0,
1348-
dev_name(&pdev->dev), ctlr);
1347+
dev_name(&pdev->dev), bs);
13491348
if (err) {
13501349
dev_err(&pdev->dev, "could not request IRQ: %d\n", err);
13511350
goto out_dma_release;

0 commit comments

Comments
 (0)