[PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls

[PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls

[Philipp Stanner]

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);
 }
 
-- 
2.47.1

Re: [PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls

[Bjorn Helgaas]

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

RE: [PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls

[Xu, Even]

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

RE: [PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls

[Xu, Even]

> -----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

Re: [PATCH] HID: intel-thc-hid: Remove deprecated PCI API calls

[Jiri Kosina]

On Tue, 28 Jan 2025, 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.
> 
> 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.

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs