From 2b1b14d40f527052047f669c06b5d742a8839eb7 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Feb 26 2024 13:04:58 +0000 Subject: bus: mhi: host: add mhi_power_down_no_destroy() (bsc#1207948). --- diff --git a/patches.suse/bus-mhi-host-add-mhi_power_down_no_destroy.patch b/patches.suse/bus-mhi-host-add-mhi_power_down_no_destroy.patch new file mode 100644 index 0000000..5fa5ce8 --- /dev/null +++ b/patches.suse/bus-mhi-host-add-mhi_power_down_no_destroy.patch @@ -0,0 +1,195 @@ +From: Baochen Qiang +Subject: [PATCH 1/3] bus: mhi: host: add mhi_power_down_no_destroy() +Date: Wed, 21 Feb 2024 11:00:24 +0800 +Message-id: <20240221030026.10553-2-quic_bqiang@quicinc.com> +Patch-mainline: Submitted, linux-wireless ML +References: bsc#1207948 + +ath11k fails to resume: + +ath11k_pci 0000:06:00.0: timeout while waiting for restart complete + +This happens because when calling mhi_sync_power_up() the MHI subsystem +eventually calls device_add() from mhi_create_devices() but the device +creation is deferred: + +mhi mhi0_IPCR: Driver qcom_mhi_qrtr force probe deferral + +The reason for deferring device creation is explained in dpm_prepare(): + + /* + * It is unsafe if probing of devices will happen during suspend or + * hibernation and system behavior will be unpredictable in this case. + * So, let's prohibit device's probing here and defer their probes + * instead. The normal behavior will be restored in dpm_complete(). + */ + +Because the device probe is deferred, the qcom_mhi_qrtr_probe() is not +called and thus MHI channels are not prepared: + +So what this means that QRTR is not delivering messages and the QMI connection +is not working between ath11k and the firmware, resulting a failure in firmware +initialization. + +To fix this add new function mhi_power_down_no_destroy() which doesn't destroy +the devices for channels during power down. This way we avoid probe defer issue +and finally can get ath11k hibernation working with the following patches. + +Actually there is an RFC version of this change and it gets positive results +from multiple users. Firstly Mani doesn't like this idea and insists that an +MHI device should be destroyed when going to suspend/hibernation, see + +https://lore.kernel.org/mhi/20231127162022.518834-1-kvalo@kernel.org/ + +Then Mani changed his mind after a further discussion with kernel PM guys, +see + +https://lore.kernel.org/all/21cd2098-97e1-4947-a5bb-a97582902ead@quicinc.com/ + +So we come up with the regular version and it is almost identical with that RFC +version. + +Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 + +Signed-off-by: Kalle Valo +Signed-off-by: Baochen Qiang +Acked-by: Takashi Iwai + +--- + drivers/bus/mhi/host/init.c | 1 + + drivers/bus/mhi/host/internal.h | 1 + + drivers/bus/mhi/host/pm.c | 36 ++++++++++++++++++++++++++++++------ + include/linux/mhi.h | 15 ++++++++++++++- + 4 files changed, 46 insertions(+), 7 deletions(-) + +--- a/drivers/bus/mhi/host/init.c ++++ b/drivers/bus/mhi/host/init.c +@@ -43,6 +43,7 @@ const char * const dev_state_tran_str[DE + [DEV_ST_TRANSITION_FP] = "FLASH PROGRAMMER", + [DEV_ST_TRANSITION_SYS_ERR] = "SYS ERROR", + [DEV_ST_TRANSITION_DISABLE] = "DISABLE", ++ [DEV_ST_TRANSITION_DISABLE_DESTROY_DEVICE] = "DISABLE (DESTROY DEVICE)", + }; + + const char * const mhi_ch_state_type_str[MHI_CH_STATE_TYPE_MAX] = { +--- a/drivers/bus/mhi/host/internal.h ++++ b/drivers/bus/mhi/host/internal.h +@@ -69,6 +69,7 @@ enum dev_st_transition { + DEV_ST_TRANSITION_FP, + DEV_ST_TRANSITION_SYS_ERR, + DEV_ST_TRANSITION_DISABLE, ++ DEV_ST_TRANSITION_DISABLE_DESTROY_DEVICE, + DEV_ST_TRANSITION_MAX, + }; + +--- a/drivers/bus/mhi/host/pm.c ++++ b/drivers/bus/mhi/host/pm.c +@@ -453,7 +453,8 @@ error_mission_mode: + } + + /* Handle shutdown transitions */ +-static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl) ++static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, ++ bool destroy_device) + { + enum mhi_pm_state cur_state; + struct mhi_event *mhi_event; +@@ -515,8 +516,10 @@ skip_mhi_reset: + dev_dbg(dev, "Waiting for all pending threads to complete\n"); + wake_up_all(&mhi_cntrl->state_event); + +- dev_dbg(dev, "Reset all active channels and remove MHI devices\n"); +- device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device); ++ if (destroy_device) { ++ dev_dbg(dev, "Reset all active channels and remove MHI devices\n"); ++ device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_destroy_device); ++ } + + mutex_lock(&mhi_cntrl->pm_mutex); + +@@ -801,7 +804,10 @@ void mhi_pm_st_worker(struct work_struct + mhi_pm_sys_error_transition(mhi_cntrl); + break; + case DEV_ST_TRANSITION_DISABLE: +- mhi_pm_disable_transition(mhi_cntrl); ++ mhi_pm_disable_transition(mhi_cntrl, false); ++ break; ++ case DEV_ST_TRANSITION_DISABLE_DESTROY_DEVICE: ++ mhi_pm_disable_transition(mhi_cntrl, true); + break; + default: + break; +@@ -1154,7 +1160,8 @@ error_exit: + } + EXPORT_SYMBOL_GPL(mhi_async_power_up); + +-void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful) ++static void __mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful, ++ bool destroy_device) + { + enum mhi_pm_state cur_state, transition_state; + struct device *dev = &mhi_cntrl->mhi_dev->dev; +@@ -1190,15 +1197,32 @@ void mhi_power_down(struct mhi_controlle + write_unlock_irq(&mhi_cntrl->pm_lock); + mutex_unlock(&mhi_cntrl->pm_mutex); + +- mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_DISABLE); ++ if (destroy_device) ++ mhi_queue_state_transition(mhi_cntrl, ++ DEV_ST_TRANSITION_DISABLE_DESTROY_DEVICE); ++ else ++ mhi_queue_state_transition(mhi_cntrl, ++ DEV_ST_TRANSITION_DISABLE); + + /* Wait for shutdown to complete */ + flush_work(&mhi_cntrl->st_worker); + + disable_irq(mhi_cntrl->irq[0]); + } ++ ++void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful) ++{ ++ __mhi_power_down(mhi_cntrl, graceful, true); ++} + EXPORT_SYMBOL_GPL(mhi_power_down); + ++void mhi_power_down_no_destroy(struct mhi_controller *mhi_cntrl, ++ bool graceful) ++{ ++ __mhi_power_down(mhi_cntrl, graceful, false); ++} ++EXPORT_SYMBOL_GPL(mhi_power_down_no_destroy); ++ + int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) + { + int ret = mhi_async_power_up(mhi_cntrl); +--- a/include/linux/mhi.h ++++ b/include/linux/mhi.h +@@ -645,13 +645,26 @@ int mhi_async_power_up(struct mhi_contro + int mhi_sync_power_up(struct mhi_controller *mhi_cntrl); + + /** +- * mhi_power_down - Start MHI power down sequence ++ * mhi_power_down - Start MHI power down sequence. See also ++ * mhi_power_down_no_destroy() which is a variant of this for ++ * suspend/hibernation. ++ * + * @mhi_cntrl: MHI controller + * @graceful: Link is still accessible, so do a graceful shutdown process + */ + void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful); + + /** ++ * mhi_power_down_no_destroy - Start MHI power down sequence but don't destroy ++ * struct devices for channels. This is a variant for mhi_power_down() and ++ * would be useful in suspend/hibernation. ++ * ++ * @mhi_cntrl: MHI controller ++ * @graceful: Link is still accessible, so do a graceful shutdown process ++ */ ++void mhi_power_down_no_destroy(struct mhi_controller *mhi_cntrl, bool graceful); ++ ++/** + * mhi_unprepare_after_power_down - Free any allocated memory after power down + * @mhi_cntrl: MHI controller + */ diff --git a/series.conf b/series.conf index 44d1aa6..ed164f9 100644 --- a/series.conf +++ b/series.conf @@ -195,6 +195,7 @@ patches.suse/wifi-ath11k-do-not-dump-SRNG-statistics-during-resum.patch patches.suse/wifi-ath11k-fix-warning-on-DMA-ring-capabilities-eve.patch patches.suse/wifi-ath11k-thermal-don-t-try-to-register-multiple-t.patch + patches.suse/bus-mhi-host-add-mhi_power_down_no_destroy.patch ######################################################## # USB