Re: [PATCH 12/22] ppc4xx: Move PLB model to ppc4xx_devs.c

[Cédric Le Goater]

On 8/13/22 17:34, BALATON Zoltan wrote:
> The PLB is shared between 405 and 440 so move it to the shared file.

Should we rename the device to Ppc4xxPlbState ?

Thanks,

C.

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/ppc405.h         | 11 -----
>   hw/ppc/ppc405_uc.c      | 93 ----------------------------------------
>   hw/ppc/ppc4xx_devs.c    | 94 +++++++++++++++++++++++++++++++++++++++++
>   include/hw/ppc/ppc4xx.h | 11 +++++
>   4 files changed, 105 insertions(+), 104 deletions(-)
>
> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
> index 31c94e4742..d85c595f9d 100644
> --- a/hw/ppc/ppc405.h
> +++ b/hw/ppc/ppc405.h
> @@ -63,17 +63,6 @@ struct ppc4xx_bd_info_t {
>       uint32_t bi_iic_fast[2];
>   };
>   
> -/* Peripheral local bus arbitrer */
> -#define TYPE_PPC405_PLB "ppc405-plb"
> -OBJECT_DECLARE_SIMPLE_TYPE(Ppc405PlbState, PPC405_PLB);
> -struct Ppc405PlbState {
> -    Ppc4xxDcrDeviceState parent_obj;
> -
> -    uint32_t acr;
> -    uint32_t bear;
> -    uint32_t besr;
> -};
> -
>   /* PLB to OPB bridge */
>   #define TYPE_PPC405_POB "ppc405-pob"
>   OBJECT_DECLARE_SIMPLE_TYPE(Ppc405PobState, PPC405_POB);
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index 922c23346f..3de6c77631 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -137,94 +137,6 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, 
> ram_addr_t ram_size)
>   
> /*****************************************************************************/
>   /* Shared peripherals */
>   
> -/*****************************************************************************/
> -/* Peripheral local bus arbitrer */
> -enum {
> -    PLB3A0_ACR = 0x077,
> -    PLB4A0_ACR = 0x081,
> -    PLB0_BESR  = 0x084,
> -    PLB0_BEAR  = 0x086,
> -    PLB0_ACR   = 0x087,
> -    PLB4A1_ACR = 0x089,
> -};
> -
> -static uint32_t dcr_read_plb(void *opaque, int dcrn)
> -{
> -    Ppc405PlbState *plb = opaque;
> -    uint32_t ret;
> -
> -    switch (dcrn) {
> -    case PLB0_ACR:
> -        ret = plb->acr;
> -        break;
> -    case PLB0_BEAR:
> -        ret = plb->bear;
> -        break;
> -    case PLB0_BESR:
> -        ret = plb->besr;
> -        break;
> -    default:
> -        /* Avoid gcc warning */
> -        ret = 0;
> -        break;
> -    }
> -
> -    return ret;
> -}
> -
> -static void dcr_write_plb(void *opaque, int dcrn, uint32_t val)
> -{
> -    Ppc405PlbState *plb = opaque;
> -
> -    switch (dcrn) {
> -    case PLB0_ACR:
> -        /* We don't care about the actual parameters written as
> -         * we don't manage any priorities on the bus
> -         */
> -        plb->acr = val & 0xF8000000;
> -        break;
> -    case PLB0_BEAR:
> -        /* Read only */
> -        break;
> -    case PLB0_BESR:
> -        /* Write-clear */
> -        plb->besr &= ~val;
> -        break;
> -    }
> -}
> -
> -static void ppc405_plb_reset(DeviceState *dev)
> -{
> -    Ppc405PlbState *plb = PPC405_PLB(dev);
> -
> -    plb->acr = 0x00000000;
> -    plb->bear = 0x00000000;
> -    plb->besr = 0x00000000;
> -}
> -
> -static void ppc405_plb_realize(DeviceState *dev, Error **errp)
> -{
> -    Ppc405PlbState *plb = PPC405_PLB(dev);
> -    Ppc4xxDcrDeviceState *dcr = PPC4xx_DCR_DEVICE(dev);
> -
> -    ppc4xx_dcr_register(dcr, PLB3A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> -    ppc4xx_dcr_register(dcr, PLB4A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> -    ppc4xx_dcr_register(dcr, PLB0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> -    ppc4xx_dcr_register(dcr, PLB0_BEAR, plb, &dcr_read_plb, &dcr_write_plb);
> -    ppc4xx_dcr_register(dcr, PLB0_BESR, plb, &dcr_read_plb, &dcr_write_plb);
> -    ppc4xx_dcr_register(dcr, PLB4A1_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> -}
> -
> -static void ppc405_plb_class_init(ObjectClass *oc, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(oc);
> -
> -    dc->realize = ppc405_plb_realize;
> -    dc->reset = ppc405_plb_reset;
> -    /* Reason: only works as function of a ppc4xx SoC */
> -    dc->user_creatable = false;
> -}
> -
>   
> /*****************************************************************************/
>   /* PLB to OPB bridge */
>   enum {
> @@ -1535,11 +1447,6 @@ static void ppc405_soc_class_init(ObjectClass *oc, void 
> *data)
>   
>   static const TypeInfo ppc405_types[] = {
>       {
> -        .name           = TYPE_PPC405_PLB,
> -        .parent         = TYPE_PPC4xx_DCR_DEVICE,
> -        .instance_size  = sizeof(Ppc405PlbState),
> -        .class_init     = ppc405_plb_class_init,
> -    }, {
>           .name           = TYPE_PPC405_POB,
>           .parent         = TYPE_PPC4xx_DCR_DEVICE,
>           .instance_size  = sizeof(Ppc405PobState),
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index 7d40c1b68a..843d759b1b 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -658,6 +658,95 @@ static void ppc4xx_mal_class_init(ObjectClass *oc, void 
> *data)
>       device_class_set_props(dc, ppc4xx_mal_properties);
>   }
>   
> +/*****************************************************************************/
> +/* Peripheral local bus arbitrer */
> +enum {
> +    PLB3A0_ACR = 0x077,
> +    PLB4A0_ACR = 0x081,
> +    PLB0_BESR  = 0x084,
> +    PLB0_BEAR  = 0x086,
> +    PLB0_ACR   = 0x087,
> +    PLB4A1_ACR = 0x089,
> +};
> +
> +static uint32_t dcr_read_plb(void *opaque, int dcrn)
> +{
> +    Ppc405PlbState *plb = opaque;
> +    uint32_t ret;
> +
> +    switch (dcrn) {
> +    case PLB0_ACR:
> +        ret = plb->acr;
> +        break;
> +    case PLB0_BEAR:
> +        ret = plb->bear;
> +        break;
> +    case PLB0_BESR:
> +        ret = plb->besr;
> +        break;
> +    default:
> +        /* Avoid gcc warning */
> +        ret = 0;
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void dcr_write_plb(void *opaque, int dcrn, uint32_t val)
> +{
> +    Ppc405PlbState *plb = opaque;
> +
> +    switch (dcrn) {
> +    case PLB0_ACR:
> +        /*
> +         * We don't care about the actual parameters written as
> +         * we don't manage any priorities on the bus
> +         */
> +        plb->acr = val & 0xF8000000;
> +        break;
> +    case PLB0_BEAR:
> +        /* Read only */
> +        break;
> +    case PLB0_BESR:
> +        /* Write-clear */
> +        plb->besr &= ~val;
> +        break;
> +    }
> +}
> +
> +static void ppc405_plb_reset(DeviceState *dev)
> +{
> +    Ppc405PlbState *plb = PPC405_PLB(dev);
> +
> +    plb->acr = 0x00000000;
> +    plb->bear = 0x00000000;
> +    plb->besr = 0x00000000;
> +}
> +
> +static void ppc405_plb_realize(DeviceState *dev, Error **errp)
> +{
> +    Ppc405PlbState *plb = PPC405_PLB(dev);
> +    Ppc4xxDcrDeviceState *dcr = PPC4xx_DCR_DEVICE(dev);
> +
> +    ppc4xx_dcr_register(dcr, PLB3A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc4xx_dcr_register(dcr, PLB4A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc4xx_dcr_register(dcr, PLB0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc4xx_dcr_register(dcr, PLB0_BEAR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc4xx_dcr_register(dcr, PLB0_BESR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc4xx_dcr_register(dcr, PLB4A1_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> +}
> +
> +static void ppc405_plb_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = ppc405_plb_realize;
> +    dc->reset = ppc405_plb_reset;
> +    /* Reason: only works as function of a ppc4xx SoC */
> +    dc->user_creatable = false;
> +}
> +
>   /* PPC4xx_DCR_DEVICE */
>   
>   void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int dcrn, void *opaque,
> @@ -694,6 +783,11 @@ static const TypeInfo ppc4xx_types[] = {
>           .instance_size  = sizeof(Ppc4xxMalState),
>           .instance_finalize = ppc4xx_mal_finalize,
>           .class_init     = ppc4xx_mal_class_init,
> +    }, {
> +        .name           = TYPE_PPC405_PLB,
> +        .parent         = TYPE_PPC4xx_DCR_DEVICE,
> +        .instance_size  = sizeof(Ppc405PlbState),
> +        .class_init     = ppc405_plb_class_init,
>       }, {
>           .name           = TYPE_PPC4xx_DCR_DEVICE,
>           .parent         = TYPE_SYS_BUS_DEVICE,
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index f40bd49bc7..e696e159f3 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -83,4 +83,15 @@ struct Ppc4xxMalState {
>       uint8_t  rxcnum;
>   };
>   
> +/* Peripheral local bus arbitrer */
> +#define TYPE_PPC405_PLB "ppc405-plb"
> +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405PlbState, PPC405_PLB);
> +struct Ppc405PlbState {
> +    Ppc4xxDcrDeviceState parent_obj;
> +
> +    uint32_t acr;
> +    uint32_t bear;
> +    uint32_t besr;
> +};
> +
>   #endif /* PPC4XX_H */