[PATCH 0/5] hw: Cleanups around PFLASH use

[PATCH 0/5] hw: Cleanups around PFLASH use

[Philippe Mathieu-Daudé]

Trivial cleanups before PFLASH refactor:

- Remove unreachable error path calling pflash_cfi01_register()
- Add FLASH_SECTOR_SIZE definitions
- Use more IEC binary prefix definition

Philippe Mathieu-Daudé (5):
  hw/ppc/sam460ex: Remove unreachable code calling
    pflash_cfi01_register()
  hw/microblaze/petalogix: Add FLASH_SECTOR_SIZE definitions
  hw/mips/malta: Add the FLASH_SECTOR_SIZE definition
  hw/sh4/r2d: Use the IEC binary prefix definitions
  hw/sh4/r2d: Add the FLASH_SECTOR_SIZE definition

 hw/microblaze/petalogix_ml605_mmu.c      |  3 ++-
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  3 ++-
 hw/mips/malta.c                          |  5 +++--
 hw/ppc/sam460ex.c                        | 12 ++++--------
 hw/sh4/r2d.c                             | 13 +++++++------
 5 files changed, 18 insertions(+), 18 deletions(-)

-- 
2.38.1

[PATCH 1/5] hw/ppc/sam460ex: Remove unreachable code calling pflash_cfi01_register()

[Philippe Mathieu-Daudé]

Since its QOM'ification in commit 368a354f02 ("pflash_cfi0x:
QOMified") the pflash_cfi01_register() function does not fail.

This call was later converted with a script to use &error_fatal,
still unable to fail. Remove the unreachable code.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/sam460ex.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 4a22ce3761..cf7213f7c9 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -103,14 +103,10 @@ static int sam460ex_load_uboot(void)
     DriveInfo *dinfo;
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-    if (!pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
-                               "sam460ex.flash", FLASH_SIZE,
-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                               64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1)) {
-        error_report("Error registering flash memory");
-        /* XXX: return an error instead? */
-        exit(1);
-    }
+    pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
+                          "sam460ex.flash", FLASH_SIZE,
+                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1));
 
     if (!dinfo) {
         /*error_report("No flash image given with the 'pflash' parameter,"
-- 
2.38.1

Re: [PATCH 1/5] hw/ppc/sam460ex: Remove unreachable code calling pflash_cfi01_register()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
> Since its QOM'ification in commit 368a354f02 ("pflash_cfi0x:
> QOMified") the pflash_cfi01_register() function does not fail.
>
> This call was later converted with a script to use &error_fatal,
> still unable to fail. Remove the unreachable code.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/ppc/sam460ex.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 4a22ce3761..cf7213f7c9 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -103,14 +103,10 @@ static int sam460ex_load_uboot(void)
>     DriveInfo *dinfo;
>
>     dinfo = drive_get(IF_PFLASH, 0, 0);
> -    if (!pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
> -                               "sam460ex.flash", FLASH_SIZE,
> -                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> -                               64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1)) {
> -        error_report("Error registering flash memory");
> -        /* XXX: return an error instead? */
> -        exit(1);
> -    }
> +    pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
> +                          "sam460ex.flash", FLASH_SIZE,
> +                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> +                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1));
>
>     if (!dinfo) {
>         /*error_report("No flash image given with the 'pflash' parameter,"
>

Re: [PATCH 1/5] hw/ppc/sam460ex: Remove unreachable code calling pflash_cfi01_register()

[Bernhard Beschow]

Am 9. Januar 2023 12:01:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Since its QOM'ification in commit 368a354f02 ("pflash_cfi0x:
>QOMified") the pflash_cfi01_register() function does not fail.
>
>This call was later converted with a script to use &error_fatal,
>still unable to fail. Remove the unreachable code.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/ppc/sam460ex.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)

It looks like there is more: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg02491.html

Best regards,
Bernhard

>
>diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
>index 4a22ce3761..cf7213f7c9 100644
>--- a/hw/ppc/sam460ex.c
>+++ b/hw/ppc/sam460ex.c
>@@ -103,14 +103,10 @@ static int sam460ex_load_uboot(void)
>     DriveInfo *dinfo;
> 
>     dinfo = drive_get(IF_PFLASH, 0, 0);
>-    if (!pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
>-                               "sam460ex.flash", FLASH_SIZE,
>-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>-                               64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1)) {
>-        error_report("Error registering flash memory");
>-        /* XXX: return an error instead? */
>-        exit(1);
>-    }
>+    pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
>+                          "sam460ex.flash", FLASH_SIZE,
>+                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>+                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1));
> 
>     if (!dinfo) {
>         /*error_report("No flash image given with the 'pflash' parameter,"

Re: [PATCH 1/5] hw/ppc/sam460ex: Remove unreachable code calling pflash_cfi01_register()

[Philippe Mathieu-Daudé]

On 9/1/23 17:42, Bernhard Beschow wrote:
> Am 9. Januar 2023 12:01:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Since its QOM'ification in commit 368a354f02 ("pflash_cfi0x:
>> QOMified") the pflash_cfi01_register() function does not fail.
>>
>> This call was later converted with a script to use &error_fatal,
>> still unable to fail. Remove the unreachable code.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/ppc/sam460ex.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
> 
> It looks like there is more: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg02491.html
Doh, my short-term memory scares me :\

[PATCH 2/5] hw/microblaze/petalogix: Add FLASH_SECTOR_SIZE definitions

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_ml605_mmu.c      | 3 ++-
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index a24fadddca..1888900156 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -45,6 +45,7 @@
 
 #define LMB_BRAM_SIZE  (128 * KiB)
 #define FLASH_SIZE     (32 * MiB)
+#define FLASH_SECTOR_SIZE   (64 * KiB)
 
 #define BINARY_DEVICE_TREE_FILE "petalogix-ml605.dtb"
 
@@ -107,7 +108,7 @@ petalogix_ml605_init(MachineState *machine)
      * 10th paremeter 0 means little-endian */
     pflash_cfi01_register(FLASH_BASEADDR, "petalogix_ml605.flash", FLASH_SIZE,
                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 2, 0x89, 0x18, 0x0000, 0x0, 0);
+                          FLASH_SECTOR_SIZE, 2, 0x89, 0x18, 0x0000, 0x0, 0);
 
 
     dev = qdev_new("xlnx.xps-intc");
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 9d959d1ad8..d14eff2514 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -40,6 +40,7 @@
 
 #define LMB_BRAM_SIZE  (128 * KiB)
 #define FLASH_SIZE     (16 * MiB)
