| Message ID | 20230203060128.19625-4-quic_mpubbise@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | Enable low power mode when WLAN is not active | expand |
Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes: > Currently, WLAN chip is powered once during driver probe and is kept > ON (powered) always even when WLAN is not active; keeping the chip > powered ON all the time will consume extra power which is not > desirable for a battery operated device. Same is the case with non-WoW > suspend, chip will never be put into low power mode when the system is > suspended resulting in higher battery drain. > > As per the recommendation, sending a PDEV suspend WMI command followed > by a QMI MODE OFF command will cease all WLAN activity and put the device > in low power mode. When WLAN interfaces are brought up, sending a QMI > MISSION MODE command would be sufficient to bring the chip out of low > power. This is a better approach than doing hif_power_down()/hif_power_up() > for every WiFi ON/OFF sequence since the turnaround time for entry/exit of > low power mode is much less. Overhead is just the time taken for sending > QMI MODE OFF & QMI MISSION MODE commands instead of going through the > entire chip boot & QMI init sequence. > > Currently the changes are applicable only for WCN6750. This can be > extended to other targets with a future patch. > > Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 > > Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> This is still crashing for me every time with WCN6855 on a NUC x86 device when I rmmod ath11k. Interestingly enough QCA6390 on a Dell XPS 13 9310 does not crash. I investigated the crash more, the crash happens in ath11k_dp_process_rxdma_err() on this line: srng = &ab->hal.srng_list[err_ring->ring_id]; Here are the debug messages before the crash (first and last are my own messages): [ 226.766111] rmmod ath11k_pci [ 227.003678] ath11k_pci 0000:06:00.0: txpower from firmware NaN, reported -2147483648 dBm [ 227.082283] ath11k_pci 0000:06:00.0: qmi wifi fw del server [ 227.195760] ath11k_pci 0000:06:00.0: cookie:0x0 [ 227.195843] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x15b894d [ 227.216022] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x0 [ 227.216086] ath11k_pci 0000:06:00.0: soc reset cause:0 [ 227.236170] ath11k_pci 0000:06:00.0: MHISTATUS 0xff04 [ 227.270816] ath11k_pci 0000:06:00.0: ext irq:167 [ 227.271231] ath11k_dp_process_rxdma_err() 4187 ab ffff888145520000 err_ring 00000000000001d0 So we get irq 167 which is: 167: 0 0 0 0 0 0 0 0 IR-PCI-MSI-0000:06:00.0 14-edge DP_EXT_IRQ But in ath11k_pcic_ext_interrupt_handler() ATH11K_FLAG_EXT_IRQ_ENABLED is still enabled so the irq is processed: if (!test_bit(ATH11K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags)) return IRQ_HANDLED; It looks like that, after applying this patch 3, whenever ath11k_pci_remove() is called we are not calling ath11k_hif_irq_disable() anymore. I checked that without patch 3 ath11k_hif_irq_disable() is always called. So this patch is definitely breaking something fundamental, but I ran out of time to invetigate further. I hope this still helps. Do note I have concerns about this patchset, it just changes quite a lot of the driver logic and I'm worried what else this breaks. Also we should definitely test with another AHB device like IPQ8074, this patchset needs extensive testing.
Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes: > Currently, WLAN chip is powered once during driver probe and is kept > ON (powered) always even when WLAN is not active; keeping the chip > powered ON all the time will consume extra power which is not > desirable for a battery operated device. Same is the case with non-WoW > suspend, chip will never be put into low power mode when the system is > suspended resulting in higher battery drain. > > As per the recommendation, sending a PDEV suspend WMI command followed > by a QMI MODE OFF command will cease all WLAN activity and put the device > in low power mode. When WLAN interfaces are brought up, sending a QMI > MISSION MODE command would be sufficient to bring the chip out of low > power. This is a better approach than doing hif_power_down()/hif_power_up() > for every WiFi ON/OFF sequence since the turnaround time for entry/exit of > low power mode is much less. Overhead is just the time taken for sending > QMI MODE OFF & QMI MISSION MODE commands instead of going through the > entire chip boot & QMI init sequence. > > Currently the changes are applicable only for WCN6750. This can be > extended to other targets with a future patch. > > Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 > > Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> [...] > +static int ath11k_ahb_core_start_ipq8074(struct ath11k_base *ab) > +{ > + /* TODO: Currently initializing the hardware/firmware only > + * during hardware recovery. Support to shutdown/turn-on > + * the hardware during Wi-Fi OFF/ON will be added later. > + */ > + if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags)) > + return 0; > + > + return ath11k_core_start_device(ab); > +} > + > +static void ath11k_ahb_core_stop_ipq8074(struct ath11k_base *ab) > +{ > + /* TODO: Currently stopping the hardware/firmware only > + * during driver unload. Support to shutdown/turn-on > + * the hardware during Wi-Fi OFF/ON will be added later. > + */ > + if (!test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) > + return; > + > + return ath11k_core_stop_device(ab); > +} Please clarify what Wi-Fi OFF/ON exactly means on these two comments, it's not clear for me. Also I want to mention that I suspect eventually we have to always power off the firmware during suspend to get hibernation working: https://bugzilla.kernel.org/show_bug.cgi?id=214649
On 2/24/2023 8:16 PM, Kalle Valo wrote: > Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes: > >> Currently, WLAN chip is powered once during driver probe and is kept >> ON (powered) always even when WLAN is not active; keeping the chip >> powered ON all the time will consume extra power which is not >> desirable for a battery operated device. Same is the case with non-WoW >> suspend, chip will never be put into low power mode when the system is >> suspended resulting in higher battery drain. >> >> As per the recommendation, sending a PDEV suspend WMI command followed >> by a QMI MODE OFF command will cease all WLAN activity and put the device >> in low power mode. When WLAN interfaces are brought up, sending a QMI >> MISSION MODE command would be sufficient to bring the chip out of low >> power. This is a better approach than doing hif_power_down()/hif_power_up() >> for every WiFi ON/OFF sequence since the turnaround time for entry/exit of >> low power mode is much less. Overhead is just the time taken for sending >> QMI MODE OFF & QMI MISSION MODE commands instead of going through the >> entire chip boot & QMI init sequence. >> >> Currently the changes are applicable only for WCN6750. This can be >> extended to other targets with a future patch. >> >> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 >> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 >> >> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> > > This is still crashing for me every time with WCN6855 on a NUC x86 > device when I rmmod ath11k. Interestingly enough QCA6390 on a Dell XPS > 13 9310 does not crash. > Surprisingly enough, I was never been able to reproduce the problem on my device which has WCN6855. > I investigated the crash more, the crash happens in > ath11k_dp_process_rxdma_err() on this line: > > srng = &ab->hal.srng_list[err_ring->ring_id]; > > Here are the debug messages before the crash (first and last are my > own messages): > > [ 226.766111] rmmod ath11k_pci > [ 227.003678] ath11k_pci 0000:06:00.0: txpower from firmware NaN, reported -2147483648 dBm > [ 227.082283] ath11k_pci 0000:06:00.0: qmi wifi fw del server > [ 227.195760] ath11k_pci 0000:06:00.0: cookie:0x0 > [ 227.195843] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x15b894d > [ 227.216022] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x0 > [ 227.216086] ath11k_pci 0000:06:00.0: soc reset cause:0 > [ 227.236170] ath11k_pci 0000:06:00.0: MHISTATUS 0xff04 > [ 227.270816] ath11k_pci 0000:06:00.0: ext irq:167 > [ 227.271231] ath11k_dp_process_rxdma_err() 4187 ab ffff888145520000 err_ring 00000000000001d0 > > So we get irq 167 which is: > > 167: 0 0 0 0 0 0 0 0 IR-PCI-MSI-0000:06:00.0 14-edge DP_EXT_IRQ > > But in ath11k_pcic_ext_interrupt_handler() ATH11K_FLAG_EXT_IRQ_ENABLED > is still enabled so the irq is processed: > > if (!test_bit(ATH11K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags)) > return IRQ_HANDLED; > > It looks like that, after applying this patch 3, whenever > ath11k_pci_remove() is called we are not calling > ath11k_hif_irq_disable() anymore. I checked that without patch 3 > ath11k_hif_irq_disable() is always called. So this patch is definitely > breaking something fundamental, but I ran out of time to invetigate > further. I hope this still helps. > This definitely helps, thanks a lot for the direction. I'll check what is missing here. > Do note I have concerns about this patchset, it just changes quite a lot > of the driver logic and I'm worried what else this breaks. Also we > should definitely test with another AHB device like IPQ8074, this > patchset needs extensive testing. > Noted. Thanks, Manikanta
On 2/24/2023 8:20 PM, Kalle Valo wrote: > Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes: > >> Currently, WLAN chip is powered once during driver probe and is kept >> ON (powered) always even when WLAN is not active; keeping the chip >> powered ON all the time will consume extra power which is not >> desirable for a battery operated device. Same is the case with non-WoW >> suspend, chip will never be put into low power mode when the system is >> suspended resulting in higher battery drain. >> >> As per the recommendation, sending a PDEV suspend WMI command followed >> by a QMI MODE OFF command will cease all WLAN activity and put the device >> in low power mode. When WLAN interfaces are brought up, sending a QMI >> MISSION MODE command would be sufficient to bring the chip out of low >> power. This is a better approach than doing hif_power_down()/hif_power_up() >> for every WiFi ON/OFF sequence since the turnaround time for entry/exit of >> low power mode is much less. Overhead is just the time taken for sending >> QMI MODE OFF & QMI MISSION MODE commands instead of going through the >> entire chip boot & QMI init sequence. >> >> Currently the changes are applicable only for WCN6750. This can be >> extended to other targets with a future patch. >> >> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 >> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 >> >> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> > > [...] > >> +static int ath11k_ahb_core_start_ipq8074(struct ath11k_base *ab) >> +{ >> + /* TODO: Currently initializing the hardware/firmware only >> + * during hardware recovery. Support to shutdown/turn-on >> + * the hardware during Wi-Fi OFF/ON will be added later. >> + */ >> + if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags)) >> + return 0; >> + >> + return ath11k_core_start_device(ab); >> +} >> + >> +static void ath11k_ahb_core_stop_ipq8074(struct ath11k_base *ab) >> +{ >> + /* TODO: Currently stopping the hardware/firmware only >> + * during driver unload. Support to shutdown/turn-on >> + * the hardware during Wi-Fi OFF/ON will be added later. >> + */ >> + if (!test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) >> + return; >> + >> + return ath11k_core_stop_device(ab); >> +} > > Please clarify what Wi-Fi OFF/ON exactly means on these two comments, > it's not clear for me. > By Wi-Fi OFF/ON I mean is the bringing the last wlan interface down/up which is nothing but the non-WoW suspend/resume. > Also I want to mention that I suspect eventually we have to always power > off the firmware during suspend to get hibernation working: > This patch will power off the firmware for WCN6750. I'm not sure how to get that working for other ath11k devices. Thanks, Manikanta
Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes: >> Also I want to mention that I suspect eventually we have to always power >> off the firmware during suspend to get hibernation working: > > This patch will power off the firmware for WCN6750. I'm not sure how > to get that working for other ath11k devices. Oh, I didn't release that. If you can, try to clarify the commit log on this part. For example, "for suspend we run command FOO_OFF which means that the power from the firmware is complete turned off". Or maybe just read them too hastily, but still having clear (and simple) commit logs help understanding the issue.
On 2/24/2023 8:16 PM, Kalle Valo wrote: > Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes: > >> Currently, WLAN chip is powered once during driver probe and is kept >> ON (powered) always even when WLAN is not active; keeping the chip >> powered ON all the time will consume extra power which is not >> desirable for a battery operated device. Same is the case with non-WoW >> suspend, chip will never be put into low power mode when the system is >> suspended resulting in higher battery drain. >> >> As per the recommendation, sending a PDEV suspend WMI command followed >> by a QMI MODE OFF command will cease all WLAN activity and put the device >> in low power mode. When WLAN interfaces are brought up, sending a QMI >> MISSION MODE command would be sufficient to bring the chip out of low >> power. This is a better approach than doing hif_power_down()/hif_power_up() >> for every WiFi ON/OFF sequence since the turnaround time for entry/exit of >> low power mode is much less. Overhead is just the time taken for sending >> QMI MODE OFF & QMI MISSION MODE commands instead of going through the >> entire chip boot & QMI init sequence. >> >> Currently the changes are applicable only for WCN6750. This can be >> extended to other targets with a future patch. >> >> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 >> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 >> >> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> > > This is still crashing for me every time with WCN6855 on a NUC x86 > device when I rmmod ath11k. Interestingly enough QCA6390 on a Dell XPS > 13 9310 does not crash. > > I investigated the crash more, the crash happens in > ath11k_dp_process_rxdma_err() on this line: > > srng = &ab->hal.srng_list[err_ring->ring_id]; > > Here are the debug messages before the crash (first and last are my > own messages): > > [ 226.766111] rmmod ath11k_pci > [ 227.003678] ath11k_pci 0000:06:00.0: txpower from firmware NaN, reported -2147483648 dBm > [ 227.082283] ath11k_pci 0000:06:00.0: qmi wifi fw del server > [ 227.195760] ath11k_pci 0000:06:00.0: cookie:0x0 > [ 227.195843] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x15b894d > [ 227.216022] ath11k_pci 0000:06:00.0: WLAON_WARM_SW_ENTRY 0x0 > [ 227.216086] ath11k_pci 0000:06:00.0: soc reset cause:0 > [ 227.236170] ath11k_pci 0000:06:00.0: MHISTATUS 0xff04 > [ 227.270816] ath11k_pci 0000:06:00.0: ext irq:167 > [ 227.271231] ath11k_dp_process_rxdma_err() 4187 ab ffff888145520000 err_ring 00000000000001d0 > > So we get irq 167 which is: > > 167: 0 0 0 0 0 0 0 0 IR-PCI-MSI-0000:06:00.0 14-edge DP_EXT_IRQ > > But in ath11k_pcic_ext_interrupt_handler() ATH11K_FLAG_EXT_IRQ_ENABLED > is still enabled so the irq is processed: > > if (!test_bit(ATH11K_FLAG_EXT_IRQ_ENABLED, &ab->dev_flags)) > return IRQ_HANDLED; > > It looks like that, after applying this patch 3, whenever > ath11k_pci_remove() is called we are not calling > ath11k_hif_irq_disable() anymore. I checked that without patch 3 > ath11k_hif_irq_disable() is always called. So this patch is definitely > breaking something fundamental, but I ran out of time to invetigate > further. I hope this still helps. > Hi Kalle, I was checking the logic around this and have added some debug logs to check if all the de-init APIs are getting called in the rmmod path. This is the function call flow with WCN6855 on my machine, ath11k_pci 0000:06:00.0: Manikanta: ath11k_core_pdev_destroy ath11k_pci 0000:06:00.0: Manikanta: ath11k_thermal_unregister ath11k_pci 0000:06:00.0: Manikanta: ath11k_mac_unregister ath11k_pci 0000:06:00.0: Manikanta: ath11k_pcic_ext_irq_disable ath11k_pci 0000:06:00.0: Manikanta: __ath11k_pcic_ext_irq_disable ath11k_pci 0000:06:00.0: Manikanta: ath11k_dp_pdev_free ath11k_pci 0000:06:00.0: Manikanta: ath11k_dp_rx_pdev_free ath11k_pci 0000:06:00.0: Manikanta: ath11k_dp_rx_pdev_mon_detach ath11k_pci 0000:06:00.0: Manikanta: ath11k_pcic_stop ath11k_pci 0000:06:00.0: Manikanta: ath11k_dp_pdev_reo_cleanup ath11k_pci 0000:06:00.0: Manikanta: ath11k_dp_free ath11k_pci 0000:06:00.0: Manikanta: ath11k_pci_power_down ath11k_pci 0000:06:00.0: Manikanta: ath11k_mac_destroy ath11k_pci 0000:06:00.0: Manikanta: ath11k_reg_free ath11k_pci 0000:06:00.0: Manikanta: ath11k_pcic_free_irq ath11k_pci 0000:06:00.0: Manikanta: ath11k_pci_free_msi ath11k_pci 0000:06:00.0: Manikanta: ath11k_hal_srng_deinit ath11k_pci 0000:06:00.0: Manikanta: ath11k_core_free In stark contrast to your observations, from the above call flow, I see that ath11k_hif_irq_disable() is getting called and the IRQs are getting disabled. ath11k_pcic_ext_irq_disable() is registered for hif_irq_disable(). I even tried the single MSI vector configuration suspecting that could be the difference. Even in single MSI case, I don't see any crashes during rmmod. I'm completely clueless as to why the same code is behaving differently with the same hardware. How can we take this forward, could you please suggest? I'm thinking to keep these changes specific to WCN6750 for now. Thanks, Manikanta
diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c index cd48eca494ed..708a49f82297 100644 --- a/drivers/net/wireless/ath/ath11k/ahb.c +++ b/drivers/net/wireless/ath/ath11k/ahb.c @@ -194,6 +194,47 @@ static const struct ath11k_pci_ops ath11k_ahb_pci_ops_wcn6750 = { .window_read32 = ath11k_ahb_window_read32_wcn6750, }; +static int ath11k_ahb_core_start_wcn6750(struct ath11k_base *ab) +{ + /* Initialize the hardware/firmware only for the first PDEV + * or during hardware recovery. + */ + if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags) && + ath11k_core_any_pdevs_on(ab)) + return 0; + + return ath11k_core_start_device(ab); +} + +static void ath11k_ahb_core_stop_wcn6750(struct ath11k_base *ab) +{ + return ath11k_core_stop_device(ab); +} + +static int ath11k_ahb_core_start_ipq8074(struct ath11k_base *ab) +{ + /* TODO: Currently initializing the hardware/firmware only + * during hardware recovery. Support to shutdown/turn-on + * the hardware during Wi-Fi OFF/ON will be added later. + */ + if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags)) + return 0; + + return ath11k_core_start_device(ab); +} + +static void ath11k_ahb_core_stop_ipq8074(struct ath11k_base *ab) +{ + /* TODO: Currently stopping the hardware/firmware only + * during driver unload. Support to shutdown/turn-on + * the hardware during Wi-Fi OFF/ON will be added later. + */ + if (!test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) + return; + + return ath11k_core_stop_device(ab); +} + static inline u32 ath11k_ahb_read32(struct ath11k_base *ab, u32 offset) { return ioread32(ab->mem + offset); @@ -793,6 +834,8 @@ static const struct ath11k_hif_ops ath11k_ahb_hif_ops_ipq8074 = { .map_service_to_pipe = ath11k_ahb_map_service_to_pipe, .power_down = ath11k_ahb_power_down, .power_up = ath11k_ahb_power_up, + .core_start = ath11k_ahb_core_start_ipq8074, + .core_stop = ath11k_ahb_core_stop_ipq8074, }; static const struct ath11k_hif_ops ath11k_ahb_hif_ops_wcn6750 = { @@ -812,6 +855,8 @@ static const struct ath11k_hif_ops ath11k_ahb_hif_ops_wcn6750 = { .resume = ath11k_ahb_hif_resume, .ce_irq_enable = ath11k_pci_enable_ce_irqs_except_wake_irq, .ce_irq_disable = ath11k_pci_disable_ce_irqs_except_wake_irq, + .core_start = ath11k_ahb_core_start_wcn6750, + .core_stop = ath11k_ahb_core_stop_wcn6750, }; static int ath11k_core_get_rproc(struct ath11k_base *ab) diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c index 5cdac963444e..0e41a54c1bdb 100644 --- a/drivers/net/wireless/ath/ath11k/core.c +++ b/drivers/net/wireless/ath/ath11k/core.c @@ -1392,7 +1392,6 @@ static int ath11k_core_soc_create(struct ath11k_base *ab) static void ath11k_core_soc_destroy(struct ath11k_base *ab) { ath11k_debugfs_soc_destroy(ab); - ath11k_dp_free(ab); ath11k_reg_free(ab); ath11k_qmi_deinit_service(ab); } @@ -1413,31 +1412,14 @@ static int ath11k_core_pdev_create(struct ath11k_base *ab) goto err_pdev_debug; } - ret = ath11k_mac_register(ab); - if (ret) { - ath11k_err(ab, "failed register the radio with mac80211: %d\n", ret); - goto err_dp_pdev_free; - } - - ret = ath11k_thermal_register(ab); - if (ret) { - ath11k_err(ab, "could not register thermal device: %d\n", - ret); - goto err_mac_unregister; - } - ret = ath11k_spectral_init(ab); if (ret) { ath11k_err(ab, "failed to init spectral %d\n", ret); - goto err_thermal_unregister; + goto err_dp_pdev_free; } return 0; -err_thermal_unregister: - ath11k_thermal_unregister(ab); -err_mac_unregister: - ath11k_mac_unregister(ab); err_dp_pdev_free: ath11k_dp_pdev_free(ab); err_pdev_debug: @@ -1448,11 +1430,8 @@ static int ath11k_core_pdev_create(struct ath11k_base *ab) static void ath11k_core_pdev_destroy(struct ath11k_base *ab) { - ath11k_spectral_deinit(ab); ath11k_thermal_unregister(ab); ath11k_mac_unregister(ab); - ath11k_hif_irq_disable(ab); - ath11k_dp_pdev_free(ab); ath11k_debugfs_pdev_destroy(ab); } @@ -1584,26 +1563,20 @@ static int ath11k_core_start_firmware(struct ath11k_base *ab, return ret; } -int ath11k_core_qmi_firmware_ready(struct ath11k_base *ab) +static int ath11k_core_setup_resources(struct ath11k_base *ab) { int ret; - ret = ath11k_core_start_firmware(ab, ATH11K_FIRMWARE_MODE_NORMAL); - if (ret) { - ath11k_err(ab, "failed to start firmware: %d\n", ret); - return ret; - } - ret = ath11k_ce_init_pipes(ab); if (ret) { ath11k_err(ab, "failed to initialize CE: %d\n", ret); - goto err_firmware_stop; + return ret; } ret = ath11k_dp_alloc(ab); if (ret) { ath11k_err(ab, "failed to init DP: %d\n", ret); - goto err_firmware_stop; + return ret; } switch (ath11k_crypto_mode) { @@ -1617,17 +1590,47 @@ int ath11k_core_qmi_firmware_ready(struct ath11k_base *ab) break; default: ath11k_info(ab, "invalid crypto_mode: %d\n", ath11k_crypto_mode); - return -EINVAL; + ret = -EINVAL; + goto err_dp_free; } if (ath11k_frame_mode == ATH11K_HW_TXRX_RAW) set_bit(ATH11K_FLAG_RAW_MODE, &ab->dev_flags); + return 0; + +err_dp_free: + ath11k_dp_free(ab); + + return ret; +} + +static void ath11k_core_free_resources(struct ath11k_base *ab) +{ + ath11k_dp_free(ab); +} + +static int ath11k_core_setup_device(struct ath11k_base *ab) +{ + int ret; + + ret = ath11k_core_start_firmware(ab, ATH11K_FIRMWARE_MODE_NORMAL); + if (ret) { + ath11k_err(ab, "failed to start firmware: %d\n", ret); + return ret; + } + + ret = ath11k_core_setup_resources(ab); + if (ret) { + ath11k_err(ab, "failed to setup resources: %d\n", ret); + goto err_firmware_stop; + } + mutex_lock(&ab->core_lock); ret = ath11k_core_start(ab); if (ret) { ath11k_err(ab, "failed to start core: %d\n", ret); - goto err_dp_free; + goto err_free_resources; } ret = ath11k_core_pdev_create(ab); @@ -1635,7 +1638,10 @@ int ath11k_core_qmi_firmware_ready(struct ath11k_base *ab) ath11k_err(ab, "failed to create pdev core: %d\n", ret); goto err_core_stop; } + ath11k_hif_irq_enable(ab); + ath11k_hif_core_stop(ab); + mutex_unlock(&ab->core_lock); return 0; @@ -1643,8 +1649,8 @@ int ath11k_core_qmi_firmware_ready(struct ath11k_base *ab) err_core_stop: ath11k_core_stop(ab); ath11k_mac_destroy(ab); -err_dp_free: - ath11k_dp_free(ab); +err_free_resources: + ath11k_core_free_resources(ab); mutex_unlock(&ab->core_lock); err_firmware_stop: ath11k_qmi_firmware_stop(ab); @@ -1652,10 +1658,43 @@ int ath11k_core_qmi_firmware_ready(struct ath11k_base *ab) return ret; } +int ath11k_core_qmi_firmware_ready(struct ath11k_base *ab) +{ + int ret; + + ret = ath11k_core_setup_device(ab); + if (ret) { + ath11k_err(ab, "failed to setup device: %d\n", ret); + return ret; + } + + ret = ath11k_mac_register(ab); + if (ret) { + ath11k_err(ab, "failed register the radio with mac80211: %d\n", ret); + goto err_free_device; + } + + ret = ath11k_thermal_register(ab); + if (ret) { + ath11k_err(ab, "could not register thermal device: %d\n", + ret); + goto err_mac_unregister; + } + + return 0; + +err_mac_unregister: + ath11k_mac_unregister(ab); +err_free_device: + ath11k_core_stop_device(ab); + ath11k_mac_destroy(ab); + + return ret; +} + static void ath11k_core_reconfigure_on_crash(struct ath11k_base *ab) { mutex_lock(&ab->core_lock); - ath11k_thermal_unregister(ab); ath11k_hif_irq_disable(ab); ath11k_dp_pdev_free(ab); ath11k_spectral_deinit(ab); @@ -1930,7 +1969,6 @@ void ath11k_core_deinit(struct ath11k_base *ab) mutex_lock(&ab->core_lock); ath11k_core_pdev_destroy(ab); - ath11k_core_stop(ab); mutex_unlock(&ab->core_lock); @@ -1998,37 +2036,140 @@ struct ath11k_base *ath11k_core_alloc(struct device *dev, size_t priv_size, } EXPORT_SYMBOL(ath11k_core_alloc); +static int ath11k_core_suspend_target(struct ath11k_base *ab, u32 suspend_opt) +{ + struct ath11k *ar; + struct ath11k_pdev *pdev; + unsigned long time_left; + int ret; + int i; + + for (i = 0; i < ab->num_radios; i++) { + pdev = &ab->pdevs[i]; + ar = pdev->ar; + + reinit_completion(&ab->htc_suspend); + + ret = ath11k_wmi_pdev_suspend(ar, suspend_opt, pdev->pdev_id); + if (ret) { + ath11k_warn(ab, "could not suspend target (%d)\n", ret); + return ret; + } + + time_left = wait_for_completion_timeout(&ab->htc_suspend, 3 * HZ); + + if (!time_left) { + ath11k_warn(ab, "suspend timed out - target pause event never came\n"); + return -ETIMEDOUT; + } + } + + return 0; +} + +void ath11k_core_stop_device(struct ath11k_base *ab) +{ + if (ab->core_stopped) + return; + + ath11k_spectral_deinit(ab); + ath11k_core_suspend_target(ab, WMI_PDEV_SUSPEND_AND_DISABLE_INTR); + ath11k_hif_irq_disable(ab); + ath11k_dp_pdev_free(ab); + ath11k_hif_stop(ab); + + if (!test_bit(ATH11K_FLAG_CRASH_FLUSH, &ab->dev_flags)) + ath11k_qmi_firmware_stop(ab); + + ath11k_wmi_detach(ab); + ath11k_dp_pdev_reo_cleanup(ab); + ath11k_dp_free(ab); + + ab->core_stopped = true; +} +EXPORT_SYMBOL(ath11k_core_stop_device); + +int ath11k_core_any_pdevs_on(struct ath11k_base *ab) +{ + struct ath11k_pdev *pdev; + struct ath11k *ar; + int i; + + for (i = 0; i < ab->num_radios; i++) { + pdev = &ab->pdevs[i]; + ar = pdev->ar; + if (!ar) + continue; + + if (ar->state == ATH11K_STATE_ON) + return true; + } + + return false; +} +EXPORT_SYMBOL(ath11k_core_any_pdevs_on); + int ath11k_core_start_device(struct ath11k_base *ab) { int ret; - if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags)) - return 0; + mutex_lock(&ab->core_lock); ath11k_hal_srng_deinit(ab); ret = ath11k_hal_srng_init(ab); if (ret) { ath11k_err(ab, "failed to init srng: %d\n", ret); - return ret; + goto err_unlock; } clear_bit(ATH11K_FLAG_CRASH_FLUSH, &ab->dev_flags); - ret = ath11k_core_qmi_firmware_ready(ab); + ret = ath11k_core_start_firmware(ab, ATH11K_FIRMWARE_MODE_NORMAL); if (ret) { - ath11k_err(ab, "failed to init core: %d\n", ret); + ath11k_err(ab, "failed to start firmware: %d\n", ret); goto err_hal_srng_deinit; } + ret = ath11k_core_setup_resources(ab); + if (ret) { + ath11k_err(ab, "failed to setup resources: %d\n", ret); + goto err_firmware_stop; + } + + ret = ath11k_core_start(ab); + if (ret) { + ath11k_err(ab, "failed to start core: %d\n", ret); + goto err_free_resources; + } + + ret = ath11k_core_pdev_create(ab); + if (ret) { + ath11k_err(ab, "failed to create pdev core: %d\n", ret); + goto err_core_stop; + } + + ath11k_hif_irq_enable(ab); clear_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags); + mutex_unlock(&ab->core_lock); + + ab->core_stopped = false; return 0; +err_core_stop: + ath11k_core_stop(ab); +err_free_resources: + ath11k_core_free_resources(ab); +err_firmware_stop: + ath11k_qmi_firmware_stop(ab); err_hal_srng_deinit: ath11k_hal_srng_deinit(ab); +err_unlock: + mutex_unlock(&ab->core_lock); return ret; } +EXPORT_SYMBOL(ath11k_core_start_device); MODULE_DESCRIPTION("Core module for Qualcomm Atheros 802.11ax wireless LAN cards."); MODULE_LICENSE("Dual BSD/GPL"); diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h index 582960deb27b..ef4ab36f84e2 100644 --- a/drivers/net/wireless/ath/ath11k/core.h +++ b/drivers/net/wireless/ath/ath11k/core.h @@ -973,6 +973,8 @@ struct ath11k_base { const struct ath11k_pci_ops *ops; } pci; + bool core_stopped; + /* must be last */ u8 drv_priv[] __aligned(sizeof(void *)); }; @@ -1168,6 +1170,8 @@ int ath11k_core_resume(struct ath11k_base *ab); int ath11k_core_suspend(struct ath11k_base *ab); void ath11k_core_pre_reconfigure_recovery(struct ath11k_base *ab); int ath11k_core_start_device(struct ath11k_base *ab); +void ath11k_core_stop_device(struct ath11k_base *ab); +int ath11k_core_any_pdevs_on(struct ath11k_base *ab); const struct firmware *ath11k_core_firmware_request(struct ath11k_base *ab, const char *filename); diff --git a/drivers/net/wireless/ath/ath11k/hif.h b/drivers/net/wireless/ath/ath11k/hif.h index 659b80d2abd4..395b3dbf79b8 100644 --- a/drivers/net/wireless/ath/ath11k/hif.h +++ b/drivers/net/wireless/ath/ath11k/hif.h @@ -30,6 +30,8 @@ struct ath11k_hif_ops { void (*ce_irq_enable)(struct ath11k_base *ab); void (*ce_irq_disable)(struct ath11k_base *ab); void (*get_ce_msi_idx)(struct ath11k_base *ab, u32 ce_id, u32 *msi_idx); + int (*core_start)(struct ath11k_base *ab); + void (*core_stop)(struct ath11k_base *ab); }; static inline void ath11k_hif_ce_irq_enable(struct ath11k_base *ab) @@ -145,4 +147,13 @@ static inline void ath11k_get_ce_msi_idx(struct ath11k_base *ab, u32 ce_id, *msi_data_idx = ce_id; } +static inline int ath11k_hif_core_start(struct ath11k_base *ab) +{ + return ab->hif.ops->core_start(ab); +} + +static inline void ath11k_hif_core_stop(struct ath11k_base *ab) +{ + return ab->hif.ops->core_stop(ab); +} #endif /* _HIF_H_ */ diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 742fbac3d54f..a3929ce6cd0f 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -5819,9 +5819,9 @@ static int ath11k_mac_op_start(struct ieee80211_hw *hw) ath11k_mac_drain_tx(ar); - ret = ath11k_core_start_device(ab); + ret = ath11k_hif_core_start(ab); if (ret) { - ath11k_err(ab, "failed to start device : %d\n", ret); + ath11k_err(ab, "failed to start core : %d\n", ret); return ret; } @@ -5982,6 +5982,10 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw) synchronize_rcu(); atomic_set(&ar->num_pending_mgmt_tx, 0); + + /* If all PDEVs on the SoC are down, then power down the device */ + if (!ath11k_core_any_pdevs_on(ar->ab)) + ath11k_hif_core_stop(ar->ab); } static void diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c index 776362d151cb..c95e25cd4fa6 100644 --- a/drivers/net/wireless/ath/ath11k/pci.c +++ b/drivers/net/wireless/ath/ath11k/pci.c @@ -682,6 +682,30 @@ static int ath11k_pci_start(struct ath11k_base *ab) return 0; } +static int ath11k_pci_core_start(struct ath11k_base *ab) +{ + /* TODO: Currently initializing the hardware/firmware only + * during hardware recovery. Support to shutdown/turn-on + * the hardware during Wi-Fi OFF/ON will be added later. + */ + if (!test_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags)) + return 0; + + return ath11k_core_start_device(ab); +} + +static void ath11k_pci_core_stop(struct ath11k_base *ab) +{ + /* TODO: Currently stopping the hardware/firmware only + * during driver unload. Support to shutdown/turn-on + * the hardware during Wi-Fi OFF/ON will be added later. + */ + if (!test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) + return; + + return ath11k_core_stop_device(ab); +} + static const struct ath11k_hif_ops ath11k_pci_hif_ops = { .start = ath11k_pci_start, .stop = ath11k_pcic_stop, @@ -700,6 +724,8 @@ static const struct ath11k_hif_ops ath11k_pci_hif_ops = { .ce_irq_enable = ath11k_pci_hif_ce_irq_enable, .ce_irq_disable = ath11k_pci_hif_ce_irq_disable, .get_ce_msi_idx = ath11k_pcic_get_ce_msi_idx, + .core_start = ath11k_pci_core_start, + .core_stop = ath11k_pci_core_stop, }; static void ath11k_pci_read_hw_version(struct ath11k_base *ab, u32 *major, u32 *minor)
Currently, WLAN chip is powered once during driver probe and is kept ON (powered) always even when WLAN is not active; keeping the chip powered ON all the time will consume extra power which is not desirable for a battery operated device. Same is the case with non-WoW suspend, chip will never be put into low power mode when the system is suspended resulting in higher battery drain. As per the recommendation, sending a PDEV suspend WMI command followed by a QMI MODE OFF command will cease all WLAN activity and put the device in low power mode. When WLAN interfaces are brought up, sending a QMI MISSION MODE command would be sufficient to bring the chip out of low power. This is a better approach than doing hif_power_down()/hif_power_up() for every WiFi ON/OFF sequence since the turnaround time for entry/exit of low power mode is much less. Overhead is just the time taken for sending QMI MODE OFF & QMI MISSION MODE commands instead of going through the entire chip boot & QMI init sequence. Currently the changes are applicable only for WCN6750. This can be extended to other targets with a future patch. Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1 Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.16 Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com> --- drivers/net/wireless/ath/ath11k/ahb.c | 45 +++++ drivers/net/wireless/ath/ath11k/core.c | 225 ++++++++++++++++++++----- drivers/net/wireless/ath/ath11k/core.h | 4 + drivers/net/wireless/ath/ath11k/hif.h | 11 ++ drivers/net/wireless/ath/ath11k/mac.c | 8 +- drivers/net/wireless/ath/ath11k/pci.c | 26 +++ 6 files changed, 275 insertions(+), 44 deletions(-)