NVMe: Don't unmap controller registers on reset
authorKeith Busch <keith.busch@intel.com>
Wed, 24 Feb 2016 16:15:52 +0000 (09:15 -0700)
committerJens Axboe <axboe@fb.com>
Thu, 3 Mar 2016 21:42:49 +0000 (14:42 -0700)
Unmapping the registers on reset or shutdown is not necessary. Keeping
the mapping simplifies reset handling.

Signed-off-by: Keith Busch <keith.busch@intel.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@fb.com>
drivers/nvme/host/pci.c

index a128672472ecf22b0abb1eb588bb12e896903a27..2ea3e398536c5ef3d993ba21ac556366d8f495b1 100644 (file)
@@ -1694,10 +1694,10 @@ static int nvme_dev_add(struct nvme_dev *dev)
        return 0;
 }
 
-static int nvme_dev_map(struct nvme_dev *dev)
+static int nvme_pci_enable(struct nvme_dev *dev)
 {
        u64 cap;
-       int bars, result = -ENOMEM;
+       int result = -ENOMEM;
        struct pci_dev *pdev = to_pci_dev(dev->dev);
 
        if (pci_enable_device_mem(pdev))
@@ -1705,24 +1705,14 @@ static int nvme_dev_map(struct nvme_dev *dev)
 
        dev->entry[0].vector = pdev->irq;
        pci_set_master(pdev);
-       bars = pci_select_bars(pdev, IORESOURCE_MEM);
-       if (!bars)
-               goto disable_pci;
-
-       if (pci_request_selected_regions(pdev, bars, "nvme"))
-               goto disable_pci;
 
        if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(64)) &&
            dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(32)))
                goto disable;
 
-       dev->bar = ioremap(pci_resource_start(pdev, 0), 8192);
-       if (!dev->bar)
-               goto disable;
-
        if (readl(dev->bar + NVME_REG_CSTS) == -1) {
                result = -ENODEV;
-               goto unmap;
+               goto disable;
        }
 
        /*
@@ -1732,7 +1722,7 @@ static int nvme_dev_map(struct nvme_dev *dev)
        if (!pdev->irq) {
                result = pci_enable_msix(pdev, dev->entry, 1);
                if (result < 0)
-                       goto unmap;
+                       goto disable;
        }
 
        cap = lo_hi_readq(dev->bar + NVME_REG_CAP);
@@ -1759,17 +1749,19 @@ static int nvme_dev_map(struct nvme_dev *dev)
        pci_save_state(pdev);
        return 0;
 
- unmap:
-       iounmap(dev->bar);
-       dev->bar = NULL;
  disable:
-       pci_release_regions(pdev);
- disable_pci:
        pci_disable_device(pdev);
        return result;
 }
 
 static void nvme_dev_unmap(struct nvme_dev *dev)
+{
+       if (dev->bar)
+               iounmap(dev->bar);
+       pci_release_regions(to_pci_dev(dev->dev));
+}
+
+static void nvme_pci_disable(struct nvme_dev *dev)
 {
        struct pci_dev *pdev = to_pci_dev(dev->dev);
 
@@ -1778,12 +1770,6 @@ static void nvme_dev_unmap(struct nvme_dev *dev)
        else if (pdev->msix_enabled)
                pci_disable_msix(pdev);
 
-       if (dev->bar) {
-               iounmap(dev->bar);
-               dev->bar = NULL;
-               pci_release_regions(pdev);
-       }
-
        if (pci_is_enabled(pdev)) {
                pci_disable_pcie_error_reporting(pdev);
                pci_disable_device(pdev);
@@ -1842,7 +1828,7 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
        nvme_dev_list_remove(dev);
 
        mutex_lock(&dev->shutdown_lock);
-       if (dev->bar) {
+       if (pci_is_enabled(to_pci_dev(dev->dev))) {
                nvme_stop_queues(&dev->ctrl);
                csts = readl(dev->bar + NVME_REG_CSTS);
        }
@@ -1855,7 +1841,7 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
                nvme_disable_io_queues(dev);
                nvme_disable_admin_queue(dev, shutdown);
        }
-       nvme_dev_unmap(dev);
+       nvme_pci_disable(dev);
 
        for (i = dev->queue_count - 1; i >= 0; i--)
                nvme_clear_queue(dev->queues[i]);
@@ -1911,12 +1897,12 @@ static void nvme_reset_work(struct work_struct *work)
         * If we're called to reset a live controller first shut it down before
         * moving on.
         */
-       if (dev->bar)
+       if (dev->ctrl.ctrl_config & NVME_CC_ENABLE)
                nvme_dev_disable(dev, false);
 
        set_bit(NVME_CTRL_RESETTING, &dev->flags);
 
-       result = nvme_dev_map(dev);
+       result = nvme_pci_enable(dev);
        if (result)
                goto out;
 
@@ -2042,6 +2028,27 @@ static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
        .free_ctrl              = nvme_pci_free_ctrl,
 };
 
+static int nvme_dev_map(struct nvme_dev *dev)
+{
+       int bars;
+       struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+       bars = pci_select_bars(pdev, IORESOURCE_MEM);
+       if (!bars)
+               return -ENODEV;
+       if (pci_request_selected_regions(pdev, bars, "nvme"))
+               return -ENODEV;
+
+       dev->bar = ioremap(pci_resource_start(pdev, 0), 8192);
+       if (!dev->bar)
+               goto release;
+
+       return 0;
+  release:
+       pci_release_regions(pdev);
+       return -ENODEV;
+}
+
 static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
        int node, result = -ENOMEM;
@@ -2066,6 +2073,10 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
        dev->dev = get_device(&pdev->dev);
        pci_set_drvdata(pdev, dev);
 
+       result = nvme_dev_map(dev);
+       if (result)
+               goto free;
+
        INIT_LIST_HEAD(&dev->node);
        INIT_WORK(&dev->scan_work, nvme_dev_scan);
        INIT_WORK(&dev->reset_work, nvme_reset_work);
@@ -2089,6 +2100,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
        nvme_release_prp_pools(dev);
  put_pci:
        put_device(dev->dev);
+       nvme_dev_unmap(dev);
  free:
        kfree(dev->queues);
        kfree(dev->entry);
@@ -2126,6 +2138,7 @@ static void nvme_remove(struct pci_dev *pdev)
        nvme_free_queues(dev, 0);
        nvme_release_cmb(dev);
        nvme_release_prp_pools(dev);
+       nvme_dev_unmap(dev);
        nvme_put_ctrl(&dev->ctrl);
 }