All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes
@ 2020-12-02 14:30 Bin Meng
  2020-12-03 12:54 ` Francisco Iglesias
  0 siblings, 1 reply; 3+ messages in thread
From: Bin Meng @ 2020-12-02 14:30 UTC (permalink / raw)
  To: Alistair Francis, Kevin Wolf, Max Reitz, qemu-block, qemu-devel
  Cc: Xuzhou Cheng, Bin Meng

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

Auto Address Increment (AAI) Word-Program is a special command of
SST flashes. AAI-WP allows multiple bytes of data to be programmed
without re-issuing the next sequential address location.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 hw/block/m25p80.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 9b36762df9..f225d9c96d 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -359,6 +359,7 @@ typedef enum {
     QPP_4 = 0x34,
     RDID_90 = 0x90,
     RDID_AB = 0xab,
+    AAI_WP = 0xad,
 
     ERASE_4K = 0x20,
     ERASE4_4K = 0x21,
@@ -449,6 +450,7 @@ struct Flash {
     bool four_bytes_address_mode;
     bool reset_enable;
     bool quad_enable;
+    bool aai_enable;
     uint8_t ear;
 
     int64_t dirty_page;
@@ -661,6 +663,7 @@ static void complete_collecting_data(Flash *s)
     case PP:
     case PP4:
     case PP4_4:
+    case AAI_WP:
         s->state = STATE_PAGE_PROGRAM;
         break;
     case READ:
@@ -1010,6 +1013,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case WRDI:
         s->write_enable = false;
+        if (get_man(s) == MAN_SST) {
+            s->aai_enable = false;
+        }
         break;
     case WREN:
         s->write_enable = true;
@@ -1162,6 +1168,17 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case RSTQIO:
         s->quad_enable = false;
         break;
+    case AAI_WP:
+        if (get_man(s) == MAN_SST && s->write_enable) {
+            if (s->aai_enable) {
+                s->state = STATE_PAGE_PROGRAM;
+            } else {
+                s->aai_enable = true;
+                s->needed_bytes = get_addr_length(s);
+                s->state = STATE_COLLECTING_DATA;
+            }
+        }
+        break;
     default:
         s->pos = 0;
         s->len = 1;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes
  2020-12-02 14:30 [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
@ 2020-12-03 12:54 ` Francisco Iglesias
  2020-12-11  6:13   ` Bin Meng
  0 siblings, 1 reply; 3+ messages in thread
From: Francisco Iglesias @ 2020-12-03 12:54 UTC (permalink / raw)
  To: Bin Meng
  Cc: Kevin Wolf, Bin Meng, qemu-block, Xuzhou Cheng, Alistair Francis,
	qemu-devel, Max Reitz

Hello Bin,

On [2020 Dec 02] Wed 22:30:37, Bin Meng wrote:
> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> 
> Auto Address Increment (AAI) Word-Program is a special command of
> SST flashes. AAI-WP allows multiple bytes of data to be programmed
> without re-issuing the next sequential address location.
> 
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>  hw/block/m25p80.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 9b36762df9..f225d9c96d 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -359,6 +359,7 @@ typedef enum {
>      QPP_4 = 0x34,
>      RDID_90 = 0x90,
>      RDID_AB = 0xab,
> +    AAI_WP = 0xad,
>  
>      ERASE_4K = 0x20,
>      ERASE4_4K = 0x21,
> @@ -449,6 +450,7 @@ struct Flash {
>      bool four_bytes_address_mode;
>      bool reset_enable;
>      bool quad_enable;
> +    bool aai_enable;

We need to add above addition also into the vmstate.

>      uint8_t ear;
>  
>      int64_t dirty_page;
> @@ -661,6 +663,7 @@ static void complete_collecting_data(Flash *s)
>      case PP:
>      case PP4:
>      case PP4_4:
> +    case AAI_WP:
>          s->state = STATE_PAGE_PROGRAM;
>          break;
>      case READ:
> @@ -1010,6 +1013,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)

Since only 3 cmds are allowed while within AAI programming sequence [1] I think
a warning migt be good have before the command switch case, similar to:

if (get_man(s) == MAN_SST && s->aai_enable && !is_valid_aai_cmd(value)) {
    qemu_log_mask(LOG_GUEST_ERROR,
                  "M25P80: Invalid cmd within AAI programming sequence");
}

>  
>      case WRDI:
>          s->write_enable = false;
> +        if (get_man(s) == MAN_SST) {
> +            s->aai_enable = false;
> +        }
>          break;
>      case WREN:
>          s->write_enable = true;
> @@ -1162,6 +1168,17 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case RSTQIO:
>          s->quad_enable = false;
>          break;
> +    case AAI_WP:
> +        if (get_man(s) == MAN_SST && s->write_enable) {
> +            if (s->aai_enable) {
> +                s->state = STATE_PAGE_PROGRAM;
> +            } else {
> +                s->aai_enable = true;
> +                s->needed_bytes = get_addr_length(s);
> +                s->state = STATE_COLLECTING_DATA;
> +            }

Perhaps a qemu_log_mask in an 'else' could be useful here also:

} else {
    qemu_log_mask(LOG_GUEST_ERROR, "M25P80: %s"
                  (get_man(s) == MAN_SST) ? "AAI_WP with write protect" :
                                            "Unknown CMD: 0xAD\n");

Lastly, [1] also says that the address shouldn't wrapp around when in AAI mode,
so we need a check before doing that also I think. 

Best regards,
Francisco Iglesias

[1] http://ww1.microchip.com/downloads/en/devicedoc/s71271_04.pdf

> +        }
> +        break;
>      default:
>          s->pos = 0;
>          s->len = 1;
> -- 
> 2.25.1
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes
  2020-12-03 12:54 ` Francisco Iglesias
