On Thu, Feb 25, 2010 at 11:55:25AM +0100, Juan Quintela wrote:
Gerd Hoffmann kra...@redhat.com wrote:
From: Izik Eidus iei...@redhat.com
Signed-off-by: Izik Eidus iei...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/pci.h | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/hw/pci.h b/hw/pci.h
index 37ebdc4..20c670e 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -301,11 +301,29 @@ pci_config_set_device_id(uint8_t *pci_config,
uint16_t val)
}
static inline void
+pci_config_set_revision(uint8_t *pci_config, uint8_t val)
+{
+pci_set_byte(pci_config[PCI_REVISION_ID], val);
+}
+
+static inline void
pci_config_set_class(uint8_t *pci_config, uint16_t val)
{
pci_set_word(pci_config[PCI_CLASS_DEVICE], val);
}
+static inline void
+pci_config_set_prog_interface(uint8_t *pci_config, uint8_t val)
+{
+pci_set_byte(pci_config[PCI_CLASS_PROG], val);
+}
+
+static inline void
+pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val)
+{
+pci_set_byte(pci_config[PCI_INTERRUPT_PIN], val);
+}
+
This one is wrong I think. Devices have no business touching
interrupt pin register.
typedef int (*pci_qdev_initfn)(PCIDevice *dev);
typedef struct {
DeviceInfo qdev;
mst had some reservations abotu this functions, but I have forgotten.
mst can you comment again?
Later, Juan.
Yes, I commented on this internally.
By the last count, we have about 600 registers. Adding incline functions
for all of them is just a source of bugs, using pci_regs directly is
better in that sense. Even worse, this creates two legal ways to
do the same thing. So no one thing you can grep for. We could convert
all devices, but it's a lot of churn for very little benefit,
and I don't see how such a change can be tested.
So I think really only the common registers should have inline
functions, and for most devices, class/revision/prog interface
should just have default values.
Finally, the APIs as proposed here just do not get us much.
You still must remember which values to put in:
pci_config_set_prog_interface(config, PCI_CLASS_SERIAL_USB)
will happily compile, and you still must verify that
value you put in does fit in.
What I would really like to see is higher level APIs.
So pci_init_legacy_vga_adapter might make sense,
it would not just set class, but also register
IO handlers for legacy ports.
--
MST
