| Message ID | 20250128101156.77868-2-phasta@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: intel-thc-hid: Remove deprecated PCI API calls | expand |
On Tue, Jan 28, 2025 at 11:11:57AM +0100, Philipp Stanner wrote: > intel-thc-hid reintroduced the already deprecated PCI API functions > > pcim_iomap_table(), > pcim_iomap_regions(), > pcim_iounmap_regions(), > > none of which should be used anymore. > ... > I'm trying to remove this API since a year. Please pay attention to the > docstrings in PCI which mark certain functions as deprecated. If you don't use it already, lei is a great tool that can help watch for things like this: https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started Or lore queries along this line: https://lore.kernel.org/all/?q=rt%3A1.month.ago..+dfb%3Apcim_iomap_table
Thanks Bjorn! Best Regards, Even Xu > -----Original Message----- > From: Bjorn Helgaas <helgaas@kernel.org> > Sent: Tuesday, January 28, 2025 11:58 PM > To: Philipp Stanner <phasta@kernel.org> > Cc: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri > Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Srinivas > Pandruvada <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > lenovo@squebb.ca>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-pci@vger.kernel.org > Subject: Re: [PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls > > On Tue, Jan 28, 2025 at 11:11:57AM +0100, Philipp Stanner wrote: > > intel-thc-hid reintroduced the already deprecated PCI API functions > > > > pcim_iomap_table(), > > pcim_iomap_regions(), > > pcim_iounmap_regions(), > > > > none of which should be used anymore. > > ... > > > I'm trying to remove this API since a year. Please pay attention to > > the docstrings in PCI which mark certain functions as deprecated. > > If you don't use it already, lei is a great tool that can help watch for things like this: > https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started > > Or lore queries along this line: > https://lore.kernel.org/all/?q=rt%3A1.month.ago..+dfb%3Apcim_iomap_table
> -----Original Message----- > From: Philipp Stanner <phasta@kernel.org> > Sent: Tuesday, January 28, 2025 6:12 PM > To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri > Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Srinivas > Pandruvada <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > lenovo@squebb.ca>; Philipp Stanner <phasta@kernel.org> > Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > pci@vger.kernel.org > Subject: [PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls > > intel-thc-hid reintroduced the already deprecated PCI API functions > > pcim_iomap_table(), > pcim_iomap_regions(), > pcim_iounmap_regions(), > > none of which should be used anymore. > > Furthermore, calling managed (pcim_*) functions in remove() and probe() for > cleanup is not necessary, since the managed functions clean up automatically. > > Replace / remove the deprecated functions. > > Fixes: 61bb2714dc3a1 ("HID: intel-thc-hid: intel-quicki2c: Add THC QuickI2C > driver skeleton") > Signed-off-by: Philipp Stanner <phasta@kernel.org> > --- > Hi, > > I'm trying to remove this API since a year. Please pay attention to the docstrings in > PCI which mark certain functions as deprecated. > > Thanks > P. > --- > .../intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 14 +++++--------- > .../intel-thc-hid/intel-quickspi/pci-quickspi.c | 14 +++++--------- > 2 files changed, 10 insertions(+), 18 deletions(-) > > diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c > b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c > index 2de93f4a25ca..fa51155ebe39 100644 > --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c > +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c > @@ -557,20 +557,19 @@ static int quicki2c_probe(struct pci_dev *pdev, > > pci_set_master(pdev); > > - ret = pcim_iomap_regions(pdev, BIT(0), KBUILD_MODNAME); > + mem_addr = pcim_iomap_region(pdev, 0, KBUILD_MODNAME); > + ret = PTR_ERR_OR_ZERO(mem_addr); > if (ret) { > dev_err_once(&pdev->dev, "Failed to get PCI regions, ret > = %d.\n", ret); > goto disable_pci_device; > } > > - mem_addr = pcim_iomap_table(pdev)[0]; > - > ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); > if (ret) { > ret = dma_set_mask_and_coherent(&pdev->dev, > DMA_BIT_MASK(32)); > if (ret) { > dev_err_once(&pdev->dev, "No usable DMA > configuration %d\n", ret); > - goto unmap_io_region; > + goto disable_pci_device; > } > } > > @@ -578,7 +577,7 @@ static int quicki2c_probe(struct pci_dev *pdev, > if (ret < 0) { > dev_err_once(&pdev->dev, > "Failed to allocate IRQ vectors. ret = %d\n", ret); > - goto unmap_io_region; > + goto disable_pci_device; > } > > pdev->irq = pci_irq_vector(pdev, 0); > @@ -587,7 +586,7 @@ static int quicki2c_probe(struct pci_dev *pdev, > if (IS_ERR(qcdev)) { > dev_err_once(&pdev->dev, "QuickI2C device init failed\n"); > ret = PTR_ERR(qcdev); > - goto unmap_io_region; > + goto disable_pci_device; > } > > pci_set_drvdata(pdev, qcdev); > @@ -666,8 +665,6 @@ static int quicki2c_probe(struct pci_dev *pdev, > quicki2c_dma_deinit(qcdev); > dev_deinit: > quicki2c_dev_deinit(qcdev); > -unmap_io_region: > - pcim_iounmap_regions(pdev, BIT(0)); > disable_pci_device: > pci_clear_master(pdev); > > @@ -697,7 +694,6 @@ static void quicki2c_remove(struct pci_dev *pdev) > > quicki2c_dev_deinit(qcdev); > > - pcim_iounmap_regions(pdev, BIT(0)); > pci_clear_master(pdev); > } > > diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c > b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c > index 4641e818dfa4..514b199cb884 100644 > --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c > +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c > @@ -575,20 +575,19 @@ static int quickspi_probe(struct pci_dev *pdev, > > pci_set_master(pdev); > > - ret = pcim_iomap_regions(pdev, BIT(0), KBUILD_MODNAME); > + mem_addr = pcim_iomap_region(pdev, 0, KBUILD_MODNAME); > + ret = PTR_ERR_OR_ZERO(mem_addr); > if (ret) { > dev_err(&pdev->dev, "Failed to get PCI regions, ret = %d.\n", > ret); > goto disable_pci_device; > } > > - mem_addr = pcim_iomap_table(pdev)[0]; > - > ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); > if (ret) { > ret = dma_set_mask_and_coherent(&pdev->dev, > DMA_BIT_MASK(32)); > if (ret) { > dev_err(&pdev->dev, "No usable DMA > configuration %d\n", ret); > - goto unmap_io_region; > + goto disable_pci_device; > } > } > > @@ -596,7 +595,7 @@ static int quickspi_probe(struct pci_dev *pdev, > if (ret < 0) { > dev_err(&pdev->dev, > "Failed to allocate IRQ vectors. ret = %d\n", ret); > - goto unmap_io_region; > + goto disable_pci_device; > } > > pdev->irq = pci_irq_vector(pdev, 0); > @@ -605,7 +604,7 @@ static int quickspi_probe(struct pci_dev *pdev, > if (IS_ERR(qsdev)) { > dev_err(&pdev->dev, "QuickSPI device init failed\n"); > ret = PTR_ERR(qsdev); > - goto unmap_io_region; > + goto disable_pci_device; > } > > pci_set_drvdata(pdev, qsdev); > @@ -668,8 +667,6 @@ static int quickspi_probe(struct pci_dev *pdev, > quickspi_dma_deinit(qsdev); > dev_deinit: > quickspi_dev_deinit(qsdev); > -unmap_io_region: > - pcim_iounmap_regions(pdev, BIT(0)); > disable_pci_device: > pci_clear_master(pdev); > > @@ -699,7 +696,6 @@ static void quickspi_remove(struct pci_dev *pdev) > > quickspi_dev_deinit(qsdev); > > - pcim_iounmap_regions(pdev, BIT(0)); > pci_clear_master(pdev); > } Thanks for the fix! Reviewed-by: Even Xu <even.xu@intel.com> > > -- > 2.47.1
diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c index 2de93f4a25ca..fa51155ebe39 100644 --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c @@ -557,20 +557,19 @@ static int quicki2c_probe(struct pci_dev *pdev, pci_set_master(pdev); - ret = pcim_iomap_regions(pdev, BIT(0), KBUILD_MODNAME); + mem_addr = pcim_iomap_region(pdev, 0, KBUILD_MODNAME); + ret = PTR_ERR_OR_ZERO(mem_addr); if (ret) { dev_err_once(&pdev->dev, "Failed to get PCI regions, ret = %d.\n", ret); goto disable_pci_device; } - mem_addr = pcim_iomap_table(pdev)[0]; - ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); if (ret) { ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); if (ret) { dev_err_once(&pdev->dev, "No usable DMA configuration %d\n", ret); - goto unmap_io_region; + goto disable_pci_device; } } @@ -578,7 +577,7 @@ static int quicki2c_probe(struct pci_dev *pdev, if (ret < 0) { dev_err_once(&pdev->dev, "Failed to allocate IRQ vectors. ret = %d\n", ret); - goto unmap_io_region; + goto disable_pci_device; } pdev->irq = pci_irq_vector(pdev, 0); @@ -587,7 +586,7 @@ static int quicki2c_probe(struct pci_dev *pdev, if (IS_ERR(qcdev)) { dev_err_once(&pdev->dev, "QuickI2C device init failed\n"); ret = PTR_ERR(qcdev); - goto unmap_io_region; + goto disable_pci_device; } pci_set_drvdata(pdev, qcdev); @@ -666,8 +665,6 @@ static int quicki2c_probe(struct pci_dev *pdev, quicki2c_dma_deinit(qcdev); dev_deinit: quicki2c_dev_deinit(qcdev); -unmap_io_region: - pcim_iounmap_regions(pdev, BIT(0)); disable_pci_device: pci_clear_master(pdev); @@ -697,7 +694,6 @@ static void quicki2c_remove(struct pci_dev *pdev) quicki2c_dev_deinit(qcdev); - pcim_iounmap_regions(pdev, BIT(0)); pci_clear_master(pdev); } diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c index 4641e818dfa4..514b199cb884 100644 --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c @@ -575,20 +575,19 @@ static int quickspi_probe(struct pci_dev *pdev, pci_set_master(pdev); - ret = pcim_iomap_regions(pdev, BIT(0), KBUILD_MODNAME); + mem_addr = pcim_iomap_region(pdev, 0, KBUILD_MODNAME); + ret = PTR_ERR_OR_ZERO(mem_addr); if (ret) { dev_err(&pdev->dev, "Failed to get PCI regions, ret = %d.\n", ret); goto disable_pci_device; } - mem_addr = pcim_iomap_table(pdev)[0]; - ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)); if (ret) { ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); if (ret) { dev_err(&pdev->dev, "No usable DMA configuration %d\n", ret); - goto unmap_io_region; + goto disable_pci_device; } } @@ -596,7 +595,7 @@ static int quickspi_probe(struct pci_dev *pdev, if (ret < 0) { dev_err(&pdev->dev, "Failed to allocate IRQ vectors. ret = %d\n", ret); - goto unmap_io_region; + goto disable_pci_device; } pdev->irq = pci_irq_vector(pdev, 0); @@ -605,7 +604,7 @@ static int quickspi_probe(struct pci_dev *pdev, if (IS_ERR(qsdev)) { dev_err(&pdev->dev, "QuickSPI device init failed\n"); ret = PTR_ERR(qsdev); - goto unmap_io_region; + goto disable_pci_device; } pci_set_drvdata(pdev, qsdev); @@ -668,8 +667,6 @@ static int quickspi_probe(struct pci_dev *pdev, quickspi_dma_deinit(qsdev); dev_deinit: quickspi_dev_deinit(qsdev); -unmap_io_region: - pcim_iounmap_regions(pdev, BIT(0)); disable_pci_device: pci_clear_master(pdev); @@ -699,7 +696,6 @@ static void quickspi_remove(struct pci_dev *pdev) quickspi_dev_deinit(qsdev); - pcim_iounmap_regions(pdev, BIT(0)); pci_clear_master(pdev); }
intel-thc-hid reintroduced the already deprecated PCI API functions pcim_iomap_table(), pcim_iomap_regions(), pcim_iounmap_regions(), none of which should be used anymore. Furthermore, calling managed (pcim_*) functions in remove() and probe() for cleanup is not necessary, since the managed functions clean up automatically. Replace / remove the deprecated functions. Fixes: 61bb2714dc3a1 ("HID: intel-thc-hid: intel-quicki2c: Add THC QuickI2C driver skeleton") Signed-off-by: Philipp Stanner <phasta@kernel.org> --- Hi, I'm trying to remove this API since a year. Please pay attention to the docstrings in PCI which mark certain functions as deprecated. Thanks P. --- .../intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 14 +++++--------- .../intel-thc-hid/intel-quickspi/pci-quickspi.c | 14 +++++--------- 2 files changed, 10 insertions(+), 18 deletions(-)