Re: [PATCH 01/22] hw/block: m25p80: Add ISSI SPI flash support

[Francisco Iglesias]

Hi Bin,

On [2020 Dec 31] Thu 19:29:49, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> This adds the ISSI SPI flash support. The number of dummy cycles in
> fast read, fast read dual output and fast read quad output commands
> is currently using the default 8. Per the datasheet [1], the number
> of dummy cycles configurable, but this is not modeled.
> 
> For flash whose size is larger than 16 MiB, the sequence of 3-byte
> address along with EXTADD bit in the bank address register (BAR) is
> not supported. Currently we assume that guest software will alawys
> use op codes with 4-byte address sequence. Fortunately this is the
> case for both U-Boot and Linux.
> 
> [1] http://www.issi.com/WW/pdf/25LP-WP256.pdf
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>  hw/block/m25p80.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 844cabea21..8a62bc4bc4 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -411,6 +411,7 @@ typedef enum {
>      MAN_NUMONYX,
>      MAN_WINBOND,
>      MAN_SST,
> +    MAN_ISSI,
>      MAN_GENERIC,
>  } Manufacturer;
>  
> @@ -486,6 +487,8 @@ static inline Manufacturer get_man(Flash *s)
>          return MAN_MACRONIX;
>      case 0xBF:
>          return MAN_SST;
> +    case 0x9D:
> +        return MAN_ISSI;
>      default:
>          return MAN_GENERIC;
>      }
> @@ -705,6 +708,9 @@ static void complete_collecting_data(Flash *s)
>          case MAN_SPANSION:
>              s->quad_enable = !!(s->data[1] & 0x02);
>              break;
> +        case MAN_ISSI:
> +            s->quad_enable = extract32(s->data[0], 6, 1);
> +            break;
>          case MAN_MACRONIX:
>              s->quad_enable = extract32(s->data[0], 6, 1);
>              if (s->len > 1) {
> @@ -897,6 +903,16 @@ static void decode_fast_read_cmd(Flash *s)
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
>          break;
> +    case MAN_ISSI:
> +        /*
> +         * The fast read instruction code is followed by address bytes and
> +         * dummy cycles, transmitted via the SI line.
> +         *
> +         * The number of dummy cycles are configurable but this is currently
> +         * unmodeled, hence the default value 8 is used.
> +         */
> +        s->needed_bytes += ((8 * 1) / 8);

According to how m25p80 models dummy clock cycles above
means that the command is being modeled with 1 dummy clock cycle (and below is
modeling the dio/qio commands with 1 and 3 dummy clock cycles). To model
the command with 8 dummy clock cycles you only add +8 above (+4 and +6
would be the values to add below). One can look into how one of the other
flashes model the commands for examples. This might also mean that the
controller will need a change and do the opposite what above calculation
does, and convert the dummy bytes into dummy clock cycles (when
transmitting on 1 line it generates 8 dummy clock cycles for each dummy
byte, when it uses 2 lines it generates 4 etc..).

Best regards,
Francisco Iglesias

> +        break;
>      default:
>          break;
>      }
> @@ -936,6 +952,16 @@ static void decode_dio_read_cmd(Flash *s)
>              break;
>          }
>          break;
> +    case MAN_ISSI:
> +        /*
> +         * The fast read dual I/O instruction code is followed by address 
> bytes
> +         * and dummy cycles, transmitted via the IO1 and IO0 line.
> +         *
> +         * The number of dummy cycles are configurable but this is currently
> +         * unmodeled, hence the default value 4 is used.
> +         */
> +        s->needed_bytes += ((4 * 2) / 8);
> +        break;
>      default:
>          break;
>      }
> @@ -976,6 +1002,16 @@ static void decode_qio_read_cmd(Flash *s)
>              break;
>          }
>          break;
> +    case MAN_ISSI:
> +        /*
> +         * The fast read quad I/O instruction code is followed by address 
> bytes
> +         * and dummy cycles, transmitted via the IO3, IO2, IO1 and IO0 line.
> +         *
> +         * The number of dummy cycles are configurable but this is currently
> +         * unmodeled, hence the default value 6 is used.
> +         */
> +        s->needed_bytes += ((6 * 4) / 8);
> +        break;
>      default:
>          break;
>      }
> @@ -1134,7 +1170,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>  
>      case RDSR:
>          s->data[0] = (!!s->write_enable) << 1;
> -        if (get_man(s) == MAN_MACRONIX) {
> +        if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
>              s->data[0] |= (!!s->quad_enable) << 6;
>          }
>          if (get_man(s) == MAN_SST) {
> -- 
> 2.25.1
> 
>