1. Patchew
  2. QEMU
  3. [PATCH 0/9] hw/block/pflash: Refactors around setting the device in read-array mode
  4. View patch

[PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method

Philippe Mathieu-Daudé posted 9 patches 4 years, 8 months ago
There is a newer version of this series
[PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago 
There is multiple places doing a device reset. Factor that
out in a common method which matches the DeviceReset prototype,
so we can also remove the reset code from the DeviceRealize()
handler. Explicit the device is set in "read array" mode on
reset.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2ba77a0171b..484b056b898 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -193,6 +193,14 @@ static void pflash_mode_read_array(PFlashCFI02 *pfl)
     memory_region_rom_device_set_romd(&pfl->orig_mem, true);
 }
 
+static void pflash_cfi02_reset(DeviceState *dev)
+{
+    PFlashCFI02 *pfl = PFLASH_CFI02(dev);
+
+    trace_pflash_reset();
+    pflash_mode_read_array(pfl);
+}
+
 static size_t pflash_regions_count(PFlashCFI02 *pfl)
 {
     return pfl->cfi_table[0x2c];
@@ -330,8 +338,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
     default:
         /* This should never happen : reset state & treat it as a read*/
         DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
-        pfl->wcycle = 0;
-        pfl->cmd = 0;
+        pflash_cfi02_reset(DEVICE(pfl));
         /* fall through to the read code */
     case 0x80: /* Erase (unlock) */
         /* We accept reads during second unlock sequence... */
@@ -710,10 +717,8 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
 
     /* Reset flash */
  reset_flash:
-    trace_pflash_reset();
     pfl->bypass = 0;
-    pfl->wcycle = 0;
-    pfl->cmd = 0;
+    pflash_cfi02_reset(DEVICE(pfl));
     return;
 
  do_bypass:
@@ -977,6 +982,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = pflash_cfi02_realize;
+    dc->reset = pflash_cfi02_reset;
     dc->unrealize = pflash_cfi02_unrealize;
     device_class_set_props(dc, pflash_cfi02_properties);
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
2.26.2
Re: [PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method
Posted by Bin Meng 4 years, 8 months ago 
On Wed, Mar 10, 2021 at 7:55 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> There is multiple places doing a device reset. Factor that
> out in a common method which matches the DeviceReset prototype,
> so we can also remove the reset code from the DeviceRealize()
> handler. Explicit the device is set in "read array" mode on
> reset.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/block/pflash_cfi02.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 2ba77a0171b..484b056b898 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -193,6 +193,14 @@ static void pflash_mode_read_array(PFlashCFI02 *pfl)
>      memory_region_rom_device_set_romd(&pfl->orig_mem, true);
>  }
>
> +static void pflash_cfi02_reset(DeviceState *dev)
> +{
> +    PFlashCFI02 *pfl = PFLASH_CFI02(dev);
> +
> +    trace_pflash_reset();
> +    pflash_mode_read_array(pfl);
> +}
> +
>  static size_t pflash_regions_count(PFlashCFI02 *pfl)
>  {
>      return pfl->cfi_table[0x2c];
> @@ -330,8 +338,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
>      default:
>          /* This should never happen : reset state & treat it as a read*/
>          DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
> -        pfl->wcycle = 0;
> -        pfl->cmd = 0;
> +        pflash_cfi02_reset(DEVICE(pfl));
>          /* fall through to the read code */
>      case 0x80: /* Erase (unlock) */
>          /* We accept reads during second unlock sequence... */
> @@ -710,10 +717,8 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
>
>      /* Reset flash */
>   reset_flash:
> -    trace_pflash_reset();
>      pfl->bypass = 0;
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0;
> +    pflash_cfi02_reset(DEVICE(pfl));

The old codes did not set pfl->rom_mode to true, but the new codes
pflash_cfi02_reset() do. Is this correct?

>      return;
>
>   do_bypass:
> @@ -977,6 +982,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->realize = pflash_cfi02_realize;
> +    dc->reset = pflash_cfi02_reset;
>      dc->unrealize = pflash_cfi02_unrealize;
>      device_class_set_props(dc, pflash_cfi02_properties);
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> --

Regards,
Bin
Re: [PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago 
On 3/10/21 9:48 AM, Bin Meng wrote:
> On Wed, Mar 10, 2021 at 7:55 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> There is multiple places doing a device reset. Factor that
>> out in a common method which matches the DeviceReset prototype,
>> so we can also remove the reset code from the DeviceRealize()
>> handler. Explicit the device is set in "read array" mode on
>> reset.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/block/pflash_cfi02.c | 16 +++++++++++-----
>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
>> index 2ba77a0171b..484b056b898 100644
>> --- a/hw/block/pflash_cfi02.c
>> +++ b/hw/block/pflash_cfi02.c
>> @@ -193,6 +193,14 @@ static void pflash_mode_read_array(PFlashCFI02 *pfl)
>>      memory_region_rom_device_set_romd(&pfl->orig_mem, true);
>>  }
>>
>> +static void pflash_cfi02_reset(DeviceState *dev)
>> +{
>> +    PFlashCFI02 *pfl = PFLASH_CFI02(dev);
>> +
>> +    trace_pflash_reset();
>> +    pflash_mode_read_array(pfl);
>> +}
>> +
>>  static size_t pflash_regions_count(PFlashCFI02 *pfl)
>>  {
>>      return pfl->cfi_table[0x2c];
>> @@ -330,8 +338,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
>>      default:
>>          /* This should never happen : reset state & treat it as a read*/
>>          DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
>> -        pfl->wcycle = 0;
>> -        pfl->cmd = 0;
>> +        pflash_cfi02_reset(DEVICE(pfl));
>>          /* fall through to the read code */
>>      case 0x80: /* Erase (unlock) */
>>          /* We accept reads during second unlock sequence... */
>> @@ -710,10 +717,8 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
>>
>>      /* Reset flash */
>>   reset_flash:
>> -    trace_pflash_reset();
>>      pfl->bypass = 0;
>> -    pfl->wcycle = 0;
>> -    pfl->cmd = 0;
>> +    pflash_cfi02_reset(DEVICE(pfl));
> 
> The old codes did not set pfl->rom_mode to true, but the new codes
> pflash_cfi02_reset() do. Is this correct?

Hmmm let's be precautious indeed. I'll better split this change.
Re: [PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method
Posted by David Edmondson 4 years, 8 months ago 
On Wednesday, 2021-03-10 at 00:50:26 +01, Philippe Mathieu-Daudé wrote:

> There is multiple places doing a device reset. Factor that
> out in a common method which matches the DeviceReset prototype,
> so we can also remove the reset code from the DeviceRealize()
> handler. Explicit the device is set in "read array" mode on
> reset.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Edmondson <david.edmondson@oracle.com>

> ---
>  hw/block/pflash_cfi02.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 2ba77a0171b..484b056b898 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -193,6 +193,14 @@ static void pflash_mode_read_array(PFlashCFI02 *pfl)
>      memory_region_rom_device_set_romd(&pfl->orig_mem, true);
>  }
>  
> +static void pflash_cfi02_reset(DeviceState *dev)
> +{
> +    PFlashCFI02 *pfl = PFLASH_CFI02(dev);
> +
> +    trace_pflash_reset();
> +    pflash_mode_read_array(pfl);
> +}
> +
>  static size_t pflash_regions_count(PFlashCFI02 *pfl)
>  {
>      return pfl->cfi_table[0x2c];
> @@ -330,8 +338,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
>      default:
>          /* This should never happen : reset state & treat it as a read*/
>          DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
> -        pfl->wcycle = 0;
> -        pfl->cmd = 0;
> +        pflash_cfi02_reset(DEVICE(pfl));
>          /* fall through to the read code */
>      case 0x80: /* Erase (unlock) */
>          /* We accept reads during second unlock sequence... */
> @@ -710,10 +717,8 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
>  
>      /* Reset flash */
>   reset_flash:
> -    trace_pflash_reset();
>      pfl->bypass = 0;
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0;
> +    pflash_cfi02_reset(DEVICE(pfl));
>      return;
>  
>   do_bypass:
> @@ -977,6 +982,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->realize = pflash_cfi02_realize;
> +    dc->reset = pflash_cfi02_reset;
>      dc->unrealize = pflash_cfi02_unrealize;
>      device_class_set_props(dc, pflash_cfi02_properties);
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> -- 
> 2.26.2

dme.
-- 
The band is just fantastic, that is really what I think.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.