Add TPCI200 and IP-Octal 232 IndustryPack emulation
v4 cover Add TPCI200 and IP-Octal 232 IndustryPack emulation
Rebased (and tested) against the current master as requested.
Quoted 7 lines · Anthony Liguori
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:
Quoted 27 lines
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
Alberto Garcia (2):
Add TEWS TPCI200 IndustryPack emulation
Add GE IP-Octal 232 IndustryPack emulation
default-configs/pci.mak | 1 + hw/Makefile.objs | 3 + hw/ipack.c | 106 ++++++++ hw/ipack.h | 75 ++++++ hw/ipoctal.c | 614 +++++++++++++++++++++++++++++++++++++++++++ hw/pci/pci_ids.h | 3 + hw/tpci200.c | 667 +++++++++++++++++++++++++++++++++++++++++++++++ 7 ficheiros modificados, 1469 adições(+) create mode 100644 hw/ipack.c create mode 100644 hw/ipack.h create mode 100644 hw/ipoctal.c create mode 100644 hw/tpci200.c
Signature · 1 line
1.7.10.4
PATCH v4 1/2 Add TEWS TPCI200 IndustryPack emulation
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.
--- 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
@@ -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
@@ -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 +106 -0
106 lines not on this page · raw message
hw/ipack.h new file +75 -0
75 lines not on this page · raw message
hw/pci/pci_ids.h +3 -0
@@ -148,4 +148,7 @@#define PCI_VENDOR_ID_NEC 0x1033#define PCI_DEVICE_ID_NEC_UPD720200 0x0194 +#define PCI_VENDOR_ID_TEWS 0x1498 +#define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8 +#endif
hw/tpci200.c new file +667 -0
667 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH v4 1/2] Add TEWS TPCI200 IndustryPack emulation
Am 08.01.2013 02:10, schrieb Alberto Garcia:
Quoted 56 lines
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.cdefault-configs/pci.mak +1
-0@@ -21,3 +21,4 @@ CONFIG_ESP=y CONFIG_ESP_PCI=y CONFIG_SERIAL=y CONFIG_SERIAL_PCI=y +CONFIG_IPACK=yhw/Makefile.objs +3
-0@@ -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.ohw/ipack.c new file +9
-0@@ -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. +*/
hw/ipack.c line 19
Quoted 10 lines
hw/ipack.c new file +10
-0+ +#include "ipack.h" + +IPackDevice *ipack_device_find (IPackBus *bus,int32_t slot) +{ +BusChild *kid; + + QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) { +DeviceState *qdev = kid->child; +IPackDevice *ip =DO_UPCAST (IPackDevice, qdev, qdev);
IPACK_DEVICE(qdev)
hw/ipack.c line 29
Quoted 10 lines
hw/ipack.c new file +10
-0+if (ip->slot == slot) { +return ip; + } + } +return NULL ; +} + +static int ipack_device_dev_init (DeviceState *qdev) +{ +IPackBus *bus =DO_UPCAST (IPackBus, qbus, qdev->parent_bus);
IPACK_BUS()
hw/ipack.c line 30
Quoted 1 line
hw/ipack.c new file +1
-0+IPackDevice *dev =DO_UPCAST (IPackDevice, qdev, qdev);
[snip]
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.
Regards,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 1/2] Add TEWS TPCI200 IndustryPack emulation
Quoted 6 lines · Andreas Färber
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?
Berto
Re: [Qemu-devel] [PATCH v4 1/2] Add TEWS TPCI200 IndustryPack emulation
Am 08.01.2013 16:57, schrieb Alberto Garcia:
Quoted 12 lines · Andreas Färber
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.
Quoted 7 lines
I also removed all accesses to internals.
And I defined
ipack_bus_new_inplace()(in the spirit ofpci_bus_new_inplace()) and updatedtpci200_initfn().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 4 lines
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?
QOM realize. My recent RFC only touched ISA but PCI will have to be converted at some point as well: instance_init can do any trivial field initializations (that cannot fail), whereas initfn / realizefn will be run after any management interactions (and might be unrealized and realized again - that's Advanced Magic though). If you don't spot anything obvious then better get the device in first, it's not yet urgent.
Regards,
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 1/2] Add TEWS TPCI200 IndustryPack emulation
Quoted 5 lines · Andreas Färber
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.
Yes, I updated both patches. Shall I resend them now or wait for more comments?
Quoted 3 lines
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.
There's only PCI code in the init function, does it make sense to split it? And am I actually supposed to write in the PCI configuration space from instance_init()?
Berto
Re: [Qemu-devel] [PATCH v4 1/2] Add TEWS TPCI200 IndustryPack emulation
In reply to Alberto GarciaAm 08.01.2013 16:57, schrieb Alberto Garcia:
Quoted 6 lines · Andreas Färber
Quoted 5 lines
IPACK_DEVICE()You have them defined in the header, please use them consistently.
Quoted 7 lines
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.
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 ofpci_bus_new_inplace()) and updatedtpci200_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.
Andreas
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 1/2] Add TEWS TPCI200 IndustryPack emulation
Quoted 4 lines · Andreas Färber
Quoted 1 line
I also removed all accesses to internals.
qdev->parent_busis not supposed to be accessed either, there isqdev_get_parent_bus()as accessor function.
Yeah, that one I had also changed :)
(I didn't find any accessor for qbus->parent, so that one I didn't change).
Quoted 6 lines
Quoted 2 lines
And I defined
ipack_bus_new_inplace()(in the spirit ofpci_bus_new_inplace()) and updatedtpci200_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.
I guess you mean https://lists.gnu.org/archive/html/qemu-devel/2012-12/msg00548.html
I anyway didn't really change anything, I just moved the qbus_create_inplace() call to ipack.c, so the fields of IPackBus are not directly accessed from tpci200.
Quoted 4 lines
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.
IPackBus is not a pointer, it is part of TPCI200State and is being initialized in-place ...
Berto
PATCH v4 2/2 Add GE IP-Octal 232 IndustryPack emulation
In reply to Alberto GarciaThe 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.
--- hw/Makefile.objs | 2 +- hw/ipoctal.c | 614 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 ficheiros modificados, 615 adições(+), 1 eliminado(-) create mode 100644 hw/ipoctal.c
hw/Makefile.objs +1 -1
@@ -101,7 +101,7 @@ common-obj-$(CONFIG_XGMAC) += xgmac.o common-obj-$(CONFIG_PCI) += wdt_i6300esb.o # IndustryPack -common-obj-$(CONFIG_IPACK) += tpci200.o ipack.o +common-obj-$(CONFIG_IPACK) += tpci200.o ipoctal.o ipack.o # PCI network cards common-obj-$(CONFIG_NE2000_PCI) += ne2000.o
hw/ipoctal.c new file +614 -0
614 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [Qemu-devel] [PATCH v4 2/2] Add GE IP-Octal 232 IndustryPack emulation
Am 08.01.2013 02:10, schrieb Alberto Garcia:
Quoted 8 lines
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 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
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.
[...]
Quoted 14 lines
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;
hw/ipoctal.c line 112
Quoted 10 lines
hw/ipoctal.c new file +10
-0+uint8_t rx_pending; +}; + +struct SCC2698Block { +uint8_t imr; +uint8_t isr; +}; + +struct IPOctalState { +IPackDevice dev;
By convention:
IPackDevice parent_obj;
SCC2698...
hw/ipoctal.c line 116
Quoted 4 lines
hw/ipoctal.c new file +4
-0+SCC2698Channel ch[N_CHANNELS ]; +SCC2698Block blk[N_BLOCKS ]; +uint8_t irq_vector; +};
I understand that this device will be used from the command line only, so the upcoming TYPE_IPOCTAL232 could remain local to this file for the cast macros (i.e., no ipoctal232.h needed).
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.
Regards,
Andreas
P.S. No need to CC [email protected] any longer, you may have noticed. ;)
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 2/2] Add GE IP-Octal 232 IndustryPack emulation
Quoted 4 lines · Andreas Färber
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.
The reason why I decided to stay with 'ipoctal' everywhere except in the visible fields (TypeInfo.name, and VMStateDescription.name) is because the other devices of the IP-Octal family are almost identical from the emulation point of view, so if a new one ever gets emulated most code will be shared.
I'm not sure if it makes sense to rename anything else in this situation...
Quoted 6 lines
hw/ipoctal.c line 112
Quoted 2 lines
hw/ipoctal.c new file +2
-0+struct IPOctalState { +IPackDevice dev;By convention:
IPackDevice
parent_obj;
I only see that in a few places, is that a recent thing? Most devices that I see use 'dev' or similar to refer to their parent objects.
Quoted 5 lines
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.has done for RTC or pending for TMP105. That could be split out in a later step though, once PCI libqos is available.
Yeah, I'm not sure if I'll have much time to work on this in the short term...
Quoted 2 lines
P.S. No need to CC [email protected] any longer, you may have noticed. ;)
Yes, I did :)
Berto