Skip to contentSkip to conversation

Sam460ex emulation

RFC PATCH In review BALATON Zoltan 6 participants 49 messages Aug 13, 2017 – Aug 23, 2017

lore
Share & tools
lore
#0 lore raw

cover Sam460ex emulation

Hello,

Here's a series to add emulation of the Sam460ex to QEMU. (This board is interesting because besides Linux, it is supported by several Amiga like OSes.) This code is based on previous work by François Revol, cleaning it up and adding some missing parts to get it to start working at least for the Linux kernel and the original U-Boot firmware of the board (which is needed to boot the Amiga like OSes easily).

This is not complete and cannot run most of the interesting OSes yet as there are still missing parts and likely some bugs I could not fix but since I have limited time and knowledge to work on this, I'd like to submit it now to get it reviewed and maybe some advice or even some help to finish it from other interested parties (especially if you have better knowledge of this hardware). Hence this RFC series, but it is probably in a shape now like some of the similar boards already in QEMU so if after review it is deemed fit I'd like to submit it for inclusion when development opens again.

Some resources and status for those who want to help:

The board is described on the manufacturer's site:
http://www.acube-systems.biz/index.php?page=hardware&pid=5
where links can also be found to the official firmware and Linux images. Only the original 2010.06.05 firmware is tested from the 20110414 updater (there is a newer version but that's only relevant for real hardware and may not work with this emulation so that's not recommended). This firmware works and starts booting but that's when problems start and you'll most likely hit some problem with most images before they boot.

AROS and Linux iso-s use the Parthenope (ub2lb) bootloader which currently fails when it calls the start_unit_scan with NULL first argument in cdrom.c:get_dev() which causes a null pointer dereference in U-Boot/board/ACube/common/sys_dep.c:start_unit_scan() which causes an exception in QEMU (vector 1300, I think it's Data TLB Error) and this leads to breakage. This may be a bug in the firmware but apparently on real hardware this works somehow so this suggests a difference in emulation that should be fixed. If I fix this in U-Boot then it hits another bug in board/ACube/common/cmd_boota.c where it references stack variable beyond its scope and gets garbled text (I wonder how this was working, maybe they used different compiler and were lucky). This causes Parthenope not find its menu.lst config. Also fixing this (here's a binary with both fixes:
http://zero.eik.bme.hu/~balaton/qemu/ ) finally it gets further booting but then AROS fails while setting up the MMU so maybe this means there are some bugs or differences between the MMU/TLB/exception emulation and real hardware (assuming it works on real hardware at all). But I don't know how real hardware works so I could not find it out. Does anyone have an idea?

Linux can also be booted via -kernel -dtb -initrd instead of booting the iso via the firmware and this gets to executing the initrd but then it seems to hang and I could not identify where and why. There are different Linux images so maybe some work better or worse but I could not fully boot a Linux system yet only to the initrd (although I've only tried a few not all of them). Also most of these images are network installers and the on board ppc4xx ethernet port is not yet emulated so this may cause the hang. Anyone wants to contribute?

MorphOS starts booting but does not find PCI devices so it fails. Unfortunately I don't know how to set kernel parameters to enable debug logging (passing them via the bootloader like on Mac does not work and MorphOS people haven't reply my query about it) so I don't care for now until other OSes easier to debug can't boot. (Besides, MorphOS could also run on the Mac emulation and may get a new version soon which might change a few things so it's low priority now; maybe I look at it again when new version is available.) I could not test AmigaOS because I don't have that but I would not be surprised if it would also fail during boot for some reason but if someone has a copy and can try it let me know how it fails (try -serial stdio to hopefully get some logs).

How can you help? (Besides fixing bugs and implementing missing parts if you can do that, but in that case you probably know what to do.) No monetary donations are solicited or accepted (I'm saying this because I got some anonymous donations in the past which I did not claim so they went to the QEMU project and did not help this work in any way, so this is not a good way to support this activity.) What would be more useful is to test on real hardware to get logs for comparison so if you have a Sam460ex (or its simpler Sam460cr version) and could gather debug logs over the serial port try getting some with an AROS iso then contact me with the log and for an AROS iso with more logging enabled to get more details.

In any case, I do this in my free time and for personal interest so don't expect any support or roadmap for this. If the info here is not enough for you to get it working for testing then it's not ready for you yet. I don't have time to help people with this so if you can't find it out you should wait until it's more developed (which may take years or could be never). Sorry for this disclaimer here but I've learned that the situation with Amiga like OSes can sometimes be interesting and I would like to avoid misunderstandings. This is very much work in progress not useful for the general users at the moment, only for interested developers.

BALATON Zoltan (12):
ppc4xx: Move MAL from ppc405_uc to ppc4xx_devs
ppc4xx: Make MAL emulation more generic
ohci: Allow sysbus version to be used as a companion
ehci: Add ppc4xx-ehci for the USB 2.0 controller in embedded PPC SoCs
ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file
ppc4xx_i2c: QOMify
ppc4xx_i2c: Implement basic I2C functions
hw/ide: Emulate SiI3112 SATA controller
ppc440: Add emulation of plb-pcix controller found in some 440 SoCs
ppc: Add 460EX embedded CPU
ppc4xx: Export ECB and PLB emulation
ppc: Add aCube Sam460ex board

 default-configs/ppcemb-softmmu.mak |    3 +
 hw/ide/Makefile.objs               |    1 +
 hw/ide/sii3112.c                   |  365 +++++++
 hw/ppc/Makefile.objs               |    4 +-
 hw/ppc/ppc405.h                    |    3 +
 hw/ppc/ppc405_uc.c                 |  527 +---------
 hw/ppc/ppc440_pcix.c               |  506 ++++++++++
 hw/ppc/ppc4xx_devs.c               |  227 +++++
 hw/ppc/ppc4xx_i2c.c                |  391 ++++++++
 hw/ppc/sam460ex.c                  | 1895 ++++++++++++++++++++++++++++++++++++
 hw/usb/hcd-ehci-sysbus.c           |   25 +
 hw/usb/hcd-ehci.h                  |    1 +
 hw/usb/hcd-ohci.c                  |   15 +-
 include/hw/i2c/ppc4xx_i2c.h        |   64 ++
 include/hw/pci/pcie_host.h         |    2 +-
 include/hw/ppc/ppc4xx.h            |    3 +
 target/ppc/cpu-models.c            |    3 +
 target/ppc/cpu-models.h            |    1 +
 target/ppc/translate_init.c        |   38 +
 19 files changed, 3557 insertions(+), 517 deletions(-)
 create mode 100644 hw/ide/sii3112.c
 create mode 100644 hw/ppc/ppc440_pcix.c
 create mode 100644 hw/ppc/ppc4xx_i2c.c
 create mode 100644 hw/ppc/sam460ex.c
 create mode 100644 include/hw/i2c/ppc4xx_i2c.h
Signature · 1 line
2.7.4
#1 lore raw

RFC PATCH 1/12 ppc4xx: Move MAL from ppc405_uc to ppc4xx_devs

This device appears in other SoCs as well not just in 405 ones

---
 hw/ppc/ppc405_uc.c      | 263 -----------------------------------------------
 hw/ppc/ppc4xx_devs.c    | 264 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/ppc4xx.h |   2 +
 3 files changed, 266 insertions(+), 263 deletions(-)
hw/ppc/ppc405_uc.c +0 -263
index f6fe3e6..3c74402 100644
@@ -42,7 +42,6 @@
 //#define DEBUG_OCM
 //#define DEBUG_I2C
 //#define DEBUG_GPT
-//#define DEBUG_MAL
 //#define DEBUG_CLOCKS
 //#define DEBUG_CLOCKS_LL
 
@@ -1513,268 +1512,6 @@ static void ppc4xx_gpt_init(hwaddr base, qemu_irq irqs[5])
 }
 
 /*****************************************************************************/
-/* MAL */
-enum {
-    MAL0_CFG      = 0x180,
-    MAL0_ESR      = 0x181,
-    MAL0_IER      = 0x182,
-    MAL0_TXCASR   = 0x184,
-    MAL0_TXCARR   = 0x185,
-    MAL0_TXEOBISR = 0x186,
-    MAL0_TXDEIR   = 0x187,
-    MAL0_RXCASR   = 0x190,
-    MAL0_RXCARR   = 0x191,
-    MAL0_RXEOBISR = 0x192,
-    MAL0_RXDEIR   = 0x193,
-    MAL0_TXCTP0R  = 0x1A0,
-    MAL0_TXCTP1R  = 0x1A1,
-    MAL0_TXCTP2R  = 0x1A2,
-    MAL0_TXCTP3R  = 0x1A3,
-    MAL0_RXCTP0R  = 0x1C0,
-    MAL0_RXCTP1R  = 0x1C1,
-    MAL0_RCBS0    = 0x1E0,
-    MAL0_RCBS1    = 0x1E1,
-};
-
-typedef struct ppc40x_mal_t ppc40x_mal_t;
-struct ppc40x_mal_t {
-    qemu_irq irqs[4];
-    uint32_t cfg;
-    uint32_t esr;
-    uint32_t ier;
-    uint32_t txcasr;
-    uint32_t txcarr;
-    uint32_t txeobisr;
-    uint32_t txdeir;
-    uint32_t rxcasr;
-    uint32_t rxcarr;
-    uint32_t rxeobisr;
-    uint32_t rxdeir;
-    uint32_t txctpr[4];
-    uint32_t rxctpr[2];
-    uint32_t rcbs[2];
-};
-
-static void ppc40x_mal_reset (void *opaque);
-
-static uint32_t dcr_read_mal (void *opaque, int dcrn)
-{
-    ppc40x_mal_t *mal;
-    uint32_t ret;
-
-    mal = opaque;
-    switch (dcrn) {
-    case MAL0_CFG:
-        ret = mal->cfg;
-        break;
-    case MAL0_ESR:
-        ret = mal->esr;
-        break;
-    case MAL0_IER:
-        ret = mal->ier;
-        break;
-    case MAL0_TXCASR:
-        ret = mal->txcasr;
-        break;
-    case MAL0_TXCARR:
-        ret = mal->txcarr;
-        break;
-    case MAL0_TXEOBISR:
-        ret = mal->txeobisr;
-        break;
-    case MAL0_TXDEIR:
-        ret = mal->txdeir;
-        break;
-    case MAL0_RXCASR:
-        ret = mal->rxcasr;
-        break;
-    case MAL0_RXCARR:
-        ret = mal->rxcarr;
-        break;
-    case MAL0_RXEOBISR:
-        ret = mal->rxeobisr;
-        break;
-    case MAL0_RXDEIR:
-        ret = mal->rxdeir;
-        break;
-    case MAL0_TXCTP0R:
-        ret = mal->txctpr[0];
-        break;
-    case MAL0_TXCTP1R:
-        ret = mal->txctpr[1];
-        break;
-    case MAL0_TXCTP2R:
-        ret = mal->txctpr[2];
-        break;
-    case MAL0_TXCTP3R:
-        ret = mal->txctpr[3];
-        break;
-    case MAL0_RXCTP0R:
-        ret = mal->rxctpr[0];
-        break;
-    case MAL0_RXCTP1R:
-        ret = mal->rxctpr[1];
-        break;
-    case MAL0_RCBS0:
-        ret = mal->rcbs[0];
-        break;
-    case MAL0_RCBS1:
-        ret = mal->rcbs[1];
-        break;
-    default:
-        ret = 0;
-        break;
-    }
-
-    return ret;
-}
-
-static void dcr_write_mal (void *opaque, int dcrn, uint32_t val)
-{
-    ppc40x_mal_t *mal;
-    int idx;
-
-    mal = opaque;
-    switch (dcrn) {
-    case MAL0_CFG:
-        if (val & 0x80000000)
-            ppc40x_mal_reset(mal);
-        mal->cfg = val & 0x00FFC087;
-        break;
-    case MAL0_ESR:
-        /* Read/clear */
-        mal->esr &= ~val;
-        break;
-    case MAL0_IER:
-        mal->ier = val & 0x0000001F;
-        break;
-    case MAL0_TXCASR:
-        mal->txcasr = val & 0xF0000000;
-        break;
-    case MAL0_TXCARR:
-        mal->txcarr = val & 0xF0000000;
-        break;
-    case MAL0_TXEOBISR:
-        /* Read/clear */
-        mal->txeobisr &= ~val;
-        break;
-    case MAL0_TXDEIR:
-        /* Read/clear */
-        mal->txdeir &= ~val;
-        break;
-    case MAL0_RXCASR:
-        mal->rxcasr = val & 0xC0000000;
-        break;
-    case MAL0_RXCARR:
-        mal->rxcarr = val & 0xC0000000;
-        break;
-    case MAL0_RXEOBISR:
-        /* Read/clear */
-        mal->rxeobisr &= ~val;
-        break;
-    case MAL0_RXDEIR:
-        /* Read/clear */
-        mal->rxdeir &= ~val;
-        break;
-    case MAL0_TXCTP0R:
-        idx = 0;
-        goto update_tx_ptr;
-    case MAL0_TXCTP1R:
-        idx = 1;
-        goto update_tx_ptr;
-    case MAL0_TXCTP2R:
-        idx = 2;
-        goto update_tx_ptr;
-    case MAL0_TXCTP3R:
-        idx = 3;
-    update_tx_ptr:
-        mal->txctpr[idx] = val;
-        break;
-    case MAL0_RXCTP0R:
-        idx = 0;
-        goto update_rx_ptr;
-    case MAL0_RXCTP1R:
-        idx = 1;
-    update_rx_ptr:
-        mal->rxctpr[idx] = val;
-        break;
-    case MAL0_RCBS0:
-        idx = 0;
-        goto update_rx_size;
-    case MAL0_RCBS1:
-        idx = 1;
-    update_rx_size:
-        mal->rcbs[idx] = val & 0x000000FF;
-        break;
-    }
-}
-
-static void ppc40x_mal_reset (void *opaque)
-{
-    ppc40x_mal_t *mal;
-
-    mal = opaque;
-    mal->cfg = 0x0007C000;
-    mal->esr = 0x00000000;
-    mal->ier = 0x00000000;
-    mal->rxcasr = 0x00000000;
-    mal->rxdeir = 0x00000000;
-    mal->rxeobisr = 0x00000000;
-    mal->txcasr = 0x00000000;
-    mal->txdeir = 0x00000000;
-    mal->txeobisr = 0x00000000;
-}
-
-static void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4])
-{
-    ppc40x_mal_t *mal;
-    int i;
-
-    mal = g_malloc0(sizeof(ppc40x_mal_t));
-    for (i = 0; i < 4; i++)
-        mal->irqs[i] = irqs[i];
-    qemu_register_reset(&ppc40x_mal_reset, mal);
-    ppc_dcr_register(env, MAL0_CFG,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_ESR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_IER,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCASR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCARR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXEOBISR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXDEIR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCASR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCARR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXEOBISR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXDEIR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP0R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP1R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP2R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP3R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCTP0R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCTP1R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RCBS0,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RCBS1,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-}
-
-/*****************************************************************************/
 /* SPR */
 void ppc40x_core_reset(PowerPCCPU *cpu)
 {
hw/ppc/ppc4xx_devs.c +264 -0

267 lines not on this page · raw message

include/hw/ppc/ppc4xx.h +2 -0
index 66e57a5..db50cfa 100644
@@ -53,6 +53,8 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
                         hwaddr *ram_sizes,
                         int do_init);
 
+void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4]);
+
 #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
 
 #endif /* PPC4XX_H */
Signature · 1 line
2.7.4
Reviewed-by on v1 1/12
#2 lore raw

Re: [Qemu-devel] [RFC PATCH 01/12] ppc4xx: Move MAL from ppc405_uc to ppc4xx_devs

Quoted 3 lines · BALATON Zoltan

This device appears in other SoCs as well not just in 405 ones

Quoted 572 lines
---
 hw/ppc/ppc405_uc.c      | 263 -----------------------------------------------
 hw/ppc/ppc4xx_devs.c    | 264 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/ppc4xx.h |   2 +
 3 files changed, 266 insertions(+), 263 deletions(-)
hw/ppc/ppc405_uc.c +0 -263
index f6fe3e6..3c74402 100644
@@ -42,7 +42,6 @@
 //#define DEBUG_OCM
 //#define DEBUG_I2C
 //#define DEBUG_GPT
-//#define DEBUG_MAL
 //#define DEBUG_CLOCKS
 //#define DEBUG_CLOCKS_LL
 
@@ -1513,268 +1512,6 @@ static void ppc4xx_gpt_init(hwaddr base, qemu_irq irqs[5])
 }
 
 /*****************************************************************************/
