Patchwork [v9,3/8] pci: clean up of pci status register

login
register
mail settings
Submitter Isaku Yamahata
Date Nov. 16, 2010, 8:26 a.m.
Message ID <80cbec110c7842fdd1b87ce41c19b4675befebf0.1289895735.git.yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/71366/
State New
Headers show

Comments

Isaku Yamahata - Nov. 16, 2010, 8:26 a.m.
This patch refine the initialization/reset of
pci status registers.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |   41 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 2 deletions(-)
Michael S. Tsirkin - Nov. 16, 2010, 2:01 p.m.
On Tue, Nov 16, 2010 at 05:26:07PM +0900, Isaku Yamahata wrote:
> This patch refine the initialization/reset of
> pci status registers.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

This one seems good. Applied with some tweaks: I cut down the comment:
we don't really need to repeat what code does IMO, rather why it does
it, and clarified the comment text.  Also split init_w1c out to a
function so that each function does one thing.

> ---
>  hw/pci.c |   41 +++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 52fe655..fba765b 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -145,6 +145,9 @@ static void pci_device_reset(PCIDevice *dev)
>      pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
>                                   pci_get_word(dev->wmask + PCI_COMMAND) |
>                                   pci_get_word(dev->w1cmask + PCI_COMMAND));
> +    pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> +                                 pci_get_word(dev->wmask + PCI_STATUS) |
> +                                 pci_get_word(dev->w1cmask + PCI_STATUS));
>      dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
>      dev->config[PCI_INTERRUPT_LINE] = 0x0;
>      for (r = 0; r < PCI_NUM_REGIONS; ++r) {
> @@ -540,7 +543,7 @@ static void pci_init_cmask(PCIDevice *dev)
>      dev->cmask[PCI_CAPABILITY_LIST] = 0xff;
>  }
>  
> -static void pci_init_wmask(PCIDevice *dev)
> +static void pci_init_wmask_w1cmask(PCIDevice *dev)
>  {
>      int config_size = pci_config_size(dev);
>  
> @@ -595,6 +598,40 @@ static void pci_init_wmask(PCIDevice *dev)
>                   PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
>                   PCI_COMMAND_INTX_DISABLE);
>  
> +    /*
> +     * bit 0-2: reserved
> +     * bit 3: PCI_STATUS_INTERRUPT: RO
> +     * bit 4: PCI_STATUS_CAP_LIST: RO
> +     * bit 5: PCI_STATUS_66MHZ: RO
> +     * bit 6: PCI_STATUS_UDF: reserved (PCI 2.2-)
> +     * bit 7: PCI_STATUS_FAST_BACK: RO
> +     * bit 8: PCI_STATUS_PARITY
> +     *        type 0: RW for bus master
> +     *        type 1: RW1C
> +     * bit 9-10: PCI_STATUS_DEVSEL: RO
> +     * bit 11: PCI_STATUS_SIG_TARGET_ABORT
> +     *         type 0: RW1C for targets that is capable of terminating
> +     *                 a transaction.
> +     *         type 1: RW1C
> +     * bit 12: PCI_STATUS_REC_TARGET_ABORT
> +     *         type 0: RW1C for masters
> +     *         type 1: RW1C
> +     * bit 13: PCI_STATUS_REC_MASTER_ABORT
> +     *         type 0: RW1C for masters
> +     *         type 1: RW1C
> +     * bit 14: PCI_STATUS_SIG_SYSTEM_ERROR
> +     *         type 0: RW1C with execptions
> +     *         type 1: RW1C
> +     * bit : PCI_STATUS_DETECTED_PARITY: RW1C
> +     *
> +     * It's okay to set w1mask even for RO=0(i.e. reserved) because
> +     * writing value 1 to w1c bit whose value is 0 has no effect.
> +     */
> +    pci_set_word(dev->w1cmask + PCI_STATUS,
> +                 PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
> +                 PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
> +                 PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
> +
>      memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
>             config_size - PCI_CONFIG_HEADER_SIZE);
>  }
> @@ -725,7 +762,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>          pci_set_default_subsystem_id(pci_dev);
>      }
>      pci_init_cmask(pci_dev);
> -    pci_init_wmask(pci_dev);
> +    pci_init_wmask_w1cmask(pci_dev);
>      if (is_bridge) {
>          pci_init_wmask_bridge(pci_dev);
>      }
> -- 
> 1.7.1.1

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 52fe655..fba765b 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -145,6 +145,9 @@  static void pci_device_reset(PCIDevice *dev)
     pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
                                  pci_get_word(dev->wmask + PCI_COMMAND) |
                                  pci_get_word(dev->w1cmask + PCI_COMMAND));
+    pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
+                                 pci_get_word(dev->wmask + PCI_STATUS) |
+                                 pci_get_word(dev->w1cmask + PCI_STATUS));
     dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
     dev->config[PCI_INTERRUPT_LINE] = 0x0;
     for (r = 0; r < PCI_NUM_REGIONS; ++r) {
@@ -540,7 +543,7 @@  static void pci_init_cmask(PCIDevice *dev)
     dev->cmask[PCI_CAPABILITY_LIST] = 0xff;
 }
 
-static void pci_init_wmask(PCIDevice *dev)
+static void pci_init_wmask_w1cmask(PCIDevice *dev)
 {
     int config_size = pci_config_size(dev);
 
@@ -595,6 +598,40 @@  static void pci_init_wmask(PCIDevice *dev)
                  PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
                  PCI_COMMAND_INTX_DISABLE);
 
+    /*
+     * bit 0-2: reserved
+     * bit 3: PCI_STATUS_INTERRUPT: RO
+     * bit 4: PCI_STATUS_CAP_LIST: RO
+     * bit 5: PCI_STATUS_66MHZ: RO
+     * bit 6: PCI_STATUS_UDF: reserved (PCI 2.2-)
+     * bit 7: PCI_STATUS_FAST_BACK: RO
+     * bit 8: PCI_STATUS_PARITY
+     *        type 0: RW for bus master
+     *        type 1: RW1C
+     * bit 9-10: PCI_STATUS_DEVSEL: RO
+     * bit 11: PCI_STATUS_SIG_TARGET_ABORT
+     *         type 0: RW1C for targets that is capable of terminating
+     *                 a transaction.
+     *         type 1: RW1C
+     * bit 12: PCI_STATUS_REC_TARGET_ABORT
+     *         type 0: RW1C for masters
+     *         type 1: RW1C
+     * bit 13: PCI_STATUS_REC_MASTER_ABORT
+     *         type 0: RW1C for masters
+     *         type 1: RW1C
+     * bit 14: PCI_STATUS_SIG_SYSTEM_ERROR
+     *         type 0: RW1C with execptions
+     *         type 1: RW1C
+     * bit : PCI_STATUS_DETECTED_PARITY: RW1C
+     *
+     * It's okay to set w1mask even for RO=0(i.e. reserved) because
+     * writing value 1 to w1c bit whose value is 0 has no effect.
+     */
+    pci_set_word(dev->w1cmask + PCI_STATUS,
+                 PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
+                 PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
+                 PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
+
     memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
            config_size - PCI_CONFIG_HEADER_SIZE);
 }
@@ -725,7 +762,7 @@  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
         pci_set_default_subsystem_id(pci_dev);
     }
     pci_init_cmask(pci_dev);
-    pci_init_wmask(pci_dev);
+    pci_init_wmask_w1cmask(pci_dev);
     if (is_bridge) {
         pci_init_wmask_bridge(pci_dev);
     }