lkml.org 
[lkml]   [2024]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 3/3] bus: mhi: host: pci_generic: Add edl callback to enter EDL
From


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

\
 
 \ /
  Last update: 2024-04-16 20:12    [W:0.115 / U:0.596 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site