On Fri, 25 Oct 2024, Philipp Stanner wrote:

On Fri, 2024-10-25 at 18:31 +0300, Ilpo Järvinen wrote:

quoted

On Fri, 25 Oct 2024, Philipp Stanner wrote:

quoted

pcim_iomap_table() and pcim_iomap_regions_request_all() have been
deprecated by the PCI subsystem in commit e354bb84a4c1 ("PCI:
Deprecate
pcim_iomap_table(), pcim_iomap_regions_request_all()").

Replace these functions with their successors, pcim_iomap() and
pcim_request_all_regions().

Signed-off-by: Philipp Stanner <redacted>
Acked-by: Kalle Valo <kvalo@kernel.org>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 16 ++++---------
---
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c

b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 3b9943eb6934..4b41613ad89d 100644

--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c

@@ -3533,7 +3533,6 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct

pci_dev *pdev,
 	struct iwl_trans_pcie *trans_pcie, **priv;
 	struct iwl_trans *trans;
 	int ret, addr_size;
-	void __iomem * const *table;
 	u32 bar0;
 
 	/* reassign our BAR 0 if invalid due to possible runtime
PM races */

@@ -3659,22 +3658,15 @@ struct iwl_trans

*iwl_trans_pcie_alloc(struct pci_dev *pdev,
 		}
 	}
 
-	ret = pcim_iomap_regions_request_all(pdev, BIT(0),
DRV_NAME);
+	ret = pcim_request_all_regions(pdev, DRV_NAME);
 	if (ret) {
-		dev_err(&pdev->dev,
"pcim_iomap_regions_request_all failed\n");
+		dev_err(&pdev->dev, "pcim_request_all_regions
failed\n");
 		goto out_no_pci;
 	}
 
-	table = pcim_iomap_table(pdev);
-	if (!table) {
-		dev_err(&pdev->dev, "pcim_iomap_table failed\n");
-		ret = -ENOMEM;
-		goto out_no_pci;
-	}
-
-	trans_pcie->hw_base = table[0];
+	trans_pcie->hw_base = pcim_iomap(pdev, 0, 0);
 	if (!trans_pcie->hw_base) {
-		dev_err(&pdev->dev, "couldn't find IO mem in first
BAR\n");
+		dev_err(&pdev->dev, "pcim_iomap failed\n");

This seems a step backwards as a human readable English error message
was 
replaced with a reference to a function name.

I think it's still an improvement because "couldn't find IO mem in
first BAR" is a nonsensical statement. What the author probably meant
was: "Couldn't find first BAR's IO mem in magic pci_iomap_table" ;)

Well, that's just spelling things on a too low level too. It's irrelevant
detail to the _user_ that kernel used some "magic table". Similarly, it's 
irrelevant to the user that function called pcim_iomap failed.

The reason I just wrote "pcim_iomap failed\n" is that this seems to be
this driver's style for those messages. See the dev_err() above, there
they also just state that this or that function failed.

The problem in using function names is they have obvious meaning for 
developers/coders but dev_err() is presented to user with varying level
of knowledge about kernel internals/code.

While users might be able to derive some information from the function 
name, it would be simply better to explain on higher level what failed 
which is what I think the original message tried to do even if it was
a bit clumsy. There is zero need to know about kernel internals to 
interpret that message (arguably one needs to know some PCI to understand 
BAR, though).

(Developers can find the internals by looking up the error message from
the code so it doesn't take away something from developers.)

-- 
 i.

I am indifferent about the message, though. Whatever the maintainer
prefers is fine.