Attention is currently required from: Arthur Heymans, Ashish Kumar Mishra, Saurabh Mishra, Subrata Banik.
2 comments:
File src/soc/intel/common/block/fast_spi/fast_spi_flash.c:
const uint8_t *byte_ptr = (const uint8_t *)data;
size_t bytes_to_copy;
union {
uint32_t full;
uint8_t bytes[4];
} dword;
for (size_t i = 0; i < len; i += 4) {
dword.full = 0;
bytes_to_copy = (len - i < 4) ? len - i : 4;
for (size_t j = 0; j < bytes_to_copy; j++)
dword.bytes[j] = byte_ptr[i + j];
write32p(ctx->mmio_base + SPIBAR_FDATA(i >> 2), dword.full);
}
I think the HW accepts at most dword transactions, but also word and byte ones. […]
This is what I suggested in CB:81959 but back then I wasn't sure if the hardware can also accept smaller width transactions:
```
static void fill_xfer_fifo(struct fast_spi_flash_ctx *ctx, const uint8_t *data,
size_t len)
{
/* FDATAn requires 32-bit accesses */
const uint32_t *dwords = (const uint32_t *)data;
for (size_t i = 0; i < len / 4; i++)
write32p(ctx->mmio_base + SPIBAR_FDATA(i), dwords[i]);
if (len % 4) {
uint32_t last_dword = 0;
for (size_t i = 0; i < len % 4; i++)
last_dword |= data[len / 4 + i] << (i * 8);
write32p(ctx->mmio_base + SPIBAR_FDATA(len / 4), last_dword);
}
}
```
But if the hardware can tolerate byte-width transactions:
```
static void fill_xfer_fifo(struct fast_spi_flash_ctx *ctx, const uint8_t *data,
size_t len)
{
const uint32_t *dwords = (const uint32_t *)data;
for (size_t i = 0; i < len / 4; i++)
write32p(ctx->mmio_base + SPIBAR_FDATA(i), dwords[i]);
for (size_t i = 0; i < len % 4; i++)
write8p(ctx->mmio_base + SPIBAR_FDATA(len / 4) + i, data[len / 4 + i])
}
```
Maximum code simplicity would be to only use byte reads/writes, but I'm not sure if there's a performance impact:
```
static void fill_xfer_fifo(struct fast_spi_flash_ctx *ctx, const uint8_t *data,
size_t len)
{
for (size_t i = 0; i < len; i++)
write8p(ctx->mmio_base + SPIBAR_FDATA(0) + i, data[i])
}
```
Patch Set #2, Line 99: static void drain_xfer_fifo(struct fast_spi_flash_ctx *ctx, void *dest,
My original suggestion from CB:81959 is:
```
static void drain_xfer_fifo(struct fast_spi_flash_ctx *ctx, uint8_t *dest,
size_t len)
{
/* FDATAn requires 32-bit accesses */
uint32_t *dwords = (uint32_t *)data;
for (size_t i = 0; i < len / 4; i++)
dwords[i] = read32p(ctx->mmio_base + SPIBAR_FDATA(i));
if (len % 4) {
const uint32_t last_dword = read32p(ctx->mmio_base + SPIBAR_FDATA(len / 4));
for (size_t i = 0; i < len % 4; i++)
data[len / 4 + i] = (uint8_t)(last_dword >> (i * 8));
}
}
```
But if the hardware can tolerate byte-width transactions:
```
static void drain_xfer_fifo(struct fast_spi_flash_ctx *ctx, uint8_t *dest,
size_t len)
{
uint32_t *dwords = (uint32_t *)data;
for (size_t i = 0; i < len / 4; i++)
dwords[i] = read32p(ctx->mmio_base + SPIBAR_FDATA(i));
for (size_t i = 0; i < len % 4; i++)
data[len / 4 + i] = read8p(ctx->mmio_base + SPIBAR_FDATA(len / 4) + i);
}
```
Maximum code simplicity would be to only use byte reads/writes, but I'm not sure if there's a performance impact:
```
static void drain_xfer_fifo(struct fast_spi_flash_ctx *ctx, uint8_t *dest,
size_t len)
{
for (size_t i = 0; i < len % 4; i++)
data[i] = read8p(ctx->mmio_base + SPIBAR_FDATA(0) + i);
}
```
To view, visit change 82079. To unsubscribe, or for help writing mail filters, visit settings.