[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST f
From: |
Francisco Iglesias |
Subject: |
Re: [PATCH] hw/block: m25p80: Implement AAI-WP command support for SST flashes |
Date: |
Thu, 3 Dec 2020 13:54:38 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
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
>
>