-/* MAL */
-enum {
-    MAL0_CFG      = 0x180,
-    MAL0_ESR      = 0x181,
-    MAL0_IER      = 0x182,
-    MAL0_TXCASR   = 0x184,
-    MAL0_TXCARR   = 0x185,
-    MAL0_TXEOBISR = 0x186,
-    MAL0_TXDEIR   = 0x187,
-    MAL0_RXCASR   = 0x190,
-    MAL0_RXCARR   = 0x191,
-    MAL0_RXEOBISR = 0x192,
-    MAL0_RXDEIR   = 0x193,
-    MAL0_TXCTP0R  = 0x1A0,
-    MAL0_TXCTP1R  = 0x1A1,
-    MAL0_TXCTP2R  = 0x1A2,
-    MAL0_TXCTP3R  = 0x1A3,
-    MAL0_RXCTP0R  = 0x1C0,
-    MAL0_RXCTP1R  = 0x1C1,
-    MAL0_RCBS0    = 0x1E0,
-    MAL0_RCBS1    = 0x1E1,
-};
-
-typedef struct ppc40x_mal_t ppc40x_mal_t;
-struct ppc40x_mal_t {
-    qemu_irq irqs[4];
-    uint32_t cfg;
-    uint32_t esr;
-    uint32_t ier;
-    uint32_t txcasr;
-    uint32_t txcarr;
-    uint32_t txeobisr;
-    uint32_t txdeir;
-    uint32_t rxcasr;
-    uint32_t rxcarr;
-    uint32_t rxeobisr;
-    uint32_t rxdeir;
-    uint32_t txctpr[4];
-    uint32_t rxctpr[2];
-    uint32_t rcbs[2];
-};
-
-static void ppc40x_mal_reset (void *opaque);
-
-static uint32_t dcr_read_mal (void *opaque, int dcrn)
-{
-    ppc40x_mal_t *mal;
-    uint32_t ret;
-
-    mal = opaque;
-    switch (dcrn) {
-    case MAL0_CFG:
-        ret = mal->cfg;
-        break;
-    case MAL0_ESR:
-        ret = mal->esr;
-        break;
-    case MAL0_IER:
-        ret = mal->ier;
-        break;
-    case MAL0_TXCASR:
-        ret = mal->txcasr;
-        break;
-    case MAL0_TXCARR:
-        ret = mal->txcarr;
-        break;
-    case MAL0_TXEOBISR:
-        ret = mal->txeobisr;
-        break;
-    case MAL0_TXDEIR:
-        ret = mal->txdeir;
-        break;
-    case MAL0_RXCASR:
-        ret = mal->rxcasr;
-        break;
-    case MAL0_RXCARR:
-        ret = mal->rxcarr;
-        break;
-    case MAL0_RXEOBISR:
-        ret = mal->rxeobisr;
-        break;
-    case MAL0_RXDEIR:
-        ret = mal->rxdeir;
-        break;
-    case MAL0_TXCTP0R:
-        ret = mal->txctpr[0];
-        break;
-    case MAL0_TXCTP1R:
-        ret = mal->txctpr[1];
-        break;
-    case MAL0_TXCTP2R:
-        ret = mal->txctpr[2];
-        break;
-    case MAL0_TXCTP3R:
-        ret = mal->txctpr[3];
-        break;
-    case MAL0_RXCTP0R:
-        ret = mal->rxctpr[0];
-        break;
-    case MAL0_RXCTP1R:
-        ret = mal->rxctpr[1];
-        break;
-    case MAL0_RCBS0:
-        ret = mal->rcbs[0];
-        break;
-    case MAL0_RCBS1:
-        ret = mal->rcbs[1];
-        break;
-    default:
-        ret = 0;
-        break;
-    }
-
-    return ret;
-}
-
-static void dcr_write_mal (void *opaque, int dcrn, uint32_t val)
-{
-    ppc40x_mal_t *mal;
-    int idx;
-
-    mal = opaque;
-    switch (dcrn) {
-    case MAL0_CFG:
-        if (val & 0x80000000)
-            ppc40x_mal_reset(mal);
-        mal->cfg = val & 0x00FFC087;
-        break;
-    case MAL0_ESR:
-        /* Read/clear */
-        mal->esr &= ~val;
-        break;
-    case MAL0_IER:
-        mal->ier = val & 0x0000001F;
-        break;
-    case MAL0_TXCASR:
-        mal->txcasr = val & 0xF0000000;
-        break;
-    case MAL0_TXCARR:
-        mal->txcarr = val & 0xF0000000;
-        break;
-    case MAL0_TXEOBISR:
-        /* Read/clear */
-        mal->txeobisr &= ~val;
-        break;
-    case MAL0_TXDEIR:
-        /* Read/clear */
-        mal->txdeir &= ~val;
-        break;
-    case MAL0_RXCASR:
-        mal->rxcasr = val & 0xC0000000;
-        break;
-    case MAL0_RXCARR:
-        mal->rxcarr = val & 0xC0000000;
-        break;
-    case MAL0_RXEOBISR:
-        /* Read/clear */
-        mal->rxeobisr &= ~val;
-        break;
-    case MAL0_RXDEIR:
-        /* Read/clear */
-        mal->rxdeir &= ~val;
-        break;
-    case MAL0_TXCTP0R:
-        idx = 0;
-        goto update_tx_ptr;
-    case MAL0_TXCTP1R:
-        idx = 1;
-        goto update_tx_ptr;
-    case MAL0_TXCTP2R:
-        idx = 2;
-        goto update_tx_ptr;
-    case MAL0_TXCTP3R:
-        idx = 3;
-    update_tx_ptr:
-        mal->txctpr[idx] = val;
-        break;
-    case MAL0_RXCTP0R:
-        idx = 0;
-        goto update_rx_ptr;
-    case MAL0_RXCTP1R:
-        idx = 1;
-    update_rx_ptr:
-        mal->rxctpr[idx] = val;
-        break;
-    case MAL0_RCBS0:
-        idx = 0;
-        goto update_rx_size;
-    case MAL0_RCBS1:
-        idx = 1;
-    update_rx_size:
-        mal->rcbs[idx] = val & 0x000000FF;
-        break;
-    }
-}
-
-static void ppc40x_mal_reset (void *opaque)
-{
-    ppc40x_mal_t *mal;
-
-    mal = opaque;
-    mal->cfg = 0x0007C000;
-    mal->esr = 0x00000000;
-    mal->ier = 0x00000000;
-    mal->rxcasr = 0x00000000;
-    mal->rxdeir = 0x00000000;
-    mal->rxeobisr = 0x00000000;
-    mal->txcasr = 0x00000000;
-    mal->txdeir = 0x00000000;
-    mal->txeobisr = 0x00000000;
-}
-
-static void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4])
-{
-    ppc40x_mal_t *mal;
-    int i;
-
-    mal = g_malloc0(sizeof(ppc40x_mal_t));
-    for (i = 0; i < 4; i++)
-        mal->irqs[i] = irqs[i];
-    qemu_register_reset(&ppc40x_mal_reset, mal);
-    ppc_dcr_register(env, MAL0_CFG,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_ESR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_IER,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCASR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCARR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXEOBISR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXDEIR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCASR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCARR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXEOBISR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXDEIR,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP0R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP1R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP2R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP3R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCTP0R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCTP1R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RCBS0,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RCBS1,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-}
-
-/*****************************************************************************/
 /* SPR */
 void ppc40x_core_reset(PowerPCCPU *cpu)
 {
hw/ppc/ppc4xx_devs.c +264 -0
index e7f413e..8e4f78e 100644
@@ -734,3 +734,267 @@ ram_addr_t ppc4xx_sdram_adjust(ram_addr_t ram_size, int nr_banks,
 
     return ram_size;
 }
+
+/*****************************************************************************/
+/* MAL */
+enum {
+    MAL0_CFG      = 0x180,
+    MAL0_ESR      = 0x181,
+    MAL0_IER      = 0x182,
+    MAL0_TXCASR   = 0x184,
+    MAL0_TXCARR   = 0x185,
+    MAL0_TXEOBISR = 0x186,
+    MAL0_TXDEIR   = 0x187,
+    MAL0_RXCASR   = 0x190,
+    MAL0_RXCARR   = 0x191,
+    MAL0_RXEOBISR = 0x192,
+    MAL0_RXDEIR   = 0x193,
+    MAL0_TXCTP0R  = 0x1A0,
+    MAL0_TXCTP1R  = 0x1A1,
+    MAL0_TXCTP2R  = 0x1A2,
+    MAL0_TXCTP3R  = 0x1A3,
+    MAL0_RXCTP0R  = 0x1C0,
+    MAL0_RXCTP1R  = 0x1C1,
+    MAL0_RCBS0    = 0x1E0,
+    MAL0_RCBS1    = 0x1E1,
+};
+
+typedef struct ppc40x_mal_t ppc40x_mal_t;
+struct ppc40x_mal_t {
+    qemu_irq irqs[4];
+    uint32_t cfg;
+    uint32_t esr;
+    uint32_t ier;
+    uint32_t txcasr;
+    uint32_t txcarr;
+    uint32_t txeobisr;
+    uint32_t txdeir;
+    uint32_t rxcasr;
+    uint32_t rxcarr;
+    uint32_t rxeobisr;
+    uint32_t rxdeir;
+    uint32_t txctpr[4];
+    uint32_t rxctpr[2];
+    uint32_t rcbs[2];
+};
+
+static void ppc40x_mal_reset(void *opaque);
+
+static uint32_t dcr_read_mal(void *opaque, int dcrn)
+{
+    ppc40x_mal_t *mal;
+    uint32_t ret;
+
+    mal = opaque;
+    switch (dcrn) {
+    case MAL0_CFG:
+        ret = mal->cfg;
+        break;
+    case MAL0_ESR:
+        ret = mal->esr;
+        break;
+    case MAL0_IER:
+        ret = mal->ier;
+        break;
+    case MAL0_TXCASR:
+        ret = mal->txcasr;
+        break;
+    case MAL0_TXCARR:
+        ret = mal->txcarr;
+        break;
+    case MAL0_TXEOBISR:
+        ret = mal->txeobisr;
+        break;
+    case MAL0_TXDEIR:
+        ret = mal->txdeir;
+        break;
+    case MAL0_RXCASR:
+        ret = mal->rxcasr;
+        break;
+    case MAL0_RXCARR:
+        ret = mal->rxcarr;
+        break;
+    case MAL0_RXEOBISR:
+        ret = mal->rxeobisr;
+        break;
+    case MAL0_RXDEIR:
+        ret = mal->rxdeir;
+        break;
+    case MAL0_TXCTP0R:
+        ret = mal->txctpr[0];
+        break;
+    case MAL0_TXCTP1R:
+        ret = mal->txctpr[1];
+        break;
+    case MAL0_TXCTP2R:
+        ret = mal->txctpr[2];
+        break;
+    case MAL0_TXCTP3R:
+        ret = mal->txctpr[3];
+        break;
+    case MAL0_RXCTP0R:
+        ret = mal->rxctpr[0];
+        break;
+    case MAL0_RXCTP1R:
+        ret = mal->rxctpr[1];
+        break;
+    case MAL0_RCBS0:
+        ret = mal->rcbs[0];
+        break;
+    case MAL0_RCBS1:
+        ret = mal->rcbs[1];
+        break;
+    default:
+        ret = 0;
+        break;
+    }
+
+    return ret;
+}
+
+static void dcr_write_mal(void *opaque, int dcrn, uint32_t val)
+{
+    ppc40x_mal_t *mal;
+    int idx;
+
+    mal = opaque;
+    switch (dcrn) {
+    case MAL0_CFG:
+        if (val & 0x80000000) {
+            ppc40x_mal_reset(mal);
+        }
+        mal->cfg = val & 0x00FFC087;
+        break;
+    case MAL0_ESR:
+        /* Read/clear */
+        mal->esr &= ~val;
+        break;
+    case MAL0_IER:
+        mal->ier = val & 0x0000001F;
+        break;
+    case MAL0_TXCASR:
+        mal->txcasr = val & 0xF0000000;
+        break;
+    case MAL0_TXCARR:
+        mal->txcarr = val & 0xF0000000;
+        break;
+    case MAL0_TXEOBISR:
+        /* Read/clear */
+        mal->txeobisr &= ~val;
+        break;
+    case MAL0_TXDEIR:
+        /* Read/clear */
+        mal->txdeir &= ~val;
+        break;
+    case MAL0_RXCASR:
+        mal->rxcasr = val & 0xC0000000;
+        break;
+    case MAL0_RXCARR:
+        mal->rxcarr = val & 0xC0000000;
+        break;
+    case MAL0_RXEOBISR:
+        /* Read/clear */
+        mal->rxeobisr &= ~val;
+        break;
+    case MAL0_RXDEIR:
+        /* Read/clear */
+        mal->rxdeir &= ~val;
+        break;
+    case MAL0_TXCTP0R:
+        idx = 0;
+        goto update_tx_ptr;
+    case MAL0_TXCTP1R:
+        idx = 1;
+        goto update_tx_ptr;
+    case MAL0_TXCTP2R:
+        idx = 2;
+        goto update_tx_ptr;
+    case MAL0_TXCTP3R:
+        idx = 3;
+    update_tx_ptr:
+        mal->txctpr[idx] = val;
+        break;
+    case MAL0_RXCTP0R:
+        idx = 0;
+        goto update_rx_ptr;
+    case MAL0_RXCTP1R:
+        idx = 1;
+    update_rx_ptr:
+        mal->rxctpr[idx] = val;
+        break;
+    case MAL0_RCBS0:
+        idx = 0;
+        goto update_rx_size;
+    case MAL0_RCBS1:
+        idx = 1;
+    update_rx_size:
+        mal->rcbs[idx] = val & 0x000000FF;
+        break;
+    }
+}
+
+static void ppc40x_mal_reset(void *opaque)
+{
+    ppc40x_mal_t *mal;
+
+    mal = opaque;
+    mal->cfg = 0x0007C000;
+    mal->esr = 0x00000000;
+    mal->ier = 0x00000000;
+    mal->rxcasr = 0x00000000;
+    mal->rxdeir = 0x00000000;
+    mal->rxeobisr = 0x00000000;
+    mal->txcasr = 0x00000000;
+    mal->txdeir = 0x00000000;
+    mal->txeobisr = 0x00000000;
+}
+
+void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4])
+{
+    ppc40x_mal_t *mal;
+    int i;
+
+    mal = g_malloc0(sizeof(ppc40x_mal_t));
+    for (i = 0; i < 4; i++) {
+        mal->irqs[i] = irqs[i];
+    }
+    qemu_register_reset(&ppc40x_mal_reset, mal);
+    ppc_dcr_register(env, MAL0_CFG,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_ESR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_IER,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXCASR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXCARR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXEOBISR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXDEIR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RXCASR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RXCARR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RXEOBISR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RXDEIR,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXCTP0R,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXCTP1R,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXCTP2R,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_TXCTP3R,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RXCTP0R,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RXCTP1R,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RCBS0,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+    ppc_dcr_register(env, MAL0_RCBS1,
+                     mal, &dcr_read_mal, &dcr_write_mal);
+}
include/hw/ppc/ppc4xx.h +2 -0
index 66e57a5..db50cfa 100644
@@ -53,6 +53,8 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
                         hwaddr *ram_sizes,
                         int do_init);
 
+void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4]);
+
 #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
 
 #endif /* PPC4XX_H */
Signature · 4 lines
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
#3 lore raw

RFC PATCH 2/12 ppc4xx: Make MAL emulation more generic

In reply to BALATON Zoltan

Allow MAL with more RX and TX channels as found in newer versions.

---
 hw/ppc/ppc405_uc.c      |   2 +-
 hw/ppc/ppc4xx_devs.c    | 171 +++++++++++++++++++-----------------------------
 include/hw/ppc/ppc4xx.h |   3 +-
 3 files changed, 70 insertions(+), 106 deletions(-)
hw/ppc/ppc405_uc.c +1 -1
index 3c74402..03856d5 100644
@@ -2281,7 +2281,7 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
     mal_irqs[1] = pic[12];
     mal_irqs[2] = pic[13];
     mal_irqs[3] = pic[14];
-    ppc405_mal_init(env, mal_irqs);
+    ppc4xx_mal_init(env, 4, 2, mal_irqs);
     /* Ethernet */
     /* Uses pic[9], pic[15], pic[17] */
     /* CPU control */
hw/ppc/ppc4xx_devs.c +67 -104

239 lines not on this page · raw message

include/hw/ppc/ppc4xx.h +2 -1
index db50cfa..cb0bb55 100644
@@ -53,7 +53,8 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
                         hwaddr *ram_sizes,
                         int do_init);
 
-void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4]);
+void ppc4xx_mal_init(CPUPPCState *env, uint8_t txcnum, uint8_t rxcnum,
+                     qemu_irq irqs[4]);
 
 #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
 
Signature · 1 line
2.7.4
Reviewed-by on v1 2/12
#4 lore raw

Re: [Qemu-devel] [RFC PATCH 02/12] ppc4xx: Make MAL emulation more generic

Quoted 3 lines · BALATON Zoltan

Allow MAL with more RX and TX channels as found in newer versions.

Looks reasonable.

Quoted 282 lines
---
 hw/ppc/ppc405_uc.c      |   2 +-
 hw/ppc/ppc4xx_devs.c    | 171 +++++++++++++++++++-----------------------------
 include/hw/ppc/ppc4xx.h |   3 +-
 3 files changed, 70 insertions(+), 106 deletions(-)
hw/ppc/ppc405_uc.c +1 -1
index 3c74402..03856d5 100644
@@ -2281,7 +2281,7 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
     mal_irqs[1] = pic[12];
     mal_irqs[2] = pic[13];
     mal_irqs[3] = pic[14];
-    ppc405_mal_init(env, mal_irqs);
+    ppc4xx_mal_init(env, 4, 2, mal_irqs);
     /* Ethernet */
     /* Uses pic[9], pic[15], pic[17] */
     /* CPU control */
hw/ppc/ppc4xx_devs.c +67 -104
index 8e4f78e..6e1cc09 100644
@@ -737,6 +737,7 @@ ram_addr_t ppc4xx_sdram_adjust(ram_addr_t ram_size, int nr_banks,
 
 /*****************************************************************************/
 /* MAL */
+
 enum {
     MAL0_CFG      = 0x180,
     MAL0_ESR      = 0x181,
@@ -750,17 +751,13 @@ enum {
     MAL0_RXEOBISR = 0x192,
     MAL0_RXDEIR   = 0x193,
     MAL0_TXCTP0R  = 0x1A0,
-    MAL0_TXCTP1R  = 0x1A1,
-    MAL0_TXCTP2R  = 0x1A2,
-    MAL0_TXCTP3R  = 0x1A3,
     MAL0_RXCTP0R  = 0x1C0,
-    MAL0_RXCTP1R  = 0x1C1,
     MAL0_RCBS0    = 0x1E0,
     MAL0_RCBS1    = 0x1E1,
 };
 
-typedef struct ppc40x_mal_t ppc40x_mal_t;
-struct ppc40x_mal_t {
+typedef struct ppc4xx_mal_t ppc4xx_mal_t;
+struct ppc4xx_mal_t {
     qemu_irq irqs[4];
     uint32_t cfg;
     uint32_t esr;
@@ -773,16 +770,32 @@ struct ppc40x_mal_t {
     uint32_t rxcarr;
     uint32_t rxeobisr;
     uint32_t rxdeir;
-    uint32_t txctpr[4];
-    uint32_t rxctpr[2];
-    uint32_t rcbs[2];
+    uint32_t *txctpr;
+    uint32_t *rxctpr;
+    uint32_t *rcbs;
+    uint8_t  txcnum;
+    uint8_t  rxcnum;
 };
 
-static void ppc40x_mal_reset(void *opaque);
+static void ppc4xx_mal_reset(void *opaque)
+{
+    ppc4xx_mal_t *mal;
+
+    mal = opaque;
+    mal->cfg = 0x0007C000;
+    mal->esr = 0x00000000;
+    mal->ier = 0x00000000;
+    mal->rxcasr = 0x00000000;
+    mal->rxdeir = 0x00000000;
+    mal->rxeobisr = 0x00000000;
+    mal->txcasr = 0x00000000;
+    mal->txdeir = 0x00000000;
+    mal->txeobisr = 0x00000000;
+}
 
 static uint32_t dcr_read_mal(void *opaque, int dcrn)
 {
-    ppc40x_mal_t *mal;
+    ppc4xx_mal_t *mal;
     uint32_t ret;
 
     mal = opaque;
@@ -820,48 +833,32 @@ static uint32_t dcr_read_mal(void *opaque, int dcrn)
     case MAL0_RXDEIR:
         ret = mal->rxdeir;
         break;
-    case MAL0_TXCTP0R:
-        ret = mal->txctpr[0];
-        break;
-    case MAL0_TXCTP1R:
-        ret = mal->txctpr[1];
-        break;
-    case MAL0_TXCTP2R:
-        ret = mal->txctpr[2];
-        break;
-    case MAL0_TXCTP3R:
-        ret = mal->txctpr[3];
-        break;
-    case MAL0_RXCTP0R:
-        ret = mal->rxctpr[0];
-        break;
-    case MAL0_RXCTP1R:
-        ret = mal->rxctpr[1];
-        break;
-    case MAL0_RCBS0:
-        ret = mal->rcbs[0];
-        break;
-    case MAL0_RCBS1:
-        ret = mal->rcbs[1];
-        break;
     default:
         ret = 0;
         break;
     }
+    if (dcrn >= MAL0_TXCTP0R && dcrn < MAL0_TXCTP0R + mal->txcnum) {
+        ret = mal->txctpr[dcrn - MAL0_TXCTP0R];
+    }
+    if (dcrn >= MAL0_RXCTP0R && dcrn < MAL0_RXCTP0R + mal->rxcnum) {
+        ret = mal->rxctpr[dcrn - MAL0_RXCTP0R];
+    }
+    if (dcrn >= MAL0_RCBS0 && dcrn < MAL0_RCBS0 + mal->rxcnum) {
+        ret = mal->rcbs[dcrn - MAL0_RCBS0];
+    }
 
     return ret;
 }
 
 static void dcr_write_mal(void *opaque, int dcrn, uint32_t val)
 {
-    ppc40x_mal_t *mal;
-    int idx;
+    ppc4xx_mal_t *mal;
 
     mal = opaque;
     switch (dcrn) {
     case MAL0_CFG:
         if (val & 0x80000000) {
-            ppc40x_mal_reset(mal);
+            ppc4xx_mal_reset(mal);
         }
         mal->cfg = val & 0x00FFC087;
         break;
@@ -900,65 +897,35 @@ static void dcr_write_mal(void *opaque, int dcrn, uint32_t val)
         /* Read/clear */
         mal->rxdeir &= ~val;
         break;
-    case MAL0_TXCTP0R:
-        idx = 0;
-        goto update_tx_ptr;
-    case MAL0_TXCTP1R:
-        idx = 1;
-        goto update_tx_ptr;
-    case MAL0_TXCTP2R:
-        idx = 2;
-        goto update_tx_ptr;
-    case MAL0_TXCTP3R:
-        idx = 3;
-    update_tx_ptr:
-        mal->txctpr[idx] = val;
-        break;
-    case MAL0_RXCTP0R:
-        idx = 0;
-        goto update_rx_ptr;
-    case MAL0_RXCTP1R:
-        idx = 1;
-    update_rx_ptr:
-        mal->rxctpr[idx] = val;
-        break;
-    case MAL0_RCBS0:
-        idx = 0;
-        goto update_rx_size;
-    case MAL0_RCBS1:
-        idx = 1;
-    update_rx_size:
-        mal->rcbs[idx] = val & 0x000000FF;
-        break;
+    }
+    if (dcrn >= MAL0_TXCTP0R && dcrn < MAL0_TXCTP0R + mal->txcnum) {
+        mal->txctpr[dcrn - MAL0_TXCTP0R] = val;
+    }
+    if (dcrn >= MAL0_RXCTP0R && dcrn < MAL0_RXCTP0R + mal->rxcnum) {
+        mal->rxctpr[dcrn - MAL0_RXCTP0R] = val;
+    }
+    if (dcrn >= MAL0_RCBS0 && dcrn < MAL0_RCBS0 + mal->rxcnum) {
+        mal->rcbs[dcrn - MAL0_RCBS0] = val & 0x000000FF;
     }
 }
 
-static void ppc40x_mal_reset(void *opaque)
-{
-    ppc40x_mal_t *mal;
-
-    mal = opaque;
-    mal->cfg = 0x0007C000;
-    mal->esr = 0x00000000;
-    mal->ier = 0x00000000;
-    mal->rxcasr = 0x00000000;
-    mal->rxdeir = 0x00000000;
-    mal->rxeobisr = 0x00000000;
-    mal->txcasr = 0x00000000;
-    mal->txdeir = 0x00000000;
-    mal->txeobisr = 0x00000000;
-}
-
-void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4])
+void ppc4xx_mal_init(CPUPPCState *env, uint8_t txcnum, uint8_t rxcnum,
+                     qemu_irq irqs[4])
 {
-    ppc40x_mal_t *mal;
+    ppc4xx_mal_t *mal;
     int i;
 
-    mal = g_malloc0(sizeof(ppc40x_mal_t));
+    assert(txcnum <= 32 && rxcnum <= 32);
+    mal = g_malloc0(sizeof(*mal));
+    mal->txcnum = txcnum;
+    mal->rxcnum = rxcnum;
+    mal->txctpr = g_new0(uint32_t, txcnum);
+    mal->rxctpr = g_new0(uint32_t, rxcnum);
+    mal->rcbs = g_new0(uint32_t, rxcnum);
     for (i = 0; i < 4; i++) {
         mal->irqs[i] = irqs[i];
     }
-    qemu_register_reset(&ppc40x_mal_reset, mal);
+    qemu_register_reset(&ppc4xx_mal_reset, mal);
     ppc_dcr_register(env, MAL0_CFG,
                      mal, &dcr_read_mal, &dcr_write_mal);
     ppc_dcr_register(env, MAL0_ESR,
@@ -981,20 +948,16 @@ void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4])
                      mal, &dcr_read_mal, &dcr_write_mal);
     ppc_dcr_register(env, MAL0_RXDEIR,
                      mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP0R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP1R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP2R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_TXCTP3R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCTP0R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RXCTP1R,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RCBS0,
-                     mal, &dcr_read_mal, &dcr_write_mal);
-    ppc_dcr_register(env, MAL0_RCBS1,
-                     mal, &dcr_read_mal, &dcr_write_mal);
+    for (i = 0; i < txcnum; i++) {
+        ppc_dcr_register(env, MAL0_TXCTP0R + i,
+                         mal, &dcr_read_mal, &dcr_write_mal);
+    }
+    for (i = 0; i < rxcnum; i++) {
+        ppc_dcr_register(env, MAL0_RXCTP0R + i,
+                         mal, &dcr_read_mal, &dcr_write_mal);
+    }
+    for (i = 0; i < rxcnum; i++) {
+        ppc_dcr_register(env, MAL0_RCBS0 + i,
+                         mal, &dcr_read_mal, &dcr_write_mal);
+    }
 }
include/hw/ppc/ppc4xx.h +2 -1
index db50cfa..cb0bb55 100644
@@ -53,7 +53,8 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
                         hwaddr *ram_sizes,
                         int do_init);
 
-void ppc405_mal_init(CPUPPCState *env, qemu_irq irqs[4]);
+void ppc4xx_mal_init(CPUPPCState *env, uint8_t txcnum, uint8_t rxcnum,
+                     qemu_irq irqs[4]);
 
 #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
 
Signature · 4 lines
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
#5 lore raw

RFC PATCH 3/12 ohci: Allow sysbus version to be used as a companion

In reply to BALATON Zoltan
---
 hw/usb/hcd-ohci.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
