Patchwork [03/20] pci: simplify pci_data_read(), pcie_mmcfg_data_read().

login
register
mail settings
Submitter Isaku Yamahata
Date Nov. 12, 2009, 5:58 a.m.
Message ID <1258005528-25383-4-git-send-email-yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/38209/
State New
Headers show

Comments

Isaku Yamahata - Nov. 12, 2009, 5:58 a.m.
simplify ugly switch by memcpy trick.
And add one assert().

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci_host.c  |   16 ++++------------
 hw/pcie_host.c |   16 ++++------------
 2 files changed, 8 insertions(+), 24 deletions(-)
Michael S. Tsirkin - Nov. 12, 2009, 11:01 a.m.
On Thu, Nov 12, 2009 at 02:58:31PM +0900, Isaku Yamahata wrote:
> simplify ugly switch by memcpy trick.
> And add one assert().
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

In fact, there's no reason to be so careful about
zeroing out high bits as far as I can tell:
in the end, the value is assigned to uint32/uint16/uint8
as appropriate and high bits are truncated.
So this can simply be val = 0xffffffff;

> ---
>  hw/pci_host.c  |   16 ++++------------
>  hw/pcie_host.c |   16 ++++------------
>  2 files changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index f4518dc..4196ebc 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -71,19 +71,11 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
>      uint32_t config_addr = pci_addr_to_config(addr);
>      uint32_t val;
>  
> +    assert(len == 1 || len == 2 || len == 4);
>      if (!pci_dev) {
> -        switch(len) {
> -        case 1:
> -            val = 0xff;
> -            break;
> -        case 2:
> -            val = 0xffff;
> -            break;
> -        default:
> -        case 4:
> -            val = 0xffffffff;
> -            break;
> -        }
> +        val = 0;
> +        memset(&val, 0xff, len);
> +        val = le32_to_cpu(val);


So the above will become simply
if (!pci_dev) {
	return ~0x0;
}


>      } else {
>          val = pci_dev->config_read(pci_dev, config_addr, len);
>          PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
> diff --git a/hw/pcie_host.c b/hw/pcie_host.c
> index b52fec6..61285da 100644
> --- a/hw/pcie_host.c
> +++ b/hw/pcie_host.c
> @@ -71,19 +71,11 @@ static uint32_t pcie_mmcfg_data_read(PCIBus *s,
>      PCIDevice *pci_dev = pcie_mmcfg_addr_to_dev(s, mmcfg_addr);
>      uint32_t val;
>  
> +    assert(len == 1 || len == 2 || len == 4);
>      if (!pci_dev) {
> -        switch(len) {
> -        case 1:
> -            val = 0xff;
> -            break;
> -        case 2:
> -            val = 0xffff;
> -            break;
> -        default:
> -        case 4:
> -            val = 0xffffffff;
> -            break;
> -        }
> +        val = 0;
> +        memset(&val, 0xff, len);
> +        val = le32_to_cpu(val);
>      } else {
>          val = pci_dev->config_read(pci_dev,
>                                     PCIE_MMCFG_CONFOFFSET(mmcfg_addr), len);
> -- 
> 1.6.0.2

Patch

diff --git a/hw/pci_host.c b/hw/pci_host.c
index f4518dc..4196ebc 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -71,19 +71,11 @@  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
     uint32_t config_addr = pci_addr_to_config(addr);
     uint32_t val;
 
+    assert(len == 1 || len == 2 || len == 4);
     if (!pci_dev) {
-        switch(len) {
-        case 1:
-            val = 0xff;
-            break;
-        case 2:
-            val = 0xffff;
-            break;
-        default:
-        case 4:
-            val = 0xffffffff;
-            break;
-        }
+        val = 0;
+        memset(&val, 0xff, len);
+        val = le32_to_cpu(val);
     } else {
         val = pci_dev->config_read(pci_dev, config_addr, len);
         PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
diff --git a/hw/pcie_host.c b/hw/pcie_host.c
index b52fec6..61285da 100644
--- a/hw/pcie_host.c
+++ b/hw/pcie_host.c
@@ -71,19 +71,11 @@  static uint32_t pcie_mmcfg_data_read(PCIBus *s,
     PCIDevice *pci_dev = pcie_mmcfg_addr_to_dev(s, mmcfg_addr);
     uint32_t val;
 
+    assert(len == 1 || len == 2 || len == 4);
     if (!pci_dev) {
-        switch(len) {
-        case 1:
-            val = 0xff;
-            break;
-        case 2:
-            val = 0xffff;
-            break;
-        default:
-        case 4:
-            val = 0xffffffff;
-            break;
-        }
+        val = 0;
+        memset(&val, 0xff, len);
+        val = le32_to_cpu(val);
     } else {
         val = pci_dev->config_read(pci_dev,
                                    PCIE_MMCFG_CONFOFFSET(mmcfg_addr), len);