Add TPCI200 and IP-Octal 232 IndustryPack emulation

Quoted 4 lines · Alberto Garcia

Ping,

Here's a QEMU image (124 MB) of a Debian system with the tpci200 and ipoctal drivers, in case you want to try them out:

Please rebase and top-post. Don't send the new series as a response to a previous thread.

Regards,

Anthony Liguori

Quoted 20 lines

http://dl.dropbox.com/u/1667385/qemu-ipoctal.qcow2

The root password is just 'root'.

The patches for QEMU are here:

http://patchwork.ozlabs.org/patch/203892/
http://patchwork.ozlabs.org/patch/203896/

And you need to run it with these options:

-device tpci200 -device ipoctal232,serialX=DEV

where X is a channel number (from 0 to 7) and DEV a host character device (e.g. "pty").

Regards,

Berto

The TPCI200 is a PCI board that supports up to 4 IndustryPack modules.

A new bus type called 'IndustryPack' has been created so any compatible module can be attached to this board.

Signed-off-by Alberto Garcia <[email protected]>

---
 default-configs/pci.mak |    1 +
 hw/Makefile.objs        |    3 +
 hw/ipack.c              |  106 ++++++++
 hw/ipack.h              |   75 ++++++
 hw/pci/pci_ids.h        |    3 +
 hw/tpci200.c            |  667 +++++++++++++++++++++++++++++++++++++++++++++++
 6 ficheiros modificados, 855 adições(+)
 create mode 100644 hw/ipack.c
 create mode 100644 hw/ipack.h
 create mode 100644 hw/tpci200.c

default-configs/pci.mak +1 -0

index ae9d1eb..ee2d18d 100644
@@ -21,3 +21,4 @@ CONFIG_ESP=y
 CONFIG_ESP_PCI=y
 CONFIG_SERIAL=y
 CONFIG_SERIAL_PCI=y
+CONFIG_IPACK=y

hw/Makefile.objs +3 -0

index b8bbed3..a42518e 100644
@@ -100,6 +100,9 @@ common-obj-$(CONFIG_XGMAC) += xgmac.o
 # PCI watchdog devices
 common-obj-$(CONFIG_PCI) += wdt_i6300esb.o
 
+# IndustryPack
+common-obj-$(CONFIG_IPACK) += tpci200.o ipack.o
+
 # PCI network cards
 common-obj-$(CONFIG_NE2000_PCI) += ne2000.o
 common-obj-$(CONFIG_EEPRO100_PCI) += eepro100.o

hw/ipack.c new file +9 -0

new file mode 100644
index 0000000..59e272b
@@ -0,0 +1,106 @@
+/*
+ * QEMU IndustryPack emulation
+ *
+ * Copyright (C) 2012 Igalia, S.L.
+ * Author: Alberto Garcia <[email protected]>
+ *
+ * This code is licensed under the GNU GPL v2 or (at your option) any
+ * later version.
+ */

IPACK_DEVICE()

You have them defined in the header, please use them consistently.

Also, please avoid accessing internals like &s->bus.qbus (BUS(s->bus)) or &s->dev.qdev (DEVICE(s->dev)).

Another thing to check (could be a follow-up) is whether the initfn can be split into instance_init (e.g., pci_set_*?) and initfn.

Quoted 7 lines

IPACK_DEVICE()

You have them defined in the header, please use them consistently.

Also, please avoid accessing internals like &s->bus.qbus (BUS(s->bus)) or &s->dev.qdev (DEVICE(s->dev)).

Ok, I'm using all the defined macros and replaced all instances of DO_UPCAST() with their checked equivalents.

I also removed all accesses to internals.

And I defined ipack_bus_new_inplace() (in the spirit of pci_bus_new_inplace()) and updated tpci200_initfn().

Tell me if there's anything else I should change, else I can post the new patches.

Quoted 2 lines

Another thing to check (could be a follow-up) is whether the initfn can be split into instance_init (e.g., pci_set_*?) and initfn.

What's exactly that for in this case?

Quoted 2 lines

Tell me if there's anything else I should change, else I can post the new patches.

Haven't looked at the latest 2/2 patch yet, same might apply there. Otherwise no issues spotted in 1/2.

Quoted 3 lines

Quoted 3 lines

Another thing to check (could be a follow-up) is whether the initfn can be split into instance_init (e.g., pci_set_*?) and initfn.

The GE IP-Octal 232 is an IndustryPack module that implements eight RS-232 serial ports, each one of which can be redirected to a character device in the host.

Signed-off-by Alberto Garcia <[email protected]>

---
 hw/Makefile.objs |    2 +-
 hw/ipoctal.c     |  614 ++++++++++++++++++++++++++++++++++++++++++++++++++++++

hw/ipoctal.c new file +14 -0

+typedef struct IPOctalState IPOctalState;
+typedef struct SCC2698Channel SCC2698Channel;
+typedef struct SCC2698Block SCC2698Block;
+
+struct SCC2698Channel {
+    IPOctalState *ipoctal;
+    CharDriverState *dev;
+    char *devpath;
+    bool rx_enabled;
+    uint8_t mr[2];
+    uint8_t mr_idx;
+    uint8_t sr;
+    uint8_t rhr[RX_FIFO_SIZE];
+    uint8_t rhr_idx;

Quoted 5 lines

IPACK_DEVICE()

You have them defined in the header, please use them consistently.

Quoted 2 lines

Also, please avoid accessing internals like &s->bus.qbus (BUS(s->bus)) or &s->dev.qdev (DEVICE(s->dev)).

Ok, I'm using all the defined macros and replaced all instances of DO_UPCAST() with their checked equivalents.

I also removed all accesses to internals.

And I defined ipack_bus_new_inplace() (in the spirit of pci_bus_new_inplace()) and updated tpci200_initfn().

On my suggestion you changed the type name from "ipoctal" to "ipoctal232" - the file name should probably then be updated, too. For the function prefixes inside the file it shouldn't matter as long as they're consistent.

hw/ipoctal.c line 112

Quoted 2 lines

hw/ipoctal.c new file +2 -0

+struct IPOctalState {
+    IPackDevice dev;

By convention:

IPackDevice parent_obj;

But if, as brought up at LinuxCon, you may want to add qtest cases it may be helpful to move some of the constants above to their own header file ipoctal232_regs.h as done for RTC or pending for TMP105. That could be split out in a later step though, once PCI libqos is available.

P.S. No need to CC [email protected] any longer, you may have noticed. ;)

Quoted 1 line

I also removed all accesses to internals.

qdev->parent_bus is not supposed to be accessed either, there is qdev_get_parent_bus() as accessor function.

Quoted 2 lines

And I defined ipack_bus_new_inplace() (in the spirit of pci_bus_new_inplace()) and updated tpci200_initfn().

Note that Paolo has a series refactoring the in-place bus init (either for qdev reset or for QOM reference counting) that you may want to take into account then.

Having the IPackBus not be a pointer but part of the exposing device initialized in-place would be desirable long-term but, like some other issues raised, not a criteria for initial patch acceptance IMO.