hw/usb/hcd-ohci.c +12 -3
index 267982e..17beedd 100644
@@ -1999,7 +1999,9 @@ typedef struct {
     /*< public >*/
 
     OHCIState ohci;
+    char *masterbus;
     uint32_t num_ports;
+    uint32_t firstport;
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
@@ -2007,10 +2009,15 @@ static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 {
     OHCISysBusState *s = SYSBUS_OHCI(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    Error *err = NULL;
 
-    /* Cannot fail as we pass NULL for masterbus */
-    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
-                  &address_space_memory, &error_abort);
+    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
+                  s->masterbus, s->firstport,
+                  &address_space_memory, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
     sysbus_init_irq(sbd, &s->ohci.irq);
     sysbus_init_mmio(sbd, &s->ohci.mem);
 }
@@ -2142,7 +2149,9 @@ static const TypeInfo ohci_pci_info = {
 };
 
 static Property ohci_sysbus_properties[] = {
+    DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
+    DEFINE_PROP_UINT32("firstport", OHCISysBusState, firstport, 0),
     DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
Signature · 1 line
2.7.4
#6 lore raw

Re: [Qemu-devel] [RFC PATCH 03/12] ohci: Allow sysbus version to be used as a companion

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

Wants a commit message - why do you need this functionality.

Patch itself looks fine to me, but you should probably CC Gerd <[email protected]> for review.

Quoted 48 lines
---
 hw/usb/hcd-ohci.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
hw/usb/hcd-ohci.c +12 -3
index 267982e..17beedd 100644
@@ -1999,7 +1999,9 @@ typedef struct {
     /*< public >*/
 
     OHCIState ohci;
+    char *masterbus;
     uint32_t num_ports;
+    uint32_t firstport;
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
@@ -2007,10 +2009,15 @@ static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 {
     OHCISysBusState *s = SYSBUS_OHCI(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    Error *err = NULL;
 
-    /* Cannot fail as we pass NULL for masterbus */
-    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
-                  &address_space_memory, &error_abort);
+    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
+                  s->masterbus, s->firstport,
+                  &address_space_memory, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
     sysbus_init_irq(sbd, &s->ohci.irq);
     sysbus_init_mmio(sbd, &s->ohci.mem);
 }
@@ -2142,7 +2149,9 @@ static const TypeInfo ohci_pci_info = {
 };
 
 static Property ohci_sysbus_properties[] = {
+    DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
+    DEFINE_PROP_UINT32("firstport", OHCISysBusState, firstport, 0),
     DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
Signature · 4 lines
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
#7 lore raw

RFC PATCH 4/12 ehci: Add ppc4xx-ehci for the USB 2.0 controller in embedded PPC SoCs

In reply to BALATON Zoltan
---
 hw/usb/hcd-ehci-sysbus.c | 25 +++++++++++++++++++++++++
 hw/usb/hcd-ehci.h        |  1 +
 2 files changed, 26 insertions(+)
hw/usb/hcd-ehci-sysbus.c +25 -0
index 6c20604..3b83beb 100644
@@ -142,6 +142,30 @@ static const TypeInfo ehci_tegra2_type_info = {
     .class_init    = ehci_tegra2_class_init,
 };
 
+static void ehci_ppc4xx_init(Object *o)
+{
+    EHCISysBusState *s = SYS_BUS_EHCI(o);
+
+    s->ehci.companion_enable = true;
+}
+
+static void ehci_ppc4xx_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    sec->capsbase = 0x0;
+    sec->opregbase = 0x10;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+}
+
+static const TypeInfo ehci_ppc4xx_type_info = {
+    .name          = TYPE_PPC4xx_EHCI,
+    .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = ehci_ppc4xx_class_init,
+    .instance_init = ehci_ppc4xx_init,
+};
+
 /*
  * Faraday FUSBH200 USB 2.0 EHCI
  */
@@ -224,6 +248,7 @@ static void ehci_sysbus_register_types(void)
     type_register_static(&ehci_xlnx_type_info);
     type_register_static(&ehci_exynos4210_type_info);
     type_register_static(&ehci_tegra2_type_info);
+    type_register_static(&ehci_ppc4xx_type_info);
     type_register_static(&ehci_fusbh200_type_info);
 }
 
hw/usb/hcd-ehci.h +1 -0
index 821f1de..0bc364b 100644
@@ -344,6 +344,7 @@ typedef struct EHCIPCIState {
 #define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
 #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
 #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
+#define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
 #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
 
 #define SYS_BUS_EHCI(obj) \
Signature · 1 line
2.7.4
#8 lore raw

Re: [Qemu-devel] [RFC PATCH 04/12] ehci: Add ppc4xx-ehci for the USB 2.0 controller in embedded PPC SoCs

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

This needs a commit message. AFAICT the new device is basically a sysbus EHCI with some altered initial values. Why do those values need to be different? Where do the new values come from?

Quoted 61 lines
---
 hw/usb/hcd-ehci-sysbus.c | 25 +++++++++++++++++++++++++
 hw/usb/hcd-ehci.h        |  1 +
 2 files changed, 26 insertions(+)
hw/usb/hcd-ehci-sysbus.c +25 -0
index 6c20604..3b83beb 100644
@@ -142,6 +142,30 @@ static const TypeInfo ehci_tegra2_type_info = {
     .class_init    = ehci_tegra2_class_init,
 };
 
+static void ehci_ppc4xx_init(Object *o)
+{
+    EHCISysBusState *s = SYS_BUS_EHCI(o);
+
+    s->ehci.companion_enable = true;
+}
+
+static void ehci_ppc4xx_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    sec->capsbase = 0x0;
+    sec->opregbase = 0x10;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+}
+
+static const TypeInfo ehci_ppc4xx_type_info = {
+    .name          = TYPE_PPC4xx_EHCI,
+    .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = ehci_ppc4xx_class_init,
+    .instance_init = ehci_ppc4xx_init,
+};
+
 /*
  * Faraday FUSBH200 USB 2.0 EHCI
  */
@@ -224,6 +248,7 @@ static void ehci_sysbus_register_types(void)
     type_register_static(&ehci_xlnx_type_info);
     type_register_static(&ehci_exynos4210_type_info);
     type_register_static(&ehci_tegra2_type_info);
+    type_register_static(&ehci_ppc4xx_type_info);
     type_register_static(&ehci_fusbh200_type_info);
 }
 
hw/usb/hcd-ehci.h +1 -0
index 821f1de..0bc364b 100644
@@ -344,6 +344,7 @@ typedef struct EHCIPCIState {
 #define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
 #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
 #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
+#define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
 #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
 
 #define SYS_BUS_EHCI(obj) \
Signature · 4 lines
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
#9 lore raw

RFC PATCH 5/12 ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file

In reply to BALATON Zoltan
---
 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc405.h      |   2 +
 hw/ppc/ppc405_uc.c   | 241 ---------------------------------------------
 hw/ppc/ppc4xx_i2c.c  | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+), 242 deletions(-)
 create mode 100644 hw/ppc/ppc4xx_i2c.c
hw/ppc/Makefile.objs +1 -1
index 7efc686..2077216 100644
@@ -13,7 +13,7 @@ endif
 obj-$(CONFIG_PSERIES) += spapr_rtas_ddw.o
 # PowerPC 4xx boards
 obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o ppc440_bamboo.o
-obj-y += ppc4xx_pci.o
+obj-y += ppc4xx_pci.o ppc4xx_i2c.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
 obj-$(CONFIG_PREP) += prep_systemio.o
hw/ppc/ppc405.h +2 -0
index a9ffc87..61ec739 100644
@@ -59,6 +59,8 @@ struct ppc4xx_bd_info_t {
 ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
                                 uint32_t flags);
 
+void ppc405_i2c_init(hwaddr base, qemu_irq irq);
+
 CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[4],
                         hwaddr ram_bases[4],
hw/ppc/ppc405_uc.c +0 -241

253 lines not on this page · raw message

hw/ppc/ppc4xx_i2c.c new file +272 -0

272 lines not on this page · raw message

Signature · 1 line
2.7.4
#10 lore raw

Re: [Qemu-devel] [RFC PATCH 05/12] ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

Needs a commit message: why is this a useful thing to do?

Quoted 572 lines
---
 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc405.h      |   2 +
 hw/ppc/ppc405_uc.c   | 241 ---------------------------------------------
 hw/ppc/ppc4xx_i2c.c  | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+), 242 deletions(-)
 create mode 100644 hw/ppc/ppc4xx_i2c.c
hw/ppc/Makefile.objs +1 -1
index 7efc686..2077216 100644
@@ -13,7 +13,7 @@ endif
 obj-$(CONFIG_PSERIES) += spapr_rtas_ddw.o
 # PowerPC 4xx boards
 obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o ppc440_bamboo.o
-obj-y += ppc4xx_pci.o
+obj-y += ppc4xx_pci.o ppc4xx_i2c.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
 obj-$(CONFIG_PREP) += prep_systemio.o
hw/ppc/ppc405.h +2 -0
index a9ffc87..61ec739 100644
@@ -59,6 +59,8 @@ struct ppc4xx_bd_info_t {
 ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
                                 uint32_t flags);
 
+void ppc405_i2c_init(hwaddr base, qemu_irq irq);
+
 CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[4],
                         hwaddr ram_bases[4],
hw/ppc/ppc405_uc.c +0 -241
index 03856d5..3925e4c 100644
@@ -40,7 +40,6 @@
 //#define DEBUG_GPIO
 //#define DEBUG_SERIAL
 //#define DEBUG_OCM
-//#define DEBUG_I2C
 //#define DEBUG_GPT
 //#define DEBUG_CLOCKS
 //#define DEBUG_CLOCKS_LL
@@ -993,246 +992,6 @@ static void ppc405_ocm_init(CPUPPCState *env)
 }
 
 /*****************************************************************************/
-/* I2C controller */
-typedef struct ppc4xx_i2c_t ppc4xx_i2c_t;
-struct ppc4xx_i2c_t {
-    qemu_irq irq;
-    MemoryRegion iomem;
-    uint8_t mdata;
-    uint8_t lmadr;
-    uint8_t hmadr;
-    uint8_t cntl;
-    uint8_t mdcntl;
-    uint8_t sts;
-    uint8_t extsts;
-    uint8_t sdata;
-    uint8_t lsadr;
-    uint8_t hsadr;
-    uint8_t clkdiv;
-    uint8_t intrmsk;
-    uint8_t xfrcnt;
-    uint8_t xtcntlss;
-    uint8_t directcntl;
-};
-
-static uint32_t ppc4xx_i2c_readb (void *opaque, hwaddr addr)
-{
-    ppc4xx_i2c_t *i2c;
-    uint32_t ret;
-
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-    i2c = opaque;
-    switch (addr) {
-    case 0x00:
-        //        i2c_readbyte(&i2c->mdata);
-        ret = i2c->mdata;
-        break;
-    case 0x02:
-        ret = i2c->sdata;
-        break;
-    case 0x04:
-        ret = i2c->lmadr;
-        break;
-    case 0x05:
-        ret = i2c->hmadr;
-        break;
-    case 0x06:
-        ret = i2c->cntl;
-        break;
-    case 0x07:
-        ret = i2c->mdcntl;
-        break;
-    case 0x08:
-        ret = i2c->sts;
-        break;
-    case 0x09:
-        ret = i2c->extsts;
-        break;
-    case 0x0A:
-        ret = i2c->lsadr;
-        break;
-    case 0x0B:
-        ret = i2c->hsadr;
-        break;
-    case 0x0C:
-        ret = i2c->clkdiv;
-        break;
-    case 0x0D:
-        ret = i2c->intrmsk;
-        break;
-    case 0x0E:
-        ret = i2c->xfrcnt;
-        break;
-    case 0x0F:
-        ret = i2c->xtcntlss;
-        break;
-    case 0x10:
-        ret = i2c->directcntl;
-        break;
-    default:
-        ret = 0x00;
-        break;
-    }
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " %02" PRIx32 "\n", __func__, addr, ret);
-#endif
-
-    return ret;
-}
-
-static void ppc4xx_i2c_writeb (void *opaque,
-                               hwaddr addr, uint32_t value)
-{
-    ppc4xx_i2c_t *i2c;
-
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
-           value);
-#endif
-    i2c = opaque;
-    switch (addr) {
-    case 0x00:
-        i2c->mdata = value;
-        //        i2c_sendbyte(&i2c->mdata);
-        break;
-    case 0x02:
-        i2c->sdata = value;
-        break;
-    case 0x04:
-        i2c->lmadr = value;
-        break;
-    case 0x05:
-        i2c->hmadr = value;
-        break;
-    case 0x06:
-        i2c->cntl = value;
-        break;
-    case 0x07:
-        i2c->mdcntl = value & 0xDF;
-        break;
-    case 0x08:
-        i2c->sts &= ~(value & 0x0A);
-        break;
-    case 0x09:
-        i2c->extsts &= ~(value & 0x8F);
-        break;
-    case 0x0A:
-        i2c->lsadr = value;
-        break;
-    case 0x0B:
-        i2c->hsadr = value;
-        break;
-    case 0x0C:
-        i2c->clkdiv = value;
-        break;
-    case 0x0D:
-        i2c->intrmsk = value;
-        break;
-    case 0x0E:
-        i2c->xfrcnt = value & 0x77;
-        break;
-    case 0x0F:
-        i2c->xtcntlss = value;
-        break;
-    case 0x10:
-        i2c->directcntl = value & 0x7;
-        break;
-    }
-}
-
-static uint32_t ppc4xx_i2c_readw (void *opaque, hwaddr addr)
-{
-    uint32_t ret;
-
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-    ret = ppc4xx_i2c_readb(opaque, addr) << 8;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 1);
-
-    return ret;
-}
-
-static void ppc4xx_i2c_writew (void *opaque,
-                               hwaddr addr, uint32_t value)
-{
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
-           value);
-#endif
-    ppc4xx_i2c_writeb(opaque, addr, value >> 8);
-    ppc4xx_i2c_writeb(opaque, addr + 1, value);
-}
-
-static uint32_t ppc4xx_i2c_readl (void *opaque, hwaddr addr)
-{
-    uint32_t ret;
-
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-    ret = ppc4xx_i2c_readb(opaque, addr) << 24;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 1) << 16;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 2) << 8;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 3);
-
-    return ret;
-}
-
-static void ppc4xx_i2c_writel (void *opaque,
-                               hwaddr addr, uint32_t value)
-{
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
-           value);
-#endif
-    ppc4xx_i2c_writeb(opaque, addr, value >> 24);
-    ppc4xx_i2c_writeb(opaque, addr + 1, value >> 16);
-    ppc4xx_i2c_writeb(opaque, addr + 2, value >> 8);
-    ppc4xx_i2c_writeb(opaque, addr + 3, value);
-}
-
-static const MemoryRegionOps i2c_ops = {
-    .old_mmio = {
-        .read = { ppc4xx_i2c_readb, ppc4xx_i2c_readw, ppc4xx_i2c_readl, },
-        .write = { ppc4xx_i2c_writeb, ppc4xx_i2c_writew, ppc4xx_i2c_writel, },
-    },
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static void ppc4xx_i2c_reset (void *opaque)
-{
-    ppc4xx_i2c_t *i2c;
-
-    i2c = opaque;
-    i2c->mdata = 0x00;
-    i2c->sdata = 0x00;
-    i2c->cntl = 0x00;
-    i2c->mdcntl = 0x00;
-    i2c->sts = 0x00;
-    i2c->extsts = 0x00;
-    i2c->clkdiv = 0x00;
-    i2c->xfrcnt = 0x00;
-    i2c->directcntl = 0x0F;
-}
-
-static void ppc405_i2c_init(hwaddr base, qemu_irq irq)
-{
-    ppc4xx_i2c_t *i2c;
-
-    i2c = g_malloc0(sizeof(ppc4xx_i2c_t));
-    i2c->irq = irq;
-#ifdef DEBUG_I2C
-    printf("%s: offset " TARGET_FMT_plx "\n", __func__, base);
-#endif
-    memory_region_init_io(&i2c->iomem, NULL, &i2c_ops, i2c, "i2c", 0x011);
-    memory_region_add_subregion(get_system_memory(), base, &i2c->iomem);
-    qemu_register_reset(ppc4xx_i2c_reset, i2c);
-}
-
-/*****************************************************************************/
 /* General purpose timers */
 typedef struct ppc4xx_gpt_t ppc4xx_gpt_t;
 struct ppc4xx_gpt_t {
hw/ppc/ppc4xx_i2c.c new file +272 -0
new file mode 100644
index 0000000..15f2dea
@@ -0,0 +1,272 @@
+/*
+ * PPC4xx I2C controller emulation
+ *
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "hw/hw.h"
+#include "exec/address-spaces.h"
+#include "hw/ppc/ppc.h"
+#include "ppc405.h"
+
+/*#define DEBUG_I2C*/
+
+typedef struct ppc4xx_i2c_t ppc4xx_i2c_t;
+struct ppc4xx_i2c_t {
+    qemu_irq irq;
+    MemoryRegion iomem;
+    uint8_t mdata;
+    uint8_t lmadr;
+    uint8_t hmadr;
+    uint8_t cntl;
+    uint8_t mdcntl;
+    uint8_t sts;
+    uint8_t extsts;
+    uint8_t sdata;
+    uint8_t lsadr;
+    uint8_t hsadr;
+    uint8_t clkdiv;
+    uint8_t intrmsk;
+    uint8_t xfrcnt;
+    uint8_t xtcntlss;
+    uint8_t directcntl;
+};
+
+static uint32_t ppc4xx_i2c_readb(void *opaque, hwaddr addr)
+{
+    ppc4xx_i2c_t *i2c;
+    uint32_t ret;
+
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+#endif
+    i2c = opaque;
+    switch (addr) {
+    case 0x00:
+        /*i2c_readbyte(&i2c->mdata);*/
+        ret = i2c->mdata;
+        break;
+    case 0x02:
+        ret = i2c->sdata;
+        break;
+    case 0x04:
+        ret = i2c->lmadr;
+        break;
+    case 0x05:
+        ret = i2c->hmadr;
+        break;
+    case 0x06:
+        ret = i2c->cntl;
+        break;
+    case 0x07:
+        ret = i2c->mdcntl;
+        break;
+    case 0x08:
+        ret = i2c->sts;
+        break;
+    case 0x09:
+        ret = i2c->extsts;
+        break;
+    case 0x0A:
+        ret = i2c->lsadr;
+        break;
+    case 0x0B:
+        ret = i2c->hsadr;
+        break;
+    case 0x0C:
+        ret = i2c->clkdiv;
+        break;
+    case 0x0D:
+        ret = i2c->intrmsk;
+        break;
+    case 0x0E:
+        ret = i2c->xfrcnt;
+        break;
+    case 0x0F:
+        ret = i2c->xtcntlss;
+        break;
+    case 0x10:
+        ret = i2c->directcntl;
+        break;
+    default:
+        ret = 0x00;
+        break;
+    }
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx " %02" PRIx32 "\n", __func__, addr, ret);
+#endif
+
+    return ret;
+}
+
+static void ppc4xx_i2c_writeb(void *opaque,
+                              hwaddr addr, uint32_t value)
+{
+    ppc4xx_i2c_t *i2c;
+
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
+           value);
+#endif
+    i2c = opaque;
+    switch (addr) {
+    case 0x00:
+        i2c->mdata = value;
+        /*i2c_sendbyte(&i2c->mdata);*/
+        break;
+    case 0x02:
+        i2c->sdata = value;
+        break;
+    case 0x04:
+        i2c->lmadr = value;
+        break;
+    case 0x05:
+        i2c->hmadr = value;
+        break;
+    case 0x06:
+        i2c->cntl = value;
+        break;
+    case 0x07:
+        i2c->mdcntl = value & 0xDF;
+        break;
+    case 0x08:
+        i2c->sts &= ~(value & 0x0A);
+        break;
+    case 0x09:
+        i2c->extsts &= ~(value & 0x8F);
+        break;
+    case 0x0A:
+        i2c->lsadr = value;
+        break;
+    case 0x0B:
+        i2c->hsadr = value;
+        break;
+    case 0x0C:
+        i2c->clkdiv = value;
+        break;
+    case 0x0D:
+        i2c->intrmsk = value;
+        break;
+    case 0x0E:
+        i2c->xfrcnt = value & 0x77;
+        break;
+    case 0x0F:
+        i2c->xtcntlss = value;
+        break;
+    case 0x10:
+        i2c->directcntl = value & 0x7;
+        break;
+    }
+}
+
+static uint32_t ppc4xx_i2c_readw(void *opaque, hwaddr addr)
+{
+    uint32_t ret;
+
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+#endif
+    ret = ppc4xx_i2c_readb(opaque, addr) << 8;
+    ret |= ppc4xx_i2c_readb(opaque, addr + 1);
+
+    return ret;
+}
+
+static void ppc4xx_i2c_writew(void *opaque,
+                              hwaddr addr, uint32_t value)
+{
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
+           value);
+#endif
+    ppc4xx_i2c_writeb(opaque, addr, value >> 8);
+    ppc4xx_i2c_writeb(opaque, addr + 1, value);
+}
+
+static uint32_t ppc4xx_i2c_readl(void *opaque, hwaddr addr)
+{
+    uint32_t ret;
+
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+#endif
+    ret = ppc4xx_i2c_readb(opaque, addr) << 24;
+    ret |= ppc4xx_i2c_readb(opaque, addr + 1) << 16;
+    ret |= ppc4xx_i2c_readb(opaque, addr + 2) << 8;
+    ret |= ppc4xx_i2c_readb(opaque, addr + 3);
+
+    return ret;
+}
+
+static void ppc4xx_i2c_writel(void *opaque,
+                              hwaddr addr, uint32_t value)
+{
+#ifdef DEBUG_I2C
+    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
+           value);
+#endif
+    ppc4xx_i2c_writeb(opaque, addr, value >> 24);
+    ppc4xx_i2c_writeb(opaque, addr + 1, value >> 16);
+    ppc4xx_i2c_writeb(opaque, addr + 2, value >> 8);
+    ppc4xx_i2c_writeb(opaque, addr + 3, value);
+}
+
+static const MemoryRegionOps i2c_ops = {
+    .old_mmio = {
+        .read = { ppc4xx_i2c_readb, ppc4xx_i2c_readw, ppc4xx_i2c_readl, },
+        .write = { ppc4xx_i2c_writeb, ppc4xx_i2c_writew, ppc4xx_i2c_writel, },
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void ppc4xx_i2c_reset(void *opaque)
+{
+    ppc4xx_i2c_t *i2c;
+
+    i2c = opaque;
+    i2c->mdata = 0x00;
+    i2c->sdata = 0x00;
+    i2c->cntl = 0x00;
+    i2c->mdcntl = 0x00;
+    i2c->sts = 0x00;
+    i2c->extsts = 0x00;
+    i2c->clkdiv = 0x00;
+    i2c->xfrcnt = 0x00;
+    i2c->directcntl = 0x0F;
+}
+
+void ppc405_i2c_init(hwaddr base, qemu_irq irq)
+{
+    ppc4xx_i2c_t *i2c;
+
+    i2c = g_malloc0(sizeof(ppc4xx_i2c_t));
+    i2c->irq = irq;
+#ifdef DEBUG_I2C
+    printf("%s: offset " TARGET_FMT_plx "\n", __func__, base);
+#endif
+    memory_region_init_io(&i2c->iomem, NULL, &i2c_ops, i2c, "i2c", 0x011);
+    memory_region_add_subregion(get_system_memory(), base, &i2c->iomem);
+    qemu_register_reset(ppc4xx_i2c_reset, i2c);
+}
Signature · 4 lines
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
#11 lore raw

Re: [Qemu-devel] [RFC PATCH 05/12] ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file

In reply to BALATON Zoltan
Quoted 8 lines · BALATON Zoltan
---
 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc405.h      |   2 +
 hw/ppc/ppc405_uc.c   | 241 ---------------------------------------------
 hw/ppc/ppc4xx_i2c.c  | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+), 242 deletions(-)
 create mode 100644 hw/ppc/ppc4xx_i2c.c

Can you move it to hw/i2c while you're at it?

Paolo

#12 lore raw

Re: [Qemu-devel] [RFC PATCH 05/12] ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file

Quoted 11 lines · Paolo Bonzini
Quoted 8 lines · BALATON Zoltan
---
 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc405.h      |   2 +
 hw/ppc/ppc405_uc.c   | 241 ---------------------------------------------
 hw/ppc/ppc4xx_i2c.c  | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+), 242 deletions(-)
 create mode 100644 hw/ppc/ppc4xx_i2c.c

