| Message ID | 20170308161957.28941-5-romain.perier@collabora.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, 2017-03-08 at 17:19 +0100, Romain Perier wrote: > The PCI pool API is deprecated. This commit replaces the PCI pool old > API by the appropriate function with the DMA pool API. > > Signed-off-by: Romain Perier <romain.perier@collabora.com> > Acked-by: Peter Senna Tschudin <peter.senna@collabora.com> > Tested-by: Peter Senna Tschudin <peter.senna@collabora.com> > --- > drivers/net/ethernet/intel/e100.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> My only concern is: - what hardware did this get tested with? Since this affects all e100 parts, it would be hard to believe that all the affected hardware was used in testing.
On Wed, Mar 08, 2017 at 02:40:25PM -0800, Jeff Kirsher wrote: > On Wed, 2017-03-08 at 17:19 +0100, Romain Perier wrote: > > The PCI pool API is deprecated. This commit replaces the PCI pool old > > API by the appropriate function with the DMA pool API. > > > > Signed-off-by: Romain Perier <romain.perier@collabora.com> > > Acked-by: Peter Senna Tschudin <peter.senna@collabora.com> > > Tested-by: Peter Senna Tschudin <peter.senna@collabora.com> > > --- > > drivers/net/ethernet/intel/e100.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > > My only concern is: > - what hardware did this get tested with? Since this affects all e100 > parts, it would be hard to believe that all the affected hardware was > used in testing. This was tested by compilation only(See https://lkml.org/lkml/2017/2/8/661). However this series removes macro definitions of the old pci_pool interface and replace call sites by what the macra was calling. Here are the macros that this series removes from include/pci.h: #define pci_pool dma_pool #define pci_pool_create(name, pdev, size, align, allocation) \ dma_pool_create(name, &pdev->dev, size, align, allocation) #define pci_pool_destroy(pool) dma_pool_destroy(pool) #define pci_pool_alloc(pool, flags, handle) dma_pool_alloc(pool, flags, handle) #define pci_pool_zalloc(pool, flags, handle) \ dma_pool_zalloc(pool, flags, handle) #define pci_pool_free(pool, vaddr, addr) dma_pool_free(pool, vaddr, add So this should not affect run time.
Hello, Le 09/03/2017 à 08:01, Peter Senna Tschudin a écrit : > On Wed, Mar 08, 2017 at 02:40:25PM -0800, Jeff Kirsher wrote: >> On Wed, 2017-03-08 at 17:19 +0100, Romain Perier wrote: >>> The PCI pool API is deprecated. This commit replaces the PCI pool old >>> API by the appropriate function with the DMA pool API. >>> >>> Signed-off-by: Romain Perier <romain.perier@collabora.com> >>> Acked-by: Peter Senna Tschudin <peter.senna@collabora.com> >>> Tested-by: Peter Senna Tschudin <peter.senna@collabora.com> >>> --- >>> drivers/net/ethernet/intel/e100.c | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> >> >> My only concern is: >> - what hardware did this get tested with? Since this affects all e100 >> parts, it would be hard to believe that all the affected hardware was >> used in testing. > This was tested by compilation only(See > https://lkml.org/lkml/2017/2/8/661). However this series removes macro > definitions of the old pci_pool interface and replace call sites by what > the macra was calling. > > Here are the macros that this series removes from include/pci.h: > > #define pci_pool dma_pool > #define pci_pool_create(name, pdev, size, align, allocation) \ > dma_pool_create(name, &pdev->dev, size, align, allocation) > #define pci_pool_destroy(pool) dma_pool_destroy(pool) > #define pci_pool_alloc(pool, flags, handle) dma_pool_alloc(pool, flags, handle) > #define pci_pool_zalloc(pool, flags, handle) \ > dma_pool_zalloc(pool, flags, handle) > #define pci_pool_free(pool, vaddr, addr) dma_pool_free(pool, vaddr, add > > So this should not affect run time. We cannot test a patch like this one on all affected platforms/drivers (at runtime). Simply because we have not the hw. As Peter said, we tested this by compilation only for now via make allyesconfig. That's up to the maintainer of the subsystem to test and ack this, imho. I agree with Peter, this should not affect runtime (as semantically it's compatible and have been validated statically and semantically by your compiler) Regards, Romain
diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c index 2b7323d..d1002c2 100644 --- a/drivers/net/ethernet/intel/e100.c +++ b/drivers/net/ethernet/intel/e100.c @@ -607,7 +607,7 @@ struct nic { struct mem *mem; dma_addr_t dma_addr; - struct pci_pool *cbs_pool; + struct dma_pool *cbs_pool; dma_addr_t cbs_dma_addr; u8 adaptive_ifs; u8 tx_threshold; @@ -1892,7 +1892,7 @@ static void e100_clean_cbs(struct nic *nic) nic->cb_to_clean = nic->cb_to_clean->next; nic->cbs_avail++; } - pci_pool_free(nic->cbs_pool, nic->cbs, nic->cbs_dma_addr); + dma_pool_free(nic->cbs_pool, nic->cbs, nic->cbs_dma_addr); nic->cbs = NULL; nic->cbs_avail = 0; } @@ -1910,7 +1910,7 @@ static int e100_alloc_cbs(struct nic *nic) nic->cb_to_use = nic->cb_to_send = nic->cb_to_clean = NULL; nic->cbs_avail = 0; - nic->cbs = pci_pool_alloc(nic->cbs_pool, GFP_KERNEL, + nic->cbs = dma_pool_alloc(nic->cbs_pool, GFP_KERNEL, &nic->cbs_dma_addr); if (!nic->cbs) return -ENOMEM; @@ -2958,8 +2958,8 @@ static int e100_probe(struct pci_dev *pdev, const struct pci_device_id *ent) netif_err(nic, probe, nic->netdev, "Cannot register net device, aborting\n"); goto err_out_free; } - nic->cbs_pool = pci_pool_create(netdev->name, - nic->pdev, + nic->cbs_pool = dma_pool_create(netdev->name, + &nic->pdev->dev, nic->params.cbs.max * sizeof(struct cb), sizeof(u32), 0); @@ -2999,7 +2999,7 @@ static void e100_remove(struct pci_dev *pdev) unregister_netdev(netdev); e100_free(nic); pci_iounmap(pdev, nic->csr); - pci_pool_destroy(nic->cbs_pool); + dma_pool_destroy(nic->cbs_pool); free_netdev(netdev); pci_release_regions(pdev); pci_disable_device(pdev);