Messages in this thread | | | Date | Tue, 16 Apr 2024 11:12:03 -0700 | Subject | Re: [PATCH v3 3/3] bus: mhi: host: pci_generic: Add edl callback to enter EDL | From | Mayank Rana <> |
| |
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@quicinc.com> >>> --- >>> 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. >>> + 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
| |