Can you move it to hw/i2c while you're at it?

I could if that's the preferred way but I've left it in hw/ppc because this is not a generic device but specific to PPC SoCs and it was easier to work with files in one dir than scattered through the tree. Should I still move it to hw/i2c?

Regards,
BALATON Zoltan

#13 lore raw

Re: [Qemu-devel] [RFC PATCH 05/12] ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file

Quoted 13 lines · BALATON Zoltan
Quoted 12 lines · Paolo Bonzini
Quoted 10 lines · BALATON Zoltan
---
 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc405.h      |   2 +
 hw/ppc/ppc405_uc.c   | 241
---------------------------------------------
 hw/ppc/ppc4xx_i2c.c  | 272
+++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+), 242 deletions(-)
 create mode 100644 hw/ppc/ppc4xx_i2c.c
Quoted 6 lines
Quoted 1 line

Can you move it to hw/i2c while you're at it?

I could if that's the preferred way but I've left it in hw/ppc because this is not a generic device but specific to PPC SoCs and it was easier to work with files in one dir than scattered through the tree. Should I still move it to hw/i2c?

Please do, having devices in hw/* helps when you do all-tree cleanups.

Paolo

#14 lore raw

Re: [Qemu-devel] [RFC PATCH 05/12] ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own file

In reply to BALATON Zoltan
Quoted 7 lines · BALATON Zoltan
Quoted 1 line · Paolo Bonzini

Can you move it to hw/i2c while you're at it?

I could if that's the preferred way but I've left it in hw/ppc because this is not a generic device but specific to PPC SoCs and it was easier to work with files in one dir than scattered through the tree. Should I still move it to hw/i2c?

Yeah, when you have a device that's both "type X" and "architecture Y" you can't put it both in hw/Y and hw/X, and the choice we've made is that hw/$ARCH is for:
* board models
* SoC container objects which just instantiate other devices
and "leaf" devices go under the relevand hw/$CATEGORY (which may be hw/misc if nothing more appropriate exists). It's a little bit arbitrary, but it follows a similar practice in the Linux kernel source tree layout.

thanks
-- PMM

#15 lore raw

RFC PATCH 6/12 ppc4xx_i2c: QOMify

In reply to BALATON Zoltan
---
 hw/ppc/ppc405.h             |   2 -
 hw/ppc/ppc405_uc.c          |   5 +-
 hw/ppc/ppc4xx_i2c.c         | 153 +++++++++++++++++---------------------------
 include/hw/i2c/ppc4xx_i2c.h |  59 +++++++++++++++++
 4 files changed, 121 insertions(+), 98 deletions(-)
 create mode 100644 include/hw/i2c/ppc4xx_i2c.h
hw/ppc/ppc405.h +0 -2
index 61ec739..a9ffc87 100644
@@ -59,8 +59,6 @@ struct ppc4xx_bd_info_t {
 ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
                                 uint32_t flags);
 
-void ppc405_i2c_init(hwaddr base, qemu_irq irq);
-
 CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[4],
                         hwaddr ram_bases[4],
hw/ppc/ppc405_uc.c +3 -2
index 3925e4c..8f44cb4 100644
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/ppc/ppc.h"
 #include "hw/boards.h"
+#include "hw/i2c/ppc4xx_i2c.h"
 #include "ppc405.h"
 #include "hw/char/serial.h"
 #include "qemu/timer.h"
@@ -1663,7 +1664,7 @@ CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                        DEVICE_BIG_ENDIAN);
     }
     /* IIC controller */
-    ppc405_i2c_init(0xef600500, pic[2]);
+    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, pic[2]);
     /* GPIO */
     ppc405_gpio_init(0xef600700);
     /* CPU control */
@@ -2010,7 +2011,7 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
     dma_irqs[3] = pic[8];
     ppc405_dma_init(env, dma_irqs);
     /* IIC controller */
-    ppc405_i2c_init(0xef600500, pic[2]);
+    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, pic[2]);
     /* GPIO */
     ppc405_gpio_init(0xef600700);
     /* Serial ports */
hw/ppc/ppc4xx_i2c.c +59 -94

204 lines not on this page · raw message

include/hw/i2c/ppc4xx_i2c.h new file +59 -0

59 lines not on this page · raw message

Signature · 1 line
2.7.4
#16 lore raw

Re: [Qemu-devel] [RFC PATCH 06/12] ppc4xx_i2c: QOMify

Quoted 1 line · BALATON Zoltan

Mostly good, one niggle below.

Quoted 133 lines
---
 hw/ppc/ppc405.h             |   2 -
 hw/ppc/ppc405_uc.c          |   5 +-
 hw/ppc/ppc4xx_i2c.c         | 153 +++++++++++++++++---------------------------
 include/hw/i2c/ppc4xx_i2c.h |  59 +++++++++++++++++
 4 files changed, 121 insertions(+), 98 deletions(-)
 create mode 100644 include/hw/i2c/ppc4xx_i2c.h
hw/ppc/ppc405.h +0 -2
index 61ec739..a9ffc87 100644
@@ -59,8 +59,6 @@ struct ppc4xx_bd_info_t {
 ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
                                 uint32_t flags);
 
-void ppc405_i2c_init(hwaddr base, qemu_irq irq);
-
 CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[4],
                         hwaddr ram_bases[4],
hw/ppc/ppc405_uc.c +3 -2
index 3925e4c..8f44cb4 100644
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/ppc/ppc.h"
 #include "hw/boards.h"
+#include "hw/i2c/ppc4xx_i2c.h"
 #include "ppc405.h"
 #include "hw/char/serial.h"
 #include "qemu/timer.h"
@@ -1663,7 +1664,7 @@ CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                        DEVICE_BIG_ENDIAN);
     }
     /* IIC controller */
-    ppc405_i2c_init(0xef600500, pic[2]);
+    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, pic[2]);
     /* GPIO */
     ppc405_gpio_init(0xef600700);
     /* CPU control */
@@ -2010,7 +2011,7 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
     dma_irqs[3] = pic[8];
     ppc405_dma_init(env, dma_irqs);
     /* IIC controller */
-    ppc405_i2c_init(0xef600500, pic[2]);
+    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500, pic[2]);
     /* GPIO */
     ppc405_gpio_init(0xef600700);
     /* Serial ports */
hw/ppc/ppc4xx_i2c.c +11 -36
index 15f2dea..fafbb34 100644
@@ -27,42 +27,19 @@
 #include "qemu-common.h"
 #include "cpu.h"
 #include "hw/hw.h"
-#include "exec/address-spaces.h"
-#include "hw/ppc/ppc.h"
-#include "ppc405.h"
+#include "hw/i2c/ppc4xx_i2c.h"
 
 /*#define DEBUG_I2C*/
 
-typedef struct ppc4xx_i2c_t ppc4xx_i2c_t;
-struct ppc4xx_i2c_t {
-    qemu_irq irq;
-    MemoryRegion iomem;
-    uint8_t mdata;
-    uint8_t lmadr;
-    uint8_t hmadr;
-    uint8_t cntl;
-    uint8_t mdcntl;
-    uint8_t sts;
-    uint8_t extsts;
-    uint8_t sdata;
-    uint8_t lsadr;
-    uint8_t hsadr;
-    uint8_t clkdiv;
-    uint8_t intrmsk;
-    uint8_t xfrcnt;
-    uint8_t xtcntlss;
-    uint8_t directcntl;
-};
+#define PPC4xx_I2C_MEM_SIZE 0x11
 
-static uint32_t ppc4xx_i2c_readb(void *opaque, hwaddr addr)
+static uint8_t ppc4xx_i2c_readb(PPC4xxI2CState *i2c, hwaddr addr)
 {
-    ppc4xx_i2c_t *i2c;
-    uint32_t ret;
+    uint8_t ret;
 
 #ifdef DEBUG_I2C
     printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
 #endif
-    i2c = opaque;
     switch (addr) {
     case 0x00:
         /*i2c_readbyte(&i2c->mdata);*/
@@ -121,16 +98,12 @@ static uint32_t ppc4xx_i2c_readb(void *opaque, hwaddr addr)
     return ret;
 }
 
-static void ppc4xx_i2c_writeb(void *opaque,
-                              hwaddr addr, uint32_t value)
+static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
 {
-    ppc4xx_i2c_t *i2c;
-
 #ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
-           value);
+    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n",
+           __func__, addr, value);
 #endif
-    i2c = opaque;
     switch (addr) {
     case 0x00:
         i2c->mdata = value;
@@ -181,71 +154,44 @@ static void ppc4xx_i2c_writeb(void *opaque,
     }
 }
 
-static uint32_t ppc4xx_i2c_readw(void *opaque, hwaddr addr)
+static uint64_t ppc4xx_i2c_read(void *opaque, hwaddr addr, unsigned int size)
 {
-    uint32_t ret;
+    PPC4xxI2CState *s = PPC4xx_I2C(opaque);
+    int i;
+    uint64_t ret = 0;

hw/ppc/ppc4xx_i2c.c static void ppc4xx_i2c_writeb(void *opaque, · line 166

Quoted 10 lines
hw/ppc/ppc4xx_i2c.c +4 -5
 
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-    ret = ppc4xx_i2c_readb(opaque, addr) << 8;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 1);
+    for (i = 0; i < size; i++) {
+        ret <<= 8;
+        ret |= ppc4xx_i2c_readb(s, addr + i);
+    }

You shouldn't need to implement your own multibyte accessors in terms of the byte accessors..

Quoted 51 lines
hw/ppc/ppc4xx_i2c.c +8 -35
     return ret;
 }
 
-static void ppc4xx_i2c_writew(void *opaque,
-                              hwaddr addr, uint32_t value)
+static void ppc4xx_i2c_write(void *opaque, hwaddr addr, uint64_t value,
+                             unsigned int size)
 {
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
-           value);
-#endif
-    ppc4xx_i2c_writeb(opaque, addr, value >> 8);
-    ppc4xx_i2c_writeb(opaque, addr + 1, value);
-}
-
-static uint32_t ppc4xx_i2c_readl(void *opaque, hwaddr addr)
-{
-    uint32_t ret;
-
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-    ret = ppc4xx_i2c_readb(opaque, addr) << 24;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 1) << 16;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 2) << 8;
-    ret |= ppc4xx_i2c_readb(opaque, addr + 3);
-
-    return ret;
-}
+    PPC4xxI2CState *s = PPC4xx_I2C(opaque);
+    int i;
 
-static void ppc4xx_i2c_writel(void *opaque,
-                              hwaddr addr, uint32_t value)
-{
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n", __func__, addr,
-           value);
-#endif
-    ppc4xx_i2c_writeb(opaque, addr, value >> 24);
-    ppc4xx_i2c_writeb(opaque, addr + 1, value >> 16);
-    ppc4xx_i2c_writeb(opaque, addr + 2, value >> 8);
-    ppc4xx_i2c_writeb(opaque, addr + 3, value);
+    for (i = 0; i < size; i++) {
+        ppc4xx_i2c_writeb(s, addr + i, value & 0xff);
+        value >>= 8;
+    }
 }
 

hw/ppc/ppc4xx_i2c.c static void ppc4xx_i2c_writeb(void *opaque, · line 187

Quoted 10 lines
hw/ppc/ppc4xx_i2c.c +5 -5
-static const MemoryRegionOps i2c_ops = {
-    .old_mmio = {
-        .read = { ppc4xx_i2c_readb, ppc4xx_i2c_readw, ppc4xx_i2c_readl, },
-        .write = { ppc4xx_i2c_writeb, ppc4xx_i2c_writew, ppc4xx_i2c_writel, },
-    },
+static const MemoryRegionOps ppc4xx_i2c_ops = {
+    .read = ppc4xx_i2c_read,
+    .write = ppc4xx_i2c_write,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 4,

.. instead you should be able to set .valid.* to one thing and .impl.* to just 1,1 and the memory acces core should break up the accesses into bytes for you.

Quoted 124 lines
hw/ppc/ppc4xx_i2c.c +31 -13
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void ppc4xx_i2c_reset(void *opaque)
+static void ppc4xx_i2c_reset(DeviceState *s)
 {
-    ppc4xx_i2c_t *i2c;
+    PPC4xxI2CState *i2c = PPC4xx_I2C(s);
 
-    i2c = opaque;
     i2c->mdata = 0x00;
     i2c->sdata = 0x00;
     i2c->cntl = 0x00;
@@ -257,16 +203,35 @@ static void ppc4xx_i2c_reset(void *opaque)
     i2c->directcntl = 0x0F;
 }
 
-void ppc405_i2c_init(hwaddr base, qemu_irq irq)
+static void ppc4xx_i2c_init(Object *o)
 {
-    ppc4xx_i2c_t *i2c;
+    PPC4xxI2CState *s = PPC4xx_I2C(o);
 
-    i2c = g_malloc0(sizeof(ppc4xx_i2c_t));
-    i2c->irq = irq;
-#ifdef DEBUG_I2C
-    printf("%s: offset " TARGET_FMT_plx "\n", __func__, base);
-#endif
-    memory_region_init_io(&i2c->iomem, NULL, &i2c_ops, i2c, "i2c", 0x011);
-    memory_region_add_subregion(get_system_memory(), base, &i2c->iomem);
-    qemu_register_reset(ppc4xx_i2c_reset, i2c);
+    memory_region_init_io(&s->iomem, OBJECT(s), &ppc4xx_i2c_ops, s,
+                          TYPE_PPC4xx_I2C, PPC4xx_I2C_MEM_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+    sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
+    s->bus = i2c_init_bus(DEVICE(s), "i2c");
 }
+
+static void ppc4xx_i2c_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = ppc4xx_i2c_reset;
+}
+
+static const TypeInfo ppc4xx_i2c_type_info = {
+    .name = TYPE_PPC4xx_I2C,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PPC4xxI2CState),
+    .instance_init = ppc4xx_i2c_init,
+    .class_init = ppc4xx_i2c_class_init,
+};
+
+static void ppc4xx_i2c_register_types(void)
+{
+    type_register_static(&ppc4xx_i2c_type_info);
+}
+
+type_init(ppc4xx_i2c_register_types)
include/hw/i2c/ppc4xx_i2c.h new file +59 -0
new file mode 100644
index 0000000..7f1e6be
@@ -0,0 +1,59 @@
+/*
+ * PPC4xx I2C controller emulation
+ *
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef PPC4XX_I2C_H
+#define PPC4XX_I2C_H
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "hw/sysbus.h"
+
+#define TYPE_PPC4xx_I2C "ppc4xx-i2c"
+#define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)
+
+typedef struct PPC4xxI2CState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    qemu_irq irq;
+    MemoryRegion iomem;
+    uint8_t mdata;
+    uint8_t lmadr;
+    uint8_t hmadr;
+    uint8_t cntl;
+    uint8_t mdcntl;
+    uint8_t sts;
+    uint8_t extsts;
+    uint8_t sdata;
+    uint8_t lsadr;
+    uint8_t hsadr;
+    uint8_t clkdiv;
+    uint8_t intrmsk;
+    uint8_t xfrcnt;
+    uint8_t xtcntlss;
+    uint8_t directcntl;
+} PPC4xxI2CState;
+
+#endif /* PPC4XX_I2C_H */
Signature · 4 lines
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
#17 lore raw

RFC PATCH 7/12 ppc4xx_i2c: Implement basic I2C functions

In reply to BALATON Zoltan

Enough to please U-Boot and make it able to detect SDRAM SPD EEPROMs

---
 hw/ppc/ppc4xx_i2c.c         | 214 +++++++++++++++++++++++++++++++++++++-------
 include/hw/i2c/ppc4xx_i2c.h |   5 ++
 2 files changed, 189 insertions(+), 30 deletions(-)
hw/ppc/ppc4xx_i2c.c +184 -30
index fafbb34..0b088e6 100644
@@ -2,6 +2,8 @@
  * PPC4xx I2C controller emulation
  *
  * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2012 François Revol
+ * Copyright (c) 2016 BALATON Zoltan
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -25,25 +27,126 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "hw/hw.h"
 #include "hw/i2c/ppc4xx_i2c.h"
 
 /*#define DEBUG_I2C*/
 
-#define PPC4xx_I2C_MEM_SIZE 0x11
+#ifdef DEBUG_I2C
+#define DPRINTF(fmt, args...) printf("[%s]%s: " fmt, TYPE_PPC4xx_I2C, \
+                                     __func__, ##args);
+#else
+#define DPRINTF(fmt, args...)
+#endif
+
+#define PPC4xx_I2C_MEM_SIZE 0x12
+
+#define IIC_CNTL_PT         (1 << 0)
+#define IIC_CNTL_READ       (1 << 1)
+#define IIC_CNTL_CHT        (1 << 2)
+#define IIC_CNTL_RPST       (1 << 3)
+
+#define IIC_STS_PT          (1 << 0)
+#define IIC_STS_ERR         (1 << 2)
+#define IIC_STS_MDBS        (1 << 5)
+
+#define IIC_EXTSTS_XFRA     (1 << 0)
+
+#define IIC_XTCNTLSS_SRST   (1 << 0)
+
+static void ppc4xx_i2c_reset(DeviceState *s)
+{
+    PPC4xxI2CState *i2c = PPC4xx_I2C(s);
+
+    /* FIXME: Should also reset bus?
+     *if (s->address != ADDR_RESET) {
+     *    i2c_end_transfer(s->bus);
+     *}
+     */
+
+    i2c->mdata = 0;
+    i2c->lmadr = 0;
+    i2c->hmadr = 0;
+    i2c->cntl = 0;
+    i2c->mdcntl = 0;
+    i2c->sts = 0;
+    i2c->extsts = 0x8f;
+    i2c->sdata = 0;
+    i2c->lsadr = 0;
+    i2c->hsadr = 0;
+    i2c->clkdiv = 0;
+    i2c->intrmsk = 0;
+    i2c->xfrcnt = 0;
+    i2c->xtcntlss = 0;
+    i2c->directcntl = 0x0f;
+    i2c->intr = 0;
+}
+
+static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
+{
+    return true;
+}
 
 static uint8_t ppc4xx_i2c_readb(PPC4xxI2CState *i2c, hwaddr addr)
 {
-    uint8_t ret;
+    int ret;
 
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
     switch (addr) {
     case 0x00:
-        /*i2c_readbyte(&i2c->mdata);*/
         ret = i2c->mdata;
+        if (ppc4xx_i2c_is_master(i2c)) {
+            ret = 0xff;
+
+            if (!(i2c->sts & IIC_STS_MDBS)) {
+                qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read "
+                              "without starting transfer\n",
+                              TYPE_PPC4xx_I2C, __func__);
+            } else {
+                int pending = (i2c->cntl >> 4) & 3;
+
+                /* get the next byte */
+                ret = i2c_recv(i2c->bus);
+                DPRINTF("received byte %04x\n", ret);
+
+                if (ret < 0) {
+                    qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: read failed "
+                                  "for device 0x%02x\n", TYPE_PPC4xx_I2C,
+                                  __func__, i2c->lmadr);
+                    ret = 0xff;
+                } else {
+                    /* Raise interrupt if enabled */
+                    /*ppc4xx_i2c_raise_interrupt(i2c)*/;
+                }
+
+                if (!pending) {
+                    i2c->sts &= ~IIC_STS_MDBS;
+                    /*i2c_end_transfer(i2c->bus);*/
+                /*} else if (i2c->cntl & (IIC_CNTL_RPST | IIC_CNTL_CHT)) {*/
+                } else if (pending) {
+                    /* current smbus implementation doesn't like
+                       multibyte xfer repeated start */
+                    i2c_end_transfer(i2c->bus);
+                    if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 1)) {
+                        /* if non zero is returned, the adress is not valid */
+                        i2c->sts &= ~IIC_STS_PT;
+                        i2c->sts |= IIC_STS_ERR;
+                        i2c->extsts |= IIC_EXTSTS_XFRA;
+                    } else {
+                        /*i2c->sts |= IIC_STS_PT;*/
+                        i2c->sts |= IIC_STS_MDBS;
+                        i2c->sts &= ~IIC_STS_ERR;
+                        i2c->extsts = 0;
+                    }
+                }
+                pending--;
+                i2c->cntl = (i2c->cntl & 0xcf) | (pending << 4);
+            }
+        } else {
+            qemu_log_mask(LOG_UNIMP, "[%s]%s: slave mode not implemented\n",
+                          TYPE_PPC4xx_I2C, __func__);
+        }
         break;
     case 0x02:
         ret = i2c->sdata;
@@ -87,39 +190,88 @@ static uint8_t ppc4xx_i2c_readb(PPC4xxI2CState *i2c, hwaddr addr)
     case 0x10:
         ret = i2c->directcntl;
         break;
+    case 0x11:
+        ret = i2c->intr;
+        break;
     default:
