Skip to content

Commit

Permalink
spi-axi-engine: Calculate buffer dimension for xfer
Browse files Browse the repository at this point in the history 
After the modification to the word length the buffer for rx and tx needed
to be adapted to the new size. Before this update the size was fixed to 8
bits and that was stored inside memory.

Example: 20 bit resolution -> entire 20 bit value stored inside 32
bit register. Value is separated into 3 parts of 8 bits and stored in the
rx buffer. The same for tx part.

Signed-off-by: Mircea Caprioru <[email protected]>
  • Loading branch information
@commodo
Mircea Caprioru authored and commodo committed Nov 7, 2018
1 parent 636d80a commit 30707b0
Showing 1 changed file with 41 additions and 6 deletions.
47 changes: 41 additions & 6 deletions drivers/spi/spi-axi-spi-engine.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ struct spi_engine {
unsigned int int_enable;

struct timer_list watchdog_timer;
unsigned int word_length;
};

static void spi_engine_program_add_cmd(struct spi_engine_program *p,
Expand Down Expand Up @@ -174,6 +175,18 @@ static unsigned int spi_engine_get_word_length(struct spi_engine *spi_engine,
return 8;
}

static void spi_engine_update_xfer_len(struct spi_engine *spi_engine,
struct spi_transfer *xfer)
{
unsigned int word_length = spi_engine->word_length;
unsigned int word_len_bytes = word_length / 8;

if ((xfer->len * 8) < word_length)
xfer->len = 1;
else
xfer->len = DIV_ROUND_UP(xfer->len, word_len_bytes);
}

static void spi_engine_gen_xfer(struct spi_engine_program *p, bool dry,
struct spi_transfer *xfer)
{
Expand Down Expand Up @@ -253,6 +266,7 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
new_word_len = spi_engine_get_word_length(spi_engine, spi, xfer);
if (new_word_len != word_len) {
word_len = new_word_len;
spi_engine->word_length = word_len;
spi_engine_program_add_cmd(p, dry,
SPI_ENGINE_CMD_WRITE(SPI_ENGINE_CMD_REG_WORD_LENGTH,
word_len));
Expand All @@ -261,6 +275,7 @@ static int spi_engine_compile_message(struct spi_engine *spi_engine,
if (cs_change)
spi_engine_gen_cs(p, dry, spi, true);

spi_engine_update_xfer_len(spi_engine, xfer);
spi_engine_gen_xfer(p, dry, xfer);
spi_engine_gen_sleep(p, dry, spi_engine, clk_div,
xfer->delay_usecs);
Expand Down Expand Up @@ -423,9 +438,32 @@ static bool spi_engine_write_cmd_fifo(struct spi_engine *spi_engine)
return spi_engine->cmd_length != 0;
}

static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)
static void spi_engine_read_buff(struct spi_engine *spi_engine, uint8_t *buf)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDI_DATA_FIFO;
uint32_t val;
uint8_t len, i;

val = readl_relaxed(addr);
len = DIV_ROUND_UP(spi_engine->word_length, 8);

for (i = 0; i < len; i++)
buf[i] = val >> (8 * i);
}

static void spi_engine_write_buff(struct spi_engine *spi_engine,
const uint8_t *buf)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDO_DATA_FIFO;
uint8_t len, i;

len = DIV_ROUND_UP(spi_engine->word_length, 8);
for (i = 0; i < len; i++)
writel_relaxed(buf[i], addr);
}

static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)
{
unsigned int n, m, i;
const uint8_t *buf;

Expand All @@ -434,7 +472,7 @@ static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)
m = min(n, spi_engine->tx_length);
buf = spi_engine->tx_buf;
for (i = 0; i < m; i++)
writel_relaxed(buf[i], addr);
spi_engine_write_buff(spi_engine, buf);
spi_engine->tx_buf += m;
spi_engine->tx_length -= m;
n -= m;
Expand All @@ -447,7 +485,6 @@ static bool spi_engine_write_tx_fifo(struct spi_engine *spi_engine)

static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine)
{
void __iomem *addr = spi_engine->base + SPI_ENGINE_REG_SDI_DATA_FIFO;
unsigned int n, m, i;
uint8_t *buf;

Expand All @@ -456,7 +493,7 @@ static bool spi_engine_read_rx_fifo(struct spi_engine *spi_engine)
m = min(n, spi_engine->rx_length);
buf = spi_engine->rx_buf;
for (i = 0; i < m; i++)
buf[i] = readl_relaxed(addr);
spi_engine_read_buff(spi_engine, buf);
spi_engine->rx_buf += m;
spi_engine->rx_length -= m;
n -= m;
Expand Down Expand Up @@ -487,7 +524,6 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
unsigned int pending;

pending = readl_relaxed(spi_engine->base + SPI_ENGINE_REG_INT_PENDING);

if (pending & SPI_ENGINE_INT_SYNC) {
writel_relaxed(SPI_ENGINE_INT_SYNC,
spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
Expand Down Expand Up @@ -674,7 +710,6 @@ static int spi_engine_probe(struct platform_device *pdev)

master->dev.of_node = pdev->dev.of_node;
master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_3WIRE;
master->bits_per_word_mask = SPI_BPW_MASK(8);
master->max_speed_hz = clk_get_rate(spi_engine->ref_clk) / 2;
master->transfer_one_message = spi_engine_transfer_one_message;
master->num_chipselect = 8;
Expand Down

4 comments on commit 30707b0

@icchalmers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this commit breaks compatibility with the standard SPI API. The length of a transfer is always specified in bytes, not words. See https://elixir.bootlin.com/linux/v5.0/source/include/linux/spi/spi.h#L700

Commit 42d22f4 and 1ba9543 then seem to try and fix this, but drivers using the SPI controller that specify transfer length in bytes will still output the wrong value.

I'm still pretty green when it comes to the kernel, so please let me know if I've got that wrong!

@mirceacaprioru
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Iain,
You where right that the other commits are fixing some issues regarding the buffer writes but those are mostly related to how we interface to the SPI Engine IP core.

The SPI api has the option to specify a word length https://elixir.bootlin.com/linux/v5.0/source/include/linux/spi/spi.h#L703

The only thing to ensure is that the transfer length covers that word length. In a case with 20 bits word length you must have a transfer of 32 bits that means that tx length will be 4 bytes.

You also have this check here https://elixir.bootlin.com/linux/v5.0/source/drivers/spi/spi.c#L2979,
https://elixir.bootlin.com/linux/v5.0/source/drivers/spi/spi.c#L2798

It even drives the CS line on a per word basis.

Regards,
Mircea

@icchalmers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. My apologies. I think during my testing I accidentally grabbed this version rather than the one with the fix applied, and then correlated recalculations on xfer->len as the culprit for MSB byte copies. I should really have checked the code more thoroughly rather than presuming, and made sure I was running the correct version!

Thanks for taking the time to reply. Very much appreciated. Keep up the good work.

Cheers,
Iain

@mirceacaprioru
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries Iain, glad to be of help.

Cheers,
Mircea

Please sign in to comment.