[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with funct
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function. |
Date: |
Wed, 15 Jun 2016 16:05:50 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.8.0 |
On 06/15/2016 03:41 PM, address@hidden wrote:
> From: Marcin Krzeminski <address@hidden>
>
> Instead of always reading and comparing jededc ID,
> replace it by function.
>
> Signed-off-by: Marcin Krzeminski <address@hidden>
Looks good to me. Some minor comments below.
Thanks,
C.
> ---
> hw/block/m25p80.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 40 insertions(+), 9 deletions(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 4c856f5..15765f5 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -307,6 +307,14 @@ typedef enum {
> STATE_READING_DATA,
> } CMDState;
>
> +typedef enum {
> + MAN_SPANSION,
> + MAN_MACRONIX,
> + MAN_NUMONYX,
> + MAN_WINBOND,
> + MAN_GENERIC,
> +} Manufacturer;
> +
> typedef struct Flash {
> SSISlave parent_obj;
>
> @@ -350,6 +358,22 @@ typedef struct M25P80Class {
> #define M25P80_GET_CLASS(obj) \
> OBJECT_GET_CLASS(M25P80Class, (obj), TYPE_M25P80)
>
> +static inline Manufacturer get_man(Flash *s)
> +{
> + switch (((s->pi->jedec >> 16) & 0xFF)) {
> + case 0x20:
> + return MAN_NUMONYX;
> + case 0xEF:
> + return MAN_WINBOND;
> + case 0x01:
> + return MAN_SPANSION;
> + case 0xC2:
> + return MAN_MACRONIX;
> + default:
> + return MAN_GENERIC;
> + }
> +}
> +
> static void blk_sync_complete(void *opaque, int ret)
> {
> /* do nothing. Masters do not directly interact with the backing store,
> @@ -562,7 +586,8 @@ static void reset_memory(Flash *s)
> s->write_enable = false;
> s->reset_enable = false;
>
> - if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
We could have kept the if ()
> + switch (get_man(s)) {
> + case MAN_NUMONYX:
> s->volatile_cfg = 0;
> s->volatile_cfg |= VCFG_DUMMY;
> s->volatile_cfg |= VCFG_WRAP_SEQUENTIAL;
> @@ -594,6 +619,9 @@ static void reset_memory(Flash *s)
> if (!(s->nonvolatile_cfg & NVCFG_LOWER_SEGMENT_MASK)) {
> s->ear = CFG_UPPER_128MB_SEG_ENABLED;
> }
> + break;
> + default:
> + break;
> }
>
> DB_PRINT_L(0, "Reset done.\n");
> @@ -634,9 +662,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> case QOR:
> case QOR4:
> s->needed_bytes = get_addr_length(s);
> - if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
> - /* Dummy cycles modeled with bytes writes instead of bits */
why remove the comment ?
> + switch (get_man(s)) {
> + case MAN_NUMONYX:
> s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> + break;
> + default:
> + break;
> }
> s->pos = 0;
> s->len = 0;
> @@ -645,9 +676,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>
> case DIOR:
> case DIOR4:
> - switch ((s->pi->jedec >> 16) & 0xFF) {
> - case JEDEC_WINBOND:
> - case JEDEC_SPANSION:
> + switch (get_man(s)) {
> + case MAN_WINBOND:
> + case MAN_SPANSION:
> s->needed_bytes = 4;
> break;
> default:
> @@ -662,9 +693,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>
> case QIOR:
> case QIOR4:
> - switch ((s->pi->jedec >> 16) & 0xFF) {
> - case JEDEC_WINBOND:
> - case JEDEC_SPANSION:
> + switch (get_man(s)) {
> + case MAN_WINBOND:
> + case MAN_SPANSION:
> s->needed_bytes = 6;
> break;
> default:
>
- [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state., (continued)
- [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID., marcin.krzeminski, 2016/06/15
- [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes., marcin.krzeminski, 2016/06/15
- [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function., marcin.krzeminski, 2016/06/15
- Re: [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function.,
Cédric Le Goater <=
- [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers., marcin.krzeminski, 2016/06/15
[Qemu-devel] [PATCH 9/9] m25p80: New flash devices., marcin.krzeminski, 2016/06/15