-        ret = 0x00;
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
+                      HWADDR_PRIx "\n", TYPE_PPC4xx_I2C, __func__, addr);
+        ret = 0;
         break;
     }
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " %02" PRIx32 "\n", __func__, addr, ret);
-#endif
 
     return ret;
 }
 
 static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
 {
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n",
-           __func__, addr, value);
-#endif
     switch (addr) {
     case 0x00:
         i2c->mdata = value;
-        /*i2c_sendbyte(&i2c->mdata);*/
+        if (!i2c_bus_busy(i2c->bus)) {
+            /* assume we start a write transfer */
+            if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 0)) {
+                /* if non zero is returned, the adress is not valid */
+                i2c->sts &= ~IIC_STS_PT;
+                i2c->sts |= IIC_STS_ERR;
+                i2c->extsts |= IIC_EXTSTS_XFRA;
+            } else {
+                i2c->sts |= IIC_STS_PT;
+                i2c->sts &= ~IIC_STS_ERR;
+                i2c->extsts = 0;
+            }
+        }
+        if (i2c_bus_busy(i2c->bus)) {
+            DPRINTF("sending byte %02x\n", i2c->mdata);
+            if (i2c_send(i2c->bus, i2c->mdata)) {
+                /* if the target return non zero then end the transfer */
+                i2c->sts &= ~IIC_STS_PT;
+                i2c->sts |= IIC_STS_ERR;
+                i2c->extsts |= IIC_EXTSTS_XFRA;
+                i2c_end_transfer(i2c->bus);
+            }
+        }
         break;
     case 0x02:
         i2c->sdata = value;
         break;
     case 0x04:
         i2c->lmadr = value;
+        if (i2c_bus_busy(i2c->bus)) {
+            i2c_end_transfer(i2c->bus);
+        }
+        DPRINTF("%s: device addr %02x\n", __func__, i2c->lmadr);
         break;
     case 0x05:
         i2c->hmadr = value;
         break;
     case 0x06:
         i2c->cntl = value;
+        if (i2c->cntl & IIC_CNTL_PT) {
+            if (i2c->cntl & IIC_CNTL_READ) {
+                DPRINTF("read xfer %d\n", ((i2c->cntl >> 4) & 3) + 1);
+                if (i2c_bus_busy(i2c->bus)) {
+                    /* end previous transfer */
+                    i2c->sts &= ~IIC_STS_PT;
+                    i2c_end_transfer(i2c->bus);
+                }
+                if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 1)) {
+                    /* if non zero is returned, the adress is not valid */
+                    i2c->sts &= ~IIC_STS_PT;
+                    i2c->sts |= IIC_STS_ERR;
+                    i2c->extsts |= IIC_EXTSTS_XFRA;
+                } else {
+                    /*i2c->sts |= IIC_STS_PT;*/
+                    i2c->sts |= IIC_STS_MDBS;
+                    i2c->sts &= ~IIC_STS_ERR;
+                    i2c->extsts = 0;
+                }
+            } else {
+                DPRINTF("write xfer %d\n", ((i2c->cntl >> 4) & 3) + 1);
+                /* we actually already did the write transfer... */
+                i2c->sts &= ~IIC_STS_PT;
+            }
+        }
         break;
     case 0x07:
         i2c->mdcntl = value & 0xDF;
@@ -132,6 +284,7 @@ static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
         break;
     case 0x0A:
         i2c->lsadr = value;
+        /*i2c_set_slave_address(i2c->bus, i2c->lsadr);*/
         break;
     case 0x0B:
         i2c->hsadr = value;
@@ -146,11 +299,23 @@ static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
         i2c->xfrcnt = value & 0x77;
         break;
     case 0x0F:
+        if (value & IIC_XTCNTLSS_SRST) {
+            /* Is it actually a full reset? U-Boot sets some regs before */
+            ppc4xx_i2c_reset(DEVICE(i2c));
+            break;
+        }
         i2c->xtcntlss = value;
         break;
     case 0x10:
         i2c->directcntl = value & 0x7;
         break;
+    case 0x11:
+        i2c->intr = value;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
+                      HWADDR_PRIx "\n", TYPE_PPC4xx_I2C, __func__, addr);
+        break;
     }
 }
 
@@ -160,10 +325,12 @@ static uint64_t ppc4xx_i2c_read(void *opaque, hwaddr addr, unsigned int size)
     int i;
     uint64_t ret = 0;
 
+    DPRINTF("read I2C " TARGET_FMT_plx " size %u\n", addr, size);
     for (i = 0; i < size; i++) {
         ret <<= 8;
         ret |= ppc4xx_i2c_readb(s, addr + i);
     }
+    DPRINTF("read I2C " TARGET_FMT_plx " %02" PRIx64 "\n", addr, ret);
 
     return ret;
 }
@@ -174,6 +341,8 @@ static void ppc4xx_i2c_write(void *opaque, hwaddr addr, uint64_t value,
     PPC4xxI2CState *s = PPC4xx_I2C(opaque);
     int i;
 
+    DPRINTF("write I2C " TARGET_FMT_plx " size %u val %08" PRIx64 "\n",
+            addr, size, value);
     for (i = 0; i < size; i++) {
         ppc4xx_i2c_writeb(s, addr + i, value & 0xff);
         value >>= 8;
@@ -188,21 +357,6 @@ static const MemoryRegionOps ppc4xx_i2c_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void ppc4xx_i2c_reset(DeviceState *s)
-{
-    PPC4xxI2CState *i2c = PPC4xx_I2C(s);
-
-    i2c->mdata = 0x00;
-    i2c->sdata = 0x00;
-    i2c->cntl = 0x00;
-    i2c->mdcntl = 0x00;
-    i2c->sts = 0x00;
-    i2c->extsts = 0x00;
-    i2c->clkdiv = 0x00;
-    i2c->xfrcnt = 0x00;
-    i2c->directcntl = 0x0F;
-}
-
 static void ppc4xx_i2c_init(Object *o)
 {
     PPC4xxI2CState *s = PPC4xx_I2C(o);
include/hw/i2c/ppc4xx_i2c.h +5 -0
index 7f1e6be..71fb392 100644
@@ -2,6 +2,8 @@
  * PPC4xx I2C controller emulation
  *
  * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2012 François Revol
+ * Copyright (c) 2016 BALATON Zoltan
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -28,6 +30,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "hw/sysbus.h"
+#include "hw/i2c/i2c.h"
 
 #define TYPE_PPC4xx_I2C "ppc4xx-i2c"
 #define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)
@@ -37,6 +40,7 @@ typedef struct PPC4xxI2CState {
     SysBusDevice parent_obj;
 
     /*< public >*/
+    I2CBus *bus;
     qemu_irq irq;
     MemoryRegion iomem;
     uint8_t mdata;
@@ -54,6 +58,7 @@ typedef struct PPC4xxI2CState {
     uint8_t xfrcnt;
     uint8_t xtcntlss;
     uint8_t directcntl;
+    uint8_t intr;
 } PPC4xxI2CState;
 
 #endif /* PPC4XX_I2C_H */
Signature · 1 line
2.7.4
Reviewed-by on v1 7/12
#18 lore raw

Re: [Qemu-devel] [RFC PATCH 07/12] ppc4xx_i2c: Implement basic I2C functions

Quoted 4 lines · BALATON Zoltan

Enough to please U-Boot and make it able to detect SDRAM SPD EEPROMs

I don't have the knowledge to review this deeply (or, rather, I don't have the time to refresh myself on the 4xx manuals). However there's nothing obviously bogus, and what we had previously was clearly a stub. So I'll be happy to apply this for 2.11 if no-one with better knowledge objects.

Quoted 362 lines
---
 hw/ppc/ppc4xx_i2c.c         | 214 +++++++++++++++++++++++++++++++++++++-------
 include/hw/i2c/ppc4xx_i2c.h |   5 ++
 2 files changed, 189 insertions(+), 30 deletions(-)
hw/ppc/ppc4xx_i2c.c +184 -30
index fafbb34..0b088e6 100644
@@ -2,6 +2,8 @@
  * PPC4xx I2C controller emulation
  *
  * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2012 François Revol
+ * Copyright (c) 2016 BALATON Zoltan
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -25,25 +27,126 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "hw/hw.h"
 #include "hw/i2c/ppc4xx_i2c.h"
 
 /*#define DEBUG_I2C*/
 
-#define PPC4xx_I2C_MEM_SIZE 0x11
+#ifdef DEBUG_I2C
+#define DPRINTF(fmt, args...) printf("[%s]%s: " fmt, TYPE_PPC4xx_I2C, \
+                                     __func__, ##args);
+#else
+#define DPRINTF(fmt, args...)
+#endif
+
+#define PPC4xx_I2C_MEM_SIZE 0x12
+
+#define IIC_CNTL_PT         (1 << 0)
+#define IIC_CNTL_READ       (1 << 1)
+#define IIC_CNTL_CHT        (1 << 2)
+#define IIC_CNTL_RPST       (1 << 3)
+
+#define IIC_STS_PT          (1 << 0)
+#define IIC_STS_ERR         (1 << 2)
+#define IIC_STS_MDBS        (1 << 5)
+
+#define IIC_EXTSTS_XFRA     (1 << 0)
+
+#define IIC_XTCNTLSS_SRST   (1 << 0)
+
+static void ppc4xx_i2c_reset(DeviceState *s)
+{
+    PPC4xxI2CState *i2c = PPC4xx_I2C(s);
+
+    /* FIXME: Should also reset bus?
+     *if (s->address != ADDR_RESET) {
+     *    i2c_end_transfer(s->bus);
+     *}
+     */
+
+    i2c->mdata = 0;
+    i2c->lmadr = 0;
+    i2c->hmadr = 0;
+    i2c->cntl = 0;
+    i2c->mdcntl = 0;
+    i2c->sts = 0;
+    i2c->extsts = 0x8f;
+    i2c->sdata = 0;
+    i2c->lsadr = 0;
+    i2c->hsadr = 0;
+    i2c->clkdiv = 0;
+    i2c->intrmsk = 0;
+    i2c->xfrcnt = 0;
+    i2c->xtcntlss = 0;
+    i2c->directcntl = 0x0f;
+    i2c->intr = 0;
+}
+
+static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c)
+{
+    return true;
+}
 
 static uint8_t ppc4xx_i2c_readb(PPC4xxI2CState *i2c, hwaddr addr)
 {
-    uint8_t ret;
+    int ret;
 
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
     switch (addr) {
     case 0x00:
-        /*i2c_readbyte(&i2c->mdata);*/
         ret = i2c->mdata;
+        if (ppc4xx_i2c_is_master(i2c)) {
+            ret = 0xff;
+
+            if (!(i2c->sts & IIC_STS_MDBS)) {
+                qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read "
+                              "without starting transfer\n",
+                              TYPE_PPC4xx_I2C, __func__);
+            } else {
+                int pending = (i2c->cntl >> 4) & 3;
+
+                /* get the next byte */
+                ret = i2c_recv(i2c->bus);
+                DPRINTF("received byte %04x\n", ret);
+
+                if (ret < 0) {
+                    qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: read failed "
+                                  "for device 0x%02x\n", TYPE_PPC4xx_I2C,
+                                  __func__, i2c->lmadr);
+                    ret = 0xff;
+                } else {
+                    /* Raise interrupt if enabled */
+                    /*ppc4xx_i2c_raise_interrupt(i2c)*/;
+                }
+
+                if (!pending) {
+                    i2c->sts &= ~IIC_STS_MDBS;
+                    /*i2c_end_transfer(i2c->bus);*/
+                /*} else if (i2c->cntl & (IIC_CNTL_RPST | IIC_CNTL_CHT)) {*/
+                } else if (pending) {
+                    /* current smbus implementation doesn't like
+                       multibyte xfer repeated start */
+                    i2c_end_transfer(i2c->bus);
+                    if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 1)) {
+                        /* if non zero is returned, the adress is not valid */
+                        i2c->sts &= ~IIC_STS_PT;
+                        i2c->sts |= IIC_STS_ERR;
+                        i2c->extsts |= IIC_EXTSTS_XFRA;
+                    } else {
+                        /*i2c->sts |= IIC_STS_PT;*/
+                        i2c->sts |= IIC_STS_MDBS;
+                        i2c->sts &= ~IIC_STS_ERR;
+                        i2c->extsts = 0;
+                    }
+                }
+                pending--;
+                i2c->cntl = (i2c->cntl & 0xcf) | (pending << 4);
+            }
+        } else {
+            qemu_log_mask(LOG_UNIMP, "[%s]%s: slave mode not implemented\n",
+                          TYPE_PPC4xx_I2C, __func__);
+        }
         break;
     case 0x02:
         ret = i2c->sdata;
@@ -87,39 +190,88 @@ static uint8_t ppc4xx_i2c_readb(PPC4xxI2CState *i2c, hwaddr addr)
     case 0x10:
         ret = i2c->directcntl;
         break;
+    case 0x11:
+        ret = i2c->intr;
+        break;
     default:
-        ret = 0x00;
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
+                      HWADDR_PRIx "\n", TYPE_PPC4xx_I2C, __func__, addr);
+        ret = 0;
         break;
     }
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " %02" PRIx32 "\n", __func__, addr, ret);
-#endif
 
     return ret;
 }
 
 static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
 {
-#ifdef DEBUG_I2C
-    printf("%s: addr " TARGET_FMT_plx " val %08" PRIx32 "\n",
-           __func__, addr, value);
-#endif
     switch (addr) {
     case 0x00:
         i2c->mdata = value;
-        /*i2c_sendbyte(&i2c->mdata);*/
+        if (!i2c_bus_busy(i2c->bus)) {
+            /* assume we start a write transfer */
+            if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 0)) {
+                /* if non zero is returned, the adress is not valid */
+                i2c->sts &= ~IIC_STS_PT;
+                i2c->sts |= IIC_STS_ERR;
+                i2c->extsts |= IIC_EXTSTS_XFRA;
+            } else {
+                i2c->sts |= IIC_STS_PT;
+                i2c->sts &= ~IIC_STS_ERR;
+                i2c->extsts = 0;
+            }
+        }
+        if (i2c_bus_busy(i2c->bus)) {
+            DPRINTF("sending byte %02x\n", i2c->mdata);
+            if (i2c_send(i2c->bus, i2c->mdata)) {
+                /* if the target return non zero then end the transfer */
+                i2c->sts &= ~IIC_STS_PT;
+                i2c->sts |= IIC_STS_ERR;
+                i2c->extsts |= IIC_EXTSTS_XFRA;
+                i2c_end_transfer(i2c->bus);
+            }
+        }
         break;
     case 0x02:
         i2c->sdata = value;
         break;
     case 0x04:
         i2c->lmadr = value;
+        if (i2c_bus_busy(i2c->bus)) {
+            i2c_end_transfer(i2c->bus);
+        }
+        DPRINTF("%s: device addr %02x\n", __func__, i2c->lmadr);
         break;
     case 0x05:
         i2c->hmadr = value;
         break;
     case 0x06:
         i2c->cntl = value;
+        if (i2c->cntl & IIC_CNTL_PT) {
+            if (i2c->cntl & IIC_CNTL_READ) {
+                DPRINTF("read xfer %d\n", ((i2c->cntl >> 4) & 3) + 1);
+                if (i2c_bus_busy(i2c->bus)) {
+                    /* end previous transfer */
+                    i2c->sts &= ~IIC_STS_PT;
+                    i2c_end_transfer(i2c->bus);
+                }
+                if (i2c_start_transfer(i2c->bus, i2c->lmadr >> 1, 1)) {
+                    /* if non zero is returned, the adress is not valid */
+                    i2c->sts &= ~IIC_STS_PT;
+                    i2c->sts |= IIC_STS_ERR;
+                    i2c->extsts |= IIC_EXTSTS_XFRA;
+                } else {
+                    /*i2c->sts |= IIC_STS_PT;*/
+                    i2c->sts |= IIC_STS_MDBS;
+                    i2c->sts &= ~IIC_STS_ERR;
+                    i2c->extsts = 0;
+                }
+            } else {
+                DPRINTF("write xfer %d\n", ((i2c->cntl >> 4) & 3) + 1);
+                /* we actually already did the write transfer... */
+                i2c->sts &= ~IIC_STS_PT;
+            }
+        }
         break;
     case 0x07:
         i2c->mdcntl = value & 0xDF;
@@ -132,6 +284,7 @@ static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
         break;
     case 0x0A:
         i2c->lsadr = value;
+        /*i2c_set_slave_address(i2c->bus, i2c->lsadr);*/
         break;
     case 0x0B:
         i2c->hsadr = value;
@@ -146,11 +299,23 @@ static void ppc4xx_i2c_writeb(PPC4xxI2CState *i2c, hwaddr addr, uint8_t value)
         i2c->xfrcnt = value & 0x77;
         break;
     case 0x0F:
+        if (value & IIC_XTCNTLSS_SRST) {
+            /* Is it actually a full reset? U-Boot sets some regs before */
+            ppc4xx_i2c_reset(DEVICE(i2c));
+            break;
+        }
         i2c->xtcntlss = value;
         break;
     case 0x10:
         i2c->directcntl = value & 0x7;
         break;
+    case 0x11:
+        i2c->intr = value;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
+                      HWADDR_PRIx "\n", TYPE_PPC4xx_I2C, __func__, addr);
+        break;
     }
 }
 
@@ -160,10 +325,12 @@ static uint64_t ppc4xx_i2c_read(void *opaque, hwaddr addr, unsigned int size)
     int i;
     uint64_t ret = 0;
 
+    DPRINTF("read I2C " TARGET_FMT_plx " size %u\n", addr, size);
     for (i = 0; i < size; i++) {
         ret <<= 8;
         ret |= ppc4xx_i2c_readb(s, addr + i);
     }
+    DPRINTF("read I2C " TARGET_FMT_plx " %02" PRIx64 "\n", addr, ret);
 
     return ret;
 }