+#define FLASH_SECTOR_SIZE   (64 * KiB)
 
 #define BINARY_DEVICE_TREE_FILE "petalogix-s3adsp1800.dtb"
 
@@ -87,7 +88,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
     pflash_cfi01_register(FLASH_BASEADDR,
                           "petalogix_s3adsp1800.flash", FLASH_SIZE,
                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
+                          FLASH_SECTOR_SIZE, 1, 0x89, 0x18, 0x0000, 0x0, 1);
 
     dev = qdev_new("xlnx.xps-intc");
     qdev_prop_set_uint32(dev, "kind-of-intr",
-- 
2.38.1

[PATCH 3/5] hw/mips/malta: Add the FLASH_SECTOR_SIZE definition

[Philippe Mathieu-Daudé]

Ease code review by using the IEC binary prefix definition
for the FLASH_SIZE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/malta.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index c0a2e0ab04..e645ba1322 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -69,7 +69,8 @@
 #define FPGA_ADDRESS        0x1f000000ULL
 #define RESET_ADDRESS       0x1fc00000ULL
 
-#define FLASH_SIZE          0x400000
+#define FLASH_SIZE          (4 * MiB)
+#define FLASH_SECTOR_SIZE   (64 * KiB)
 
 typedef struct {
     MemoryRegion iomem;
@@ -1289,7 +1290,7 @@ void mips_malta_init(MachineState *machine)
     fl = pflash_cfi01_register(FLASH_ADDRESS, "mips_malta.bios",
                                FLASH_SIZE,
                                dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                               65536,
+                               FLASH_SECTOR_SIZE,
                                4, 0x0000, 0x0000, 0x0000, 0x0000, be);
     bios = pflash_cfi01_get_memory(fl);
     fl_idx++;
-- 
2.38.1

[PATCH 4/5] hw/sh4/r2d: Use the IEC binary prefix definitions

[Philippe Mathieu-Daudé]

IEC binary prefixes ease code review: the unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sh4/r2d.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 39fc4f19d9..b3667e9b12 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -47,10 +47,10 @@
 #define FLASH_BASE 0x00000000
 #define FLASH_SIZE (16 * MiB)
 
-#define SDRAM_BASE 0x0c000000 /* Physical location of SDRAM: Area 3 */
-#define SDRAM_SIZE 0x04000000
+#define SDRAM_BASE          (192 * MiB) /* Physical location of SDRAM: Area 3 */
+#define SDRAM_SIZE          (64 * MiB)
 
-#define SM501_VRAM_SIZE 0x800000
+#define SM501_VRAM_SIZE     (8 * MiB)
 
 #define BOOT_PARAMS_OFFSET 0x0010000
 /* CONFIG_BOOT_LINK_OFFSET of Linux kernel */
-- 
2.38.1

Re: [PATCH 4/5] hw/sh4/r2d: Use the IEC binary prefix definitions

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/sh4/r2d.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> index 39fc4f19d9..b3667e9b12 100644
> --- a/hw/sh4/r2d.c
> +++ b/hw/sh4/r2d.c
> @@ -47,10 +47,10 @@
> #define FLASH_BASE 0x00000000
> #define FLASH_SIZE (16 * MiB)
>
> -#define SDRAM_BASE 0x0c000000 /* Physical location of SDRAM: Area 3 */
> -#define SDRAM_SIZE 0x04000000
> +#define SDRAM_BASE          (192 * MiB) /* Physical location of SDRAM: Area 3 */
> +#define SDRAM_SIZE          (64 * MiB)

I don't think changing these help as the docs probably have memory map 
with the hex numbers rather than sizes so it's easier to match as it is 
now.

> -#define SM501_VRAM_SIZE 0x800000
> +#define SM501_VRAM_SIZE     (8 * MiB)

This one is OK but since it's only used once in

qdev_prop_set_uint32(dev, "vram-size", SM501_VRAM_SIZE);

you might as well just inline it there and remove the define which is then 
pretty clear and easier to see without needing to look up the define far 
away from its usage.

Regards,
BALATON Zoltan

> #define BOOT_PARAMS_OFFSET 0x0010000
> /* CONFIG_BOOT_LINK_OFFSET of Linux kernel */
>

Re: [PATCH 4/5] hw/sh4/r2d: Use the IEC binary prefix definitions

[Philippe Mathieu-Daudé]

On 9/1/23 13:46, BALATON Zoltan wrote:
> On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
>> IEC binary prefixes ease code review: the unit is explicit.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/sh4/r2d.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
>> index 39fc4f19d9..b3667e9b12 100644
>> --- a/hw/sh4/r2d.c
>> +++ b/hw/sh4/r2d.c
>> @@ -47,10 +47,10 @@
>> #define FLASH_BASE 0x00000000
>> #define FLASH_SIZE (16 * MiB)
>>
>> -#define SDRAM_BASE 0x0c000000 /* Physical location of SDRAM: Area 3 */
>> -#define SDRAM_SIZE 0x04000000
>> +#define SDRAM_BASE          (192 * MiB) /* Physical location of 
>> SDRAM: Area 3 */
>> +#define SDRAM_SIZE          (64 * MiB)
> 
> I don't think changing these help as the docs probably have memory map 
> with the hex numbers rather than sizes so it's easier to match as it is 
> now.
> 
>> -#define SM501_VRAM_SIZE 0x800000
>> +#define SM501_VRAM_SIZE     (8 * MiB)
> 
> This one is OK but since it's only used once in
> 
> qdev_prop_set_uint32(dev, "vram-size", SM501_VRAM_SIZE);
> 
> you might as well just inline it there and remove the define which is 
> then pretty clear and easier to see without needing to look up the 
> define far away from its usage.

I did this change after Peter's feedbacks on:
https://lore.kernel.org/qemu-devel/CAFEAcA8MSO4YMEq2FqvpJKUVYE_1CqaB2KLD1YN-YebOhJVgEg@mail.gmail.com/

But maybe I misunderstood him. Peter, looking at it again, maybe you
asked for a definition because when using pflash_cfi01_register() it
isn't explicit what means each argument? So in this case, since the
properties gives a hint on what is the value ("vram-size") it would
be OK to inline?

Thanks,

Phil.

Re: [PATCH 4/5] hw/sh4/r2d: Use the IEC binary prefix definitions

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2195 bytes --]

On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
> On 9/1/23 13:46, BALATON Zoltan wrote:
>> On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
>>> IEC binary prefixes ease code review: the unit is explicit.
>>> 
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/sh4/r2d.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
>>> index 39fc4f19d9..b3667e9b12 100644
>>> --- a/hw/sh4/r2d.c
>>> +++ b/hw/sh4/r2d.c
>>> @@ -47,10 +47,10 @@
>>> #define FLASH_BASE 0x00000000
>>> #define FLASH_SIZE (16 * MiB)
>>> 
>>> -#define SDRAM_BASE 0x0c000000 /* Physical location of SDRAM: Area 3 */
>>> -#define SDRAM_SIZE 0x04000000
>>> +#define SDRAM_BASE          (192 * MiB) /* Physical location of SDRAM: 
>>> Area 3 */
>>> +#define SDRAM_SIZE          (64 * MiB)
>> 
>> I don't think changing these help as the docs probably have memory map with 
>> the hex numbers rather than sizes so it's easier to match as it is now.
>> 
>>> -#define SM501_VRAM_SIZE 0x800000
>>> +#define SM501_VRAM_SIZE     (8 * MiB)
>> 
>> This one is OK but since it's only used once in
>> 
>> qdev_prop_set_uint32(dev, "vram-size", SM501_VRAM_SIZE);
>> 
>> you might as well just inline it there and remove the define which is then 
>> pretty clear and easier to see without needing to look up the define far 
>> away from its usage.
>
> I did this change after Peter's feedbacks on:
> https://lore.kernel.org/qemu-devel/CAFEAcA8MSO4YMEq2FqvpJKUVYE_1CqaB2KLD1YN-YebOhJVgEg@mail.gmail.com/
>
> But maybe I misunderstood him. Peter, looking at it again, maybe you
> asked for a definition because when using pflash_cfi01_register() it
> isn't explicit what means each argument? So in this case, since the
> properties gives a hint on what is the value ("vram-size") it would
> be OK to inline?

I think since you'll change it later to set properties then it will be 
clear again without defines so I don't think single use defines are needed 
in this case. It's just the many arguments of pflash_cfi01_register() 
function that made it unclear but that will be gone by the end of the 
series.

Regards,
BALATON Zoltan

[PATCH 5/5] hw/sh4/r2d: Add the FLASH_SECTOR_SIZE definition

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sh4/r2d.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index b3667e9b12..6e0c65124a 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -45,7 +45,8 @@
 #include "hw/block/flash.h"
 
 #define FLASH_BASE 0x00000000
-#define FLASH_SIZE (16 * MiB)
+#define FLASH_SIZE          (16 * MiB)
+#define FLASH_SECTOR_SIZE   (128 * KiB)
 
 #define SDRAM_BASE          (192 * MiB) /* Physical location of SDRAM: Area 3 */
 #define SDRAM_SIZE          (64 * MiB)
@@ -304,8 +305,8 @@ static void r2d_init(MachineState *machine)
     dinfo = drive_get(IF_PFLASH, 0, 0);
     pflash_cfi02_register(0x0, "r2d.flash", FLASH_SIZE,
                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                          64 * KiB, 1, 2, 0x0001, 0x227e, 0x2220, 0x2200,
-                          0x555, 0x2aa, 0);
+                          FLASH_SECTOR_SIZE, 1, 2,
+                          0x0001, 0x227e, 0x2220, 0x2200, 0x555, 0x2aa, 0);
 
     /* NIC: rtl8139 on-board, and 2 slots. */
     for (i = 0; i < nb_nics; i++)
