Re: [PATCH 7/7] spi: bcm2835: Speed up FIFO access if fill level is known

[Lukas Wunner <lukas@xxxxxxxxx>]

On Sat, Nov 10, 2018 at 12:25:34PM +0100, Stefan Wahren wrote:
> > On 08.11.2018, at 08:06, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > > +#define BCM2835_SPI_FIFO_SIZE		64
> > > +#define BCM2835_SPI_FIFO_SIZE_3_4	48
> 
> I only have doubts about the naming FIFO_SIZE_3_4 because it describe
> a fill level not a size.

Hm, it's three quarters of the FIFO's size, so seems sufficiently apt?


On Sat, Nov 10, 2018 at 11:03:17AM +0100, kernel@xxxxxxxxxxxxxxxx wrote:
> On 08.11.2018, at 08:06, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > @@ -700,8 +756,10 @@ static int bcm2835_spi_transfer_one_poll(struct spi_master *master,
> > 					    jiffies - timeout,
> > 					    bs->tx_len, bs->rx_len);
> > 			/* fall back to interrupt mode */
> > -			return bcm2835_spi_transfer_one_irq(master, spi,
> > -							    tfr, cs);
> > +			cs |= BCM2835_SPI_CS_INTR | BCM2835_SPI_CS_INTD;
> > +			bcm2835_wr(bs, BCM2835_SPI_CS, cs);
> > +			/* tell SPI core to wait for completion */
> > +			return 1;
> > 		}
> > 	}
> 
> What is wrong about using bcm2835_spi_transfer_one_irq?
> It makes the intention clear that we switch to irq mode.
> 
> Why are you open-coding it instead?
> 
> If there is a slight different behavior now then I would recommend amending
> the transfer_one_irq to handle the behavior.
> 
> If it is done by adding a ???fill bytes??? argument to transfer_one_irq that 
> may be set to 0 in this case then the compiler should be able to optimize
> the unnecessary code away.

Ok, I've folded the requested changes into the patch for v2.
Will wait a few more days for further comments though.

Thanks!

Lukas