Re: [Qemu-devel] [PULL 24/27] hw/block/pflash_cfi02: Implement erase suspend/resume

[Philippe Mathieu-Daudé]

On 7/11/19 2:35 PM, Peter Maydell wrote:
> On Tue, 2 Jul 2019 at 04:29, Philippe Mathieu-Daudé <address@hidden> wrote:
>>
>> From: Stephen Checkoway <address@hidden>
>>
>> During a sector erase (but not a chip erase), the embeded erase program
>> can be suspended. Once suspended, the sectors not selected for erasure
>> may be read and programmed. Autoselect mode is allowed during erase
>> suspend mode. Presumably, CFI queries are similarly allowed so this
>> commit allows them as well.
>>
>> Since guest firmware can use status bits DQ7, DQ6, DQ3, and DQ2 to
>> determine the current state of sector erasure, these bits are properly
>> implemented.
>>
>> @@ -305,13 +364,16 @@ static uint64_t pflash_read(void *opaque, hwaddr 
>> offset, unsigned int width)
>>          }
>>          DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx64 "\n", __func__, boff, 
>> ret);
>>          break;
>> -    case 0xA0:
>>      case 0x10:
>>      case 0x30:
>> +        /* Toggle bit 2 during erase, but not program. */
>> +        toggle_dq2(pfl);
>> +    case 0xA0:
>> +        /* Toggle bit 6 */
>> +        toggle_dq6(pfl);
>>          /* Status register read */
>>          ret = pfl->status;
>>          DPRINTF("%s: status %" PRIx64 "\n", __func__, ret);
>> -        toggle_dq6(pfl);
>>          break;
>>      case 0x98:
>>          /* CFI query mode */
> 
> Hi; Coverity (CID 1403012) flags up the case 0x30 as an implicit
> fallthrough. Should it have an explicit "break" or a "/* fall through */"
> comment?

Yes, it is an implicit fallthrough. I'll send a patch.

Thanks,

Phil.