On Thu, Dec 08, 2016 at 2:23:39PM +0200, Yuval Shaia wrote: > + > > +static int pvrdma_pci_probe(struct pci_dev *pdev, > > + const struct pci_device_id *id) > > +{ > > + struct pci_dev *pdev_net; > > + struct pvrdma_dev *dev; > > + int ret; > > + unsigned long start; > > + unsigned long len; > > + unsigned int version; > > + dma_addr_t slot_dma = 0; > > + > > + dev_dbg(&pdev->dev, "initializing driver %s\n", pci_name(pdev)); > > + > > + /* Allocate zero-out device */ > > + dev = (struct pvrdma_dev *)ib_alloc_device(sizeof(*dev)); > > + if (!dev) { > > + dev_err(&pdev->dev, "failed to allocate IB device\n"); > > + return -ENOMEM; > > + } > > + > > + mutex_lock(&pvrdma_device_list_lock); > > + list_add(&dev->device_link, &pvrdma_device_list); > > + mutex_unlock(&pvrdma_device_list_lock); > > + > > + ret = pvrdma_init_device(dev); > > + if (ret) > > + goto err_free_device; > > + > > + dev->pdev = pdev; > > + pci_set_drvdata(pdev, dev); > > + > > + ret = pci_enable_device(pdev); > > + if (ret) { > > + dev_err(&pdev->dev, "cannot enable PCI device\n"); > > + goto err_free_device; > > + } > > + > > + dev_dbg(&pdev->dev, "PCI resource flags BAR0 %#lx\n", > > + pci_resource_flags(pdev, 0)); > > + dev_dbg(&pdev->dev, "PCI resource len %#llx\n", > > + (unsigned long long)pci_resource_len(pdev, 0)); > > + dev_dbg(&pdev->dev, "PCI resource start %#llx\n", > > + (unsigned long long)pci_resource_start(pdev, 0)); > > + dev_dbg(&pdev->dev, "PCI resource flags BAR1 %#lx\n", > > + pci_resource_flags(pdev, 1)); > > + dev_dbg(&pdev->dev, "PCI resource len %#llx\n", > > + (unsigned long long)pci_resource_len(pdev, 1)); > > + dev_dbg(&pdev->dev, "PCI resource start %#llx\n", > > + (unsigned long long)pci_resource_start(pdev, 1)); > > + > > + if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM) || > > + !(pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) { > > + dev_err(&pdev->dev, "PCI BAR region not MMIO\n"); > > + ret = -ENOMEM; > > + goto err_free_device; > > + } > > + > > + ret = pci_request_regions(pdev, DRV_NAME); > > + if (ret) { > > + dev_err(&pdev->dev, "cannot request PCI resources\n"); > > + goto err_disable_pdev; > > + } > > + > > + /* Enable 64-Bit DMA */ > > + if (pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) == 0) { > > + ret = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)); > > + if (ret != 0) { > > + dev_err(&pdev->dev, > > + "pci_set_consistent_dma_mask failed\n"); > > + goto err_free_resource; > > + } > > + } else { > > + ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32)); > > + if (ret != 0) { > > + dev_err(&pdev->dev, > > + "pci_set_dma_mask failed\n"); > > + goto err_free_resource; > > + } > > + } > > + > > + pci_set_master(pdev); > > + > > + /* Map register space */ > > + start = pci_resource_start(dev->pdev, PVRDMA_PCI_RESOURCE_REG); > > + len = pci_resource_len(dev->pdev, PVRDMA_PCI_RESOURCE_REG); > > + dev->regs = ioremap(start, len); > > + if (!dev->regs) { > > + dev_err(&pdev->dev, "register mapping failed\n"); > > + ret = -ENOMEM; > > + goto err_free_resource; > > + } > > + > > + /* Setup per-device UAR. */ > > + dev->driver_uar.index = 0; > > + dev->driver_uar.pfn = > > + pci_resource_start(dev->pdev, PVRDMA_PCI_RESOURCE_UAR) >> > > + PAGE_SHIFT; > > + dev->driver_uar.map = > > + ioremap(dev->driver_uar.pfn << PAGE_SHIFT, PAGE_SIZE); > > + if (!dev->driver_uar.map) { > > + dev_err(&pdev->dev, "failed to remap UAR pages\n"); > > + ret = -ENOMEM; > > + goto err_unmap_regs; > > + } > > + > > + version = pvrdma_read_reg(dev, PVRDMA_REG_VERSION); > > + dev_info(&pdev->dev, "device version %d, driver version %d\n", > > + version, PVRDMA_VERSION); > > + if (version < PVRDMA_VERSION) { > > + dev_err(&pdev->dev, "incompatible device version\n"); > > + goto err_uar_unmap; > > + } > > + > > + dev->dsr = dma_alloc_coherent(&pdev->dev, sizeof(*dev->dsr), > > + &dev->dsrbase, GFP_KERNEL); > > + if (!dev->dsr) { > > + dev_err(&pdev->dev, "failed to allocate shared region\n"); > > + ret = -ENOMEM; > > + goto err_uar_unmap; > > + } > > + > > + /* Setup the shared region */ > > + memset(dev->dsr, 0, sizeof(*dev->dsr)); > > + dev->dsr->driver_version = PVRDMA_VERSION; > > + dev->dsr->gos_info.gos_bits = sizeof(void *) == 4 ? > > + PVRDMA_GOS_BITS_32 : > > + PVRDMA_GOS_BITS_64; > > + dev->dsr->gos_info.gos_type = PVRDMA_GOS_TYPE_LINUX; > > + dev->dsr->gos_info.gos_ver = 1; > > + dev->dsr->uar_pfn = dev->driver_uar.pfn; > > + > > + /* Command slot. */ > > + dev->cmd_slot = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, > > + &slot_dma, GFP_KERNEL); > > + if (!dev->cmd_slot) { > > + ret = -ENOMEM; > > + goto err_free_dsr; > > + } > > + > > + dev->dsr->cmd_slot_dma = (u64)slot_dma; > > + > > + /* Response slot. */ > > + dev->resp_slot = dma_alloc_coherent(&pdev->dev, PAGE_SIZE, > > + &slot_dma, GFP_KERNEL); > > + if (!dev->resp_slot) { > > + ret = -ENOMEM; > > + goto err_free_slots; > > + } > > + > > + dev->dsr->resp_slot_dma = (u64)slot_dma; > > + > > + /* Async event ring */ > > + dev->dsr->async_ring_pages.num_pages = 4; > > Why 4? > Actually question is why driver hard-coded this value as apposed to other > HW resources rings (such as CQ, QP) that is calculated based on element > size and max element number that supported by the HW? In order to support finding these pages dynamically, we would have to split out the way our driver <-> device interaction works. We would have to pass down the event ring info after the hw caps are read. I dont think its worthwhile to do that since the rings are used only between the driver and the device. I can turn this into a macro so atleast its defined only in one place. Thanks, Adit -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html