|
| From: | mar.krzeminski |
| Subject: | Re: [Qemu-devel] [PATCH for-2.9 17/30] aspeed/smc: handle SPI flash Command mode |
| Date: | Mon, 5 Dec 2016 16:33:36 +0100 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
W dniu 05.12.2016 o 15:07, Cédric Le Goater pisze:
On 12/04/2016 05:31 PM, mar.krzeminski wrote:Hi Cedric, Since there is no public datasheet user guide for SMC I would ask some question regarding HW itself because I got impression that you are implementing in this model a part functionality that is done by Bootrom. W dniu 29.11.2016 o 16:43, Cédric Le Goater pisze:The Aspeed SMC controllers have a mode (Command mode) in which accesses to the flash content are no different than doing MMIOs. The controller generates all the necessary commands to load (or store) data in memory. However, accesses are restricted to the segment window assigned the the flash module by the controller. This window is defined by the Segment Address Register. Signed-off-by: Cédric Le Goater <address@hidden> Reviewed-by: Andrew Jeffery <address@hidden> --- hw/ssi/aspeed_smc.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 162 insertions(+), 12 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 72a44150b0a1..eec087199a22 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -69,6 +69,7 @@ #define R_CTRL0 (0x10 / 4) #define CTRL_CMD_SHIFT 16 #define CTRL_CMD_MASK 0xff +#define CTRL_AST2400_SPI_4BYTE (1 << 13) #define CTRL_CE_STOP_ACTIVE (1 << 2) #define CTRL_CMD_MODE_MASK 0x3 #define CTRL_READMODE 0x0 @@ -135,6 +136,16 @@ #define ASPEED_SOC_SPI_FLASH_BASE 0x30000000 #define ASPEED_SOC_SPI2_FLASH_BASE 0x38000000+/* Flash opcodes. */+#define SPI_OP_READ 0x03 /* Read data bytes (low frequency) */ +#define SPI_OP_WRDI 0x04 /* Write disable */ +#define SPI_OP_RDSR 0x05 /* Read status register */ +#define SPI_OP_WREN 0x06 /* Write enable */Are you sure that the controller is aware af all above commands (especially WD/WE and RDS)?HW is aware of SPI_OP_READ which is the default command for the "normal" read mode. For other modes, fast and write, the command op is configured in the CEx Control Register. These ops are used in the model : * SPI_OP_READ_FAST, for dummies * SPI_OP_EN4B, might be useless if we expect software to send this command before using this mode. * SPI_OP_WREN, same comment. The rest I should remove as it is unused.
I think only SPI_OP_READ should stay in the model, rest goes to guest.
I believe this check is not needed at all because m25p80c will tell if it has+ +/* Used for Macronix and Winbond flashes. */ +#define SPI_OP_EN4B 0xb7 /* Enter 4-byte mode */ +#define SPI_OP_EX4B 0xe9 /* Exit 4-byte mode */ +Same as above but I think 4byte address mode bit changes onlu nymber of bytes that is sent after instruction./* * Default segments mapping addresses and size for each slave per * controller. These can be changed when board is initialized with the @@ -357,6 +368,98 @@ static inline bool aspeed_smc_is_writable(const AspeedSMCFlash *fl) return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + fl->id)); }+static inline int aspeed_smc_flash_cmd(const AspeedSMCFlash *fl)+{ + AspeedSMCState *s = fl->controller; + int cmd = (s->regs[s->r_ctrl0 + fl->id] >> CTRL_CMD_SHIFT) & CTRL_CMD_MASK; + + /* This is the default value for read mode. In other modes, the + * command should be defined */ + if (aspeed_smc_flash_mode(fl) == CTRL_READMODE) { + cmd = SPI_OP_READ; + } + + if (!cmd) {cmd == 0 => NOP command for some flash (eg. mx66l1g45g).So I should use another default value, OxFF ?
unsupported instruction and HW should accept all register values, isn't it?
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: no command defined for mode %d\n", + __func__, aspeed_smc_flash_mode(fl)); + } + + return cmd; +} + +static inline int aspeed_smc_flash_is_4byte(const AspeedSMCFlash *fl) +{ + AspeedSMCState *s = fl->controller; + + if (s->ctrl->segments == aspeed_segments_spi) { + return s->regs[s->r_ctrl0] & CTRL_AST2400_SPI_4BYTE; + } else { + return s->regs[s->r_ce_ctrl] & (1 << (CTRL_EXTENDED0 + fl->id)); + } +} + +static void aspeed_smc_flash_select(const AspeedSMCFlash *fl) +{ + AspeedSMCState *s = fl->controller; + + s->regs[s->r_ctrl0 + fl->id] &= ~CTRL_CE_STOP_ACTIVE; + qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); +} + +static void aspeed_smc_flash_unselect(const AspeedSMCFlash *fl) +{ + AspeedSMCState *s = fl->controller; + + s->regs[s->r_ctrl0 + fl->id] |= CTRL_CE_STOP_ACTIVE; + qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); +} + +static uint32_t aspeed_smc_check_segment_addr(AspeedSMCFlash *fl, uint32_t addr) +{ + AspeedSMCState *s = fl->controller; + AspeedSegments seg; + + aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + fl->id], &seg); + if ((addr & (seg.size - 1)) != addr) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid address 0x%08x for CS%d segment : " + "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n", + s->ctrl->name, addr, fl->id, seg.addr, + seg.addr + seg.size); + } + + addr &= seg.size - 1; + return addr; +} + +static void aspeed_smc_flash_setup_read(AspeedSMCFlash *fl, uint32_t addr) +{ + AspeedSMCState *s = fl->controller; + uint8_t cmd = aspeed_smc_flash_cmd(fl); + + /* + * To be checked: I am not sure the Aspeed SPI controller needs to + * enable writes when running in READ/FREAD command mode + */ + + /* access can not exceed CS segment */ + addr = aspeed_smc_check_segment_addr(fl, addr); + + /* TODO: do we have to send 4BYTE each time ? */I am not aware of any flash that needs that, this command should be send only once.yes. That is what I think also. it also means that a preliminary 4BYTE command should be sent before using that mode.
Generally there are two ways to access more than 16MiB in flash:- 4byte address mode: all commands change to accept 4byte address, not supported by all flash devices. - 4byte opcodes: different opcode is used to signal 4-byte long address (eg. 0x3 three bytes, 0x13 four). Also not all flash support that. If the HW does not use any of those, ones should be removed from this model.
I also think that HW does not send this command (see above comment).+ if (aspeed_smc_flash_is_4byte(fl)) { + ssi_transfer(s->spi, SPI_OP_EN4B); + } + + ssi_transfer(s->spi, cmd); + + if (aspeed_smc_flash_is_4byte(fl)) { + ssi_transfer(s->spi, (addr >> 24) & 0xff); + } + ssi_transfer(s->spi, (addr >> 16) & 0xff); + ssi_transfer(s->spi, (addr >> 8) & 0xff); + ssi_transfer(s->spi, (addr & 0xff)); +} + static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) { AspeedSMCFlash *fl = opaque; @@ -364,19 +467,55 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) uint64_t ret = 0; int i;- if (aspeed_smc_is_usermode(fl)) {+ switch (aspeed_smc_flash_mode(fl)) { + case CTRL_USERMODE: for (i = 0; i < size; i++) { ret |= ssi_transfer(s->spi, 0x0) << (8 * i); } - } else { - qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n", - __func__); - ret = -1; + break; + case CTRL_READMODE: + case CTRL_FREADMODE:CTRL_FREADMODE should not sent dummy bytes?yes it should. this is in a following patch.
Yes, I noticed that after sending this email :) Thanks, Marcin
Thanks, C.Thanks, Marcin+ aspeed_smc_flash_select(fl); + aspeed_smc_flash_setup_read(fl, addr); + + for (i = 0; i < size; i++) { + ret |= ssi_transfer(s->spi, 0x0) << (8 * i); + } + + aspeed_smc_flash_unselect(fl); + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid flash mode %d\n", + __func__, aspeed_smc_flash_mode(fl)); }return ret;}+static void aspeed_smc_flash_setup_write(AspeedSMCFlash *fl, uint32_t addr)+{ + AspeedSMCState *s = fl->controller; + uint8_t cmd = aspeed_smc_flash_cmd(fl); + + /* Flash access can not exceed CS segment */ + addr = aspeed_smc_check_segment_addr(fl, addr); + + /* TODO: do we have to send 4BYTE each time ? */ + if (aspeed_smc_flash_is_4byte(fl)) { + ssi_transfer(s->spi, SPI_OP_EN4B); + } + + ssi_transfer(s->spi, SPI_OP_WREN); + ssi_transfer(s->spi, cmd); + + if (aspeed_smc_flash_is_4byte(fl)) { + ssi_transfer(s->spi, (addr >> 24) & 0xff); + } + ssi_transfer(s->spi, (addr >> 16) & 0xff); + ssi_transfer(s->spi, (addr >> 8) & 0xff); + ssi_transfer(s->spi, (addr & 0xff)); +} + static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, unsigned size) { @@ -390,14 +529,25 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, return; }- if (!aspeed_smc_is_usermode(fl)) {- qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n", - __func__); - return; - } + switch (aspeed_smc_flash_mode(fl)) { + case CTRL_USERMODE: + for (i = 0; i < size; i++) { + ssi_transfer(s->spi, (data >> (8 * i)) & 0xff); + } + break; + case CTRL_WRITEMODE: + aspeed_smc_flash_select(fl); + aspeed_smc_flash_setup_write(fl, addr); + + for (i = 0; i < size; i++) { + ssi_transfer(s->spi, (data >> (8 * i)) & 0xff); + }- for (i = 0; i < size; i++) {- ssi_transfer(s->spi, (data >> (8 * i)) & 0xff); + aspeed_smc_flash_unselect(fl); + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid flash mode %d\n", + __func__, aspeed_smc_flash_mode(fl)); } }
| [Prev in Thread] | Current Thread | [Next in Thread] |