[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled()
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled() |
Date: |
Wed, 21 Jan 2015 13:50:59 +0200 |
On Thu, Dec 11, 2014 at 10:20:24AM +0800, Hu Tao wrote:
> This makes code more readable.
>
> Signed-off-by: Hu Tao <address@hidden>
> Reviewed-by: Marcel Apfelbaum <address@hidden>
> ---
> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v3 2/6] pci: introduce pci_host_config_enabled(),
Michael S. Tsirkin <=