lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 16 Apr 2024 11:50:15 +0800
From: Qiang Yu <quic_qianyu@...cinc.com>
To: Mayank Rana <quic_mrana@...cinc.com>, <mani@...nel.org>,
        <quic_jhugo@...cinc.com>
CC: <mhi@...ts.linux.dev>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <quic_cang@...cinc.com>
Subject: Re: [PATCH v3 3/3] bus: mhi: host: pci_generic: Add edl callback to
 enter EDL


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@...cinc.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

>> +    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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