| 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
| ||
|
Message-ID: <a901549e-c3ec-47b5-9523-a6342eca455f@quicinc.com>
Date: Fri, 12 Apr 2024 15:13:33 +0800
From: Qiang Yu <quic_qianyu@...cinc.com>
To: Jeffrey Hugo <quic_jhugo@...cinc.com>, <mani@...nel.org>
CC: <mhi@...ts.linux.dev>, <linux-arm-msm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <quic_cang@...cinc.com>,
<quic_mrana@...cinc.com>
Subject: Re: [PATCH v2 2/2] bus: mhi: host: pci_generic: Add edl callback to
enter EDL
On 4/11/2024 10:46 PM, Jeffrey Hugo wrote:
> On 4/10/2024 9:15 PM, 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 | 50
>> ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 50 insertions(+)
>>
>> diff --git a/drivers/bus/mhi/host/pci_generic.c
>> b/drivers/bus/mhi/host/pci_generic.c
>> index 51639bf..a529815 100644
>> --- a/drivers/bus/mhi/host/pci_generic.c
>> +++ b/drivers/bus/mhi/host/pci_generic.c
>> @@ -27,12 +27,23 @@
>> #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)
>> +
>> +#define CHDBOFF 0x18
>
> This is already in drivers/bus/mhi/common.h why duplicate it here?
I only see common.h be included in ep/internal.h host/internal.h and
host/trace.h. So I thought it can only be used by MHI stack. Can we
include common.h in pci_generic.c?
>
>> +#define CHDBOFF_CHDBOFF_MASK 0xFFFFFFFF
>> +#define CHDBOFF_CHDBOFF_SHIFT 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 +55,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 +304,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 +315,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 +326,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 +943,38 @@ 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)
>> +{
>> + int ret;
>> + u32 val;
>> + void __iomem *edl_db;
>> + void __iomem *base = mhi_cntrl->regs;
>
> It looks like this file follows reverse christmas tree, but this
> function does not. you should fix it.
Will fix it in next version patch.
>
>> +
>> + 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);
>> +
>> + mhi_cntrl->read_reg(mhi_cntrl, base + CHDBOFF, &val);
>> + val = (val & CHDBOFF_CHDBOFF_MASK) >> CHDBOFF_CHDBOFF_SHIFT;
>> +
>> + 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 +1009,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;
>
Powered by blists - more mailing lists