Re: [PATCH v3 3/3] bus: mhi: host: pci_generic: Add edl callback to enter EDL
From: Qiang Yu
Date: Tue Apr 16 2024 - 23:02:01 EST
On 4/17/2024 2:12 AM, Mayank Rana wrote:
On 4/15/2024 8:50 PM, Qiang Yu wrote:
On 4/16/2024 7:53 AM, Mayank Rana wrote:
Hi Qiang
On 4/15/2024 1:49 AM, Qiang Yu wrote:
Add mhi_pci_generic_edl_trigger as edl_trigger for some devices
(eg. SDX65)
to enter EDL mode by writing the 0xEDEDEDED cookie to the channel 91
doorbell register and forcing an SOC reset afterwards.
Signed-off-by: Qiang Yu <quic_qianyu@xxxxxxxxxxx>
---
drivers/bus/mhi/host/pci_generic.c | 47
++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/bus/mhi/host/pci_generic.c
b/drivers/bus/mhi/host/pci_generic.c
index 51639bf..cbf8a58 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -27,12 +27,19 @@
#define PCI_VENDOR_ID_THALES 0x1269
#define PCI_VENDOR_ID_QUECTEL 0x1eac
+#define MHI_EDL_DB 91
+#define MHI_EDL_COOKIE 0xEDEDEDED
+
+/* Device can enter EDL by first setting edl cookie then issuing
inband reset*/
+#define MHI_PCI_GENERIC_EDL_TRIGGER BIT(0)
+
/**
* 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: each bit represents a different way to enter EDL
* @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 +51,7 @@ struct mhi_pci_dev_info {
const char *name;
const char *fw;
const char *edl;
+ unsigned int edl_trigger;
unsigned int bar_num;
unsigned int dma_data_width;
unsigned int mru_default;
@@ -292,6 +300,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 = MHI_PCI_GENERIC_EDL_TRIGGER,
.config = &modem_qcom_v2_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
.dma_data_width = 32,
@@ -302,6 +311,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 = MHI_PCI_GENERIC_EDL_TRIGGER,
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
.dma_data_width = 32,
@@ -312,6 +322,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 = MHI_PCI_GENERIC_EDL_TRIGGER,
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
.dma_data_width = 32,
@@ -928,6 +939,39 @@ 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, "Wake up device fail before
trigger EDL\n");
+ return ret;
+ }
+
+ pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0);
+ mhi_cntrl->runtime_get(mhi_cntrl);
+
+ ret = mhi_get_channel_doorbell(mhi_cntrl, &val);
+ if (ret)
+ return ret;
Don't we need error handling part here i.e. calling
mhi_cntrl->runtime_put() as well mhi_device_put() ?
Hi Mayank
After soc_reset, device will reboot to EDL mode and MHI state will be
SYSERR. So host will fail to suspend
anyway. The "error handling" we need here is actually to enter EDL
mode, this will be done by SYSERR irq.
Here, mhi_cntrl->runtime_put() and mhi_device_put() are only to
balance mhi_cntrl->runtime_get() and
mhi_device_get_sync().
Thanks,
Qiang
I am saying is there possibility that mhi_get_channel_doorbell()
returns error ?
If yes, then why don't we need error handling as part of it. you are
exiting if this API return error without doing anything.
I think here mhi_get_channel_doorbell will not return error. But I still
add a error checking because it invoked mhi_read_reg, which is a must check
API. For modem mhi controller, this API eventually does a memory read.
This memory read will return a normal value if link is up and all f's if
link
is bad.
Thanks,
Qiang
+ 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);
+
+ mhi_cntrl->runtime_put(mhi_cntrl);
+ mhi_device_put(mhi_cntrl->mhi_dev);
+
+ return 0;
+}
+
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 +1006,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_PCI_GENERIC_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;
Regards,
Mayank