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

Bin Meng posted 22 patches 4 years, 10 months ago
There is a newer version of this series
[PATCH 01/22] hw/block: m25p80: Add ISSI SPI flash support
Posted by Bin Meng 4 years, 10 months ago
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);
+        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


Re: [PATCH 01/22] hw/block: m25p80: Add ISSI SPI flash support
Posted by Francisco Iglesias 4 years, 10 months ago
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
> 
> 

Re: [PATCH 01/22] hw/block: m25p80: Add ISSI SPI flash support
Posted by Bin Meng 4 years, 10 months ago
Hi Francisco,

On Tue, Jan 5, 2021 at 12:00 AM Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
>
> 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..).
>

I now am more inclined to say something is wrong in the Xilinx SPI
models and I just don't have time to check what is wrong there. As I
mentioned here [1] for SPI controllers that use fifo to stuff the
dummy bits on the line, there is no way for hardware to tell what is
dummy bits in the fifo or not. The hardware just sends whatever in the
fifo which is prepared by the software. Hence the only way to get such
hardware to work is to use bytes, instead of bits in QEMU.

[1] http://patchwork.ozlabs.org/project/qemu-devel/patch/1606704602-59435-1-git-send-email-bmeng.cn@gmail.com/

Regards,
Bin