Re: [Qemu-devel] [PATCH] ppc440_pcix: Add dummy implementation of BRDGOPT registers

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

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

On Wed, Mar 07, 2018 at 09:43:59PM +0100, BALATON Zoltan wrote:
> I don't know what should be the correct implementation for these so
> these are just stored and returned as is without doing anything for
> now only to silence warnings when u-boot accesses these registers.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

I'm a bit dubious about putting in a dummy register implementation.
Specifically, I think the case needs to be made that the dummy
implementation is preferable to just putting up with the test errors.

At the very least there should be a comment in the code indicating
that it's just a dummy stub implementation.

> ---
>  hw/ppc/ppc440_pcix.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index ab2626a..3f177d3 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -55,6 +55,8 @@ typedef struct PPC440PCIXState {
>      PCIDevice *dev;
>      struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
>      struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
> +    uint32_t brdgopt1;
> +    uint32_t brdgopt2;
>      uint32_t sts;
>      qemu_irq irq[PCI_NUM_PINS];
>      AddressSpace bm_as;
> @@ -95,6 +97,8 @@ typedef struct PPC440PCIXState {
>  #define PCIX0_PIM0SAH       0xf8
>  #define PCIX0_PIM2SAH       0xfc
>  
> +#define PCIX0_BRDGOPT1      0x40
> +#define PCIX0_BRDGOPT2      0x44
>  #define PCIX0_STS           0xe0
>  
>  #define PCI_ALL_SIZE        (PPC440_REG_BASE + PPC440_REG_SIZE)
> @@ -270,6 +274,12 @@ static void ppc440_pcix_reg_write4(void *opaque, hwaddr addr,
>          ppc440_pcix_update_pim(s, 2);
>          break;
>  
> +    case PCIX0_BRDGOPT1:
> +        s->brdgopt1 = val;
> +        break;
> +    case PCIX0_BRDGOPT2:
> +        s->brdgopt2 = val;
> +        break;
>      case PCIX0_STS:
>          s->sts = val;
>          break;
> @@ -365,6 +375,12 @@ static uint64_t ppc440_pcix_reg_read4(void *opaque, hwaddr addr,
>          val = s->pim[2].la >> 32;
>          break;
>  
> +    case PCIX0_BRDGOPT1:
> +        val = s->brdgopt1;
> +        break;
> +    case PCIX0_BRDGOPT2:
> +        val = s->brdgopt2;
> +        break;
>      case PCIX0_STS:
>          val = s->sts;
>          break;
> @@ -408,6 +424,8 @@ static void ppc440_pcix_reset(DeviceState *dev)
>      for (i = 0; i < PPC440_PCIX_NR_PIMS; i++) {
>          s->pim[i].sa = 0xffffffff00000000ULL;
>      }
> +    s->brdgopt1 = 0;
> +    s->brdgopt2 = 0;
>      s->sts = 0;
>  }
>  

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