Re: [PATCH] hw/intc/ppc-uic: Make default dcr-base 0xc0, not 0x30

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Jan 11, 2021 at 09:30:07PM +0000, Peter Maydell wrote:
> In commit 34d0831f38fd8 the ppc-uic device was added, with a dcr-base
> property. The intention was that the default value of dcr-base should be
> the one that most of our boards need, so that in the common case they
> don't need to specify a property value.
> 
> All QEMU boards with a UIC use a dcr-base of 0xc0, with the exception of
> sam460ex which has four UICs and so puts them at 0xc0, 0xd0, 0xe0, 0xf0.
> So 0xc0 is the obvious right choice for the default dcr-base.
> 
> The board code conversions in commits 0270d74ef88623505 (bamboo) and
> c5ac9dc64fa552a6 (virtex_ml507) assumed that default was 0xc0. Unfortunately
> the actual default in 34d0831f38fd8 was 0x30, by mistake, so the
> bamboo and virtex_ml507 boards were broken as they were converted
> away from ppcuic_init() (which always specifies the dcr_base property
> value explicitly).
> 
> Set the default dcr-base to 0xc0 as was intended, fixing bamboo and
> virtex_ml507.
> 
> Fixes: 34d0831f38fd8
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Applied, thanks.

> ---
> This is the right way to fix the bug I was trying to fix with
> "hw/ppc/ppc400_bamboo: Set dcr-base correctly when creating UIC".
> 
> David: you probably want to put this patch in your ppc queue
> before "hw/ppc/ppc405_uc: Drop use of ppcuic_init()", as that patch
> also assumes the 0xc0 default.

Done.

> 
>  hw/intc/ppc-uic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/intc/ppc-uic.c b/hw/intc/ppc-uic.c
> index b21951eea83..7171de7b355 100644
> --- a/hw/intc/ppc-uic.c
> +++ b/hw/intc/ppc-uic.c
> @@ -274,7 +274,7 @@ static void ppc_uic_realize(DeviceState *dev, Error **errp)
>  
>  static Property ppc_uic_properties[] = {
>      DEFINE_PROP_LINK("cpu", PPCUIC, cpu, TYPE_CPU, CPUState *),
> -    DEFINE_PROP_UINT32("dcr-base", PPCUIC, dcr_base, 0x30),
> +    DEFINE_PROP_UINT32("dcr-base", PPCUIC, dcr_base, 0xc0),
>      DEFINE_PROP_BOOL("use-vectors", PPCUIC, use_vectors, true),
>      DEFINE_PROP_END_OF_LIST()
>  };

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]