diff mbox series

[1/5] hw/mips/malta: Fix the DEBUG_BOARD_INIT code

Message ID 20190305162829.20079-2-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series mips_malta: Clean up definition of flash memory size | expand

Commit Message

Philippe Mathieu-Daudé March 5, 2019, 4:28 p.m. UTC
Commit fa1d36df746 missed to convert this ifdef'ed out code.
Introduce the pflash_blk variable.

This fixes:

  hw/mips/mips_malta.c:1273:16: error: implicit declaration of function ‘blk_name’; did you mean ‘basename’? [-Werror=implicit-function-declaration]
                blk_name(dinfo->bdrv), fl_sectors);
                ^~~~~~~~
  hw/mips/mips_malta.c:1273:16: error: nested extern declaration of ‘blk_name’ [-Werror=nested-externs]
  hw/mips/mips_malta.c:1273:30: error: ‘DriveInfo’ {aka ‘struct DriveInfo’} has no member named ‘bdrv’
                blk_name(dinfo->bdrv), fl_sectors);
                                ^~

Fixes: fa1d36df746
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/mips/mips_malta.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Markus Armbruster March 6, 2019, 1:20 p.m. UTC | #1
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Commit fa1d36df746 missed to convert this ifdef'ed out code.

My fault...

> Introduce the pflash_blk variable.
>
> This fixes:
>
>   hw/mips/mips_malta.c:1273:16: error: implicit declaration of function ‘blk_name’; did you mean ‘basename’? [-Werror=implicit-function-declaration]
>                 blk_name(dinfo->bdrv), fl_sectors);
>                 ^~~~~~~~
>   hw/mips/mips_malta.c:1273:16: error: nested extern declaration of ‘blk_name’ [-Werror=nested-externs]
>   hw/mips/mips_malta.c:1273:30: error: ‘DriveInfo’ {aka ‘struct DriveInfo’} has no member named ‘bdrv’
>                 blk_name(dinfo->bdrv), fl_sectors);
>                                 ^~
>
> Fixes: fa1d36df746
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/mips/mips_malta.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 2827074e9b..b4cd8f02ad 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -40,6 +40,7 @@
>  #include "hw/pci/pci.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/arch_init.h"
> +#include "sysemu/block-backend.h"
>  #include "qemu/log.h"
>  #include "hw/mips/bios.h"
>  #include "hw/ide.h"
> @@ -1205,6 +1206,7 @@ void mips_malta_init(MachineState *machine)
>      qemu_irq cbus_irq, i8259_irq;
>      int piix4_devfn;
>      I2CBus *smbus;
> +    BlockBackend *pflash_blk = NULL;
>      DriveInfo *dinfo;
>      DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      int fl_idx = 0;
> @@ -1265,17 +1267,18 @@ void mips_malta_init(MachineState *machine)
>  
>      /* Load firmware in flash / BIOS. */
>      dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> -#ifdef DEBUG_BOARD_INIT
>      if (dinfo) {
> +        pflash_blk = blk_by_legacy_dinfo(dinfo);
> +#ifdef DEBUG_BOARD_INIT
>          printf("Register parallel flash %d size " TARGET_FMT_lx " at "
> -               "addr %08llx '%s' %x\n",
> -               fl_idx, bios_size, FLASH_ADDRESS,
> -               blk_name(dinfo->bdrv), fl_sectors);
> -    }
> +               "addr %08llx '%s'\n",
> +               fl_idx, blk_getlength(pflash_blk), FLASH_ADDRESS,
> +               blk_name(pflash_blk));

You stop printing @fl_sectors.  Intentional?

Would the next patch be a better fit?

>  #endif
> +    }
>      fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
>                                 BIOS_SIZE,
> -                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> +                               pflash_blk,
>                                 65536, fl_sectors,
>                                 4, 0x0000, 0x0000, 0x0000, 0x0000, be);
>      bios = pflash_cfi01_get_memory(fl);