@@ -174,6 +341,8 @@ static void ppc4xx_i2c_write(void *opaque, hwaddr addr, uint64_t value,
     PPC4xxI2CState *s = PPC4xx_I2C(opaque);
     int i;
 
+    DPRINTF("write I2C " TARGET_FMT_plx " size %u val %08" PRIx64 "\n",
+            addr, size, value);
     for (i = 0; i < size; i++) {
         ppc4xx_i2c_writeb(s, addr + i, value & 0xff);
         value >>= 8;
@@ -188,21 +357,6 @@ static const MemoryRegionOps ppc4xx_i2c_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void ppc4xx_i2c_reset(DeviceState *s)
-{
-    PPC4xxI2CState *i2c = PPC4xx_I2C(s);
-
-    i2c->mdata = 0x00;
-    i2c->sdata = 0x00;
-    i2c->cntl = 0x00;
-    i2c->mdcntl = 0x00;
-    i2c->sts = 0x00;
-    i2c->extsts = 0x00;
-    i2c->clkdiv = 0x00;
-    i2c->xfrcnt = 0x00;
-    i2c->directcntl = 0x0F;
-}
-
 static void ppc4xx_i2c_init(Object *o)
 {
     PPC4xxI2CState *s = PPC4xx_I2C(o);
include/hw/i2c/ppc4xx_i2c.h +5 -0
index 7f1e6be..71fb392 100644
@@ -2,6 +2,8 @@
  * PPC4xx I2C controller emulation
  *
  * Copyright (c) 2007 Jocelyn Mayer
+ * Copyright (c) 2012 François Revol
+ * Copyright (c) 2016 BALATON Zoltan
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -28,6 +30,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "hw/sysbus.h"
+#include "hw/i2c/i2c.h"
 
 #define TYPE_PPC4xx_I2C "ppc4xx-i2c"
 #define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)
@@ -37,6 +40,7 @@ typedef struct PPC4xxI2CState {
     SysBusDevice parent_obj;
 
     /*< public >*/
+    I2CBus *bus;
     qemu_irq irq;
     MemoryRegion iomem;
     uint8_t mdata;
@@ -54,6 +58,7 @@ typedef struct PPC4xxI2CState {
     uint8_t xfrcnt;
     uint8_t xtcntlss;
     uint8_t directcntl;
+    uint8_t intr;
 } PPC4xxI2CState;
 
 #endif /* PPC4XX_I2C_H */
Signature · 4 lines
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
#19 lore raw

RFC PATCH 8/12 hw/ide: Emulate SiI3112 SATA controller

In reply to BALATON Zoltan
---
 hw/ide/Makefile.objs |   1 +
 hw/ide/sii3112.c     | 365 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 366 insertions(+)
 create mode 100644 hw/ide/sii3112.c
hw/ide/Makefile.objs +1 -0
index 729e9bd..76f3d6d 100644
@@ -10,3 +10,4 @@ common-obj-$(CONFIG_IDE_VIA) += via.o
 common-obj-$(CONFIG_MICRODRIVE) += microdrive.o
 common-obj-$(CONFIG_AHCI) += ahci.o
 common-obj-$(CONFIG_AHCI) += ich.o
+common-obj-$(CONFIG_IDE_SII3112) += sii3112.o
hw/ide/sii3112.c new file +365 -0

365 lines not on this page · raw message

Signature · 1 line
2.7.4
#20 lore raw

Re: [Qemu-devel] [RFC PATCH 08/12] hw/ide: Emulate SiI3112 SATA controller

Quoted 1 line · BALATON Zoltan

Needs a commit message: where does this controller appear? what's distinctive about it? is there a link to a datasheet or programming manual?

Quoted 386 lines
---
 hw/ide/Makefile.objs |   1 +
 hw/ide/sii3112.c     | 365 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 366 insertions(+)
 create mode 100644 hw/ide/sii3112.c
hw/ide/Makefile.objs +1 -0
index 729e9bd..76f3d6d 100644
@@ -10,3 +10,4 @@ common-obj-$(CONFIG_IDE_VIA) += via.o
 common-obj-$(CONFIG_MICRODRIVE) += microdrive.o
 common-obj-$(CONFIG_AHCI) += ahci.o
 common-obj-$(CONFIG_AHCI) += ich.o
+common-obj-$(CONFIG_IDE_SII3112) += sii3112.o
hw/ide/sii3112.c new file +365 -0
new file mode 100644
index 0000000..d3f34b5
@@ -0,0 +1,365 @@
+/*
+ * QEMU SiI3112A PCI to Serial ATA Controller Emulation
+ *
+ * Copyright (C) 2017 BALATON Zoltan <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <qemu/osdep.h>
+#include <hw/ide/pci.h>
+
+#ifdef DEBUG_IDE
+#define DPRINTF(fmt, ...) fprintf(stderr, fmt, ## __VA_ARGS__);
+#else
+#define DPRINTF(fmt, ...)
+#endif /* DEBUG */
+
+#define TYPE_SII3112_PCI "sii3112"
+#define SII3112_PCI(obj) OBJECT_CHECK(SiI3112PCIState, (obj), \
+                         TYPE_SII3112_PCI)
+
+typedef struct SiI3112Regs {
+    uint32_t confstat;
+    uint32_t scontrol;
+    uint16_t sien;
+    uint8_t swdata;
+} SiI3112Regs;
+
+typedef struct SiI3112PCIState {
+    PCIIDEState i;
+    MemoryRegion mmio;
+    SiI3112Regs regs[2];
+} SiI3112PCIState;
+
+static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
+                                unsigned int size)
+{
+    SiI3112PCIState *d = opaque;
+    uint64_t val = 0;
+
+    switch (addr) {
+    case 0x00:
+        val = d->i.bmdma[0].cmd;
+        break;
+    case 0x01:
+        val = d->regs[0].swdata;
+        break;
+    case 0x02:
+        val = d->i.bmdma[0].status;
+        break;
+    case 0x03:
+        val = 0;
+        break;
+    case 0x04 ... 0x07:
+        val = bmdma_addr_ioport_ops.read(&d->i.bmdma[0], addr - 4, size);
+        break;
+    case 0x08:
+        val = d->i.bmdma[1].cmd;
+        break;
+    case 0x09:
+        val = d->regs[1].swdata;
+        break;
+    case 0x0a:
+        val = d->i.bmdma[1].status;
+        break;
+    case 0x0b:
+        val = 0;
+        break;
+    case 0x0c ... 0x0f:
+        val = bmdma_addr_ioport_ops.read(&d->i.bmdma[1], addr - 12, size);
+        break;
+    case 0x10:
+        val = d->i.bmdma[0].cmd;
+        val |= (d->regs[0].confstat & (1UL << 11) ? (1 << 4) : 0); /*SATAINT0*/
+        val |= (d->regs[1].confstat & (1UL << 11) ? (1 << 6) : 0); /*SATAINT1*/
+        val |= (d->i.bmdma[1].status & BM_STATUS_INT ? (1 << 14) : 0);
+        val |= d->i.bmdma[0].status << 16;
+        val |= d->i.bmdma[1].status << 24;
+        break;
+    case 0x18:
+        val = d->i.bmdma[1].cmd;
+        val |= (d->regs[1].confstat & (1UL << 11) ? (1 << 4) : 0);
+        val |= d->i.bmdma[1].status << 16;
+        break;
+    case 0x80 ... 0x87:
+        if (size == 1) {
+            val = ide_ioport_read(&d->i.bus[0], addr - 0x80);
+        } else if (addr == 0x80) {
+            val = (size == 2) ? ide_data_readw(&d->i.bus[0], 0) :
+                                ide_data_readl(&d->i.bus[0], 0);
+        } else {
+            val = (1ULL << (size * 8)) - 1;
+        }
+        break;
+    case 0x8a:
+        val = (size == 1) ? ide_status_read(&d->i.bus[0], 4) :
+                            (1ULL << (size * 8)) - 1;
+        break;
+    case 0xa0:
+        val = d->regs[0].confstat;
+        break;
+    case 0xc0 ... 0xc7:
+        if (size == 1) {
+            val = ide_ioport_read(&d->i.bus[1], addr - 0xc0);
+        } else if (addr == 0xc0) {
+            val = (size == 2) ? ide_data_readw(&d->i.bus[1], 0) :
+                                ide_data_readl(&d->i.bus[1], 0);
+        } else {
+            val = (1ULL << (size * 8)) - 1;
+        }
+        break;
+    case 0xca:
+        val = (size == 1) ? ide_status_read(&d->i.bus[0], 4) :
+                            (1ULL << (size * 8)) - 1;
+        break;
+    case 0xe0:
+        val = d->regs[1].confstat;
+        break;
+    case 0x100:
+        val = d->regs[0].scontrol;
+        break;
+    case 0x104:
+        val = (d->i.bus[0].ifs[0].blk) ? 0x113 : 0;
+        break;
+    case 0x148:
+        val = d->regs[0].sien << 16;
+        break;
+    case 0x180:
+        val = d->regs[1].scontrol;
+        break;
+    case 0x184:
+        val = (d->i.bus[1].ifs[0].blk) ? 0x113 : 0;
+        break;
+    case 0x1c8:
+        val = d->regs[1].sien << 16;
+        break;
+    default:
+        val = 0;
+    }
+    DPRINTF("%s: addr 0x%"PRIx64 " size %d = %"PRIx64 "\n",
+            __func__, addr, size, val);
+    return val;
+}
+
+static void sii3112_reg_write(void *opaque, hwaddr addr,
+                              uint64_t val, unsigned int size)
+{
+    SiI3112PCIState *d = opaque;
+
+    DPRINTF("%s: addr 0x%"PRIx64 " size %d = %"PRIx64 "\n",
+            __func__, addr, size, val);
+    switch (addr) {
+    case 0x00:
+    case 0x10:
+        bmdma_cmd_writeb(&d->i.bmdma[0], val);
+        break;
+    case 0x01:
+    case 0x11:
+        d->regs[0].swdata = val & 0x3f;
+        break;
+    case 0x02:
+    case 0x12:
+        d->i.bmdma[0].status = (val & 0x60) | (d->i.bmdma[0].status & 1) |
+                               (d->i.bmdma[0].status & ~val & 6);
+        break;
+    case 0x04 ... 0x07:
+        bmdma_addr_ioport_ops.write(&d->i.bmdma[0], addr - 4, val, size);
+        break;
+    case 0x08:
+    case 0x18:
+        bmdma_cmd_writeb(&d->i.bmdma[1], val);
+        break;
+    case 0x09:
+    case 0x19:
+        d->regs[1].swdata = val & 0x3f;
+        break;
+    case 0x0a:
+    case 0x1a:
+        d->i.bmdma[1].status = (val & 0x60) | (d->i.bmdma[1].status & 1) |
+                               (d->i.bmdma[1].status & ~val & 6);
+        break;
+    case 0x0c ... 0x0f:
+        bmdma_addr_ioport_ops.write(&d->i.bmdma[1], addr - 12, val, size);
+        break;
+    case 0x80 ... 0x87:
+        if (size == 1) {
+            ide_ioport_write(&d->i.bus[0], addr - 0x80, val);
+        } else if (addr == 0x80) {
+            if (size == 2) {
+                ide_data_writew(&d->i.bus[0], 0, val);
+            } else {
+                ide_data_writel(&d->i.bus[0], 0, val);
+            }
+        }
+        break;
+    case 0x8a:
+        if (size == 1) {
+            ide_cmd_write(&d->i.bus[0], 4, val);
+        }
+        break;
+    case 0xc0 ... 0xc7:
+        if (size == 1) {
+            ide_ioport_write(&d->i.bus[1], addr - 0xc0, val);
+        } else if (addr == 0xc0) {
+            if (size == 2) {
+                ide_data_writew(&d->i.bus[1], 0, val);
+            } else {
+                ide_data_writel(&d->i.bus[1], 0, val);
+            }
+        }
+        break;
+    case 0xca:
+        if (size == 1) {
+            ide_cmd_write(&d->i.bus[1], 4, val);
+        }
+        break;
+    case 0x100:
+        d->regs[0].scontrol = val & 0xfff;
+        if (val & 1) {
+            ide_bus_reset(&d->i.bus[0]);
+        }
+        break;
+    case 0x148:
+        d->regs[0].sien = (val >> 16) & 0x3eed;
+        break;
+    case 0x180:
+        d->regs[1].scontrol = val & 0xfff;
+        if (val & 1) {
+            ide_bus_reset(&d->i.bus[1]);
+        }
+        break;
+    case 0x1c8:
+        d->regs[1].sien = (val >> 16) & 0x3eed;
+        break;
+    default:
+        val = 0;
+    }
+}
+
+static const MemoryRegionOps sii3112_reg_ops = {
+    .read = sii3112_reg_read,
+    .write = sii3112_reg_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+/* the PCI irq level is the logical OR of the two channels */
+static void sii3112_update_irq(SiI3112PCIState *s)
+{
+    int i, set = 0;
+
+    for (i = 0; i < 2; i++) {
+        set |= s->regs[i].confstat & (1UL << 11);
+    }
+    pci_set_irq(PCI_DEVICE(s), (set ? 1 : 0));
+}
+
+static void sii3112_set_irq(void *opaque, int channel, int level)
+{
+    SiI3112PCIState *s = opaque;
+
+    DPRINTF("%s: channel %d level %d\n", __func__, channel, level);
+    if (level) {
+        s->regs[channel].confstat |= (1UL << 11);
+    } else {
+        s->regs[channel].confstat &= ~(1UL << 11);
+    }
+
+    sii3112_update_irq(s);
+}
+
+static void sii3112_reset(void *opaque)
+{
+    SiI3112PCIState *s = opaque;
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        s->regs[i].confstat = 0x6515 << 16;
+        ide_bus_reset(&s->i.bus[i]);
+    }
+}
+
+static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
+{
+    SiI3112PCIState *d = SII3112_PCI(dev);
+    PCIIDEState *s = PCI_IDE(dev);
+    MemoryRegion *mr;
+    qemu_irq *irq;
+    int i;
+
+    pci_config_set_interrupt_pin(dev->config, 1);
+    pci_set_byte(dev->config + PCI_CACHE_LINE_SIZE, 8);
+
+    memory_region_init_io(&d->mmio, OBJECT(d), &sii3112_reg_ops, d,
+                         "sii3112.bar5", 0x200);
+    pci_register_bar(dev, 5, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
+
+    mr = g_new(MemoryRegion, 1);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar0", &d->mmio, 0x80, 8);
+    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    mr = g_new(MemoryRegion, 1);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar1", &d->mmio, 0x88, 4);
+    pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    mr = g_new(MemoryRegion, 1);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar2", &d->mmio, 0xc0, 8);
+    pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    mr = g_new(MemoryRegion, 1);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar3", &d->mmio, 0xc8, 4);
+    pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    mr = g_new(MemoryRegion, 1);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &d->mmio, 0, 16);
+    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
+
+    irq = qemu_allocate_irqs(sii3112_set_irq, d, 2);
+    for (i = 0; i < 2; i++) {
+        ide_bus_new(&s->bus[i], sizeof(s->bus[i]), DEVICE(dev), i, 1);
+        ide_init2(&s->bus[i], irq[i]);
+
+        bmdma_init(&s->bus[i], &s->bmdma[i], s);
+        s->bmdma[i].bus = &s->bus[i];
+        ide_register_restart_cb(&s->bus[i]);
+    }
+    qemu_register_reset(sii3112_reset, s);
+}
+
+static void sii3112_pci_exitfn(PCIDevice *dev)
+{
+    PCIIDEState *d = PCI_IDE(dev);
+    int i;
+
+    for (i = 0; i < 2; ++i) {
+        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
+        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
+    }
+}
+
+static void sii3112_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pd = PCI_DEVICE_CLASS(klass);
+
+    pd->vendor_id = 0x1095;
+    pd->device_id = 0x3112;
+    pd->class_id = PCI_CLASS_STORAGE_RAID;
+    pd->revision = 1;
+    pd->realize = sii3112_pci_realize;
+    pd->exit = sii3112_pci_exitfn;
+    dc->desc = "SiI3112A SATA controller";
+    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+}
+
+static const TypeInfo sii3112_pci_info = {
+    .name = TYPE_SII3112_PCI,
+    .parent = TYPE_PCI_IDE,
+    .instance_size = sizeof(SiI3112PCIState),
+    .class_init = sii3112_pci_class_init,
+};
+
+static void sii3112_register_types(void)
+{
+    type_register_static(&sii3112_pci_info);
+}
+
+type_init(sii3112_register_types)
Signature · 4 lines
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
#21 lore raw

Re: [Qemu-devel] [RFC PATCH 08/12] hw/ide: Emulate SiI3112 SATA controller

Quoted 5 lines · David Gibson
Quoted 1 line · BALATON Zoltan

Needs a commit message: where does this controller appear? what's distinctive about it? is there a link to a datasheet or programming manual?

This is just a generic PCI SATA controller, quite common and also used in PCs. Nothing special about it apart from guests running on Sam460ex usually have a driver for it (unlike other already emulated SATA controllers) and it is simpler to implement than the on board SATA controller of the 460EX and also has two ports instead of one so more useful to have.

I could add this as commit message if you think that would be better or just submit it separately through the IDE branch then use from these patch series.

Regards,
BALATON Zoltan

#22 lore raw

Re: [Qemu-devel] [RFC PATCH 08/12] hw/ide: Emulate SiI3112 SATA controller

Quoted 13 lines · BALATON Zoltan
Quoted 5 lines · David Gibson
Quoted 1 line · BALATON Zoltan

Needs a commit message: where does this controller appear? what's distinctive about it? is there a link to a datasheet or programming manual?

This is just a generic PCI SATA controller, quite common and also used in PCs. Nothing special about it apart from guests running on Sam460ex usually have a driver for it (unlike other already emulated SATA controllers) and it is simpler to implement than the on board SATA controller of the 460EX and also has two ports instead of one so more useful to have.

Quoted 3 lines

I could add this as commit message if you think that would be better or just submit it separately through the IDE branch then use from these patch series.

Please do add that as a commit message. We probably should get an ack from Jon Snow for this.

Signature · 4 lines
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
#23 lore raw

RFC PATCH 9/12 ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

In reply to BALATON Zoltan
---
 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc440_pcix.c | 506 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 507 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/ppc440_pcix.c
hw/ppc/Makefile.objs +1 -1
index 2077216..9c5d58a 100644
@@ -13,7 +13,7 @@ endif
 obj-$(CONFIG_PSERIES) += spapr_rtas_ddw.o
 # PowerPC 4xx boards
 obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o ppc440_bamboo.o
-obj-y += ppc4xx_pci.o ppc4xx_i2c.o
+obj-y += ppc4xx_pci.o ppc440_pcix.o ppc4xx_i2c.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
 obj-$(CONFIG_PREP) += prep_systemio.o
hw/ppc/ppc440_pcix.c new file +506 -0

506 lines not on this page · raw message

Signature · 1 line
2.7.4
#24 lore raw

Re: [Qemu-devel] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

You know I'm going to say it, right: needs a commit message.

What's a "plb-pcix", and what's an example of a 440 SoCs which has it.

This is basically a new device, so I'm pretty willing to merge for 2.11 with minimal review once rebased with the rest of the series.

Couple of comments below

[snip]

hw/ppc/ppc440_pcix.c line 407

Quoted 7 lines
hw/ppc/ppc440_pcix.c new file +7 -0
+static void ppc440_pcix_reset(DeviceState *dev)
+{
+    struct PPC440PCIXState *s = PPC440_PCIX_HOST_BRIDGE(dev);
+    int i;
+
+    memset(s->pom, 0, sizeof(s->pom));
+    memset(s->pim, 0, sizeof(s->pim));

Is it safe to just memset() the memory region objects within the pim/pom arrays without cleaning them up? I'm guessing not..

Quoted 15 lines
hw/ppc/ppc440_pcix.c new file +15 -0
+    for (i = 0; i < PPC440_PCIX_NR_PIMS; i++) {
+        s->pim[i].sa = 0xffffffff00000000ULL;
+    }
+    s->sts = 0;
+}
+
+/* All pins from each slot are tied to a single board IRQ.
+ * This may need further refactoring for other boards. */
+static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+    int slot = pci_dev->devfn >> 3;
+
+    DPRINTF("%s: devfn %x irq %d -> %d\n", __func__,
+            pci_dev->devfn, irq_num, slot);
+

hw/ppc/ppc440_pcix.c line 432

Quoted 10 lines
hw/ppc/ppc440_pcix.c new file +10 -0
+    return slot - 1;
+}
+
+static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
+{
+    qemu_irq *pci_irqs = opaque;
+
+    DPRINTF("%s: PCI irq %d\n", __func__, irq_num);
+    if (irq_num < 0) {
+        fprintf(stderr, "%s: PCI irq %d\n", __func__, irq_num);

Use report_error() please.

Quoted 74 lines
hw/ppc/ppc440_pcix.c new file +74 -0
+        return;
+    }
+    qemu_set_irq(pci_irqs[irq_num], level);
+}
+
+static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
+{
+    PPC440PCIXState *s = opaque;
+
+    return &s->bm_as;
+}
+
+static int ppc440_pcix_initfn(SysBusDevice *dev)
+{
+    PPC440PCIXState *s;
+    PCIHostState *h;
+    int i;
+
+    h = PCI_HOST_BRIDGE(dev);
+    s = PPC440_PCIX_HOST_BRIDGE(dev);
+
+    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+        sysbus_init_irq(dev, &s->irq[i]);
+    }
+
+    memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
+    h->bus = pci_register_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq,
+                         ppc440_pcix_map_irq, s->irq, &s->busmem,
+                         get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+
+    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
+
+    memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
+    memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
+    address_space_init(&s->bm_as, &s->bm, "pci-bm");
+    pci_setup_iommu(h->bus, ppc440_pcix_set_iommu, s);
+
+    memory_region_init(&s->container, OBJECT(s), "pci-container", PCI_ALL_SIZE);
+    memory_region_init_io(&h->conf_mem, OBJECT(s), &pci_host_conf_le_ops, h,
+                          "pci-conf-idx", 4);
+    memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops, h,
+                          "pci-conf-data", 4);
+    memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
+                          "pci.reg", PPC440_REG_SIZE);
+    memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem);
+    memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem);
+    memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem);
+    sysbus_init_mmio(dev, &s->container);
+
+    return 0;
+}
+
+static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
+{
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    k->init = ppc440_pcix_initfn;
+    dc->reset = ppc440_pcix_reset;
+}
+
+static const TypeInfo ppc440_pcix_info = {
+    .name          = TYPE_PPC440_PCIX_HOST_BRIDGE,
+    .parent        = TYPE_PCI_HOST_BRIDGE,
+    .instance_size = sizeof(PPC440PCIXState),
+    .class_init    = ppc440_pcix_class_init,
+};
+
+static void ppc440_pcix_register_types(void)
+{
+    type_register_static(&ppc440_pcix_info);
+}
+
+type_init(ppc440_pcix_register_types)
Signature · 4 lines
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
#25 lore raw

Re: [Qemu-devel] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

Hi,

Quoted 5 lines · David Gibson

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

You know I'm going to say it, right: needs a commit message.

What's a "plb-pcix", and what's an example of a 440 SoCs which has it.

IIRC that's the PCI(express) controller, which is not memory-mapped but inside the DCR address space, so accessed from the PLB.

See:

http://www.embeddeddeveloper.com/assets/processors/amcc/datasheets/PP460EX_DS2063.pdf

page 9, 13.

François.

#26 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

Quoted 9 lines · François Revol
Quoted 5 lines · David Gibson

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

You know I'm going to say it, right: needs a commit message.

What's a "plb-pcix", and what's an example of a 440 SoCs which has it.

IIRC that's the PCI(express) controller, which is not memory-mapped but inside the DCR address space, so accessed from the PLB.

