[PATCH 1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt

[PATCH 1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt

[Douglas Anderson]

For the same reasons talked about in commit 9503a1fc123d ("ath9k: Use
platform_get_irq() to get the interrupt"), we should be using
platform_get_irq() in ath11k. Let's make the switch.

Without this change, WiFi wasn't coming up on my Qualcomm sc7280-based
hardware. Specifically, "platform_get_resource(pdev, IORESOURCE_IRQ,
i)" was failing even for i=0. Digging into the platform device there
truly were no IRQs present in the list of resources when the call was
made.

I didn't dig into what changed between 5.15 (where
platform_get_resource() seems to work) and mainline Linux (where it
doesn't). Given the zeal robot report for ath9k I assume it's a known
issue. I'll mark this as "fixing" the patch that introduced the
platform_get_resource() call since it should have always been fine to
just call platform_get_irq() and that'll make sure it goes back as far
as it needs to go.

Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1

Fixes: 00402f49d26f ("ath11k: Add support for WCN6750 device")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/net/wireless/ath/ath11k/ahb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
index d34a4d6325b2..f70a119bb5c8 100644
--- a/drivers/net/wireless/ath/ath11k/ahb.c
+++ b/drivers/net/wireless/ath/ath11k/ahb.c
@@ -859,11 +859,11 @@ static int ath11k_ahb_setup_msi_resources(struct ath11k_base *ab)
 	ab->pci.msi.ep_base_data = int_prop + 32;
 
 	for (i = 0; i < ab->pci.msi.config->total_vectors; i++) {
-		res = platform_get_resource(pdev, IORESOURCE_IRQ, i);
-		if (!res)
-			return -ENODEV;
+		ret = platform_get_irq(pdev, i);
+		if (ret < 0)
+			return ret;
 
-		ab->pci.msi.irqs[i] = res->start;
+		ab->pci.msi.irqs[i] = ret;
 	}
 
 	set_bit(ATH11K_FLAG_MULTI_MSI_VECTORS, &ab->dev_flags);
-- 
2.39.1.405.gd4c25cc71f-goog

[PATCH 2/2] wifi: ath5k: Use platform_get_irq() to get the interrupt

[Douglas Anderson]

For the same reasons talked about in commit 9503a1fc123d ("ath9k: Use
platform_get_irq() to get the interrupt"), we should be using
platform_get_irq() in ath5k. Let's make the switch.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I'm not setup to actually test this, but I figured that I might as
well go all the way and fix all the instances of the same pattern that
I found in the ath drivers since the old call was actually breaking me
in ath11k. I did at least confirm that the code compiles for me.

If folks would rather not land an untested patch like this, though,
feel free to drop this and just land patch #1 as long as that one
looks OK.

 drivers/net/wireless/ath/ath5k/ahb.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
index 2c9cec8b53d9..28a1e5eff204 100644
--- a/drivers/net/wireless/ath/ath5k/ahb.c
+++ b/drivers/net/wireless/ath/ath5k/ahb.c
@@ -113,15 +113,13 @@ static int ath_ahb_probe(struct platform_device *pdev)
 		goto err_out;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res == NULL) {
-		dev_err(&pdev->dev, "no IRQ resource found\n");
-		ret = -ENXIO;
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no IRQ resource found: %d\n", irq);
+		ret = irq;
 		goto err_iounmap;
 	}
 
-	irq = res->start;
-
 	hw = ieee80211_alloc_hw(sizeof(struct ath5k_hw), &ath5k_hw_ops);
 	if (hw == NULL) {
 		dev_err(&pdev->dev, "no memory for ieee80211_hw\n");
-- 
2.39.1.405.gd4c25cc71f-goog

Re: [PATCH 1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt

[Jonas Gorski]

On Tue, 24 Jan 2023 at 20:05, Douglas Anderson <dianders@chromium.org> wrote:
>
> For the same reasons talked about in commit 9503a1fc123d ("ath9k: Use
> platform_get_irq() to get the interrupt"), we should be using
> platform_get_irq() in ath11k. Let's make the switch.
>
> Without this change, WiFi wasn't coming up on my Qualcomm sc7280-based
> hardware. Specifically, "platform_get_resource(pdev, IORESOURCE_IRQ,
> i)" was failing even for i=0. Digging into the platform device there
> truly were no IRQs present in the list of resources when the call was
> made.
>
> I didn't dig into what changed between 5.15 (where
> platform_get_resource() seems to work) and mainline Linux (where it
> doesn't). Given the zeal robot report for ath9k I assume it's a known
> issue. I'll mark this as "fixing" the patch that introduced the
> platform_get_resource() call since it should have always been fine to
> just call platform_get_irq() and that'll make sure it goes back as far
> as it needs to go.

Since I recently stumbled upon this in a different (external) driver,
it's likely a1a2b7125e10 ("of/platform: Drop static setup of IRQ
resource from DT core").

Regards
Jonas

Re: [PATCH 1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt

[Luis Chamberlain]

On Tue, Jan 24, 2023 at 11:01:00AM -0800, Douglas Anderson wrote:
> For the same reasons talked about in commit 9503a1fc123d ("ath9k: Use
> platform_get_irq() to get the interrupt"), we should be using
> platform_get_irq() in ath11k. Let's make the switch.

The commit log is rather weak, it is better to re-state what the commit
log in 9503a1fc123d states as it is stronger, and very clear.

To that end. Why not write an SmPL Coccinelle grammer patch for this
and put it on scripts/coccinelle/api ? Then hunt / convert things which
will use DT as well and where this is actually useful / likely buggy.

  Luis

Re: [PATCH 1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt

[Doug Anderson]

Hi,

On Thu, Jan 26, 2023 at 11:01 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Tue, Jan 24, 2023 at 11:01:00AM -0800, Douglas Anderson wrote:
> > For the same reasons talked about in commit 9503a1fc123d ("ath9k: Use
> > platform_get_irq() to get the interrupt"), we should be using
> > platform_get_irq() in ath11k. Let's make the switch.
>
> The commit log is rather weak, it is better to re-state what the commit
> log in 9503a1fc123d states as it is stronger, and very clear.

Sure. Adding in the info that Jonas provided about what commit
specifically broke me would also be nice. I'll try to send out a new
CL with improved wording tomorrow.


> To that end. Why not write an SmPL Coccinelle grammer patch for this
> and put it on scripts/coccinelle/api ? Then hunt / convert things which
> will use DT as well and where this is actually useful / likely buggy.

That sounds like a great idea. ...but not something I'm going to do.
I'm not personally on a mission to track down everyone hitting this
particular issue. Hopefully those that were involved in commit
a1a2b7125e10 ("of/platform: Drop static setup of IRQ resource from DT
core") made some effort to hunt problems down and it seems like,
maybe, the zeal robot was part of that effort? In my case, the ath11k
bug hit me and that's what I need fixed. I tried to be a friendly
citizen and also fixup ath5k because it was super obvious that it was
the same issue and the same code.

-Doug

Re: [PATCH 1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt

[Luis Chamberlain]

On Thu, Jan 26, 2023 at 04:14:42PM -0800, Doug Anderson wrote:
> > To that end. Why not write an SmPL Coccinelle grammer patch for this
> > and put it on scripts/coccinelle/api ? Then hunt / convert things which
> > will use DT as well and where this is actually useful / likely buggy.
> 
> That sounds like a great idea. ...but not something I'm going to do.

:*(

  Luis