W dniu 17.03.2016 o 18:24, Peter Crosthwaite pisze: > On Mon, Feb 22, 2016 at 12:03 AM, <marcin.krzemin...@nokia.com> wrote: >> From: Marcin Krzeminski <marcin.krzemin...@nokia.com> >> >> This patch adds both volatile and non volatile configuration registers >> and commands to allow modify them. It is needed for proper handling >> dummy cycles. Initialization of those registers and flash state >> has been included as well. >> Some of this registers are used by kernel. >> >> Signed-off-by: Marcin Krzeminski <marcin.krzemin...@nokia.com> >> --- >> hw/block/m25p80.c | 110 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 110 insertions(+) >> >> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c >> index 0698e7b..9d5a071 100644 >> --- a/hw/block/m25p80.c >> +++ b/hw/block/m25p80.c >> @@ -26,6 +26,7 @@ >> #include "sysemu/block-backend.h" >> #include "sysemu/blockdev.h" >> #include "hw/ssi/ssi.h" >> +#include "qemu/bitops.h" >> >> #ifndef M25P80_ERR_DEBUG >> #define M25P80_ERR_DEBUG 0 >> @@ -245,6 +246,15 @@ typedef enum { >> >> RESET_ENABLE = 0x66, >> RESET_MEMORY = 0x99, >> + >> + RNVCR = 0xB5, >> + WNVCR = 0xB1, >> + >> + RVCR = 0x85, >> + WVCR = 0x81, >> + >> + REVCR = 0x65, >> + WEVCR = 0x61, >> } FlashCMD; >> >> typedef enum { >> @@ -271,6 +281,9 @@ typedef struct Flash { >> uint8_t needed_bytes; >> uint8_t cmd_in_progress; >> uint64_t cur_addr; >> + uint32_t nonvolatile_cfg; >> + uint32_t volatile_cfg; >> + uint32_t enh_volatile_cfg; >> bool write_enable; >> bool four_bytes_address_mode; >> bool reset_enable; >> @@ -459,6 +472,15 @@ static void complete_collecting_data(Flash *s) >> case EXTEND_ADDR_WRITE: >> s->ear = s->data[0]; >> break; >> + case WNVCR: >> + s->nonvolatile_cfg = s->data[0] | (s->data[1] << 8); >> + break; >> + case WVCR: >> + s->volatile_cfg = s->data[0]; >> + break; >> + case WEVCR: >> + s->enh_volatile_cfg = s->data[0]; >> + break; >> default: >> break; >> } >> @@ -477,6 +499,42 @@ static void reset_memory(Flash *s) >> s->write_enable = false; >> s->reset_enable = false; >> >> + if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) { >> + s->volatile_cfg = 0; >> + /* WRAP & reserved*/ > > These bitfield masks and their values need some macrofication. Then > the comments explaining what bits are what can be dropped. > >> + s->volatile_cfg |= 0x3; > > Why you set a reserved bit? >From datasheet: The device ships from the factory with all bits erased to 1 (FFFFh). > > >> + /* XIP */ >> + if (extract32(s->nonvolatile_cfg, 9, 3) != 0x7) { > > An example, > #define NVCFG_XIP_MODE_DISABLED 0x7 > >> + s->volatile_cfg |= (1 << 3); >> + } >> + /* Number of dummy cycles */ >> + s->volatile_cfg |= deposit32(s->volatile_cfg, >> + 4, 4, extract32(s->nonvolatile_cfg, 12, 4)); >> + s->enh_volatile_cfg = 0; >> + /* Output driver strength */ >> + s->enh_volatile_cfg |= 0x7; >> + /* Vpp accelerator */ >> + s->enh_volatile_cfg |= (1 << 3); > > #define EVCFG_VPP_ACCELERATOR (1 << 3) ... Macros here is a good idea. I will add them in v5.
Thanks, Marcin > > > I am aware of my unreliability to get the review timely but the patch > intent looks good, > > so with the macroification changes (globally to this function): > > Acked-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> > > Regards, > Peter