| Message ID | 20250109-fix_reboot_issues_with_hw_grouping-v1-4-fb39ec03451e@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | wifi: ath12k: fixes for rmmod and recovery issues with hardware grouping | expand |
On 1/8/2025 8:25 PM, Aditya Kumar Singh wrote: > At present, during PCI shutdown, the power down is only executed for a > single device. However, when operating in a group, all devices need to be > powered down simultaneously. Failure to do so will result in a firmware > assertion. > > Hence, introduce a new ath12k_pci_hw_group_power_down() and call it during > power down. This will ensure that all partner devices are properly powered > down. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 > > Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> > --- > drivers/net/wireless/ath/ath12k/pci.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c > index 837be309cd45a2d037ee8c3bba8f7be0f457d6b2..7f6521a56ffc0f1e9687c94d6829a9c1f1887661 100644 > --- a/drivers/net/wireless/ath/ath12k/pci.c > +++ b/drivers/net/wireless/ath/ath12k/pci.c > @@ -1751,13 +1751,34 @@ static void ath12k_pci_remove(struct pci_dev *pdev) > ath12k_core_free(ab); > } > > +static void ath12k_pci_hw_group_power_down(struct ath12k_hw_group *ag) don't you end up calling this for every device in the group? what prevents ath12k_pci_power_down(ab, false) from being called multiple times for the same ab? > +{ > + struct ath12k_base *ab; > + int i; > + > + if (!ag) > + return; > + > + mutex_lock(&ag->mutex); > + > + for (i = 0; i < ag->num_devices; i++) { > + ab = ag->ab[i]; > + if (!ab) > + continue; > + > + ath12k_pci_power_down(ab, false); > + } > + > + mutex_unlock(&ag->mutex); > +} > + > static void ath12k_pci_shutdown(struct pci_dev *pdev) > { > struct ath12k_base *ab = pci_get_drvdata(pdev); > struct ath12k_pci *ab_pci = ath12k_pci_priv(ab); > > ath12k_pci_set_irq_affinity_hint(ab_pci, NULL); > - ath12k_pci_power_down(ab, false); > + ath12k_pci_hw_group_power_down(ab->ag); > } > > static __maybe_unused int ath12k_pci_pm_suspend(struct device *dev) >
On 1/14/25 00:42, Jeff Johnson wrote: > On 1/8/2025 8:25 PM, Aditya Kumar Singh wrote: >> At present, during PCI shutdown, the power down is only executed for a >> single device. However, when operating in a group, all devices need to be >> powered down simultaneously. Failure to do so will result in a firmware >> assertion. >> >> Hence, introduce a new ath12k_pci_hw_group_power_down() and call it during >> power down. This will ensure that all partner devices are properly powered >> down. >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 >> >> Signed-off-by: Aditya Kumar Singh<quic_adisi@quicinc.com> >> --- >> drivers/net/wireless/ath/ath12k/pci.c | 23 ++++++++++++++++++++++- >> 1 file changed, 22 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c >> index 837be309cd45a2d037ee8c3bba8f7be0f457d6b2..7f6521a56ffc0f1e9687c94d6829a9c1f1887661 100644 >> --- a/drivers/net/wireless/ath/ath12k/pci.c >> +++ b/drivers/net/wireless/ath/ath12k/pci.c >> @@ -1751,13 +1751,34 @@ static void ath12k_pci_remove(struct pci_dev *pdev) >> ath12k_core_free(ab); >> } >> >> +static void ath12k_pci_hw_group_power_down(struct ath12k_hw_group *ag) > don't you end up calling this for every device in the group? > what prevents ath12k_pci_power_down(ab, false) from being called multiple > times for the same ab? That's true. ath12k_pci_power_down() has logic already that if device is powered down, it will ignore the further call. This is handled via previous patch.
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c index 837be309cd45a2d037ee8c3bba8f7be0f457d6b2..7f6521a56ffc0f1e9687c94d6829a9c1f1887661 100644 --- a/drivers/net/wireless/ath/ath12k/pci.c +++ b/drivers/net/wireless/ath/ath12k/pci.c @@ -1751,13 +1751,34 @@ static void ath12k_pci_remove(struct pci_dev *pdev) ath12k_core_free(ab); } +static void ath12k_pci_hw_group_power_down(struct ath12k_hw_group *ag) +{ + struct ath12k_base *ab; + int i; + + if (!ag) + return; + + mutex_lock(&ag->mutex); + + for (i = 0; i < ag->num_devices; i++) { + ab = ag->ab[i]; + if (!ab) + continue; + + ath12k_pci_power_down(ab, false); + } + + mutex_unlock(&ag->mutex); +} + static void ath12k_pci_shutdown(struct pci_dev *pdev) { struct ath12k_base *ab = pci_get_drvdata(pdev); struct ath12k_pci *ab_pci = ath12k_pci_priv(ab); ath12k_pci_set_irq_affinity_hint(ab_pci, NULL); - ath12k_pci_power_down(ab, false); + ath12k_pci_hw_group_power_down(ab->ag); } static __maybe_unused int ath12k_pci_pm_suspend(struct device *dev)
At present, during PCI shutdown, the power down is only executed for a single device. However, when operating in a group, all devices need to be powered down simultaneously. Failure to do so will result in a firmware assertion. Hence, introduce a new ath12k_pci_hw_group_power_down() and call it during power down. This will ensure that all partner devices are properly powered down. Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> --- drivers/net/wireless/ath/ath12k/pci.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)