From 813e0ae613d6ee1b3e11f1c41f8b9e9df8ef0493 Mon Sep 17 00:00:00 2001 From: Baochen Qiang Date: Tue, 5 Mar 2024 10:13:18 +0800 Subject: [PATCH 1/4] bus: mhi: host: Add mhi_power_down_keep_dev() API to support system suspend/hibernation Currently, ath11k fails to resume from system suspend/hibernation on some the x86 host machines with below error message: ``` ath11k_pci 0000:06:00.0: timeout while waiting for restart complete ``` This happens because, ath11k powers down the MHI stack during suspend and that leads to destruction of the struct device associated with the MHI channels. And during resume, ath11k calls calling mhi_sync_power_up() to power up the MHI subsystem and that eventually calls the driver framework's device_add() API from mhi_create_devices(). But the PM framework blocks the struct device creation during device_add() and this leads to probe deferral as below: ``` mhi mhi0_IPCR: Driver qcom_mhi_qrtr force probe deferral ``` The reason for deferring device creation during resume 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(). */ Due to the device probe deferral, qcom_mhi_qrtr_probe() API is not getting called during resume and thus MHI channels are not prepared. So this blocks the QMI messages from being transferred between ath11k and firmware, resulting in a firmware initialization failure. After consulting with Rafael, it was decided to not destroy the struct device for the MHI channels during system suspend/hibernation because the device is bound to appear again during resume. So to achieve this, a new API called mhi_power_down_keep_dev() is introduced for MHI controllers to keep the struct device when required. This API is similar to the existing mhi_power_down() API, except that it keeps the struct device associated with MHI channels instead of destroying them. Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.30 Signed-off-by: Baochen Qiang Reviewed-by: Manivannan Sadhasivam Reviewed-by: Jeff Johnson Link: https://lore.kernel.org/r/20240305021320.3367-2-quic_bqiang@quicinc.com [mani: reworded the commit message and subject] Signed-off-by: Manivannan Sadhasivam --- drivers/bus/mhi/host/internal.h | 4 +++- drivers/bus/mhi/host/pm.c | 42 ++++++++++++++++++++++++++++----- include/linux/mhi.h | 18 +++++++++++++- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h index 5fe49311b8eb..aaad40a07f69 100644 --- a/drivers/bus/mhi/host/internal.h +++ b/drivers/bus/mhi/host/internal.h @@ -80,6 +80,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, }; @@ -90,7 +91,8 @@ enum dev_st_transition { dev_st_trans(MISSION_MODE, "MISSION MODE") \ dev_st_trans(FP, "FLASH PROGRAMMER") \ dev_st_trans(SYS_ERR, "SYS ERROR") \ - dev_st_trans_end(DISABLE, "DISABLE") + dev_st_trans(DISABLE, "DISABLE") \ + dev_st_trans_end(DISABLE_DESTROY_DEVICE, "DISABLE (DESTROY DEVICE)") extern const char * const dev_state_tran_str[DEV_ST_TRANSITION_MAX]; #define TO_DEV_STATE_TRANS_STR(state) (((state) >= DEV_ST_TRANSITION_MAX) ? \ diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c index 8b40d3f01acc..11c0e751f223 100644 --- a/drivers/bus/mhi/host/pm.c +++ b/drivers/bus/mhi/host/pm.c @@ -468,7 +468,8 @@ static int mhi_pm_mission_mode_transition(struct mhi_controller *mhi_cntrl) } /* 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; @@ -530,8 +531,16 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl) 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); + /* + * Only destroy the 'struct device' for channels if indicated by the + * 'destroy_device' flag. Because, during system suspend or hibernation + * state, there is no need to destroy the 'struct device' as the endpoint + * device would still be physically attached to the machine. + */ + 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); @@ -821,7 +830,10 @@ void mhi_pm_st_worker(struct work_struct *work) 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; @@ -1175,7 +1187,8 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl) } 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; @@ -1211,15 +1224,32 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful) 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_keep_dev(struct mhi_controller *mhi_cntrl, + bool graceful) +{ + __mhi_power_down(mhi_cntrl, graceful, false); +} +EXPORT_SYMBOL_GPL(mhi_power_down_keep_dev); + int mhi_sync_power_up(struct mhi_controller *mhi_cntrl) { int ret = mhi_async_power_up(mhi_cntrl); diff --git a/include/linux/mhi.h b/include/linux/mhi.h index 77b8c0a26674..cde01e133a1b 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -630,12 +630,28 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl); int mhi_sync_power_up(struct mhi_controller *mhi_cntrl); /** - * mhi_power_down - Start MHI power down sequence + * mhi_power_down - Power down the MHI device and also destroy the + * 'struct device' for the channels associated with it. + * See also mhi_power_down_keep_dev() which is a variant + * of this API that keeps the 'struct device' for channels + * (useful during 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_keep_dev - Power down the MHI device but keep the 'struct + * device' for the channels associated with it. + * This is a variant of 'mhi_power_down()' and + * useful in scenarios such as suspend/hibernation + * where destroying of the 'struct device' is not + * needed. + * @mhi_cntrl: MHI controller + * @graceful: Link is still accessible, so do a graceful shutdown process + */ +void mhi_power_down_keep_dev(struct mhi_controller *mhi_cntrl, bool graceful); + /** * mhi_unprepare_after_power_down - Free any allocated memory after power down * @mhi_cntrl: MHI controller From 17553ba8e19dee8770b3dcc597d49dcc3418f3b0 Mon Sep 17 00:00:00 2001 From: Qiang Yu Date: Wed, 24 Apr 2024 11:21:53 +0800 Subject: [PATCH 2/4] bus: mhi: host: Add sysfs entry to force device to enter EDL Add sysfs entry to allow users of MHI bus to force device to enter EDL (Emergency Download) mode to download the device firmware. Since there is no guarantee that all the devices will support EDL mode, the sysfs entry is kept as an optional one and will appear only for the supported devices. Controllers supporting the EDL mode are expected to provide edl_trigger() callback that puts the device into EDL mode. Signed-off-by: Qiang Yu Reviewed-by: Jeffrey Hugo Reviewed-by: Manivannan Sadhasivam Link: https://lore.kernel.org/r/1713928915-18229-2-git-send-email-quic_qianyu@quicinc.com [mani: fixed the kernel version and reworded the commit message] Signed-off-by: Manivannan Sadhasivam --- Documentation/ABI/stable/sysfs-bus-mhi | 13 ++++++++++ drivers/bus/mhi/host/init.c | 33 ++++++++++++++++++++++++++ include/linux/mhi.h | 2 ++ 3 files changed, 48 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-bus-mhi b/Documentation/ABI/stable/sysfs-bus-mhi index 1a47f9e0cc84..8b9698fa0beb 100644 --- a/Documentation/ABI/stable/sysfs-bus-mhi +++ b/Documentation/ABI/stable/sysfs-bus-mhi @@ -29,3 +29,16 @@ Description: Initiates a SoC reset on the MHI controller. A SoC reset is This can be useful as a method of recovery if the device is non-responsive, or as a means of loading new firmware as a system administration task. + +What: /sys/bus/mhi/devices/.../trigger_edl +Date: April 2024 +KernelVersion: 6.10 +Contact: mhi@lists.linux.dev +Description: Writing a non-zero value to this file will force devices to + enter EDL (Emergency Download) mode. This entry only exists for + devices capable of entering the EDL mode using the standard EDL + triggering mechanism defined in the MHI spec v1.2. Once in EDL + mode, the flash programmer image can be downloaded to the + device to enter the flash programmer execution environment. + This can be useful if user wants to use QDL (Qualcomm Download, + which is used to download firmware over EDL) to update firmware. diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c index 44f934981de8..7104c1856987 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -127,6 +127,30 @@ static ssize_t soc_reset_store(struct device *dev, } static DEVICE_ATTR_WO(soc_reset); +static ssize_t trigger_edl_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct mhi_device *mhi_dev = to_mhi_device(dev); + struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; + unsigned long val; + int ret; + + ret = kstrtoul(buf, 10, &val); + if (ret < 0) + return ret; + + if (!val) + return -EINVAL; + + ret = mhi_cntrl->edl_trigger(mhi_cntrl); + if (ret) + return ret; + + return count; +} +static DEVICE_ATTR_WO(trigger_edl); + static struct attribute *mhi_dev_attrs[] = { &dev_attr_serial_number.attr, &dev_attr_oem_pk_hash.attr, @@ -1018,6 +1042,12 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl, if (ret) goto err_release_dev; + if (mhi_cntrl->edl_trigger) { + ret = sysfs_create_file(&mhi_dev->dev.kobj, &dev_attr_trigger_edl.attr); + if (ret) + goto err_release_dev; + } + mhi_cntrl->mhi_dev = mhi_dev; mhi_create_debugfs(mhi_cntrl); @@ -1051,6 +1081,9 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl) mhi_deinit_free_irq(mhi_cntrl); mhi_destroy_debugfs(mhi_cntrl); + if (mhi_cntrl->edl_trigger) + sysfs_remove_file(&mhi_dev->dev.kobj, &dev_attr_trigger_edl.attr); + destroy_workqueue(mhi_cntrl->hiprio_wq); kfree(mhi_cntrl->mhi_cmd); kfree(mhi_cntrl->mhi_event); diff --git a/include/linux/mhi.h b/include/linux/mhi.h index cde01e133a1b..d968e1ab44dc 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -353,6 +353,7 @@ struct mhi_controller_config { * @read_reg: Read a MHI register via the physical link (required) * @write_reg: Write a MHI register via the physical link (required) * @reset: Controller specific reset function (optional) + * @edl_trigger: CB function to trigger EDL mode (optional) * @buffer_len: Bounce buffer length * @index: Index of the MHI controller instance * @bounce_buf: Use of bounce buffer @@ -435,6 +436,7 @@ struct mhi_controller { void (*write_reg)(struct mhi_controller *mhi_cntrl, void __iomem *addr, u32 val); void (*reset)(struct mhi_controller *mhi_cntrl); + int (*edl_trigger)(struct mhi_controller *mhi_cntrl); size_t buffer_len; int index; From 553f94fc7667259e47a9318e9e5702c9a814d637 Mon Sep 17 00:00:00 2001 From: Qiang Yu Date: Wed, 24 Apr 2024 11:21:54 +0800 Subject: [PATCH 3/4] bus: mhi: host: Add a new API for getting channel doorbell offset Some controllers may want to access a specific doorbell register. Hence add a new API that reads the CHDBOFF register and returns the offset of the doorbell registers from MMIO base, so that the controller can calculate the address of the specific doorbell register by adding the register offset with doorbell offset and MMIO base address. Signed-off-by: Qiang Yu Reviewed-by: Jeffrey Hugo Link: https://lore.kernel.org/r/1713928915-18229-3-git-send-email-quic_qianyu@quicinc.com [mani: reworded commit message and Kdoc] Signed-off-by: Manivannan Sadhasivam --- drivers/bus/mhi/host/init.c | 8 +++----- drivers/bus/mhi/host/main.c | 16 ++++++++++++++++ include/linux/mhi.h | 9 +++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c index 7104c1856987..173f79918741 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -541,11 +541,9 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl) dev_dbg(dev, "Initializing MHI registers\n"); /* Read channel db offset */ - ret = mhi_read_reg(mhi_cntrl, base, CHDBOFF, &val); - if (ret) { - dev_err(dev, "Unable to read CHDBOFF register\n"); - return -EIO; - } + ret = mhi_get_channel_doorbell_offset(mhi_cntrl, &val); + if (ret) + return ret; if (val >= mhi_cntrl->reg_len - (8 * MHI_DEV_WAKE_DB)) { dev_err(dev, "CHDB offset: 0x%x is out of range: 0x%zx\n", diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c index 15d657af9b5b..4de75674f193 100644 --- a/drivers/bus/mhi/host/main.c +++ b/drivers/bus/mhi/host/main.c @@ -1691,3 +1691,19 @@ void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev) } } EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer); + +int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset) +{ + struct device *dev = &mhi_cntrl->mhi_dev->dev; + void __iomem *base = mhi_cntrl->regs; + int ret; + + ret = mhi_read_reg(mhi_cntrl, base, CHDBOFF, chdb_offset); + if (ret) { + dev_err(dev, "Unable to read CHDBOFF register\n"); + return -EIO; + } + + return 0; +} +EXPORT_SYMBOL_GPL(mhi_get_channel_doorbell_offset); diff --git a/include/linux/mhi.h b/include/linux/mhi.h index d968e1ab44dc..b573f15762f8 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -816,4 +816,13 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum dma_data_direction dir, */ bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir); +/** + * mhi_get_channel_doorbell_offset - Get the channel doorbell offset + * @mhi_cntrl: MHI controller + * @chdb_offset: Read channel doorbell offset + * + * Return: 0 if the read succeeds, a negative error code otherwise + */ +int mhi_get_channel_doorbell_offset(struct mhi_controller *mhi_cntrl, u32 *chdb_offset); + #endif /* _MHI_H_ */ From 48f98496b1de132f2e056605b5330209136066dc Mon Sep 17 00:00:00 2001 From: Qiang Yu Date: Wed, 24 Apr 2024 11:21:55 +0800 Subject: [PATCH 4/4] bus: mhi: host: pci_generic: Add generic edl_trigger to allow devices to enter EDL mode Some of the MHI modems like SDX65 based ones are capable of entering the EDL mode as per the standard triggering mechanism defined in the MHI spec v1.2. So let's add a common mhi_pci_generic_edl_trigger() function that triggers the EDL mode in the device when user writes to the /sys/bus/mhi/devices/.../trigger_edl file. As per the spec, the EDL mode can be triggered by writing a cookie to the EDL doorbell register and then resetting the device. Devices supporting this standard way of entering EDL mode can set the mhi_pci_dev_info::edl_trigger flag. Signed-off-by: Qiang Yu Reviewed-by: Jeffrey Hugo Reviewed-by: Manivannan Sadhasivam Link: https://lore.kernel.org/r/1713928915-18229-4-git-send-email-quic_qianyu@quicinc.com [mani: reworded commit message] Signed-off-by: Manivannan Sadhasivam --- drivers/bus/mhi/host/pci_generic.c | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c index 51639bfcfec7..08844ee79654 100644 --- a/drivers/bus/mhi/host/pci_generic.c +++ b/drivers/bus/mhi/host/pci_generic.c @@ -27,12 +27,16 @@ #define PCI_VENDOR_ID_THALES 0x1269 #define PCI_VENDOR_ID_QUECTEL 0x1eac +#define MHI_EDL_DB 91 +#define MHI_EDL_COOKIE 0xEDEDEDED + /** * struct mhi_pci_dev_info - MHI PCI device specific information * @config: MHI controller configuration * @name: name of the PCI module * @fw: firmware path (if any) * @edl: emergency download mode firmware path (if any) + * @edl_trigger: capable of triggering EDL mode in the device (if supported) * @bar_num: PCI base address register to use for MHI MMIO register space * @dma_data_width: DMA transfer word size (32 or 64 bits) * @mru_default: default MRU size for MBIM network packets @@ -44,6 +48,7 @@ struct mhi_pci_dev_info { const char *name; const char *fw; const char *edl; + bool edl_trigger; unsigned int bar_num; unsigned int dma_data_width; unsigned int mru_default; @@ -292,6 +297,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx75_info = { .name = "qcom-sdx75m", .fw = "qcom/sdx75m/xbl.elf", .edl = "qcom/sdx75m/edl.mbn", + .edl_trigger = true, .config = &modem_qcom_v2_mhiv_config, .bar_num = MHI_PCI_DEFAULT_BAR_NUM, .dma_data_width = 32, @@ -302,6 +308,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { .name = "qcom-sdx65m", .fw = "qcom/sdx65m/xbl.elf", .edl = "qcom/sdx65m/edl.mbn", + .edl_trigger = true, .config = &modem_qcom_v1_mhiv_config, .bar_num = MHI_PCI_DEFAULT_BAR_NUM, .dma_data_width = 32, @@ -312,6 +319,7 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { .name = "qcom-sdx55m", .fw = "qcom/sdx55m/sbl1.mbn", .edl = "qcom/sdx55m/edl.mbn", + .edl_trigger = true, .config = &modem_qcom_v1_mhiv_config, .bar_num = MHI_PCI_DEFAULT_BAR_NUM, .dma_data_width = 32, @@ -928,6 +936,40 @@ static void health_check(struct timer_list *t) mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD); } +static int mhi_pci_generic_edl_trigger(struct mhi_controller *mhi_cntrl) +{ + void __iomem *base = mhi_cntrl->regs; + void __iomem *edl_db; + int ret; + u32 val; + + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev); + if (ret) { + dev_err(mhi_cntrl->cntrl_dev, "Failed to wakeup the device\n"); + return ret; + } + + pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0); + mhi_cntrl->runtime_get(mhi_cntrl); + + ret = mhi_get_channel_doorbell_offset(mhi_cntrl, &val); + if (ret) + goto err_get_chdb; + + edl_db = base + val + (8 * MHI_EDL_DB); + + mhi_cntrl->write_reg(mhi_cntrl, edl_db + 4, upper_32_bits(MHI_EDL_COOKIE)); + mhi_cntrl->write_reg(mhi_cntrl, edl_db, lower_32_bits(MHI_EDL_COOKIE)); + + mhi_soc_reset(mhi_cntrl); + +err_get_chdb: + mhi_cntrl->runtime_put(mhi_cntrl); + mhi_device_put(mhi_cntrl->mhi_dev); + + return ret; +} + static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *) id->driver_data; @@ -962,6 +1004,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) mhi_cntrl->runtime_put = mhi_pci_runtime_put; mhi_cntrl->mru = info->mru_default; + if (info->edl_trigger) + mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger; + if (info->sideband_wake) { mhi_cntrl->wake_get = mhi_pci_wake_get_nop; mhi_cntrl->wake_put = mhi_pci_wake_put_nop;