Re: [Qemu-ppc] [Qemu-devel] [PATCH 4/4] uninorth: remove token register from uninorth device

[Philippe Mathieu-Daudé]

On 05/06/2018 11:20 AM, Mark Cave-Ayland wrote:
> From observation of various OS sources it can be seen that the token register
> introduced in 4e46dcdbd3 "PPC: Newworld: Add uninorth token register" is not
> required, since the only register currently implemented is the uninorth 
> hardware
> version which is read-only.
> 
> Remove the token register implementation and instead return the uninorth
> version corresponding to the hardware.

Thanks for this cleanup :)

> 
> Signed-off-by: Mark Cave-Ayland <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  hw/pci-host/uninorth.c         | 11 +++++------
>  include/hw/pci-host/uninorth.h |  4 +++-
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index a658f9230a..abebfaf755 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -524,19 +524,18 @@ static void unin_write(void *opaque, hwaddr addr, 
> uint64_t value,
>                         unsigned size)
>  {
>      trace_unin_write(addr, value);
> -    if (addr == 0x0) {
> -        *(int *)opaque = value;
> -    }
>  }
>  
>  static uint64_t unin_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      uint32_t value;
>  
> -    value = 0;
>      switch (addr) {
>      case 0:
> -        value = *(int *)opaque;
> +        value = UNINORTH_VERSION_10A;
> +        break;
> +    default:
> +        value = 0;
>      }
>  
>      trace_unin_read(addr, value);
> @@ -559,7 +558,7 @@ static void unin_init(Object *obj)
>      UNINState *s = UNI_NORTH(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>  
> -    memory_region_init_io(&s->mem, obj, &unin_ops, &s->token, "unin", 
> 0x1000);
> +    memory_region_init_io(&s->mem, obj, &unin_ops, s, "unin", 0x1000);
>  
>      sysbus_init_mmio(sbd, &s->mem);
>  }
> diff --git a/include/hw/pci-host/uninorth.h b/include/hw/pci-host/uninorth.h
> index f6654bad9b..2a1cf9f284 100644
> --- a/include/hw/pci-host/uninorth.h
> +++ b/include/hw/pci-host/uninorth.h
> @@ -29,6 +29,9 @@
>  
>  #include "hw/ppc/openpic.h"
>  
> +/* UniNorth version */
> +#define UNINORTH_VERSION_10A    0x7
> +
>  #define TYPE_UNI_NORTH_PCI_HOST_BRIDGE "uni-north-pci-pcihost"
>  #define TYPE_UNI_NORTH_AGP_HOST_BRIDGE "uni-north-agp-pcihost"
>  #define TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE 
> "uni-north-internal-pci-pcihost"
> @@ -57,7 +60,6 @@ typedef struct UNINState {
>      SysBusDevice parent_obj;
>  
>      MemoryRegion mem;
> -    int token[1];
>  } UNINState;
>  
>  #define TYPE_UNI_NORTH "uni-north"
>