This is the PCI(X) controller only, the PCIe(xpress) controller is (partially) implemented in sam460ex.c TYPE_PPC460EX_PCIE_HOST/"ppc460ex-pcie-host". The plb-pcix is the compatible name the fdt refers to. I only know of the 460EX that has it but probably other similar SoCs (like some 440 variants) also have this kind of PCI controller (according to #defines in U-Boot source but don't remember the details).

Regards,
BALATON Zoltan

#27 lore raw

Re: [Qemu-devel] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

In reply to François Revol
Quoted 9 lines · François Revol

Hi,

Quoted 5 lines · David Gibson

On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:

You know I'm going to say it, right: needs a commit message.

What's a "plb-pcix", and what's an example of a 440 SoCs which has it.

Quoted 8 lines

IIRC that's the PCI(express) controller, which is not memory-mapped but inside the DCR address space, so accessed from the PLB.

See:

http://www.embeddeddeveloper.com/assets/processors/amcc/datasheets/PP460EX_DS2063.pdf

page 9, 13.

Sure, I can guess that. But my point is that this is the sort of information (including the link) that ought to be in the commit message.

Signature · 4 lines
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
#28 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

In reply to BALATON Zoltan

hi Balaton,
i can help with amigaos4 for sam .
i can test it on P5040 book3e and on G5 quad book3s machine.
note sam for boot amigaos need a special modified version of uboot that is available on acube website it is a firmware update. without modified uboot will be not posdible auto boot amigaos. idont know if is possible use standard uboot incuded in qemu for boot amigaos from uboot command line. i can ask help to Enrico Vidale as my FB friend but i dont know if he will be available for this kind of helping.

bye
luigi

Inviato da iPad

Quoted 538 lines

Il giorno 14 ago 2017, alle ore 01:30, BALATON Zoltan <[email protected]> ha scritto:

---
hw/ppc/Makefile.objs | 2 +-
hw/ppc/ppc440_pcix.c | 506 +++++++++++++++++++++++++++++++++++++++++++++++++++

2 files changed, 507 insertions(+), 1 deletion(-)

create mode 100644 hw/ppc/ppc440_pcix.c

hw/ppc/Makefile.objs +0 -0
index 2077216..9c5d58a 100644
@@ -13,7 +13,7 @@ endif

obj-$(CONFIG_PSERIES) += spapr_rtas_ddw.o
# PowerPC 4xx boards
obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o ppc440_bamboo.o

hw/ppc/Makefile.objs +1 -1
-obj-y += ppc4xx_pci.o ppc4xx_i2c.o
+obj-y += ppc4xx_pci.o ppc440_pcix.o ppc4xx_i2c.o

# PReP
obj-$(CONFIG_PREP) += prep.o
obj-$(CONFIG_PREP) += prep_systemio.o

hw/ppc/ppc440_pcix.c new file +506 -0
new file mode 100644
index 0000000..3abd0d0
@@ -0,0 +1,506 @@
+/*
+ * Emulation of the ibm,plb-pcix PCI controller
+ * This is found in some 440 SoCs e.g. the 460EX.
+ *
+ * Copyright (c) 2016 BALATON Zoltan
+ *
+ * Derived from ppc4xx_pci.c and pci-host/ppce500.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "hw/ppc/ppc.h"
+#include "hw/ppc/ppc4xx.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
+#include "exec/address-spaces.h"
+
+/*#define DEBUG_PCI*/
+
+#ifdef DEBUG_PCI
+#define DPRINTF(fmt, ...) fprintf(stderr, fmt, ## __VA_ARGS__);
+#else
+#define DPRINTF(fmt, ...)
+#endif /* DEBUG */
+
+struct PLBOutMap {
+    uint64_t la;
+    uint64_t pcia;
+    uint32_t sa;
+    MemoryRegion mr;
+};
+
+struct PLBInMap {
+    uint64_t sa;
+    uint64_t la;
+    MemoryRegion mr;
+};
+
+#define TYPE_PPC440_PCIX_HOST_BRIDGE "ppc440-pcix-host"
+#define PPC440_PCIX_HOST_BRIDGE(obj) \
+    OBJECT_CHECK(PPC440PCIXState, (obj), TYPE_PPC440_PCIX_HOST_BRIDGE)
+
+#define PPC440_PCIX_NR_POMS 3
+#define PPC440_PCIX_NR_PIMS 3
+
+typedef struct PPC440PCIXState {
+    PCIHostState parent_obj;
+
+    PCIDevice *dev;
+    struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
+    struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
+    uint32_t sts;
+    qemu_irq irq[PCI_NUM_PINS];
+    AddressSpace bm_as;
+    MemoryRegion bm;
+
+    MemoryRegion container;
+    MemoryRegion iomem;
+    MemoryRegion busmem;
+} PPC440PCIXState;
+
+#define PPC440_REG_BASE     0x80000
+#define PPC440_REG_SIZE     0xff
+
+#define PCIC0_CFGADDR       0x0
+#define PCIC0_CFGDATA       0x4
+
+#define PCIX0_POM0LAL       0x68
+#define PCIX0_POM0LAH       0x6c
+#define PCIX0_POM0SA        0x70
+#define PCIX0_POM0PCIAL     0x74
+#define PCIX0_POM0PCIAH     0x78
+#define PCIX0_POM1LAL       0x7c
+#define PCIX0_POM1LAH       0x80
+#define PCIX0_POM1SA        0x84
+#define PCIX0_POM1PCIAL     0x88
+#define PCIX0_POM1PCIAH     0x8c
+#define PCIX0_POM2SA        0x90
+
+#define PCIX0_PIM0SAL       0x98
+#define PCIX0_PIM0LAL       0x9c
+#define PCIX0_PIM0LAH       0xa0
+#define PCIX0_PIM1SA        0xa4
+#define PCIX0_PIM1LAL       0xa8
+#define PCIX0_PIM1LAH       0xac
+#define PCIX0_PIM2SAL       0xb0
+#define PCIX0_PIM2LAL       0xb4
+#define PCIX0_PIM2LAH       0xb8
+#define PCIX0_PIM0SAH       0xf8
+#define PCIX0_PIM2SAH       0xfc
+
+#define PCIX0_STS           0xe0
+
+#define PCI_ALL_SIZE        (PPC440_REG_BASE + PPC440_REG_SIZE)
+
+/* DMA mapping */
+static void ppc440_pcix_update_pim(PPC440PCIXState *s, int idx)
+{
+    MemoryRegion *mem = &s->pim[idx].mr;
+    char *name;
+    uint64_t size;
+
+    if (memory_region_is_mapped(mem)) {
+        /* Before we modify anything, unmap and destroy the region */
+        memory_region_del_subregion(&s->bm, mem);
+        object_unparent(OBJECT(mem));
+    }
+
+    if (!(s->pim[idx].sa & 1)) {
+        /* Not enabled, nothing to do */
+        return;
+    }
+
+    name = g_strdup_printf("PCI Inbound Window %d", idx);
+    size = ~(s->pim[idx].sa & ~7ULL) + 1;
+    memory_region_init_alias(mem, OBJECT(s), name, get_system_memory(),
+                             s->pim[idx].la, size);
+    memory_region_add_subregion_overlap(&s->bm, 0, mem, -1);
+    g_free(name);
+
+    DPRINTF("%s: Added window %d of size=%#"PRIx64" to CPU=%#"PRIx64"\n",
+            __func__, idx, size, s->pim[idx].la);
+}
+
+/* BAR mapping */
+static void ppc440_pcix_update_pom(PPC440PCIXState *s, int idx)
+{
+    MemoryRegion *mem = &s->pom[idx].mr;
+    MemoryRegion *address_space_mem = get_system_memory();
+    char *name;
+    uint32_t size;
+
+    if (memory_region_is_mapped(mem)) {
+        /* Before we modify anything, unmap and destroy the region */
+        memory_region_del_subregion(address_space_mem, mem);
+        object_unparent(OBJECT(mem));
+    }
+
+    if (!(s->pom[idx].sa & 1)) {
+        /* Not enabled, nothing to do */
+        return;
+    }
+
+    name = g_strdup_printf("PCI Outbound Window %d", idx);
+    size = ~(s->pom[idx].sa & 0xfffffffe) + 1;
+    if (!size) {
+        size = 0xffffffff;
+    }
+    memory_region_init_alias(mem, OBJECT(s), name, &s->busmem,
+                             s->pom[idx].pcia, size);
+    memory_region_add_subregion(address_space_mem, s->pom[idx].la, mem);
+    g_free(name);
+
+    DPRINTF("%s: Added window %d of size=%#x from CPU=%#"PRIx64
+            " to PCI=%#"PRIx64"\n", __func__, idx, size, s->pom[idx].la,
+            s->pom[idx].pcia);
+}
+
+static void ppc440_pcix_reg_write4(void *opaque, hwaddr addr,
+                                   uint64_t val, unsigned size)
+{
+    struct PPC440PCIXState *s = opaque;
+
+    DPRINTF("%s: addr 0x%"PRIx64 " = %"PRIx64 "\n", __func__, addr, val);
+    switch (addr) {
+    case PCI_VENDOR_ID ... PCI_MAX_LAT:
+        stl_le_p(s->dev->config + addr, val);
+        break;
+
+    case PCIX0_POM0LAL:
+        s->pom[0].la &= 0xffffffff00000000ULL;
+        s->pom[0].la |= val;
+        ppc440_pcix_update_pom(s, 0);
+        break;
+    case PCIX0_POM0LAH:
+        s->pom[0].la &= 0xffffffffULL;
+        s->pom[0].la |= val << 32;
+        ppc440_pcix_update_pom(s, 0);
+        break;
+    case PCIX0_POM0SA:
+        s->pom[0].sa = val;
+        ppc440_pcix_update_pom(s, 0);
+        break;
+    case PCIX0_POM0PCIAL:
+        s->pom[0].pcia &= 0xffffffff00000000ULL;
+        s->pom[0].pcia |= val;
+        ppc440_pcix_update_pom(s, 0);
+        break;
+    case PCIX0_POM0PCIAH:
+        s->pom[0].pcia &= 0xffffffffULL;
+        s->pom[0].pcia |= val << 32;
+        ppc440_pcix_update_pom(s, 0);
+        break;
+    case PCIX0_POM1LAL:
+        s->pom[1].la &= 0xffffffff00000000ULL;
+        s->pom[1].la |= val;
+        ppc440_pcix_update_pom(s, 1);
+        break;
+    case PCIX0_POM1LAH:
+        s->pom[1].la &= 0xffffffffULL;
+        s->pom[1].la |= val << 32;
+        ppc440_pcix_update_pom(s, 1);
+        break;
+    case PCIX0_POM1SA:
+        s->pom[1].sa = val;
+        ppc440_pcix_update_pom(s, 1);
+        break;
+    case PCIX0_POM1PCIAL:
+        s->pom[1].pcia &= 0xffffffff00000000ULL;
+        s->pom[1].pcia |= val;
+        ppc440_pcix_update_pom(s, 1);
+        break;
+    case PCIX0_POM1PCIAH:
+        s->pom[1].pcia &= 0xffffffffULL;
+        s->pom[1].pcia |= val << 32;
+        ppc440_pcix_update_pom(s, 1);
+        break;
+    case PCIX0_POM2SA:
+        s->pom[2].sa = val;
+        break;
+
+    case PCIX0_PIM0SAL:
+        s->pim[0].sa &= 0xffffffff00000000ULL;
+        s->pim[0].sa |= val;
+        ppc440_pcix_update_pim(s, 0);
+        break;
+    case PCIX0_PIM0LAL:
+        s->pim[0].la &= 0xffffffff00000000ULL;
+        s->pim[0].la |= val;
+        ppc440_pcix_update_pim(s, 0);
+        break;
+    case PCIX0_PIM0LAH:
+        s->pim[0].la &= 0xffffffffULL;
+        s->pim[0].la |= val << 32;
+        ppc440_pcix_update_pim(s, 0);
+        break;
+    case PCIX0_PIM1SA:
+        s->pim[1].sa = val;
+        ppc440_pcix_update_pim(s, 1);
+        break;
+    case PCIX0_PIM1LAL:
+        s->pim[1].la &= 0xffffffff00000000ULL;
+        s->pim[1].la |= val;
+        ppc440_pcix_update_pim(s, 1);
+        break;
+    case PCIX0_PIM1LAH:
+        s->pim[1].la &= 0xffffffffULL;
+        s->pim[1].la |= val << 32;
+        ppc440_pcix_update_pim(s, 1);
+        break;
+    case PCIX0_PIM2SAL:
+        s->pim[2].sa &= 0xffffffff00000000ULL;
+        s->pim[2].sa = val;
+        ppc440_pcix_update_pim(s, 2);
+        break;
+    case PCIX0_PIM2LAL:
+        s->pim[2].la &= 0xffffffff00000000ULL;
+        s->pim[2].la |= val;
+        ppc440_pcix_update_pim(s, 2);
+        break;
+    case PCIX0_PIM2LAH:
+        s->pim[2].la &= 0xffffffffULL;
+        s->pim[2].la |= val << 32;
+        ppc440_pcix_update_pim(s, 2);
+        break;
+
+    case PCIX0_STS:
+        s->sts = val;
+        break;
+
+    case PCIX0_PIM0SAH:
+        s->pim[0].sa &= 0xffffffffULL;
+        s->pim[0].sa |= val << 32;
+        ppc440_pcix_update_pim(s, 0);
+        break;
+    case PCIX0_PIM2SAH:
+        s->pim[2].sa &= 0xffffffffULL;
+        s->pim[2].sa |= val << 32;
+        ppc440_pcix_update_pim(s, 2);
+        break;
+
+    default:
+        printf("%s: unhandled PCI internal register 0x%lx\n", __func__,
+               (unsigned long)addr);
+        break;
+    }
+}
+
+static uint64_t ppc440_pcix_reg_read4(void *opaque, hwaddr addr,
+                                     unsigned size)
+{
+    struct PPC440PCIXState *s = opaque;
+    uint32_t val;
+
+    switch (addr) {
+    case PCI_VENDOR_ID ... PCI_MAX_LAT:
+        val = ldl_le_p(s->dev->config + addr);
+        break;
+
+    case PCIX0_POM0LAL:
+        val = s->pom[0].la;
+        break;
+    case PCIX0_POM0LAH:
+        val = s->pom[0].la >> 32;
+        break;
+    case PCIX0_POM0SA:
+        val = s->pom[0].sa;
+        break;
+    case PCIX0_POM0PCIAL:
+        val = s->pom[0].pcia;
+        break;
+    case PCIX0_POM0PCIAH:
+        val = s->pom[0].pcia >> 32;
+        break;
+    case PCIX0_POM1LAL:
+        val = s->pom[1].la;
+        break;
+    case PCIX0_POM1LAH:
+        val = s->pom[1].la >> 32;
+        break;
+    case PCIX0_POM1SA:
+        val = s->pom[1].sa;
+        break;
+    case PCIX0_POM1PCIAL:
+        val = s->pom[1].pcia;
+        break;
+    case PCIX0_POM1PCIAH:
+        val = s->pom[1].pcia >> 32;
+        break;
+    case PCIX0_POM2SA:
+        val = s->pom[2].sa;
+        break;
+
+    case PCIX0_PIM0SAL:
+        val = s->pim[0].sa;
+        break;
+    case PCIX0_PIM0LAL:
+        val = s->pim[0].la;
+        break;
+    case PCIX0_PIM0LAH:
+        val = s->pim[0].la >> 32;
+        break;
+    case PCIX0_PIM1SA:
+        val = s->pim[1].sa;
+        break;
+    case PCIX0_PIM1LAL:
+        val = s->pim[1].la;
+        break;
+    case PCIX0_PIM1LAH:
+        val = s->pim[1].la >> 32;
+        break;
+    case PCIX0_PIM2SAL:
+        val = s->pim[2].sa;
+        break;
+    case PCIX0_PIM2LAL:
+        val = s->pim[2].la;
+        break;
+    case PCIX0_PIM2LAH:
+        val = s->pim[2].la >> 32;
+        break;
+
+    case PCIX0_STS:
+        val = s->sts;
+        break;
+
+    case PCIX0_PIM0SAH:
+        val = s->pim[0].sa  >> 32;
+        break;
+    case PCIX0_PIM2SAH:
+        val = s->pim[2].sa  >> 32;
+        break;
+
+    default:
+        printf("%s: invalid PCI internal register 0x%lx\n", __func__,
+               (unsigned long)addr);
+        val = 0;
+    }
+
+    DPRINTF("%s: addr 0x%"PRIx64 " = %"PRIx32 "\n", __func__, addr, val);
+    return val;
+}
+
+static const MemoryRegionOps pci_reg_ops = {
+    .read = ppc440_pcix_reg_read4,
+    .write = ppc440_pcix_reg_write4,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void ppc440_pcix_reset(DeviceState *dev)
+{
+    struct PPC440PCIXState *s = PPC440_PCIX_HOST_BRIDGE(dev);
+    int i;
+
+    memset(s->pom, 0, sizeof(s->pom));
+    memset(s->pim, 0, sizeof(s->pim));
+    for (i = 0; i < PPC440_PCIX_NR_PIMS; i++) {
+        s->pim[i].sa = 0xffffffff00000000ULL;
+    }
+    s->sts = 0;
+}
+
+/* All pins from each slot are tied to a single board IRQ.
+ * This may need further refactoring for other boards. */
+static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+    int slot = pci_dev->devfn >> 3;
+
+    DPRINTF("%s: devfn %x irq %d -> %d\n", __func__,
+            pci_dev->devfn, irq_num, slot);
+
+    return slot - 1;
+}
+
+static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
+{
+    qemu_irq *pci_irqs = opaque;
+
+    DPRINTF("%s: PCI irq %d\n", __func__, irq_num);
+    if (irq_num < 0) {
+        fprintf(stderr, "%s: PCI irq %d\n", __func__, irq_num);
+        return;
+    }
+    qemu_set_irq(pci_irqs[irq_num], level);
+}
+
+static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
+{
+    PPC440PCIXState *s = opaque;
+
+    return &s->bm_as;
+}
+
+static int ppc440_pcix_initfn(SysBusDevice *dev)
+{
+    PPC440PCIXState *s;
+    PCIHostState *h;
+    int i;
+
+    h = PCI_HOST_BRIDGE(dev);
+    s = PPC440_PCIX_HOST_BRIDGE(dev);
+
+    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+        sysbus_init_irq(dev, &s->irq[i]);
+    }
+
+    memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
+    h->bus = pci_register_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq,
+                         ppc440_pcix_map_irq, s->irq, &s->busmem,
+                         get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+
+    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
+
+    memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
+    memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
+    address_space_init(&s->bm_as, &s->bm, "pci-bm");
+    pci_setup_iommu(h->bus, ppc440_pcix_set_iommu, s);
+
+    memory_region_init(&s->container, OBJECT(s), "pci-container", PCI_ALL_SIZE);
+    memory_region_init_io(&h->conf_mem, OBJECT(s), &pci_host_conf_le_ops, h,
+                          "pci-conf-idx", 4);
+    memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops, h,
+                          "pci-conf-data", 4);
+    memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
+                          "pci.reg", PPC440_REG_SIZE);
+    memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem);
+    memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem);
+    memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem);
+    sysbus_init_mmio(dev, &s->container);
+
+    return 0;
+}
+
+static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
+{
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    k->init = ppc440_pcix_initfn;
+    dc->reset = ppc440_pcix_reset;
+}
+
+static const TypeInfo ppc440_pcix_info = {
+    .name          = TYPE_PPC440_PCIX_HOST_BRIDGE,
+    .parent        = TYPE_PCI_HOST_BRIDGE,
+    .instance_size = sizeof(PPC440PCIXState),
+    .class_init    = ppc440_pcix_class_init,
+};
+
+static void ppc440_pcix_register_types(void)
+{
+    type_register_static(&ppc440_pcix_info);
+}
+
+type_init(ppc440_pcix_register_types)
Signature · 1 line
2.7.4
#29 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

Hi,

Quoted 9 lines · luigi burdo

hi Balaton, i can help with amigaos4 for sam . i can test it on P5040 book3e and on G5 quad book3s machine. note sam for boot amigaos need a special modified version of uboot that is available on acube website it is a firmware update. without modified uboot will be not posdible auto boot amigaos. idont know if is possible use standard uboot incuded in qemu for boot amigaos from uboot command line. i can ask help to Enrico Vidale as my FB friend but i dont know if he will be available for this kind of helping.

Yes they use their own custom U-Boot, with a lot of changes and code duplication, and added x86 emulation to run a VGA BIOS, and never upstreamed the changes... much like any other vendor, sadly.

I once managed to build a binary from their published source code with an old compiler, although it's not exactly the same of the binary used, which I think is actually run with the on-chip RAM or something weird alike.

François.

#30 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

Quoted 2 lines · François Revol

I once managed to build a binary from their published source code with an old compiler,

It builds here with the patch I've posted next to the bin image.

Quoted 2 lines

although it's not exactly the same of the binary used,
which I think is actually run with the on-chip RAM or something weird alike.

Maybe it's the same but we may have a problem with MMU emulation so it behaves differently (or they've compiled with different compiler/version which generates different code, I haven't checked that deep).

But AROS has a problem with MMU so it may be a bug. I'll send a separate message about that.

Regards,
BALATON Zoltan

#31 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

In reply to luigi burdo
Quoted 1 line · luigi burdo

hi Balaton,

Ciao Luigi,

Quoted 2 lines

i can help with amigaos4 for sam .
i can test it on P5040 book3e and on G5 quad book3s machine.

Thanks for the offer but unfortunately that would not help because these are different CPUs than the 460EX used in the Sam board. I'd definitely need logs from Sam460ex or Sam460cr, other logs are not the same and not help to find differences between real hardware and emulation.

Quoted 1 line

note sam for boot amigaos need a special modified version of uboot that is available on acube website it is a firmware update. without modified uboot will be not posdible auto boot amigaos.

I know about this and if you read the cover message (RFC PATCH 00/12) I link to a fixed version of that U-Boot firmware (rebuilt from source with the mentioned patches) which works with this emulation. (The original one from the updater also starts but cannot boot due to some problems which are fixed by these patches in my image. This is described in the cover message.) With that fixed firmware image it should be possible to at least try booting AmigaOS on the emulation and get some logs. I'd be surprised if it also worked at this point but I could not try it as it needs an appropriate AmigaOS version that runs on the Sam460. It should produce some logs though with -serial stdio which may help finding what is missing.

Quoted 1 line

idont know if is possible use standard uboot incuded in qemu for boot amigaos from uboot command line. i can ask help to Enrico Vidale as my FB friend but i dont know if he will be available for this kind of helping.

I think it would not be possible to use the U-Boot in QEMU now as that is for e500 CPU but this is not needed either. See above, we are aiming to emulate enough of the board that it can run the original firmware which should be able to boot these Amiga like OSes and normal Linux images used on the Sam460ex. This already works but things fail after or during boot currently. This is what needs to be debugged. So we'd need someone who has a Sam460 (ex or cr) board and can test on that to get logs from OSes on real hardware for comparison.

Regards,
BALATON Zoltan

#32 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

hi Balaton,

Quoted 1 line

I know about this and if you read the cover message (RFC PATCH 00/12) I link to a fixed version of that U-Boot firmware (rebuilt from source with the mentioned patches) which works with this emulation. (The original one from the updater also starts but cannot boot due to some problems which are fixed by these patches in my image. This is described in the cover message.) With that fixed firmware image it should be possible to at least try booting AmigaOS on the emulation and get some logs. I'd be surprised if it also worked at this point but I could not try it as it needs an appropriate AmigaOS version that runs on the Sam460. It should produce some logs though with -serial stdio which may help finding what is missing.

i have sam460 amigaos cdrom iso as my backup and can test it inside qemu. sharing the logs if you needed.

Quoted 1 line

I think it would not be possible to use the U-Boot in QEMU now as that is for e500 CPU but this is not needed either. See above, we are aiming to emulate enough of the board that it can run the original firmware which should be able to boot these Amiga like OSes and normal Linux images used on the Sam460ex. This already works but things fail after or during boot currently. This is what needs to be debugged. So we'd need someone who has a Sam460 (ex or cr) board and can test on that to get logs from OSes on real hardware for comparison.

about real hardware no problem i will ask inside amigan community if someone have the opportunity to share the serial debug of sam 460 runinng amigaos.

can i ask you something ?
why you dont try to integrate in qemu the pegasos 2 or the efika machine. i have the feeling that probably it can be more simple because more old machine and components.

bye
luigi

#33 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

Quoted 4 lines · luigi burdo

can i ask you something ? why you dont try to integrate in qemu the pegasos 2 or the efika machine. i have the feeling that probably it can be more simple because more old machine and components.

Except they are antique, and the Pegasos at least has an horribly buggy OpenFirmware implementation.

I'd rather work on BeBox support ;-)

François.

#34 lore raw

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 09/12] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

In reply to luigi burdo
Quoted 1 line · luigi burdo

i have sam460 amigaos cdrom iso as my backup and can test it inside qemu. sharing the logs if you needed.

It may help if you can do that. You need to compile QEMU from git with this patch series applied and copy the u-boot-sam460-20100605-fixed.bin linked from the cover message as u-boot-sam460-20100605.bin to the current dir. Then try booting the CD and see if it shows something. It should print some messages to console where it's started with -serial stdio when you start AmigaOS with debug (you may need to set os4_commandline in U-Boot for this but I don't know AmigaOS).

Quoted 1 line

about real hardware no problem i will ask inside amigan community if someone have the opportunity to share the serial debug of sam 460 runinng amigaos.

Do you know about a good place for such requests? Is there some forum or mailing list I should be aware of?

Quoted 2 lines

can i ask you something ?
why you dont try to integrate in qemu the pegasos 2 or the efika machine. i have the feeling that probably it can be more simple because more old machine and components.

The age of a machine and components has nothing to do with how easy it is to emulate. It depends on what is already emulated in QEMU. If none of those old components are implemented by QEMU already then it's a lot of work to write emulation of them. (Such as the Marvell system controller chip in Pegasos or the SoC the Efika uses. Besides, does AmigaOS run on Efika?) If there are some previous work we can build on then it's easier. QEMU already had ppc440 emulation including a similar board and Francois has started sam460ex emulation so it was easier to finish this than making a new one. (Also firmware and graphics emulation may be more difficult for those other boards.)

