On Sun, Aug 17, 2014 at 12:41:04AM +0200, Rafał Miłecki wrote:
> +static inline unsigned int bcm53xxspi_calc_timeout(size_t len)
> +{
> + return (len * 9000 / 13500000 * 110 / 100) + 1;
> +}
Magic numbersr!
> +static int bcm53xxspi_wait(struct bcm53xxspi *b53spi, unsigned int
> timeout_ms)
> +{
> + unsigned long deadline;
> + u32 tmp;
> +
> + /* SPE bit has to be 0 before we read MSPI STATUS */
> + while (1) {
> + tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
> + if (!(tmp & B53SPI_MSPI_SPCR2_SPE))
> + break;
> + }
This could block for ever, there should be a timeout of some kind. I'd
also include a cpu_relax() in here since it's a busy wait.
> +static void bcm53xxspi_buf_write(struct bcm53xxspi *b53spi, u8 *w_buf,
> + size_t len, bool cont)
> +{
> + u32 tmp;
> + int i;
> +
> + for (i = 0; i < len; i++) {
> + /* Transmit Register File MSB */
> + bcm53xxspi_write(b53spi, B53SPI_MSPI_TXRAM + 4 * (i * 2),
> + (unsigned int)w_buf[i]);
> + }
> +
> + for (i = 0; i < len; i++) {
> + tmp = B53SPI_CDRAM_CONT | B53SPI_CDRAM_PCS_DISABLE_ALL |
> + B53SPI_CDRAM_PCS_DSCK;
> + if (!cont && i == len - 1)
> + tmp &= ~B53SPI_CDRAM_CONT;
> + tmp &= ~0x1;
> + /* Command Register File */
> + bcm53xxspi_write(b53spi, B53SPI_MSPI_CDRAM + 4 * i, tmp);
> + }
> +
> + /* Set queue pointers */
> + bcm53xxspi_write(b53spi, B53SPI_MSPI_NEWQP, 0);
> + bcm53xxspi_write(b53spi, B53SPI_MSPI_ENDQP, len - 1);
> +
> + if (cont)
> + bcm53xxspi_write(b53spi, B53SPI_MSPI_WRITE_LOCK, 1);
> +
> + /* Start SPI transfer */
> + tmp = bcm53xxspi_read(b53spi, B53SPI_MSPI_SPCR2);
> + tmp |= B53SPI_MSPI_SPCR2_SPE;
> + if (cont)
> + tmp |= B53SPI_MSPI_SPCR2_CONT_AFTER_CMD;
> + bcm53xxspi_write(b53spi, B53SPI_MSPI_SPCR2, tmp);
> +
> + /* Wait for SPI to finish */
> + bcm53xxspi_wait(b53spi, bcm53xxspi_calc_timeout(len));
This means we can only do unidirectonal transfers from the look of it -
is that a limitation of the hardware (from a quick read it seems like it
might be)?
> + /* Wait for SPI to finish */
> + bcm53xxspi_wait(b53spi, bcm53xxspi_calc_timeout(len));
No interrupts? This will busy wait for the entire transfer which seems
a bit much.
I'm also not seeing a set_cs() operation here.
> + err = spi_register_master(master);
> + if (err) {
devm_spi_register_master().
signature.asc
Description: Digital signature
