[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 06/11] block: m25p80: Add configuration regis
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v4 06/11] block: m25p80: Add configuration registers |
Date: |
Thu, 17 Mar 2016 10:24:35 -0700 |
On Mon, Feb 22, 2016 at 12:03 AM, <address@hidden> wrote:
> From: Marcin Krzeminski <address@hidden>
>
> 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 <address@hidden>
> ---
> 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?
> + /* 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) ...
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 <address@hidden>
Regards,
Peter
> + /* Reset/hold & reserved */
> + s->enh_volatile_cfg |= (1 << 4);
> + /* Dual I/O protocol */
> + if ((s->nonvolatile_cfg >> 1) & 0x1) {
> + s->enh_volatile_cfg |= (1 << 6);
> + }
> + /* Quad I/O protocol */
> + if ((s->nonvolatile_cfg >> 3) & 0x1) {
> + s->enh_volatile_cfg |= (1 << 7);
> + }
> +
> + if (!(s->nonvolatile_cfg & 0x1)) {
> + s->four_bytes_address_mode = true;
> + }
> +
> + if (!((s->nonvolatile_cfg >> 1) & 0x1)) {
> + s->ear = 0x3;
> + }
> + }
> +
> DB_PRINT_L(0, "Reset done.\n");
> }
>
> @@ -617,6 +675,49 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> s->state = STATE_COLLECTING_DATA;
> }
> break;
> + case RNVCR:
> + s->data[0] = s->nonvolatile_cfg & 0xFF;
> + s->data[1] = (s->nonvolatile_cfg >> 8) & 0xFF;
> + s->pos = 0;
> + s->len = 2;
> + s->state = STATE_READING_DATA;
> + break;
> + case WNVCR:
> + if (s->write_enable) {
> + s->needed_bytes = 2;
> + s->pos = 0;
> + s->len = 0;
> + s->state = STATE_COLLECTING_DATA;
> + }
> + break;
> + case RVCR:
> + s->data[0] = s->volatile_cfg & 0xFF;
> + s->pos = 0;
> + s->len = 1;
> + s->state = STATE_READING_DATA;
> + break;
> + case WVCR:
> + if (s->write_enable) {
> + s->needed_bytes = 1;
> + s->pos = 0;
> + s->len = 0;
> + s->state = STATE_COLLECTING_DATA;
> + }
> + break;
> + case REVCR:
> + s->data[0] = s->enh_volatile_cfg & 0xFF;
> + s->pos = 0;
> + s->len = 1;
> + s->state = STATE_READING_DATA;
> + break;
> + case WEVCR:
> + if (s->write_enable) {
> + s->needed_bytes = 1;
> + s->pos = 0;
> + s->len = 0;
> + s->state = STATE_COLLECTING_DATA;
> + }
> + break;
> case RESET_ENABLE:
> s->reset_enable = true;
> break;
> @@ -738,6 +839,11 @@ static void m25p80_pre_save(void *opaque)
> flash_sync_dirty((Flash *)opaque, -1);
> }
>
> +static Property m25p80_properties[] = {
> + DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static const VMStateDescription vmstate_m25p80 = {
> .name = "xilinx_spi",
> .version_id = 2,
> @@ -755,6 +861,9 @@ static const VMStateDescription vmstate_m25p80 = {
> VMSTATE_BOOL(four_bytes_address_mode, Flash),
> VMSTATE_UINT8(ear, Flash),
> VMSTATE_BOOL(reset_enable, Flash),
> + VMSTATE_UINT32(nonvolatile_cfg, Flash),
> + VMSTATE_UINT32(volatile_cfg, Flash),
> + VMSTATE_UINT32(enh_volatile_cfg, Flash),
> VMSTATE_END_OF_LIST()
> }
> };
> @@ -770,6 +879,7 @@ static void m25p80_class_init(ObjectClass *klass, void
> *data)
> k->set_cs = m25p80_cs;
> k->cs_polarity = SSI_CS_LOW;
> dc->vmsd = &vmstate_m25p80;
> + dc->props = m25p80_properties;
> mc->pi = data;
> }
>
> --
> 2.5.0
>
- Re: [Qemu-devel] [PATCH v4 06/11] block: m25p80: Add configuration registers,
Peter Crosthwaite <=