At the end it does not matter which machine we emulate if the goal is to run these OSes. If we have any of the supported boards they will run so we don't need more of these just one and it was the sam460 which was nearest to start working. (The MorphOS team or one of the developers may have some Pegasos 2 emulation but they don't intend to publish that and I don't know about previous work on Efika so the Sam460ex seems like a good choice with support for a lot of OSes, including porting Haiku which was why Francois has started emulating it so this may help that activity as well increasing the available OSes.)

Regards,
BALATON Zoltan

#35 lore raw

RFC PATCH 10/12 ppc: Add 460EX embedded CPU

In reply to BALATON Zoltan

Despite its name it is a 440 core CPU

---
 target/ppc/cpu-models.c     |  3 +++
 target/ppc/cpu-models.h     |  1 +
 target/ppc/translate_init.c | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)
target/ppc/cpu-models.c +3 -0
index 4d3e635..7c368fc 100644
@@ -348,6 +348,8 @@
                 "PowerPC 440 EPb")
     POWERPC_DEF("440EPX",        CPU_POWERPC_440EPX,                 440EP,
                 "PowerPC 440 EPX")
+    POWERPC_DEF("460EXb",        CPU_POWERPC_460EXb,                 460EX,
+                "PowerPC 460 EXb")
 #if defined(TODO_USER_ONLY)
     POWERPC_DEF("440GPb",        CPU_POWERPC_440GPb,                 440GP,
                 "PowerPC 440 GPb")
@@ -1238,6 +1240,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
     { "440GP", "440GPc" },
     { "440GR", "440GRa" },
     { "440GX", "440GXf" },
+    { "460EX", "460EXb" },
 
     { "RCPU", "MPC5xx" },
     /* MPC5xx microcontrollers */
target/ppc/cpu-models.h +1 -0
index b563c45..92f99c0 100644
@@ -241,6 +241,7 @@ enum {
     CPU_POWERPC_440SP              = 0x53221850,
     CPU_POWERPC_440SP2             = 0x53221891,
     CPU_POWERPC_440SPE             = 0x53421890,
+    CPU_POWERPC_460EXb             = 0x130218A4, /* called 460 but 440 core */
     /* PowerPC 460 family */
 #if 0
     /* Generic PowerPC 464 */
target/ppc/translate_init.c +38 -0
index 8fb407e..1e0d566 100644
@@ -3832,6 +3832,44 @@ POWERPC_FAMILY(440EP)(ObjectClass *oc, void *data)
                  POWERPC_FLAG_DE | POWERPC_FLAG_BUS_CLK;
 }
 
+POWERPC_FAMILY(460EX)(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+
+    dc->desc = "PowerPC 460 EX";
+    pcc->init_proc = init_proc_440EP;
+    pcc->check_pow = check_pow_nocheck;
+    pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
+                       PPC_FLOAT | PPC_FLOAT_FRES | PPC_FLOAT_FSEL |
+                       PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
+                       PPC_FLOAT_STFIWX |
+                       PPC_DCR | PPC_DCRX | PPC_WRTEE | PPC_RFMCI |
+                       PPC_CACHE | PPC_CACHE_ICBI |
+                       PPC_CACHE_DCBZ | PPC_CACHE_DCBA |
+                       PPC_MEM_TLBSYNC | PPC_MFTB |
+                       PPC_BOOKE | PPC_4xx_COMMON | PPC_405_MAC |
+                       PPC_440_SPEC;
+    pcc->msr_mask = (1ull << MSR_POW) |
+                    (1ull << MSR_CE) |
+                    (1ull << MSR_EE) |
+                    (1ull << MSR_PR) |
+                    (1ull << MSR_FP) |
+                    (1ull << MSR_ME) |
+                    (1ull << MSR_FE0) |
+                    (1ull << MSR_DWE) |
+                    (1ull << MSR_DE) |
+                    (1ull << MSR_FE1) |
+                    (1ull << MSR_IR) |
+                    (1ull << MSR_DR);
+    pcc->mmu_model = POWERPC_MMU_BOOKE;
+    pcc->excp_model = POWERPC_EXCP_BOOKE;
+    pcc->bus_model = PPC_FLAGS_INPUT_BookE;
+    pcc->bfd_mach = bfd_mach_ppc_403;
+    pcc->flags = POWERPC_FLAG_CE | POWERPC_FLAG_DWE |
+                 POWERPC_FLAG_DE | POWERPC_FLAG_BUS_CLK;
+}
+
 static void init_proc_440GP(CPUPPCState *env)
 {
     /* Time base */
Signature · 1 line
2.7.4
Reviewed-by on v1 10/12
#36 lore raw

Re: [Qemu-devel] [RFC PATCH 10/12] ppc: Add 460EX embedded CPU

Quoted 3 lines · BALATON Zoltan

Despite its name it is a 440 core CPU

Quoted 88 lines
---
 target/ppc/cpu-models.c     |  3 +++
 target/ppc/cpu-models.h     |  1 +
 target/ppc/translate_init.c | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)
target/ppc/cpu-models.c +3 -0
index 4d3e635..7c368fc 100644
@@ -348,6 +348,8 @@
                 "PowerPC 440 EPb")
     POWERPC_DEF("440EPX",        CPU_POWERPC_440EPX,                 440EP,
                 "PowerPC 440 EPX")
+    POWERPC_DEF("460EXb",        CPU_POWERPC_460EXb,                 460EX,
+                "PowerPC 460 EXb")
 #if defined(TODO_USER_ONLY)
     POWERPC_DEF("440GPb",        CPU_POWERPC_440GPb,                 440GP,
                 "PowerPC 440 GPb")
@@ -1238,6 +1240,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
     { "440GP", "440GPc" },
     { "440GR", "440GRa" },
     { "440GX", "440GXf" },
+    { "460EX", "460EXb" },
 
     { "RCPU", "MPC5xx" },
     /* MPC5xx microcontrollers */
target/ppc/cpu-models.h +1 -0
index b563c45..92f99c0 100644
@@ -241,6 +241,7 @@ enum {
     CPU_POWERPC_440SP              = 0x53221850,
     CPU_POWERPC_440SP2             = 0x53221891,
     CPU_POWERPC_440SPE             = 0x53421890,
+    CPU_POWERPC_460EXb             = 0x130218A4, /* called 460 but 440 core */
     /* PowerPC 460 family */
 #if 0
     /* Generic PowerPC 464 */
target/ppc/translate_init.c +38 -0
index 8fb407e..1e0d566 100644
@@ -3832,6 +3832,44 @@ POWERPC_FAMILY(440EP)(ObjectClass *oc, void *data)
                  POWERPC_FLAG_DE | POWERPC_FLAG_BUS_CLK;
 }
 
+POWERPC_FAMILY(460EX)(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+
+    dc->desc = "PowerPC 460 EX";
+    pcc->init_proc = init_proc_440EP;
+    pcc->check_pow = check_pow_nocheck;
+    pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
+                       PPC_FLOAT | PPC_FLOAT_FRES | PPC_FLOAT_FSEL |
+                       PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
+                       PPC_FLOAT_STFIWX |
+                       PPC_DCR | PPC_DCRX | PPC_WRTEE | PPC_RFMCI |
+                       PPC_CACHE | PPC_CACHE_ICBI |
+                       PPC_CACHE_DCBZ | PPC_CACHE_DCBA |
+                       PPC_MEM_TLBSYNC | PPC_MFTB |
+                       PPC_BOOKE | PPC_4xx_COMMON | PPC_405_MAC |
+                       PPC_440_SPEC;
+    pcc->msr_mask = (1ull << MSR_POW) |
+                    (1ull << MSR_CE) |
+                    (1ull << MSR_EE) |
+                    (1ull << MSR_PR) |
+                    (1ull << MSR_FP) |
+                    (1ull << MSR_ME) |
+                    (1ull << MSR_FE0) |
+                    (1ull << MSR_DWE) |
+                    (1ull << MSR_DE) |
+                    (1ull << MSR_FE1) |
+                    (1ull << MSR_IR) |
+                    (1ull << MSR_DR);
+    pcc->mmu_model = POWERPC_MMU_BOOKE;
+    pcc->excp_model = POWERPC_EXCP_BOOKE;
+    pcc->bus_model = PPC_FLAGS_INPUT_BookE;
+    pcc->bfd_mach = bfd_mach_ppc_403;
+    pcc->flags = POWERPC_FLAG_CE | POWERPC_FLAG_DWE |
+                 POWERPC_FLAG_DE | POWERPC_FLAG_BUS_CLK;
+}
+
 static void init_proc_440GP(CPUPPCState *env)
 {
     /* Time base */
Signature · 4 lines
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
#37 lore raw

RFC PATCH 11/12 ppc4xx: Export ECB and PLB emulation

In reply to BALATON Zoltan

These devices are found in some other ppc4xx cores too. Also add some more PLB registers for 460EX.

---
 hw/ppc/ppc405.h    |  3 +++
 hw/ppc/ppc405_uc.c | 16 +++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)
hw/ppc/ppc405.h +3 -0
index a9ffc87..7ed25cf 100644
@@ -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],
hw/ppc/ppc405_uc.c +11 -5
index 8f44cb4..8e58065 100644
@@ -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;
 
Signature · 1 line
2.7.4
#38 lore raw

Re: [Qemu-devel] [RFC PATCH 11/12] ppc4xx: Export ECB and PLB emulation

Quoted 1 line · BALATON Zoltan

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?

Quoted 1 line

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?

Quoted 69 lines
---
 hw/ppc/ppc405.h    |  3 +++
 hw/ppc/ppc405_uc.c | 16 +++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)
hw/ppc/ppc405.h +3 -0
index a9ffc87..7ed25cf 100644
@@ -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],
hw/ppc/ppc405_uc.c +11 -5
index 8f44cb4..8e58065 100644
@@ -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;
 
Signature · 4 lines
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
#39 lore raw

Re: [Qemu-devel] [RFC PATCH 11/12] ppc4xx: Export ECB and PLB emulation

Quoted 2 lines · David Gibson
Quoted 1 line · BALATON Zoltan

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.

Quoted 2 lines

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.

Quoted 5 lines
Quoted 1 line

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

#40 lore raw

Re: [Qemu-devel] [RFC PATCH 11/12] ppc4xx: Export ECB and PLB emulation

Quoted 10 lines · BALATON Zoltan
Quoted 2 lines · David Gibson
Quoted 1 line · BALATON Zoltan

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.

Quoted 2 lines

Elsewhere in the series you moved things that were used elsewhere out of 405_uc, why not do that here as well?

Quoted 6 lines

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.

Quoted 10 lines
Quoted 5 lines
Quoted 1 line

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.

Signature · 4 lines
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
#41 lore raw

RFC PATCH 12/12 ppc: Add aCube Sam460ex board

In reply to BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

---
 default-configs/ppcemb-softmmu.mak |    3 +
 hw/ppc/Makefile.objs               |    2 +
 hw/ppc/sam460ex.c                  | 1895 ++++++++++++++++++++++++++++++++++++
 include/hw/pci/pcie_host.h         |    2 +-
 4 files changed, 1901 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/sam460ex.c
default-configs/ppcemb-softmmu.mak +3 -0
index 94340de..9a0a177 100644
@@ -16,3 +16,6 @@ CONFIG_XILINX=y
 CONFIG_XILINX_ETHLITE=y
 CONFIG_LIBDECNUMBER=y
 CONFIG_SM501=y
+CONFIG_USB_EHCI_SYSBUS=y
+CONFIG_IDE_SII3112=y
+CONFIG_SAM460EX=y
hw/ppc/Makefile.objs +2 -0
index 9c5d58a..a242a59 100644
@@ -27,3 +27,5 @@ obj-$(CONFIG_E500) += e500.o mpc8544ds.o e500plat.o
 obj-$(CONFIG_E500) += mpc8544_guts.o ppce500_spin.o
 # PowerPC 440 Xilinx ML507 reference board.
 obj-$(CONFIG_XILINX) += virtex_ml507.o
+# ACube Sam460ex board.
+obj-$(CONFIG_SAM460EX) += sam460ex.o
hw/ppc/sam460ex.c new file +1895 -0

1,895 lines not on this page · raw message

include/hw/pci/pcie_host.h +1 -1
index 4d23c80..3f7b988 100644
@@ -65,7 +65,7 @@ void pcie_host_mmcfg_update(PCIExpressHost *e,
  * bit 12 - 14: function number
  * bit  0 - 11: offset in configuration space of a given device
  */
-#define PCIE_MMCFG_SIZE_MAX             (1ULL << 28)
+#define PCIE_MMCFG_SIZE_MAX             (1ULL << 29)
 #define PCIE_MMCFG_SIZE_MIN             (1ULL << 20)
 #define PCIE_MMCFG_BUS_BIT              20
 #define PCIE_MMCFG_BUS_MASK             0x1ff
Signature · 1 line
2.7.4
#42 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

Quoted 6 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

Signature · 4 lines
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
#43 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

Quoted 5 lines · David Gibson
Quoted 4 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

Quoted 9 lines
Quoted 2 lines

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

Indeed, I never thought it would need that many devices to run, and I intended to move that out but never did.

While it'd make it easier to review, I also recall waiting 3 years to get a simple patch in just because someone wanted the code to first be refactored (hint: wacom) :p

François.

#44 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

In reply to David Gibson
Quoted 5 lines · David Gibson
Quoted 4 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

Quoted 8 lines
Quoted 2 lines

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

I thought it's simpler to review a series with 12 reasonably sized patches than one with twice as many which only modify a few lines here and there each. Also adding a lot of code scattered in hw directories is probably less clear than having them all at one place. But of course each approach can be reasoned. I thought this might have to be split up but I've left it one place for now for first review to get some advice on what's preferred.

Maybe I should put things that belong to the SoC in ppc440_uc.c (similar to ppc405uc.c we already have) and move common devices used by both to ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup is needed that could be done separately afterwards, I don't think it's a good idea to mix in too much cleanup now to keep patches relatively simple. (I already have some moving around included as clean up patches but I'd like to focus on actual functions than clean up at this point).

Does putting these devices from board code to ppc440_uc.c sound acceptable?

Regards,
BALATON Zoltan

#45 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

Quoted 15 lines · BALATON Zoltan
Quoted 5 lines · David Gibson
Quoted 4 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

Quoted 8 lines
Quoted 2 lines

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

Quoted 6 lines

I thought it's simpler to review a series with 12 reasonably sized patches than one with twice as many which only modify a few lines here and there each. Also adding a lot of code scattered in hw directories is probably less clear than having them all at one place. But of course each approach can be reasoned. I thought this might have to be split up but I've left it one place for now for first review to get some advice on what's preferred.

Well, it depends on the content of the patches. If splitting it up means a lot of looking between patches to make sense of what's going on then that's certainly not good. But if the small patches are independent of each other and can be assessed on their own merits, then that usually makes it easier.

In this case the various new 440 devices should be pretty much independent of each other, so I think splitting is the better option.

Quoted 10 lines

Maybe I should put things that belong to the SoC in ppc440_uc.c (similar to ppc405uc.c we already have) and move common devices used by both to ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup is needed that could be done separately afterwards, I don't think it's a good idea to mix in too much cleanup now to keep patches relatively simple. (I already have some moving around included as clean up patches but I'd like to focus on actual functions than clean up at this point).

Does putting these devices from board code to ppc440_uc.c sound acceptable?

That'd be ok - though again, I'd prefer to see each device as a separate patch. It would probably be preferable to put each device in a separate file as well though, unless they're _really_ tiny. Nothing inherently wrong with small .c files, if they're still more or less self contained.

Signature · 4 lines
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
#46 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

Quoted 40 lines · David Gibson
Quoted 15 lines · BALATON Zoltan
Quoted 5 lines · David Gibson
Quoted 4 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

Quoted 8 lines
Quoted 2 lines

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

Quoted 6 lines

I thought it's simpler to review a series with 12 reasonably sized patches than one with twice as many which only modify a few lines here and there each. Also adding a lot of code scattered in hw directories is probably less clear than having them all at one place. But of course each approach can be reasoned. I thought this might have to be split up but I've left it one place for now for first review to get some advice on what's preferred.

Well, it depends on the content of the patches. If splitting it up means a lot of looking between patches to make sense of what's going on then that's certainly not good. But if the small patches are independent of each other and can be assessed on their own merits, then that usually makes it easier.

In this case the various new 440 devices should be pretty much independent of each other, so I think splitting is the better option.

Quoted 8 lines

Maybe I should put things that belong to the SoC in ppc440_uc.c (similar to ppc405uc.c we already have) and move common devices used by both to ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup is needed that could be done separately afterwards, I don't think it's a good idea to mix in too much cleanup now to keep patches relatively simple. (I already have some moving around included as clean up patches but I'd like to focus on actual functions than clean up at this point).

Quoted 8 lines
Quoted 2 lines

Does putting these devices from board code to ppc440_uc.c sound acceptable?

That'd be ok - though again, I'd prefer to see each device as a separate patch. It would probably be preferable to put each device in a separate file as well though, unless they're _really_ tiny. Nothing inherently wrong with small .c files, if they're still more or less self contained.

Since most of these are just stubs for now and not really full emulation of the device I'd leave them in one file similar to ppc405 for now. They both could be cleaned up later if it's found necessary or devices that are deemed ready and big enough to be moved out could be split off (like I did with i2c and pcix). If you insist, I can split it now but I think it's more difficult to work with a lot of small files scattered in hw instead of everything related to ppc440 in one place until there's still missing fucntionality. I consider splitting it up cleanup which could be done later separately unless it's a requirement to get this series in.

#47 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

Quoted 50 lines · BALATON Zoltan
Quoted 40 lines · David Gibson
Quoted 15 lines · BALATON Zoltan
Quoted 5 lines · David Gibson
Quoted 4 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

Quoted 8 lines
Quoted 2 lines

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

Quoted 6 lines

I thought it's simpler to review a series with 12 reasonably sized patches than one with twice as many which only modify a few lines here and there each. Also adding a lot of code scattered in hw directories is probably less clear than having them all at one place. But of course each approach can be reasoned. I thought this might have to be split up but I've left it one place for now for first review to get some advice on what's preferred.

Well, it depends on the content of the patches. If splitting it up means a lot of looking between patches to make sense of what's going on then that's certainly not good. But if the small patches are independent of each other and can be assessed on their own merits, then that usually makes it easier.

In this case the various new 440 devices should be pretty much independent of each other, so I think splitting is the better option.

Quoted 8 lines

Maybe I should put things that belong to the SoC in ppc440_uc.c (similar to ppc405uc.c we already have) and move common devices used by both to ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup is needed that could be done separately afterwards, I don't think it's a good idea to mix in too much cleanup now to keep patches relatively simple. (I already have some moving around included as clean up patches but I'd like to focus on actual functions than clean up at this point).

Quoted 8 lines
Quoted 2 lines

Does putting these devices from board code to ppc440_uc.c sound acceptable?

That'd be ok - though again, I'd prefer to see each device as a separate patch. It would probably be preferable to put each device in a separate file as well though, unless they're _really_ tiny. Nothing inherently wrong with small .c files, if they're still more or less self contained.

Quoted 9 lines

Since most of these are just stubs for now and not really full emulation of the device I'd leave them in one file similar to ppc405 for now. They both could be cleaned up later if it's found necessary or devices that are deemed ready and big enough to be moved out could be split off (like I did with i2c and pcix). If you insist, I can split it now but I think it's more difficult to work with a lot of small files scattered in hw instead of everything related to ppc440 in one place until there's still missing fucntionality. I consider splitting it up cleanup which could be done later separately unless it's a requirement to get this series in.

Ok, that's a reasonable argument. Go ahead and put them in a ppc440_uc.c file.

Signature · 4 lines
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
#48 lore raw

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

Quoted 61 lines · David Gibson
Quoted 50 lines · BALATON Zoltan
Quoted 40 lines · David Gibson
Quoted 15 lines · BALATON Zoltan
Quoted 5 lines · David Gibson
Quoted 4 lines · BALATON Zoltan

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC. This is not a full implementation yet with a lot of components still missing but enough to start a Linux kernel and the U-Boot firmware.

Quoted 8 lines
Quoted 2 lines

There are a *lot* of devices defined here. Most of them look like they belong to the SoC, not the board (since they use DCRs), so it doesn't really make sense to define them in a board file. It would also make it easier to review if they were split up into separate patches.

Quoted 6 lines

I thought it's simpler to review a series with 12 reasonably sized patches than one with twice as many which only modify a few lines here and there each. Also adding a lot of code scattered in hw directories is probably less clear than having them all at one place. But of course each approach can be reasoned. I thought this might have to be split up but I've left it one place for now for first review to get some advice on what's preferred.

Well, it depends on the content of the patches. If splitting it up means a lot of looking between patches to make sense of what's going on then that's certainly not good. But if the small patches are independent of each other and can be assessed on their own merits, then that usually makes it easier.

In this case the various new 440 devices should be pretty much independent of each other, so I think splitting is the better option.

Quoted 8 lines

Maybe I should put things that belong to the SoC in ppc440_uc.c (similar to ppc405uc.c we already have) and move common devices used by both to ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup is needed that could be done separately afterwards, I don't think it's a good idea to mix in too much cleanup now to keep patches relatively simple. (I already have some moving around included as clean up patches but I'd like to focus on actual functions than clean up at this point).

Quoted 8 lines
Quoted 2 lines

Does putting these devices from board code to ppc440_uc.c sound acceptable?

That'd be ok - though again, I'd prefer to see each device as a separate patch. It would probably be preferable to put each device in a separate file as well though, unless they're _really_ tiny. Nothing inherently wrong with small .c files, if they're still more or less self contained.

Quoted 9 lines

Since most of these are just stubs for now and not really full emulation of the device I'd leave them in one file similar to ppc405 for now. They both could be cleaned up later if it's found necessary or devices that are deemed ready and big enough to be moved out could be split off (like I did with i2c and pcix). If you insist, I can split it now but I think it's more difficult to work with a lot of small files scattered in hw instead of everything related to ppc440 in one place until there's still missing fucntionality. I consider splitting it up cleanup which could be done later separately unless it's a requirement to get this series in.

Quoted 2 lines

Ok, that's a reasonable argument. Go ahead and put them in a ppc440_uc.c file.

I already did in the non RFC submission of the series I've sent last Sunday (in [PATCH 14/15] ppc4xx: Add device models found in PPC440 core).