Re: [Qemu-devel] [PATCH] hw/m25p80.c: add WRSR(0x01) support

[Peter Crosthwaite]

Hi Dante,

On Fri, Jan 25, 2013 at 5:25 PM, Peter Crosthwaite
<address@hidden> wrote:
> Hi Dante,
>
> Sorry about the delay, and thanks for the contribution. Please run the
> patch through the provided checkpatch script (scripts/checkpatch.pl).
> There are a few whitespace errors that need to be fixed. Please also
> see the comment below.
>
> Regards,
> Peter
>
> On Thu, Jan 17, 2013 at 10:17 PM, Dante <address@hidden> wrote:
>> Atmel, SST and Intel/Numonyx serial flash tend to power up
>> with the software protection bits set.
>> And thus the new m25p80.c in linux kernel would always tries
>> to use WREN(0x06) + WRSR(0x01) to turn-off the protection.
>> The WEL(0x02) of status register is supposed to be cleared
>> after WRSR(0x01).
>> There are some drivers (i.e my own tiny driver for RTOSes) would
>> check the WEL(0x02) in status register to make sure the protection
>> is correctly turned off, so this patch is mandatory to me.
>>
>> Signed-off-by: Kuo-Jung Su <address@hidden>
>> ---
>>  hw/m25p80.c |   16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/hw/m25p80.c b/hw/m25p80.c
>> index d392656..c8d0411 100644
>> --- a/hw/m25p80.c
>> +++ b/hw/m25p80.c
>> @@ -184,6 +184,7 @@ static const FlashPartInfo known_devices[] = {
>>
>>  typedef enum {
>>      NOP = 0,
>> +    WRSR = 0x1,
>>      WRDI = 0x4,
>>      RDSR = 0x5,
>>      WREN = 0x6,
>> @@ -377,6 +378,12 @@ static void complete_collecting_data(Flash *s)
>>      case ERASE_SECTOR:
>>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>>          break;
>> +    case WRSR:
>> +       if (s->write_enable) {
>> +               s->state = STATE_IDLE;
>
> Returning to the idle state should be unconditional, if
> (!s->write_enable), then if you issue a WRSR, you will get stuck in
> COLLECTING_DATA.
>

Reviewing this a little more, transitioning back to IDLE is handled by
the de-assertion of the chip-select. So you should by able to just
remove this s->state = STATE_IDLE altogether.

Regards,
Peter

> Regards,
> Peter
>
>> +               s->write_enable = false;
>> +       }
>> +       break;
>>      default:
>>          break;
>>      }
>> @@ -440,6 +447,15 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>          s->len = 0;
>>          s->state = STATE_COLLECTING_DATA;
>>          break;
>> +
>> +       case WRSR:
>> +               if (s->write_enable) {
>> +                       s->needed_bytes = 1;
>> +                       s->pos = 0;
>> +               s->len = 0;
>> +                       s->state = STATE_COLLECTING_DATA;
>> +               }
>> +        break;
>>
>>      case WRDI:
>>          s->write_enable = false;
>> --
>> 1.7.9.5
>>
>>
>> ********************* Confidentiality Notice ************************
>> This electronic message and any attachments may contain
>> confidential and legally privileged information or
>> information which is otherwise protected from disclosure.
>> If you are not the intended recipient,please do not disclose
>> the contents, either in whole or in part, to anyone,and
>> immediately delete the message and any attachments from
>> your computer system and destroy all hard copies.
>> Thank you for your cooperation.
>> ***********************************************************************
>>