We should probably join lines here.
Markus Armbruster March 7, 2019, 7:06 a.m. UTC | #2
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Commit fa1d36df746 missed to convert this ifdef'ed out code.
> Introduce the pflash_blk variable.
>
> This fixes:
>
>   hw/mips/mips_malta.c:1273:16: error: implicit declaration of function ‘blk_name’; did you mean ‘basename’? [-Werror=implicit-function-declaration]
>                 blk_name(dinfo->bdrv), fl_sectors);
>                 ^~~~~~~~
>   hw/mips/mips_malta.c:1273:16: error: nested extern declaration of ‘blk_name’ [-Werror=nested-externs]
>   hw/mips/mips_malta.c:1273:30: error: ‘DriveInfo’ {aka ‘struct DriveInfo’} has no member named ‘bdrv’
>                 blk_name(dinfo->bdrv), fl_sectors);
>                                 ^~
>
> Fixes: fa1d36df746
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Question for the maintainers: Aurelien, Aleksandar, fix the
DEBUG_BOARD_INIT code or drop it?  It looks rather stale...
Aleksandar Markovic March 7, 2019, 1:48 p.m. UTC | #3
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH 1/5] hw/mips/malta: Fix the DEBUG_BOARD_INIT code
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
> > Commit fa1d36df746 missed to convert this ifdef'ed out code.
> > Introduce the pflash_blk variable.
> > 
> > This fixes:
> > 
> >    hw/mips/mips_malta.c:1273:16: error: implicit declaration of function ‘blk_name’; did you mean ‘basename’? [-Werror=implicit-function-declaration]
> >                 blk_name(dinfo->bdrv), fl_sectors);
> >                 ^~~~~~~~
> >   hw/mips/mips_malta.c:1273:16: error: nested extern declaration of ‘blk_name’ [-Werror=nested-externs]
> >   hw/mips/mips_malta.c:1273:30: error: ‘DriveInfo’ {aka ‘struct DriveInfo’} has no member named ‘bdrv’
> >                 blk_name(dinfo->bdrv), fl_sectors);
> >                                 ^~
> > 
> > Fixes: fa1d36df746
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Question for the maintainers: Aurelien, Aleksandar, fix the
> DEBUG_BOARD_INIT code or drop it?  It looks rather stale...

Markus,

My vote is: drop it.

And, Markus, yes, if you agree with any patch from this series, please let them all go though your pull request.

Regards,
Aleksandar
Aleksandar Markovic March 7, 2019, 1:53 p.m. UTC | #4
> > From: Markus Armbruster <armbru@redhat.com>
> > Subject: Re: [Qemu-devel] [PATCH 1/5] hw/mips/malta: Fix the DEBUG_BOARD_INIT code
> > 
> > Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> > 
> > > Commit fa1d36df746 missed to convert this ifdef'ed out code.
> > > Introduce the pflash_blk variable.
> > > 
> > > This fixes:
> > > 
> > >    hw/mips/mips_malta.c:1273:16: error: implicit declaration of function ‘blk_name’; > did you mean ‘basename’? [-Werror=implicit-function-declaration]
> > >                 blk_name(dinfo->bdrv), fl_sectors);
> > >                 ^~~~~~~~
> > >   hw/mips/mips_malta.c:1273:16: error: nested extern declaration of ‘blk_name’
> [-Werror=nested-externs]
> > >   hw/mips/mips_malta.c:1273:30: error: ‘DriveInfo’ {aka ‘struct DriveInfo’} has no
> member named ‘bdrv’
> > >                 blk_name(dinfo->bdrv), fl_sectors);
> > >                                 ^~
> > > 
> > > Fixes: fa1d36df746
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > 
> > Question for the maintainers: Aurelien, Aleksandar, fix the
> > DEBUG_BOARD_INIT code or drop it?  It looks rather stale...
> 
> Markus,
> 
> My vote is: drop it.
> 
> And, Markus, yes, if you agree with any patch from this series,
> please let them all go > though your pull request.
> 
> Regards,
> Aleksandar

Markus, if you decide to drop the code segment, you have my

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

in advance.

I believe the whole patch series was also reviewed by Richard.

Sincerely,
Aleksandar
diff mbox series

Patch

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 2827074e9b..b4cd8f02ad 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -40,6 +40,7 @@ 
 #include "hw/pci/pci.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/arch_init.h"
+#include "sysemu/block-backend.h"
 #include "qemu/log.h"
 #include "hw/mips/bios.h"
 #include "hw/ide.h"
@@ -1205,6 +1206,7 @@  void mips_malta_init(MachineState *machine)
     qemu_irq cbus_irq, i8259_irq;
     int piix4_devfn;
     I2CBus *smbus;
+    BlockBackend *pflash_blk = NULL;
     DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     int fl_idx = 0;
@@ -1265,17 +1267,18 @@  void mips_malta_init(MachineState *machine)
 
     /* Load firmware in flash / BIOS. */
     dinfo = drive_get(IF_PFLASH, 0, fl_idx);
-#ifdef DEBUG_BOARD_INIT
     if (dinfo) {
+        pflash_blk = blk_by_legacy_dinfo(dinfo);
+#ifdef DEBUG_BOARD_INIT
         printf("Register parallel flash %d size " TARGET_FMT_lx " at "
-               "addr %08llx '%s' %x\n",
-               fl_idx, bios_size, FLASH_ADDRESS,
-               blk_name(dinfo->bdrv), fl_sectors);
-    }
+               "addr %08llx '%s'\n",
+               fl_idx, blk_getlength(pflash_blk), FLASH_ADDRESS,
+               blk_name(pflash_blk));
 #endif
+    }
     fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
                                BIOS_SIZE,
-                               dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+                               pflash_blk,
                                65536, fl_sectors,
                                4, 0x0000, 0x0000, 0x0000, 0x0000, be);
     bios = pflash_cfi01_get_memory(fl);