Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.

2009-05-26 Thread Michael S. Tsirkin
On Tue, May 26, 2009 at 11:33:37AM +0900, Isaku Yamahata wrote:
 On Mon, May 25, 2009 at 03:25:33PM +0300, Michael S. Tsirkin wrote:
  Add inline routines for convenient access to pci devices
  with correct (little) endianness. Will be used by MSI-X support.

 Just a minor comment.
 How about to add pci_[sg]et_byte() for consistency?

I don't see that it makes sense - pci_set_long(config, value)
is shorter than *((uint32_t *)config) = cpu_to_le32(value),
but single bytes don't have endianness, and *config = value
is shorter.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.

2009-05-26 Thread Avi Kivity
Michael S. Tsirkin wrote:
 On Tue, May 26, 2009 at 11:33:37AM +0900, Isaku Yamahata wrote:
   
 On Mon, May 25, 2009 at 03:25:33PM +0300, Michael S. Tsirkin wrote:
 
 Add inline routines for convenient access to pci devices
 with correct (little) endianness. Will be used by MSI-X support.
   
 Just a minor comment.
 How about to add pci_[sg]et_byte() for consistency?
 

 I don't see that it makes sense - pci_set_long(config, value)
 is shorter than *((uint32_t *)config) = cpu_to_le32(value),
 but single bytes don't have endianness, and *config = value
 is shorter.
   

It's nice to have consistent APIs though.

-- 
error compiling committee.c: too many arguments to function

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.

2009-05-25 Thread Isaku Yamahata
Just a minor comment.
How about to add pci_[sg]et_byte() for consistency?

On Mon, May 25, 2009 at 03:25:33PM +0300, Michael S. Tsirkin wrote:
 Add inline routines for convenient access to pci devices
 with correct (little) endianness. Will be used by MSI-X support.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/pci.h |   30 +++---
  1 files changed, 27 insertions(+), 3 deletions(-)
 
 diff --git a/hw/pci.h b/hw/pci.h
 index 40137c6..4e112a3 100644
 --- a/hw/pci.h
 +++ b/hw/pci.h
 @@ -240,21 +240,45 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, 
 uint16_t vid, uint16_t did,
  pci_map_irq_fn map_irq, const char *name);
  
  static inline void
 +pci_set_word(uint8_t *config, uint16_t val)
 +{
 +cpu_to_le16wu((uint16_t *)config, val);
 +}
 +
 +static inline uint16_t
 +pci_get_word(uint8_t *config)
 +{
 +return le16_to_cpupu((uint16_t *)config);
 +}
 +
 +static inline void
 +pci_set_long(uint8_t *config, uint32_t val)
 +{
 +cpu_to_le32wu((uint32_t *)config, val);
 +}
 +
 +static inline uint32_t
 +pci_get_long(uint8_t *config)
 +{
 +return le32_to_cpupu((uint32_t *)config);
 +}
 +
 +static inline void
  pci_config_set_vendor_id(uint8_t *pci_config, uint16_t val)
  {
 -cpu_to_le16wu((uint16_t *)pci_config[PCI_VENDOR_ID], val);
 +pci_set_word(pci_config[PCI_VENDOR_ID], val);
  }
  
  static inline void
  pci_config_set_device_id(uint8_t *pci_config, uint16_t val)
  {
 -cpu_to_le16wu((uint16_t *)pci_config[PCI_DEVICE_ID], val);
 +pci_set_word(pci_config[PCI_DEVICE_ID], val);
  }
  
  static inline void
  pci_config_set_class(uint8_t *pci_config, uint16_t val)
  {
 -cpu_to_le16wu((uint16_t *)pci_config[PCI_CLASS_DEVICE], val);
 +pci_set_word(pci_config[PCI_CLASS_DEVICE], val);
  }
  
  typedef void (*pci_qdev_initfn)(PCIDevice *dev);

-- 
yamahata
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization