[PATCH v2 12/12] hw/ppc/Kconfig: Add dependency PEGASOS2 -> ATI_VGA

Philippe Mathieu-Daudé posted 12 patches 4 years, 8 months ago
[PATCH v2 12/12] hw/ppc/Kconfig: Add dependency PEGASOS2 -> ATI_VGA
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
From: Philippe Mathieu-Daudé <f4bug@amsat.org>

While the ATI VGA device isn't a requisite (no crash without it):

  $ qemu-system-ppc -M pegasos2
  qemu-system-ppc: standard VGA not available

it is useful to have it with the Pegasos2 machine:

  $ qemu-system-ppc -M pegasos2 -vga none -bios pegasos2.rom -device ati-vga,romfile=
  qemu-system-ppc: -device ati-vga,romfile=: 'ati-vga' is not a valid device model name

Add it as an implicit Kconfig dependency.

Fixes: ba7e5ac18e7 ("hw/ppc: Add emulation of Genesi/bPlan Pegasos II")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ppc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 2e4c56eb770..e36db08665a 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -77,6 +77,7 @@ config PEGASOS2
     select SMBUS_EEPROM
 # This should come with VT82C686
     select ACPI_X86
+    imply ATI_VGA
 
 config PREP
     bool
-- 
2.26.3

Re: [PATCH v2 12/12] hw/ppc/Kconfig: Add dependency PEGASOS2 -> ATI_VGA
Posted by BALATON Zoltan 4 years, 8 months ago
On Sat, 15 May 2021, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> While the ATI VGA device isn't a requisite (no crash without it):
>
>  $ qemu-system-ppc -M pegasos2
>  qemu-system-ppc: standard VGA not available
>
> it is useful to have it with the Pegasos2 machine:
>
>  $ qemu-system-ppc -M pegasos2 -vga none -bios pegasos2.rom -device ati-vga,romfile=
>  qemu-system-ppc: -device ati-vga,romfile=: 'ati-vga' is not a valid device model name
>
> Add it as an implicit Kconfig dependency.
>
> Fixes: ba7e5ac18e7 ("hw/ppc: Add emulation of Genesi/bPlan Pegasos II")

You can list it as a fix but I regard this more an enhancement or 
amandment to that commit as it was not broken in this regard as the commit 
message above also explains. To me Fixes tag means more that something was 
broken in that commit that this one patches up but I don't care much about 
this tag. It would probably make more sense in your other commits fixing 
missing dependencies (although not clear which commit those fix as the 
missing dependencies were probably also missing before the latest clean 
ups).

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
> hw/ppc/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 2e4c56eb770..e36db08665a 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -77,6 +77,7 @@ config PEGASOS2
>     select SMBUS_EEPROM
> # This should come with VT82C686
>     select ACPI_X86
> +    imply ATI_VGA
>
> config PREP
>     bool
>
Re: [PATCH v2 12/12] hw/ppc/Kconfig: Add dependency PEGASOS2 -> ATI_VGA
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 5/15/21 9:41 PM, BALATON Zoltan wrote:
> On Sat, 15 May 2021, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> While the ATI VGA device isn't a requisite (no crash without it):
>>
>>  $ qemu-system-ppc -M pegasos2
>>  qemu-system-ppc: standard VGA not available
>>
>> it is useful to have it with the Pegasos2 machine:
>>
>>  $ qemu-system-ppc -M pegasos2 -vga none -bios pegasos2.rom -device
>> ati-vga,romfile=
>>  qemu-system-ppc: -device ati-vga,romfile=: 'ati-vga' is not a valid
>> device model name
>>
>> Add it as an implicit Kconfig dependency.
>>
>> Fixes: ba7e5ac18e7 ("hw/ppc: Add emulation of Genesi/bPlan Pegasos II")
> 
> You can list it as a fix but I regard this more an enhancement or
> amandment to that commit as it was not broken in this regard as the
> commit message above also explains. To me Fixes tag means more that
> something was broken in that commit that this one patches up but I don't
> care much about this tag. It would probably make more sense in your
> other commits fixing missing dependencies (although not clear which
> commit those fix as the missing dependencies were probably also missing
> before the latest clean ups).

OK, I'll reword.

Thanks for reviewing this series!

> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>

Re: [PATCH v2 12/12] hw/ppc/Kconfig: Add dependency PEGASOS2 -> ATI_VGA
Posted by Bin Meng 4 years, 8 months ago
On Sun, May 16, 2021 at 3:41 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Sat, 15 May 2021, Philippe Mathieu-Daudé wrote:
> > From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >
> > While the ATI VGA device isn't a requisite (no crash without it):
> >
> >  $ qemu-system-ppc -M pegasos2
> >  qemu-system-ppc: standard VGA not available
> >
> > it is useful to have it with the Pegasos2 machine:
> >
> >  $ qemu-system-ppc -M pegasos2 -vga none -bios pegasos2.rom -device ati-vga,romfile=
> >  qemu-system-ppc: -device ati-vga,romfile=: 'ati-vga' is not a valid device model name
> >
> > Add it as an implicit Kconfig dependency.
> >
> > Fixes: ba7e5ac18e7 ("hw/ppc: Add emulation of Genesi/bPlan Pegasos II")
>
> You can list it as a fix but I regard this more an enhancement or
> amandment to that commit as it was not broken in this regard as the commit
> message above also explains. To me Fixes tag means more that something was

Agree. This patch is more like a feature, instead of a fix. So the
"Fixes" tag isn't appropriate.

> broken in that commit that this one patches up but I don't care much about
> this tag. It would probably make more sense in your other commits fixing
> missing dependencies (although not clear which commit those fix as the
> missing dependencies were probably also missing before the latest clean
> ups).
>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>

FWIW:

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

Re: [PATCH v2 12/12] hw/ppc/Kconfig: Add dependency PEGASOS2 -> ATI_VGA
Posted by David Gibson 4 years, 8 months ago
On Sat, May 15, 2021 at 07:37:16PM +0200, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> While the ATI VGA device isn't a requisite (no crash without it):
> 
>   $ qemu-system-ppc -M pegasos2
>   qemu-system-ppc: standard VGA not available
> 
> it is useful to have it with the Pegasos2 machine:
> 
>   $ qemu-system-ppc -M pegasos2 -vga none -bios pegasos2.rom -device ati-vga,romfile=
>   qemu-system-ppc: -device ati-vga,romfile=: 'ati-vga' is not a valid device model name
> 
> Add it as an implicit Kconfig dependency.
> 
> Fixes: ba7e5ac18e7 ("hw/ppc: Add emulation of Genesi/bPlan Pegasos II")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 2e4c56eb770..e36db08665a 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -77,6 +77,7 @@ config PEGASOS2
>      select SMBUS_EEPROM
>  # This should come with VT82C686
>      select ACPI_X86
> +    imply ATI_VGA
>  
>  config PREP
>      bool

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson