| Message ID | 675e091387248b49c97ff99ef07eb23ed316aefb.1502643878.git.balaton@eik.bme.hu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote: > These devices are found in some other ppc4xx cores too. Elsewhere in the series you moved things that were used elsewhere out of 405_uc, why not do that here as well? > Also add some more PLB registers for 460EX. Separate patch for that please, it's logically unrelated. Plus the commit message should say what these registers are, where they appear, why do we need them? > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/ppc/ppc405.h | 3 +++ > hw/ppc/ppc405_uc.c | 16 +++++++++++----- > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h > index a9ffc87..7ed25cf 100644 > --- a/hw/ppc/ppc405.h > +++ b/hw/ppc/ppc405.h > @@ -59,6 +59,9 @@ struct ppc4xx_bd_info_t { > ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd, > uint32_t flags); > > +void ppc4xx_plb_init(CPUPPCState *env); > +void ppc405_ebc_init(CPUPPCState *env); > + > CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem, > MemoryRegion ram_memories[4], > hwaddr ram_bases[4], > diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c > index 8f44cb4..8e58065 100644 > --- a/hw/ppc/ppc405_uc.c > +++ b/hw/ppc/ppc405_uc.c > @@ -105,9 +105,12 @@ ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd, > /*****************************************************************************/ > /* Peripheral local bus arbitrer */ > enum { > - PLB0_BESR = 0x084, > - PLB0_BEAR = 0x086, > - PLB0_ACR = 0x087, > + PLB3A0_ACR = 0x077, > + PLB4A0_ACR = 0x081, > + PLB0_BESR = 0x084, > + PLB0_BEAR = 0x086, > + PLB0_ACR = 0x087, > + PLB4A1_ACR = 0x089, > }; > > typedef struct ppc4xx_plb_t ppc4xx_plb_t; > @@ -174,14 +177,17 @@ static void ppc4xx_plb_reset (void *opaque) > plb->besr = 0x00000000; > } > > -static void ppc4xx_plb_init(CPUPPCState *env) > +void ppc4xx_plb_init(CPUPPCState *env) > { > ppc4xx_plb_t *plb; > > plb = g_malloc0(sizeof(ppc4xx_plb_t)); > + ppc_dcr_register(env, PLB3A0_ACR, plb, &dcr_read_plb, &dcr_write_plb); > + ppc_dcr_register(env, PLB4A0_ACR, plb, &dcr_read_plb, &dcr_write_plb); > ppc_dcr_register(env, PLB0_ACR, plb, &dcr_read_plb, &dcr_write_plb); > ppc_dcr_register(env, PLB0_BEAR, plb, &dcr_read_plb, &dcr_write_plb); > ppc_dcr_register(env, PLB0_BESR, plb, &dcr_read_plb, &dcr_write_plb); > + ppc_dcr_register(env, PLB4A1_ACR, plb, &dcr_read_plb, &dcr_write_plb); > qemu_register_reset(ppc4xx_plb_reset, plb); > } > > @@ -585,7 +591,7 @@ static void ebc_reset (void *opaque) > ebc->cfg = 0x80400000; > } > > -static void ppc405_ebc_init(CPUPPCState *env) > +void ppc405_ebc_init(CPUPPCState *env) > { > ppc4xx_ebc_t *ebc; >
On Mon, 14 Aug 2017, David Gibson wrote: > On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote: >> These devices are found in some other ppc4xx cores too. Thanks for the quick review, hope more will follow for other patches too maybe also from others. > Elsewhere in the series you moved things that were used elsewhere out > of 405_uc, why not do that here as well? I've moved those because those devices needed to be modified extensively while these two in this patch are used basically unchanged (they may not even be completely correct for 460EX but seem to work well enough) so I've left them here. I could add this explanation to the commit message or maybe move them to ppc4xx_devs to make it clearer that they are not only used by 405 emulation. >> Also add some more PLB registers for 460EX. > > Separate patch for that please, it's logically unrelated. Plus the > commit message should say what these registers are, where they appear, > why do we need them? OK. I don't really know what these registers are. I guess we need them because U-Boot or other guests poke them but they are only added to avoid the crash, not really emulated (just read zero, ignore write) for now. Regards, BALATON Zoltan
On Mon, Aug 14, 2017 at 01:06:42PM +0200, BALATON Zoltan wrote: > On Mon, 14 Aug 2017, David Gibson wrote: > > On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote: > > > These devices are found in some other ppc4xx cores too. > > Thanks for the quick review, hope more will follow for other patches too > maybe also from others. > > > Elsewhere in the series you moved things that were used elsewhere out > > of 405_uc, why not do that here as well? > > I've moved those because those devices needed to be modified extensively > while these two in this patch are used basically unchanged (they may not > even be completely correct for 460EX but seem to work well enough) so I've > left them here. I could add this explanation to the commit message or maybe > move them to ppc4xx_devs to make it clearer that they are not only used by > 405 emulation. I think move them. Probably to their own file - I think smaller files are usually going to be more readable than one big file with heaps of 4xx devices. > > > Also add some more PLB registers for 460EX. > > > > Separate patch for that please, it's logically unrelated. Plus the > > commit message should say what these registers are, where they appear, > > why do we need them? > > OK. I don't really know what these registers are. I guess we need them > because U-Boot or other guests poke them but they are only added to avoid > the crash, not really emulated (just read zero, ignore write) for > now. Ok, so say that in the commit message.
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h index a9ffc87..7ed25cf 100644 --- a/hw/ppc/ppc405.h +++ b/hw/ppc/ppc405.h @@ -59,6 +59,9 @@ struct ppc4xx_bd_info_t { ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd, uint32_t flags); +void ppc4xx_plb_init(CPUPPCState *env); +void ppc405_ebc_init(CPUPPCState *env); + CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem, MemoryRegion ram_memories[4], hwaddr ram_bases[4], diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c index 8f44cb4..8e58065 100644 --- a/hw/ppc/ppc405_uc.c +++ b/hw/ppc/ppc405_uc.c @@ -105,9 +105,12 @@ ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd, /*****************************************************************************/ /* Peripheral local bus arbitrer */ enum { - PLB0_BESR = 0x084, - PLB0_BEAR = 0x086, - PLB0_ACR = 0x087, + PLB3A0_ACR = 0x077, + PLB4A0_ACR = 0x081, + PLB0_BESR = 0x084, + PLB0_BEAR = 0x086, + PLB0_ACR = 0x087, + PLB4A1_ACR = 0x089, }; typedef struct ppc4xx_plb_t ppc4xx_plb_t; @@ -174,14 +177,17 @@ static void ppc4xx_plb_reset (void *opaque) plb->besr = 0x00000000; } -static void ppc4xx_plb_init(CPUPPCState *env) +void ppc4xx_plb_init(CPUPPCState *env) { ppc4xx_plb_t *plb; plb = g_malloc0(sizeof(ppc4xx_plb_t)); + ppc_dcr_register(env, PLB3A0_ACR, plb, &dcr_read_plb, &dcr_write_plb); + ppc_dcr_register(env, PLB4A0_ACR, plb, &dcr_read_plb, &dcr_write_plb); ppc_dcr_register(env, PLB0_ACR, plb, &dcr_read_plb, &dcr_write_plb); ppc_dcr_register(env, PLB0_BEAR, plb, &dcr_read_plb, &dcr_write_plb); ppc_dcr_register(env, PLB0_BESR, plb, &dcr_read_plb, &dcr_write_plb); + ppc_dcr_register(env, PLB4A1_ACR, plb, &dcr_read_plb, &dcr_write_plb); qemu_register_reset(ppc4xx_plb_reset, plb); } @@ -585,7 +591,7 @@ static void ebc_reset (void *opaque) ebc->cfg = 0x80400000; } -static void ppc405_ebc_init(CPUPPCState *env) +void ppc405_ebc_init(CPUPPCState *env) { ppc4xx_ebc_t *ebc;
These devices are found in some other ppc4xx cores too. Also add some more PLB registers for 460EX. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/ppc/ppc405.h | 3 +++ hw/ppc/ppc405_uc.c | 16 +++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-)