| Message ID | 20230124110057.1.I69cf3d56c97098287fe3a70084ee515098390b70@changeid (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/2] wifi: ath11k: Use platform_get_irq() to get the interrupt | expand |
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
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
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
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 --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);
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(-)