@ 2020-12-11  6:13   ` Bin Meng
  0 siblings, 0 replies; 3+ messages in thread
From: Bin Meng @ 2020-12-11  6:13 UTC (permalink / raw)
  To: Francisco Iglesias
  Cc: Kevin Wolf, Bin Meng, Qemu-block, Xuzhou Cheng, Alistair Francis,
	qemu-devel@nongnu.org Developers, Max Reitz

Hi Francisco,

On Thu, Dec 3, 2020 at 8:54 PM Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
>
> Hello Bin,
>
> On [2020 Dec 02] Wed 22:30:37, Bin Meng wrote:
> > From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> >
> > Auto Address Increment (AAI) Word-Program is a special command of
> > SST flashes. AAI-WP allows multiple bytes of data to be programmed
> > without re-issuing the next sequential address location.
> >
> > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  hw/block/m25p80.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> > index 9b36762df9..f225d9c96d 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -359,6 +359,7 @@ typedef enum {
> >      QPP_4 = 0x34,
> >      RDID_90 = 0x90,
> >      RDID_AB = 0xab,
> > +    AAI_WP = 0xad,
> >
> >      ERASE_4K = 0x20,
> >      ERASE4_4K = 0x21,
> > @@ -449,6 +450,7 @@ struct Flash {
> >      bool four_bytes_address_mode;
> >      bool reset_enable;
> >      bool quad_enable;
> > +    bool aai_enable;
>
> We need to add above addition also into the vmstate.
>
> >      uint8_t ear;
> >
> >      int64_t dirty_page;
> > @@ -661,6 +663,7 @@ static void complete_collecting_data(Flash *s)
> >      case PP:
> >      case PP4:
> >      case PP4_4:
> > +    case AAI_WP:
> >          s->state = STATE_PAGE_PROGRAM;
> >          break;
> >      case READ:
> > @@ -1010,6 +1013,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>
> Since only 3 cmds are allowed while within AAI programming sequence [1] I think
> a warning migt be good have before the command switch case, similar to:
>
> if (get_man(s) == MAN_SST && s->aai_enable && !is_valid_aai_cmd(value)) {
>     qemu_log_mask(LOG_GUEST_ERROR,
>                   "M25P80: Invalid cmd within AAI programming sequence");
> }
>
> >
> >      case WRDI:
> >          s->write_enable = false;
> > +        if (get_man(s) == MAN_SST) {
> > +            s->aai_enable = false;
> > +        }
> >          break;
> >      case WREN:
> >          s->write_enable = true;
> > @@ -1162,6 +1168,17 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> >      case RSTQIO:
> >          s->quad_enable = false;
> >          break;
> > +    case AAI_WP:
> > +        if (get_man(s) == MAN_SST && s->write_enable) {
> > +            if (s->aai_enable) {
> > +                s->state = STATE_PAGE_PROGRAM;
> > +            } else {
> > +                s->aai_enable = true;
> > +                s->needed_bytes = get_addr_length(s);
> > +                s->state = STATE_COLLECTING_DATA;
> > +            }
>
> Perhaps a qemu_log_mask in an 'else' could be useful here also:
>
> } else {
>     qemu_log_mask(LOG_GUEST_ERROR, "M25P80: %s"
>                   (get_man(s) == MAN_SST) ? "AAI_WP with write protect" :
>                                             "Unknown CMD: 0xAD\n");
>
> Lastly, [1] also says that the address shouldn't wrapp around when in AAI mode,
> so we need a check before doing that also I think.
>
> Best regards,
> Francisco Iglesias
>
> [1] http://ww1.microchip.com/downloads/en/devicedoc/s71271_04.pdf

Thanks for the review. All of your comments will be addressed in v2.

Regards,
Bin


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-11  6:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 14:30 [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
2020-12-03 12:54 ` Francisco Iglesias
2020-12-11  6:13   ` Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.