On 12/04/2016 05:46 PM, mar.krzeminski wrote: > > > W dniu 29.11.2016 o 16:44, Cédric Le Goater pisze: >> When doing fast read, a certain amount of dummy bytes should be sent >> before the read. This number is configurable in the controler CE0 >> Control Register and needs to be modeled using fake transfers the >> flash module. >> >> When the controller is configured for Command mode, the SPI command >> used to do the read is stored in the CE0 control register but, in User >> mode, we need to snoop into the flow of bytes to catch the command. It >> should be the first byte after CS select. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> Reviewed-by: Andrew Jeffery <and...@aj.id.au> >> --- >> hw/ssi/aspeed_smc.c | 58 >> ++++++++++++++++++++++++++++++++++++++------- >> include/hw/ssi/aspeed_smc.h | 1 + >> 2 files changed, 51 insertions(+), 8 deletions(-) >> >> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c >> index 9596ea94a3bc..733fd8b09c06 100644 >> --- a/hw/ssi/aspeed_smc.c >> +++ b/hw/ssi/aspeed_smc.c >> @@ -71,7 +71,9 @@ >> #define R_CTRL0 (0x10 / 4) >> #define CTRL_CMD_SHIFT 16 >> #define CTRL_CMD_MASK 0xff >> +#define CTRL_DUMMY_HIGH_SHIFT 14 >> #define CTRL_AST2400_SPI_4BYTE (1 << 13) >> +#define CTRL_DUMMY_LOW_SHIFT 6 /* 2 bits [7:6] */ >> #define CTRL_CE_STOP_ACTIVE (1 << 2) >> #define CTRL_CMD_MODE_MASK 0x3 >> #define CTRL_READMODE 0x0 >> @@ -151,6 +153,7 @@ >> #define SPI_OP_WRDI 0x04 /* Write disable */ >> #define SPI_OP_RDSR 0x05 /* Read status register */ >> #define SPI_OP_WREN 0x06 /* Write enable */ >> +#define SPI_OP_READ_FAST 0x0b /* Read data bytes (high frequency) */ >> >> /* Used for Macronix and Winbond flashes. */ >> #define SPI_OP_EN4B 0xb7 /* Enter 4-byte mode */ >> @@ -510,6 +513,12 @@ static void aspeed_smc_flash_setup_read(AspeedSMCFlash >> *fl, uint32_t addr) >> /* access can not exceed CS segment */ >> addr = aspeed_smc_check_segment_addr(fl, addr); >> >> + /* >> + * Remember command as we might need to send dummy bytes before >> + * reading data >> + */ >> + fl->cmd = cmd; >> + >> /* TODO: do we have to send 4BYTE each time ? */ >> if (aspeed_smc_flash_is_4byte(fl)) { >> ssi_transfer(s->spi, SPI_OP_EN4B); >> @@ -525,27 +534,50 @@ static void aspeed_smc_flash_setup_read(AspeedSMCFlash >> *fl, uint32_t addr) >> ssi_transfer(s->spi, (addr & 0xff)); >> } >> >> +static int aspeed_smc_flash_dummies(const AspeedSMCFlash *fl) >> +{ >> + AspeedSMCState *s = fl->controller; >> + uint32_t r_ctrl0 = s->regs[s->r_ctrl0 + fl->id]; >> + uint32_t dummy_high = (r_ctrl0 >> CTRL_DUMMY_HIGH_SHIFT) & 0x1; >> + uint32_t dummy_low = (r_ctrl0 >> CTRL_DUMMY_LOW_SHIFT) & 0x3; >> + >> + return ((dummy_high << 2) | dummy_low) * 8; >> +} >> + >> +static uint64_t aspeed_smc_flash_do_read(AspeedSMCFlash *fl, unsigned size) >> +{ >> + AspeedSMCState *s = fl->controller; >> + uint64_t ret = 0; >> + int i; >> + >> + if (fl->cmd == SPI_OP_READ_FAST) { >> + for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) { >> + ssi_transfer(s->spi, 0x0); >> + } >> + } > Generally this is wrong, controller should be not aware of any command in > user mode. > Currently you are forced to do this by m25p80c dummy cycles implementation. > I had no time to improve this since it need to update in Xilinx models, but > maybe it is good place to talk about the solution. > I was thinking to add to ssi function transferN, and use it in flash models. > Firs introduce new state for dummy cycles in m25p80 and then:
This new state would depend on the command op ? fastread would put the object in a dummy_cycle state ? > a) in case caller use ssi_transfer(transfer8 in flsh models) dummy count will > be incremented by 8. > This should solve the problem when flash is connected to controller that is > not aware about dummy cycles, > like this mode or clear SPI controller. > b) when caller use ssi_trasferN length (in bits) will be the number of dummy > cycles. > > What is your opinion? when you have some time, please send a quick rfc patch for the API :) so that I can experiment on the aspeed controller. Thanks, C. >> + fl->cmd = 0; >> + >> + for (i = 0; i < size; i++) { >> + ret |= ssi_transfer(s->spi, 0x0) << (8 * i); >> + } >> + return ret; >> +} >> + >> static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned >> size) >> { >> AspeedSMCFlash *fl = opaque; >> - const AspeedSMCState *s = fl->controller; >> uint64_t ret = 0; >> - int i; >> >> switch (aspeed_smc_flash_mode(fl)) { >> case CTRL_USERMODE: >> - for (i = 0; i < size; i++) { >> - ret |= ssi_transfer(s->spi, 0x0) << (8 * i); >> - } >> + ret = aspeed_smc_flash_do_read(fl, size); >> break; >> case CTRL_READMODE: >> case CTRL_FREADMODE: >> 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); >> - } >> + ret = aspeed_smc_flash_do_read(fl, size); >> >> aspeed_smc_flash_unselect(fl); >> break; >> @@ -596,6 +628,15 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr >> addr, uint64_t data, >> >> switch (aspeed_smc_flash_mode(fl)) { >> case CTRL_USERMODE: >> + /* >> + * First write after chip select is the chip command. Remember >> + * it as we might need to send dummy bytes before reading >> + * data. It will be reseted when the chip is unselected. >> + */ >> + if (!fl->cmd) { >> + fl->cmd = data & 0xff; >> + } >> + >> for (i = 0; i < size; i++) { >> ssi_transfer(s->spi, (data >> (8 * i)) & 0xff); >> } >> @@ -629,6 +670,7 @@ static const MemoryRegionOps aspeed_smc_flash_ops = { >> static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl) >> { >> AspeedSMCState *s = fl->controller; >> + fl->cmd = 0; >> qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); >> } >> >> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h >> index 88a904849801..3ae0a369073d 100644 >> --- a/include/hw/ssi/aspeed_smc.h >> +++ b/include/hw/ssi/aspeed_smc.h >> @@ -52,6 +52,7 @@ typedef struct AspeedSMCFlash { >> >> uint8_t id; >> uint32_t size; >> + uint8_t cmd; >> >> MemoryRegion mmio; >> DeviceState *flash; >