-- 
2.38.1

Re: [PATCH 5/5] hw/sh4/r2d: Add the FLASH_SECTOR_SIZE definition

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/sh4/r2d.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> index b3667e9b12..6e0c65124a 100644
> --- a/hw/sh4/r2d.c
> +++ b/hw/sh4/r2d.c
> @@ -45,7 +45,8 @@
> #include "hw/block/flash.h"
>
> #define FLASH_BASE 0x00000000
> -#define FLASH_SIZE (16 * MiB)
> +#define FLASH_SIZE          (16 * MiB)
> +#define FLASH_SECTOR_SIZE   (128 * KiB)

I'm not a fan of single use defines when it makes harder to see what value 
is used than using a constant. This one is border line case as the 
pflash_cfi02_register() function has so many arguments that it's not clear 
which is which but I'm not sure it worth the churn but I don't care too 
much either.

Regards,
BALATON Zoltan

> #define SDRAM_BASE          (192 * MiB) /* Physical location of SDRAM: Area 3 */
> #define SDRAM_SIZE          (64 * MiB)
> @@ -304,8 +305,8 @@ static void r2d_init(MachineState *machine)
>     dinfo = drive_get(IF_PFLASH, 0, 0);
>     pflash_cfi02_register(0x0, "r2d.flash", FLASH_SIZE,
>                           dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> -                          64 * KiB, 1, 2, 0x0001, 0x227e, 0x2220, 0x2200,
> -                          0x555, 0x2aa, 0);
> +                          FLASH_SECTOR_SIZE, 1, 2,
> +                          0x0001, 0x227e, 0x2220, 0x2200, 0x555, 0x2aa, 0);
>
>     /* NIC: rtl8139 on-board, and 2 slots. */
>     for (i = 0; i < nb_nics; i++)
>