diff mbox series

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

Message ID 20230124110057.1.I69cf3d56c97098287fe3a70084ee515098390b70@changeid
State New
Headers
Series [1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt |

Commit Message

Doug Anderson Jan. 24, 2023, 7:01 p.m. UTC 
  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(-)

Comments

Jonas Gorski Jan. 26, 2023, 10:16 a.m. UTC | #1 
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
Luis Chamberlain Jan. 26, 2023, 7:01 p.m. UTC | #2 
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
Doug Anderson Jan. 27, 2023, 12:14 a.m. UTC | #3 
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
Luis Chamberlain Jan. 27, 2023, 12:27 a.m. UTC | #4 
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
diff mbox series

Patch

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