Message ID | ec9dbeae26f421f4e2e7a3d110f252329c2113ce.1418264106.git.hutao@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
On Thu, Dec 11, 2014 at 10:20:24AM +0800, Hu Tao wrote: > This makes code more readable. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com> > --- > hw/mips/gt64xxx_pci.c | 4 ++-- > hw/pci/pci_host.c | 5 +++-- > include/hw/pci/pci_host.h | 5 +++++ > 3 files changed, 10 insertions(+), 4 deletions(-) We have a ton of other places hard-coding 1<<31, why special-case these? > diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c > index 1f2fe5f..f118c9c 100644 > --- a/hw/mips/gt64xxx_pci.c > +++ b/hw/mips/gt64xxx_pci.c > @@ -564,7 +564,7 @@ static void gt64120_writel (void *opaque, hwaddr addr, > if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { > val = bswap32(val); > } > - if (phb->config_reg & (1u << 31)) { > + if (pci_host_config_enabled(phb)) { > pci_data_write(phb->bus, phb->config_reg, val, 4); > } > break; > @@ -804,7 +804,7 @@ static uint64_t gt64120_readl (void *opaque, > val = phb->config_reg; > break; > case GT_PCI0_CFGDATA: > - if (!(phb->config_reg & (1 << 31))) { > + if (!pci_host_config_enabled(phb)) { > val = 0xffffffff; > } else { > val = pci_data_read(phb->bus, phb->config_reg, 4); > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > index 3e26f92..9bc47d8 100644 > --- a/hw/pci/pci_host.c > +++ b/hw/pci/pci_host.c > @@ -133,8 +133,9 @@ static void pci_host_data_write(void *opaque, hwaddr addr, > PCIHostState *s = opaque; > PCI_DPRINTF("write addr " TARGET_FMT_plx " len %d val %x\n", > addr, len, (unsigned)val); > - if (s->config_reg & (1u << 31)) > + if (pci_host_config_enabled(s)) { > pci_data_write(s->bus, s->config_reg | (addr & 3), val, len); > + } > } > > static uint64_t pci_host_data_read(void *opaque, > @@ -142,7 +143,7 @@ static uint64_t pci_host_data_read(void *opaque, > { > PCIHostState *s = opaque; > uint32_t val; > - if (!(s->config_reg & (1U << 31))) { > + if (!pci_host_config_enabled(s)) { > return 0xffffffff; > } > val = pci_data_read(s->bus, s->config_reg | (addr & 3), len); > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > index ba31595..b48791d 100644 > --- a/include/hw/pci/pci_host.h > +++ b/include/hw/pci/pci_host.h > @@ -65,6 +65,11 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr, > void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len); > uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len); > > +static inline bool pci_host_config_enabled(struct PCIHostState *pci_host) > +{ > + return pci_host->config_reg & (1U << 31); > +} > + Better: #define PCI_HOST_CONFIG_ENABLE (1U << 31) then everyone can just do s->config_reg & PCI_HOST_CONFIG_ENABLE better as it'll work for code like this: phb->config_reg = (pciaddr) | (1u << 31); > extern const MemoryRegionOps pci_host_conf_le_ops; > extern const MemoryRegionOps pci_host_conf_be_ops; > extern const MemoryRegionOps pci_host_data_le_ops; > -- > 1.9.3
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index 1f2fe5f..f118c9c 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -564,7 +564,7 @@ static void gt64120_writel (void *opaque, hwaddr addr, if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { val = bswap32(val); } - if (phb->config_reg & (1u << 31)) { + if (pci_host_config_enabled(phb)) { pci_data_write(phb->bus, phb->config_reg, val, 4); } break; @@ -804,7 +804,7 @@ static uint64_t gt64120_readl (void *opaque, val = phb->config_reg; break; case GT_PCI0_CFGDATA: - if (!(phb->config_reg & (1 << 31))) { + if (!pci_host_config_enabled(phb)) { val = 0xffffffff; } else { val = pci_data_read(phb->bus, phb->config_reg, 4); diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index 3e26f92..9bc47d8 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -133,8 +133,9 @@ static void pci_host_data_write(void *opaque, hwaddr addr, PCIHostState *s = opaque; PCI_DPRINTF("write addr " TARGET_FMT_plx " len %d val %x\n", addr, len, (unsigned)val); - if (s->config_reg & (1u << 31)) + if (pci_host_config_enabled(s)) { pci_data_write(s->bus, s->config_reg | (addr & 3), val, len); + } } static uint64_t pci_host_data_read(void *opaque, @@ -142,7 +143,7 @@ static uint64_t pci_host_data_read(void *opaque, { PCIHostState *s = opaque; uint32_t val; - if (!(s->config_reg & (1U << 31))) { + if (!pci_host_config_enabled(s)) { return 0xffffffff; } val = pci_data_read(s->bus, s->config_reg | (addr & 3), len); diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h index ba31595..b48791d 100644 --- a/include/hw/pci/pci_host.h +++ b/include/hw/pci/pci_host.h @@ -65,6 +65,11 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr, void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len); uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len); +static inline bool pci_host_config_enabled(struct PCIHostState *pci_host) +{ + return pci_host->config_reg & (1U << 31); +} + extern const MemoryRegionOps pci_host_conf_le_ops; extern const MemoryRegionOps pci_host_conf_be_ops; extern const MemoryRegionOps pci_host_data_le_ops;